Skip to content

Commit e2dffa3

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #5321 from k8s-infra-cherrypick-robot/cherry-pick-5175-to-release-2.6
[release-2.6] 🐛: Tags defined in subnet spec should be applied
2 parents 6170acc + f66b66f commit e2dffa3

File tree

2 files changed

+185
-68
lines changed

2 files changed

+185
-68
lines changed

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

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,6 @@ func (s *Service) reconcileSubnets() error {
5858
existing infrav1.Subnets
5959
)
6060

61-
// Describing the VPC Subnets tags the resources.
62-
if s.scope.TagUnmanagedNetworkResources() {
63-
// Describe subnets in the vpc.
64-
if existing, err = s.describeVpcSubnets(); err != nil {
65-
return err
66-
}
67-
}
68-
6961
unmanagedVPC := s.scope.VPC().IsUnmanaged(s.scope.Name())
7062

7163
if len(subnets) == 0 {
@@ -93,12 +85,9 @@ func (s *Service) reconcileSubnets() error {
9385
}
9486
}
9587

96-
// Describing the VPC Subnets tags the resources.
97-
if !s.scope.TagUnmanagedNetworkResources() {
98-
// Describe subnets in the vpc.
99-
if existing, err = s.describeVpcSubnets(); err != nil {
100-
return err
101-
}
88+
// Describe subnets in the vpc.
89+
if existing, err = s.describeVpcSubnets(); err != nil {
90+
return err
10291
}
10392

10493
if s.scope.SecondaryCidrBlock() != nil {
@@ -139,11 +128,12 @@ func (s *Service) reconcileSubnets() error {
139128
existingSubnet.ID = sub.ID
140129
}
141130

131+
// Make sure tags are up-to-date.
132+
subnetTags := sub.Tags
133+
142134
// Update subnet spec with the existing subnet details
143135
existingSubnet.DeepCopyInto(sub)
144136

145-
// Make sure tags are up-to-date.
146-
subnetTags := sub.Tags
147137
if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) {
148138
buildParams := s.getSubnetTagParams(unmanagedVPC, existingSubnet.GetResourceID(), existingSubnet.IsPublic, existingSubnet.AvailabilityZone, subnetTags, existingSubnet.IsEdge())
149139
tagsBuilder := tags.New(&buildParams, tags.WithEC2(s.EC2Client))

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

Lines changed: 179 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,7 @@ func TestReconcileSubnets(t *testing.T) {
6363
{ID: "subnet-private-us-east-1-wl1-nyc-wlz-1", AvailabilityZone: "us-east-1-wl1-nyc-wlz-1", CidrBlock: "10.0.7.0/24", IsPublic: false},
6464
{ID: "subnet-public-us-east-1-wl1-nyc-wlz-1", AvailabilityZone: "us-east-1-wl1-nyc-wlz-1", CidrBlock: "10.0.8.0/24", IsPublic: true},
6565
}
66-
// TODO(mtulio): replace by slices.Concat(...) on go 1.22+
67-
stubSubnetsAllZones := stubSubnetsAvailabilityZone
68-
stubSubnetsAllZones = append(stubSubnetsAllZones, stubSubnetsLocalZone...)
66+
stubSubnetsAllZones := append(stubSubnetsAvailabilityZone, stubSubnetsLocalZone...)
6967
stubSubnetsAllZones = append(stubSubnetsAllZones, stubSubnetsWavelengthZone...)
7068

7169
// NetworkSpec with subnets in zone type availability-zone
@@ -1057,55 +1055,7 @@ func TestReconcileSubnets(t *testing.T) {
10571055
},
10581056
Subnets: []infrav1.SubnetSpec{},
10591057
}).WithTagUnmanagedNetworkResources(true),
1060-
expect: func(m *mocks.MockEC2APIMockRecorder) {
1061-
m.DescribeSubnetsWithContext(context.TODO(), gomock.Eq(&ec2.DescribeSubnetsInput{
1062-
Filters: []*ec2.Filter{
1063-
{
1064-
Name: aws.String("state"),
1065-
Values: []*string{aws.String("pending"), aws.String("available")},
1066-
},
1067-
{
1068-
Name: aws.String("vpc-id"),
1069-
Values: []*string{aws.String(subnetsVPCID)},
1070-
},
1071-
},
1072-
})).
1073-
Return(&ec2.DescribeSubnetsOutput{
1074-
Subnets: []*ec2.Subnet{
1075-
{
1076-
VpcId: aws.String(subnetsVPCID),
1077-
SubnetId: aws.String("subnet-1"),
1078-
AvailabilityZone: aws.String("us-east-1a"),
1079-
CidrBlock: aws.String("10.0.10.0/24"),
1080-
MapPublicIpOnLaunch: aws.Bool(false),
1081-
},
1082-
{
1083-
VpcId: aws.String(subnetsVPCID),
1084-
SubnetId: aws.String("subnet-2"),
1085-
AvailabilityZone: aws.String("us-east-1a"),
1086-
CidrBlock: aws.String("10.0.20.0/24"),
1087-
MapPublicIpOnLaunch: aws.Bool(false),
1088-
},
1089-
},
1090-
}, nil)
1091-
m.DescribeRouteTablesWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.DescribeRouteTablesInput{})).
1092-
Return(&ec2.DescribeRouteTablesOutput{}, nil)
1093-
1094-
m.DescribeNatGatewaysPagesWithContext(context.TODO(),
1095-
gomock.Eq(&ec2.DescribeNatGatewaysInput{
1096-
Filter: []*ec2.Filter{
1097-
{
1098-
Name: aws.String("vpc-id"),
1099-
Values: []*string{aws.String(subnetsVPCID)},
1100-
},
1101-
{
1102-
Name: aws.String("state"),
1103-
Values: []*string{aws.String("pending"), aws.String("available")},
1104-
},
1105-
},
1106-
}),
1107-
gomock.Any()).Return(nil)
1108-
},
1058+
expect: func(m *mocks.MockEC2APIMockRecorder) {},
11091059
errorExpected: true,
11101060
tagUnmanagedNetworkResources: true,
11111061
},
@@ -2625,6 +2575,183 @@ func TestReconcileSubnets(t *testing.T) {
26252575
}, nil).AnyTimes()
26262576
},
26272577
},
2578+
{
2579+
name: "Managed VPC, existing public and private subnets, 2 subnets in spec, custom tags in spec should be created",
2580+
input: NewClusterScope().WithNetwork(&infrav1.NetworkSpec{
2581+
VPC: infrav1.VPCSpec{
2582+
ID: subnetsVPCID,
2583+
Tags: infrav1.Tags{
2584+
infrav1.ClusterTagKey("test-cluster"): "owned",
2585+
},
2586+
},
2587+
Subnets: []infrav1.SubnetSpec{
2588+
{
2589+
ID: "subnet-1",
2590+
AvailabilityZone: "us-east-1a",
2591+
IsPublic: true,
2592+
Tags: map[string]string{"this-tag-is-in-the-spec": "but-its-not-on-aws"},
2593+
},
2594+
{
2595+
ID: "subnet-2",
2596+
AvailabilityZone: "us-east-1a",
2597+
IsPublic: false,
2598+
Tags: map[string]string{"subnet-2-this-tag-is-in-the-spec": "subnet-2-but-its-not-on-aws"},
2599+
},
2600+
},
2601+
}),
2602+
expect: func(m *mocks.MockEC2APIMockRecorder) {
2603+
tagsOnSubnet1 := []*ec2.Tag{
2604+
{
2605+
Key: aws.String("Name"),
2606+
Value: aws.String("test-cluster-subnet-public"),
2607+
},
2608+
{
2609+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
2610+
Value: aws.String("owned"),
2611+
},
2612+
{
2613+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
2614+
Value: aws.String("public"),
2615+
},
2616+
}
2617+
tagsOnSubnet2 := []*ec2.Tag{
2618+
{
2619+
Key: aws.String("Name"),
2620+
Value: aws.String("test-cluster-subnet-private"),
2621+
},
2622+
{
2623+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
2624+
Value: aws.String("owned"),
2625+
},
2626+
{
2627+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
2628+
Value: aws.String("private"),
2629+
},
2630+
}
2631+
m.DescribeSubnetsWithContext(context.TODO(), gomock.Eq(&ec2.DescribeSubnetsInput{
2632+
Filters: []*ec2.Filter{
2633+
{
2634+
Name: aws.String("state"),
2635+
Values: []*string{aws.String("pending"), aws.String("available")},
2636+
},
2637+
{
2638+
Name: aws.String("vpc-id"),
2639+
Values: []*string{aws.String(subnetsVPCID)},
2640+
},
2641+
},
2642+
})).
2643+
Return(&ec2.DescribeSubnetsOutput{
2644+
Subnets: []*ec2.Subnet{
2645+
{
2646+
VpcId: aws.String(subnetsVPCID),
2647+
SubnetId: aws.String("subnet-1"),
2648+
AvailabilityZone: aws.String("us-east-1a"),
2649+
CidrBlock: aws.String("10.0.0.0/17"),
2650+
Tags: tagsOnSubnet1,
2651+
},
2652+
{
2653+
VpcId: aws.String(subnetsVPCID),
2654+
SubnetId: aws.String("subnet-2"),
2655+
AvailabilityZone: aws.String("us-east-1a"),
2656+
CidrBlock: aws.String("10.0.128.0/17"),
2657+
Tags: tagsOnSubnet2,
2658+
},
2659+
},
2660+
}, nil)
2661+
2662+
m.DescribeRouteTablesWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.DescribeRouteTablesInput{})).
2663+
Return(&ec2.DescribeRouteTablesOutput{}, nil)
2664+
2665+
m.DescribeNatGatewaysPagesWithContext(context.TODO(),
2666+
gomock.Eq(&ec2.DescribeNatGatewaysInput{
2667+
Filter: []*ec2.Filter{
2668+
{
2669+
Name: aws.String("vpc-id"),
2670+
Values: []*string{aws.String(subnetsVPCID)},
2671+
},
2672+
{
2673+
Name: aws.String("state"),
2674+
Values: []*string{aws.String("pending"), aws.String("available")},
2675+
},
2676+
},
2677+
}),
2678+
gomock.Any()).Return(nil)
2679+
2680+
// Public subnet
2681+
expectedAppliedAwsTagsForSubnet1 := []*ec2.Tag{
2682+
{
2683+
Key: aws.String("Name"),
2684+
Value: aws.String("test-cluster-subnet-public-us-east-1a"),
2685+
},
2686+
{
2687+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
2688+
Value: aws.String("owned"),
2689+
},
2690+
{
2691+
Key: aws.String("kubernetes.io/role/elb"),
2692+
Value: aws.String("1"),
2693+
},
2694+
{
2695+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
2696+
Value: aws.String("owned"),
2697+
},
2698+
{
2699+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
2700+
Value: aws.String("public"),
2701+
},
2702+
{
2703+
Key: aws.String("this-tag-is-in-the-spec"),
2704+
Value: aws.String("but-its-not-on-aws"),
2705+
}}
2706+
m.CreateTagsWithContext(context.TODO(), gomock.Eq(&ec2.CreateTagsInput{
2707+
Resources: aws.StringSlice([]string{"subnet-1"}),
2708+
Tags: expectedAppliedAwsTagsForSubnet1,
2709+
})).
2710+
Return(nil, nil)
2711+
2712+
// Private subnet
2713+
expectedAppliedAwsTagsForSubnet2 := []*ec2.Tag{
2714+
{
2715+
Key: aws.String("Name"),
2716+
Value: aws.String("test-cluster-subnet-private-us-east-1a"),
2717+
},
2718+
{
2719+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
2720+
Value: aws.String("owned"),
2721+
},
2722+
{
2723+
Key: aws.String("kubernetes.io/role/internal-elb"),
2724+
Value: aws.String("1"),
2725+
},
2726+
{
2727+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
2728+
Value: aws.String("owned"),
2729+
},
2730+
{
2731+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
2732+
Value: aws.String("private"),
2733+
},
2734+
{
2735+
Key: aws.String("subnet-2-this-tag-is-in-the-spec"),
2736+
Value: aws.String("subnet-2-but-its-not-on-aws"),
2737+
}}
2738+
m.CreateTagsWithContext(context.TODO(), gomock.Eq(&ec2.CreateTagsInput{
2739+
Resources: aws.StringSlice([]string{"subnet-2"}),
2740+
Tags: expectedAppliedAwsTagsForSubnet2,
2741+
})).
2742+
Return(nil, nil)
2743+
2744+
m.DescribeAvailabilityZonesWithContext(context.TODO(), gomock.Any()).
2745+
Return(&ec2.DescribeAvailabilityZonesOutput{
2746+
AvailabilityZones: []*ec2.AvailabilityZone{
2747+
{
2748+
ZoneName: aws.String("us-east-1a"),
2749+
ZoneType: aws.String("availability-zone"),
2750+
},
2751+
},
2752+
}, nil).AnyTimes()
2753+
},
2754+
},
26282755
{
26292756
name: "With ManagedControlPlaneScope, Managed VPC, no existing subnets exist, two az's, expect two private and two public from default, created with tag including eksClusterName not a name of Cluster resource",
26302757
input: NewManagedControlPlaneScope().

0 commit comments

Comments
 (0)