Skip to content

Commit 702324d

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4341 from Jacobious52/main
check TagUmanagedNetworkResources feature gate before tagging subnets for LBs
2 parents a4000d7 + 9220681 commit 702324d

File tree

3 files changed

+112
-21
lines changed

3 files changed

+112
-21
lines changed

‎controllers/helpers_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,8 @@ func getClusterScope(awsCluster infrav1.AWSCluster) (*scope.ClusterScope, error)
174174
Name: "test-cluster",
175175
},
176176
},
177-
AWSCluster: &awsCluster,
177+
AWSCluster: &awsCluster,
178+
TagUnmanagedNetworkResources: true,
178179
},
179180
)
180181
}

‎pkg/cloud/services/network/subnets.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -508,20 +508,20 @@ func (s *Service) getSubnetTagParams(unmanagedVPC bool, id string, public bool,
508508
var role string
509509
additionalTags := make(map[string]string)
510510

511-
if !unmanagedVPC {
511+
if !unmanagedVPC || s.scope.TagUnmanagedNetworkResources() {
512512
additionalTags = s.scope.AdditionalTags()
513-
}
514513

515-
if public {
516-
role = infrav1.PublicRoleTagValue
517-
additionalTags[externalLoadBalancerTag] = "1"
518-
} else {
519-
role = infrav1.PrivateRoleTagValue
520-
additionalTags[internalLoadBalancerTag] = "1"
521-
}
514+
if public {
515+
role = infrav1.PublicRoleTagValue
516+
additionalTags[externalLoadBalancerTag] = "1"
517+
} else {
518+
role = infrav1.PrivateRoleTagValue
519+
additionalTags[internalLoadBalancerTag] = "1"
520+
}
522521

523-
// Add tag needed for Service type=LoadBalancer
524-
additionalTags[infrav1.ClusterAWSCloudProviderTagKey(s.scope.KubernetesClusterName())] = string(infrav1.ResourceLifecycleShared)
522+
// Add tag needed for Service type=LoadBalancer
523+
additionalTags[infrav1.ClusterAWSCloudProviderTagKey(s.scope.KubernetesClusterName())] = string(infrav1.ResourceLifecycleShared)
524+
}
525525

526526
if !unmanagedVPC {
527527
for k, v := range manualTags {

‎pkg/cloud/services/network/subnets_test.go

Lines changed: 99 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,90 @@ func TestReconcileSubnets(t *testing.T) {
4848
errorExpected bool
4949
tagUnmanagedNetworkResources bool
5050
}{
51+
{
52+
name: "Unmanaged VPC, disable TagUnmanagedNetworkResources, 2 existing subnets in vpc, 2 subnet in spec, subnets match, with routes, should succeed",
53+
input: NewClusterScope().WithNetwork(&infrav1.NetworkSpec{
54+
VPC: infrav1.VPCSpec{
55+
ID: subnetsVPCID,
56+
},
57+
Subnets: []infrav1.SubnetSpec{
58+
{
59+
ID: "subnet-1",
60+
},
61+
{
62+
ID: "subnet-2",
63+
},
64+
},
65+
}).WithTagUnmanagedNetworkResources(false),
66+
expect: func(m *mocks.MockEC2APIMockRecorder) {
67+
m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{
68+
Filters: []*ec2.Filter{
69+
{
70+
Name: aws.String("state"),
71+
Values: []*string{aws.String("pending"), aws.String("available")},
72+
},
73+
{
74+
Name: aws.String("vpc-id"),
75+
Values: []*string{aws.String(subnetsVPCID)},
76+
},
77+
},
78+
})).
79+
Return(&ec2.DescribeSubnetsOutput{
80+
Subnets: []*ec2.Subnet{
81+
{
82+
VpcId: aws.String(subnetsVPCID),
83+
SubnetId: aws.String("subnet-1"),
84+
AvailabilityZone: aws.String("us-east-1a"),
85+
CidrBlock: aws.String("10.0.10.0/24"),
86+
MapPublicIpOnLaunch: aws.Bool(false),
87+
},
88+
{
89+
VpcId: aws.String(subnetsVPCID),
90+
SubnetId: aws.String("subnet-2"),
91+
AvailabilityZone: aws.String("us-east-1a"),
92+
CidrBlock: aws.String("10.0.20.0/24"),
93+
MapPublicIpOnLaunch: aws.Bool(false),
94+
},
95+
},
96+
}, nil)
97+
98+
m.DescribeRouteTables(gomock.AssignableToTypeOf(&ec2.DescribeRouteTablesInput{})).
99+
Return(&ec2.DescribeRouteTablesOutput{
100+
RouteTables: []*ec2.RouteTable{
101+
{
102+
VpcId: aws.String(subnetsVPCID),
103+
Associations: []*ec2.RouteTableAssociation{
104+
{
105+
SubnetId: aws.String("subnet-1"),
106+
RouteTableId: aws.String("rt-12345"),
107+
},
108+
},
109+
Routes: []*ec2.Route{
110+
{
111+
GatewayId: aws.String("igw-12345"),
112+
},
113+
},
114+
},
115+
},
116+
}, nil)
117+
118+
m.DescribeNatGatewaysPages(
119+
gomock.Eq(&ec2.DescribeNatGatewaysInput{
120+
Filter: []*ec2.Filter{
121+
{
122+
Name: aws.String("vpc-id"),
123+
Values: []*string{aws.String(subnetsVPCID)},
124+
},
125+
{
126+
Name: aws.String("state"),
127+
Values: []*string{aws.String("pending"), aws.String("available")},
128+
},
129+
},
130+
}),
131+
gomock.Any()).Return(nil)
132+
},
133+
tagUnmanagedNetworkResources: false,
134+
},
51135
{
52136
name: "Unmanaged VPC, 2 existing subnets in vpc, 2 subnet in spec, subnets match, with routes, should succeed",
53137
input: NewClusterScope().WithNetwork(&infrav1.NetworkSpec{
@@ -62,7 +146,7 @@ func TestReconcileSubnets(t *testing.T) {
62146
ID: "subnet-2",
63147
},
64148
},
65-
}),
149+
}).WithTagUnmanagedNetworkResources(true),
66150
expect: func(m *mocks.MockEC2APIMockRecorder) {
67151
m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{
68152
Filters: []*ec2.Filter{
@@ -160,6 +244,7 @@ func TestReconcileSubnets(t *testing.T) {
160244
})).
161245
Return(&ec2.CreateTagsOutput{}, nil)
162246
},
247+
tagUnmanagedNetworkResources: true,
163248
},
164249
{
165250
name: "IPv6 enabled vpc with default subnets should succeed",
@@ -179,7 +264,7 @@ func TestReconcileSubnets(t *testing.T) {
179264
IPv6CidrBlock: "2001:db8:1234:1a02::/64",
180265
},
181266
},
182-
}),
267+
}).WithTagUnmanagedNetworkResources(true),
183268
expect: func(m *mocks.MockEC2APIMockRecorder) {
184269
m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{
185270
Filters: []*ec2.Filter{
@@ -297,6 +382,7 @@ func TestReconcileSubnets(t *testing.T) {
297382
})).
298383
Return(&ec2.CreateTagsOutput{}, nil)
299384
},
385+
tagUnmanagedNetworkResources: true,
300386
},
301387
{
302388
name: "Unmanaged VPC, 2 existing subnets in vpc, 2 subnet in spec, subnets match, no routes, should succeed",
@@ -313,7 +399,7 @@ func TestReconcileSubnets(t *testing.T) {
313399
ID: "subnet-2",
314400
},
315401
},
316-
}),
402+
}).WithTagUnmanagedNetworkResources(true),
317403
expect: func(m *mocks.MockEC2APIMockRecorder) {
318404
m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{
319405
Filters: []*ec2.Filter{
@@ -394,7 +480,8 @@ func TestReconcileSubnets(t *testing.T) {
394480
})).
395481
Return(&ec2.CreateTagsOutput{}, nil)
396482
},
397-
errorExpected: false,
483+
errorExpected: false,
484+
tagUnmanagedNetworkResources: true,
398485
},
399486
{
400487
name: "Unmanaged VPC, 2 existing matching subnets, subnet tagging fails, should succeed",
@@ -410,7 +497,7 @@ func TestReconcileSubnets(t *testing.T) {
410497
ID: "subnet-2",
411498
},
412499
},
413-
}),
500+
}).WithTagUnmanagedNetworkResources(true),
414501
expect: func(m *mocks.MockEC2APIMockRecorder) {
415502
m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{
416503
Filters: []*ec2.Filter{
@@ -493,6 +580,7 @@ func TestReconcileSubnets(t *testing.T) {
493580
})).
494581
Return(&ec2.CreateTagsOutput{}, fmt.Errorf("tagging failed"))
495582
},
583+
tagUnmanagedNetworkResources: true,
496584
},
497585
{
498586
name: "Unmanaged VPC, 2 existing subnets in vpc, 0 subnet in spec, should fail",
@@ -573,7 +661,7 @@ func TestReconcileSubnets(t *testing.T) {
573661
IsPublic: true,
574662
},
575663
},
576-
}),
664+
}).WithTagUnmanagedNetworkResources(true),
577665
expect: func(m *mocks.MockEC2APIMockRecorder) {
578666
m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{
579667
Filters: []*ec2.Filter{
@@ -607,7 +695,8 @@ func TestReconcileSubnets(t *testing.T) {
607695
}),
608696
gomock.Any()).Return(nil)
609697
},
610-
errorExpected: true,
698+
errorExpected: true,
699+
tagUnmanagedNetworkResources: true,
611700
},
612701
{
613702
name: "Unmanaged VPC, 2 subnets exist, 2 private subnet in spec, should succeed",
@@ -627,7 +716,7 @@ func TestReconcileSubnets(t *testing.T) {
627716
IsPublic: false,
628717
},
629718
},
630-
}),
719+
}).WithTagUnmanagedNetworkResources(true),
631720
expect: func(m *mocks.MockEC2APIMockRecorder) {
632721
m.DescribeSubnets(gomock.Eq(&ec2.DescribeSubnetsInput{
633722
Filters: []*ec2.Filter{
@@ -708,7 +797,8 @@ func TestReconcileSubnets(t *testing.T) {
708797
})).
709798
Return(&ec2.CreateTagsOutput{}, nil)
710799
},
711-
errorExpected: false,
800+
errorExpected: false,
801+
tagUnmanagedNetworkResources: true,
712802
},
713803
{
714804
name: "Managed VPC, no subnets exist, 1 private and 1 public subnet in spec, create both",

0 commit comments

Comments
 (0)