Skip to content

Commit c489d36

Browse files
nikimanoledakinikimanoledaki
committed
Adding AssignIpv6AddressOnCreation task after cluster creation due to CF bug
- AssignIpv6AddressOnCreation also needs to be set on public subnets, but due to a current bug in CF, this cannot be set alongside MapPublicIpOnLaunch at create time. This means we need to add it "manually" by hitting the VPC API to update each public subnet after launch. - Added extra validation that NAT is nil
1 parent c67cbd7 commit c489d36

File tree

7 files changed

+228
-77
lines changed

7 files changed

+228
-77
lines changed

‎integration/tests/ipv6/ipv6_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,10 @@ var _ = Describe("(Integration) [EKS IPv6 test]", func() {
4747
BeforeSuite(func() {
4848
clusterConfig = api.NewClusterConfig()
4949
clusterConfig.Metadata.Name = clusterName
50-
clusterConfig.Metadata.Version = "latest"
50+
clusterConfig.Metadata.Version = "1.21"
5151
clusterConfig.Metadata.Region = params.Region
5252
clusterConfig.VPC.IPFamily = aws.String("IPv6")
53+
clusterConfig.VPC.NAT = nil
5354
clusterConfig.IAM.WithOIDC = api.Enabled()
5455
clusterConfig.Addons = []*api.Addon{
5556
{

‎pkg/cfn/manager/create_tasks.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,15 @@ func (c *StackCollection) NewTasksToCreateClusterWithNodeGroups(nodeGroups []*ap
3333
},
3434
)
3535

36+
if c.spec.VPC.IPFamily != nil && *c.spec.VPC.IPFamily == string(api.IPV6Family) {
37+
taskTree.Append(
38+
&AssignIpv6AddressOnCreationTask{
39+
ClusterConfig: c.spec,
40+
EC2API: c.ec2API,
41+
},
42+
)
43+
}
44+
3645
appendNodeGroupTasksTo := func(taskTree *tasks.TaskTree) {
3746
vpcImporter := vpc.NewStackConfigImporter(c.MakeClusterStackName())
3847
nodeGroupTasks := c.NewUnmanagedNodeGroupTask(nodeGroups, false, vpcImporter)

‎pkg/cfn/manager/create_tasks_test.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package manager_test
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/aws/aws-sdk-go/aws"
7+
"github.com/aws/aws-sdk-go/service/ec2"
8+
. "github.com/onsi/ginkgo"
9+
. "github.com/onsi/gomega"
10+
"github.com/stretchr/testify/mock"
11+
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
12+
"github.com/weaveworks/eksctl/pkg/cfn/manager"
13+
"github.com/weaveworks/eksctl/pkg/testutils/mockprovider"
14+
)
15+
16+
var _ = Describe("CreateTasks", func() {
17+
var subnetIDs = []string{"123", "456"}
18+
var clusterConfig *api.ClusterConfig
19+
Context("AssignIpv6AddressOnCreationTask", func() {
20+
BeforeEach(func() {
21+
clusterConfig = api.NewClusterConfig()
22+
clusterConfig.VPC.Subnets = &api.ClusterSubnets{}
23+
clusterConfig.VPC.Subnets.Public = map[string]api.AZSubnetSpec{
24+
"0": {ID: subnetIDs[0]},
25+
"1": {ID: subnetIDs[1]},
26+
}
27+
})
28+
29+
It("sets AssignIpv6AddressOnCreation to true for all public subnets", func() {
30+
modifySubnetAttributeCallCount := 0
31+
p := mockprovider.NewMockProvider()
32+
mockCall1 := p.MockEC2().On("ModifySubnetAttribute", &ec2.ModifySubnetAttributeInput{
33+
AssignIpv6AddressOnCreation: &ec2.AttributeBooleanValue{
34+
Value: aws.Bool(true),
35+
},
36+
SubnetId: aws.String(subnetIDs[0]),
37+
}).Return(&ec2.ModifySubnetAttributeOutput{}, nil)
38+
39+
mockCall1.RunFn = func(_ mock.Arguments) {
40+
modifySubnetAttributeCallCount++
41+
}
42+
43+
mockCall2 := p.MockEC2().On("ModifySubnetAttribute", &ec2.ModifySubnetAttributeInput{
44+
AssignIpv6AddressOnCreation: &ec2.AttributeBooleanValue{
45+
Value: aws.Bool(true),
46+
},
47+
SubnetId: aws.String(subnetIDs[1]),
48+
}).Return(&ec2.ModifySubnetAttributeOutput{}, nil)
49+
50+
mockCall2.RunFn = func(_ mock.Arguments) {
51+
modifySubnetAttributeCallCount++
52+
}
53+
54+
task := manager.AssignIpv6AddressOnCreationTask{
55+
EC2API: p.EC2(),
56+
ClusterConfig: clusterConfig,
57+
}
58+
errorCh := make(chan error)
59+
err := task.Do(errorCh)
60+
Expect(err).NotTo(HaveOccurred())
61+
Expect(modifySubnetAttributeCallCount).To(Equal(2))
62+
63+
By("closing the error channel")
64+
Eventually(errorCh).Should(BeClosed())
65+
})
66+
67+
When("the API call errors", func() {
68+
It("errors", func() {
69+
p := mockprovider.NewMockProvider()
70+
p.MockEC2().On("ModifySubnetAttribute", &ec2.ModifySubnetAttributeInput{
71+
AssignIpv6AddressOnCreation: &ec2.AttributeBooleanValue{
72+
Value: aws.Bool(true),
73+
},
74+
SubnetId: aws.String(subnetIDs[0]),
75+
}).Return(&ec2.ModifySubnetAttributeOutput{}, fmt.Errorf("foo"))
76+
77+
task := manager.AssignIpv6AddressOnCreationTask{
78+
EC2API: p.EC2(),
79+
ClusterConfig: clusterConfig,
80+
}
81+
errorCh := make(chan error)
82+
err := task.Do(errorCh)
83+
Expect(err).To(MatchError("failed to update subnet \"123\": foo"))
84+
85+
By("closing the error channel")
86+
Eventually(errorCh).Should(BeClosed())
87+
})
88+
})
89+
})
90+
})

‎pkg/cfn/manager/tasks.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import (
66
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
77
"k8s.io/client-go/kubernetes"
88

9+
"github.com/aws/aws-sdk-go/aws"
10+
"github.com/aws/aws-sdk-go/service/ec2"
11+
"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
912
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
1013
iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc"
1114
kubewrapper "github.com/weaveworks/eksctl/pkg/kubernetes"
@@ -131,3 +134,28 @@ func (t *kubernetesTask) Do(errs chan error) error {
131134
close(errs)
132135
return err
133136
}
137+
138+
type AssignIpv6AddressOnCreationTask struct {
139+
EC2API ec2iface.EC2API
140+
ClusterConfig *api.ClusterConfig
141+
}
142+
143+
func (t *AssignIpv6AddressOnCreationTask) Describe() string {
144+
return "set AssignIpv6AddressOnCreation to true for public subnets"
145+
}
146+
147+
func (t *AssignIpv6AddressOnCreationTask) Do(errs chan error) error {
148+
defer close(errs)
149+
for _, subnet := range t.ClusterConfig.VPC.Subnets.Public.WithIDs() {
150+
_, err := t.EC2API.ModifySubnetAttribute(&ec2.ModifySubnetAttributeInput{
151+
AssignIpv6AddressOnCreation: &ec2.AttributeBooleanValue{
152+
Value: aws.Bool(true),
153+
},
154+
SubnetId: aws.String(subnet),
155+
})
156+
if err != nil {
157+
return fmt.Errorf("failed to update subnet %q: %v", subnet, err)
158+
}
159+
}
160+
return nil
161+
}

‎pkg/cfn/manager/tasks_test.go‎

Lines changed: 79 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package manager
33
import (
44
"fmt"
55

6+
"github.com/aws/aws-sdk-go/aws"
67
. "github.com/onsi/ginkgo"
78
. "github.com/onsi/gomega"
89
"github.com/pkg/errors"
@@ -59,83 +60,67 @@ var _ = Describe("StackCollection Tasks", func() {
5960
}
6061

6162
Describe("TaskTree", func() {
62-
Context("With real tasks", func() {
63-
64-
BeforeEach(func() {
65-
66-
p = mockprovider.NewMockProvider()
63+
BeforeEach(func() {
64+
p = mockprovider.NewMockProvider()
65+
cfg = newClusterConfig("test-cluster")
66+
stackManager = NewStackCollection(p, cfg)
67+
})
6768

68-
cfg = newClusterConfig("test-cluster")
69+
It("should have nice description", func() {
70+
fakeVPCImporter := new(vpcfakes.FakeImporter)
71+
// TODO use DescribeTable
72+
73+
// The supportsManagedNodes argument has no effect on the Describe call, so the values are alternated
74+
// in these tests
75+
{
76+
tasks := stackManager.NewUnmanagedNodeGroupTask(makeNodeGroups("bar", "foo"), false, fakeVPCImporter)
77+
Expect(tasks.Describe()).To(Equal(`2 parallel tasks: { create nodegroup "bar", create nodegroup "foo" }`))
78+
}
79+
{
80+
tasks := stackManager.NewUnmanagedNodeGroupTask(makeNodeGroups("bar"), false, fakeVPCImporter)
81+
Expect(tasks.Describe()).To(Equal(`1 task: { create nodegroup "bar" }`))
82+
}
83+
{
84+
tasks := stackManager.NewUnmanagedNodeGroupTask(makeNodeGroups("foo"), false, fakeVPCImporter)
85+
Expect(tasks.Describe()).To(Equal(`1 task: { create nodegroup "foo" }`))
86+
}
87+
{
88+
tasks := stackManager.NewUnmanagedNodeGroupTask(nil, false, fakeVPCImporter)
89+
Expect(tasks.Describe()).To(Equal(`no tasks`))
90+
}
91+
{
92+
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar", "foo"), nil, true)
93+
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", 2 parallel sub-tasks: { create nodegroup "bar", create nodegroup "foo" } }`))
94+
}
95+
{
96+
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar"), nil, false)
97+
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", create nodegroup "bar" }`))
98+
}
99+
{
100+
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(nil, nil, true)
101+
Expect(tasks.Describe()).To(Equal(`1 task: { create cluster control plane "test-cluster" }`))
102+
}
103+
{
104+
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar", "foo"), makeManagedNodeGroups("m1", "m2"), false)
105+
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", 4 parallel sub-tasks: { create nodegroup "bar", create nodegroup "foo", create managed nodegroup "m1", create managed nodegroup "m2" } }`))
106+
}
107+
{
108+
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("foo"), makeManagedNodeGroups("m1"), true)
109+
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", 2 parallel sub-tasks: { create nodegroup "foo", create managed nodegroup "m1" } }`))
110+
}
111+
{
112+
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar"), nil, false, &task{id: 1})
113+
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", 2 sequential sub-tasks: { task 1, create nodegroup "bar" } }`))
114+
}
115+
})
69116

70-
stackManager = NewStackCollection(p, cfg)
117+
When("IPFamily is set to ipv6", func() {
118+
BeforeEach(func() {
119+
cfg.VPC.IPFamily = aws.String(string(api.IPV6Family))
71120
})
72-
73-
It("should have nice description", func() {
74-
makeNodeGroups := func(names ...string) []*api.NodeGroup {
75-
var nodeGroups []*api.NodeGroup
76-
for _, name := range names {
77-
ng := api.NewNodeGroup()
78-
ng.Name = name
79-
nodeGroups = append(nodeGroups, ng)
80-
}
81-
return nodeGroups
82-
}
83-
84-
makeManagedNodeGroups := func(names ...string) []*api.ManagedNodeGroup {
85-
var managedNodeGroups []*api.ManagedNodeGroup
86-
for _, name := range names {
87-
ng := api.NewManagedNodeGroup()
88-
ng.Name = name
89-
managedNodeGroups = append(managedNodeGroups, ng)
90-
}
91-
return managedNodeGroups
92-
}
93-
94-
fakeVPCImporter := new(vpcfakes.FakeImporter)
95-
// TODO use DescribeTable
96-
97-
// The supportsManagedNodes argument has no effect on the Describe call, so the values are alternated
98-
// in these tests
99-
{
100-
tasks := stackManager.NewUnmanagedNodeGroupTask(makeNodeGroups("bar", "foo"), false, fakeVPCImporter)
101-
Expect(tasks.Describe()).To(Equal(`2 parallel tasks: { create nodegroup "bar", create nodegroup "foo" }`))
102-
}
103-
{
104-
tasks := stackManager.NewUnmanagedNodeGroupTask(makeNodeGroups("bar"), false, fakeVPCImporter)
105-
Expect(tasks.Describe()).To(Equal(`1 task: { create nodegroup "bar" }`))
106-
}
107-
{
108-
tasks := stackManager.NewUnmanagedNodeGroupTask(makeNodeGroups("foo"), false, fakeVPCImporter)
109-
Expect(tasks.Describe()).To(Equal(`1 task: { create nodegroup "foo" }`))
110-
}
111-
{
112-
tasks := stackManager.NewUnmanagedNodeGroupTask(nil, false, fakeVPCImporter)
113-
Expect(tasks.Describe()).To(Equal(`no tasks`))
114-
}
115-
{
116-
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar", "foo"), nil, true)
117-
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", 2 parallel sub-tasks: { create nodegroup "bar", create nodegroup "foo" } }`))
118-
}
119-
{
120-
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar"), nil, false)
121-
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", create nodegroup "bar" }`))
122-
}
123-
{
124-
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(nil, nil, true)
125-
Expect(tasks.Describe()).To(Equal(`1 task: { create cluster control plane "test-cluster" }`))
126-
}
127-
{
128-
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar", "foo"), makeManagedNodeGroups("m1", "m2"), false)
129-
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", 4 parallel sub-tasks: { create nodegroup "bar", create nodegroup "foo", create managed nodegroup "m1", create managed nodegroup "m2" } }`))
130-
}
131-
{
132-
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("foo"), makeManagedNodeGroups("m1"), true)
133-
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", 2 parallel sub-tasks: { create nodegroup "foo", create managed nodegroup "m1" } }`))
134-
}
135-
{
136-
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar"), nil, false, &task{id: 1})
137-
Expect(tasks.Describe()).To(Equal(`2 sequential tasks: { create cluster control plane "test-cluster", 2 sequential sub-tasks: { task 1, create nodegroup "bar" } }`))
138-
}
121+
It("appends the AssignIpv6AddressOnCreation task to occur after the cluster creation", func() {
122+
tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(makeNodeGroups("bar", "foo"), nil, true)
123+
Expect(tasks.Describe()).To(Equal(`3 sequential tasks: { create cluster control plane "test-cluster", set AssignIpv6AddressOnCreation to true for public subnets, 2 parallel sub-tasks: { create nodegroup "bar", create nodegroup "foo" } }`))
139124
})
140125
})
141126
})
@@ -175,3 +160,23 @@ var _ = Describe("StackCollection Tasks", func() {
175160
})
176161
})
177162
})
163+
164+
func makeNodeGroups(names ...string) []*api.NodeGroup {
165+
var nodeGroups []*api.NodeGroup
166+
for _, name := range names {
167+
ng := api.NewNodeGroup()
168+
ng.Name = name
169+
nodeGroups = append(nodeGroups, ng)
170+
}
171+
return nodeGroups
172+
}
173+
174+
func makeManagedNodeGroups(names ...string) []*api.ManagedNodeGroup {
175+
var managedNodeGroups []*api.ManagedNodeGroup
176+
for _, name := range names {
177+
ng := api.NewManagedNodeGroup()
178+
ng.Name = name
179+
managedNodeGroups = append(managedNodeGroups, ng)
180+
}
181+
return managedNodeGroups
182+
}

‎pkg/ctl/cmdutils/configfile.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,12 @@ func NewCreateClusterLoader(cmd *Cmd, ngFilter *filter.NodeGroupFilter, ng *api.
238238
}
239239

240240
if clusterConfig.VPC.NAT == nil {
241-
clusterConfig.VPC.NAT = api.DefaultClusterNAT()
241+
if clusterConfig.VPC.IPFamily == nil || *clusterConfig.VPC.IPFamily != string(api.IPV6Family) {
242+
clusterConfig.VPC.NAT = api.DefaultClusterNAT()
243+
}
242244
}
243245

244-
if !api.IsSetAndNonEmptyString(clusterConfig.VPC.NAT.Gateway) {
246+
if clusterConfig.VPC.NAT != nil && !api.IsSetAndNonEmptyString(clusterConfig.VPC.NAT.Gateway) {
245247
*clusterConfig.VPC.NAT.Gateway = api.ClusterSingleNAT
246248
}
247249

‎pkg/ctl/cmdutils/configfile_test.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,22 @@ var _ = Describe("cmdutils configfile", func() {
200200
}
201201
})
202202

203+
When("using ipv6", func() {
204+
It("should default VPC.NAT to nil", func() {
205+
cmd := &Cmd{
206+
CobraCommand: newCmd(),
207+
ClusterConfigFile: filepath.Join(examplesDir, "29-vpc-with-ip-family.yaml"),
208+
ClusterConfig: api.NewClusterConfig(),
209+
ProviderConfig: api.ProviderConfig{},
210+
}
211+
params := &CreateClusterCmdParams{WithoutNodeGroup: true, CreateManagedNGOptions: CreateManagedNGOptions{
212+
Managed: false,
213+
}}
214+
Expect(NewCreateClusterLoader(cmd, filter.NewNodeGroupFilter(), nil, params).Load()).To(Succeed())
215+
Expect(cmd.ClusterConfig.VPC.NAT).To(BeNil())
216+
})
217+
})
218+
203219
It("loader should handle named and unnamed nodegroups without config file", func() {
204220
unnamedNG := api.NewNodeGroup()
205221

0 commit comments

Comments
 (0)