Skip to content

Commit fb86d3f

Browse files
authored
Relax cluster tag requirement on subnets during auto-discovery (#1773)
* relax cluster tag requirement on subnets * handle subnets with multiple cluster tags
1 parent bf40902 commit fb86d3f

File tree

2 files changed

+272
-38
lines changed

2 files changed

+272
-38
lines changed

pkg/networking/subnet_resolver.go

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,7 @@ func (r *defaultSubnetsResolver) ResolveViaDiscovery(ctx context.Context, opts .
112112
case elbv2model.LoadBalancerSchemeInternetFacing:
113113
subnetRoleTagKey = TagKeySubnetPublicELB
114114
}
115-
clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName)
116115
req := &ec2sdk.DescribeSubnetsInput{Filters: []*ec2sdk.Filter{
117-
{
118-
Name: awssdk.String("tag:" + clusterResourceTagKey),
119-
Values: awssdk.StringSlice([]string{"owned", "shared"}),
120-
},
121116
{
122117
Name: awssdk.String("tag:" + subnetRoleTagKey),
123118
Values: awssdk.StringSlice([]string{"", "1"}),
@@ -128,17 +123,31 @@ func (r *defaultSubnetsResolver) ResolveViaDiscovery(ctx context.Context, opts .
128123
},
129124
}}
130125

131-
subnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req)
126+
allSubnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req)
132127
if err != nil {
133128
return nil, err
134129
}
130+
var subnets []*ec2sdk.Subnet
131+
for _, subnet := range allSubnets {
132+
if r.checkSubnetIsNotTaggedForOtherClusters(subnet) {
133+
subnets = append(subnets, subnet)
134+
}
135+
}
135136
subnetsByAZ := mapSDKSubnetsByAZ(subnets)
136137
chosenSubnets := make([]*ec2sdk.Subnet, 0, len(subnetsByAZ))
137138
for az, subnets := range subnetsByAZ {
138139
if len(subnets) == 1 {
139140
chosenSubnets = append(chosenSubnets, subnets[0])
140141
} else if len(subnets) > 1 {
141142
sort.Slice(subnets, func(i, j int) bool {
143+
clusterTagI := r.checkSubnetHasClusterTag(subnets[i])
144+
clusterTagJ := r.checkSubnetHasClusterTag(subnets[j])
145+
if clusterTagI != clusterTagJ {
146+
if clusterTagI {
147+
return true
148+
}
149+
return false
150+
}
142151
return awssdk.StringValue(subnets[i].SubnetId) < awssdk.StringValue(subnets[j].SubnetId)
143152
})
144153
r.logger.Info("multiple subnet in the same AvailabilityZone", "AvailabilityZone", az,
@@ -298,3 +307,38 @@ func sortSubnetsByID(subnets []*ec2sdk.Subnet) {
298307
return awssdk.StringValue(subnets[i].SubnetId) < awssdk.StringValue(subnets[j].SubnetId)
299308
})
300309
}
310+
311+
// checkSubnetHasClusterTag checks if the subnet is tagged for the current cluster
312+
func (r *defaultSubnetsResolver) checkSubnetHasClusterTag(subnet *ec2sdk.Subnet) bool {
313+
clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName)
314+
for _, tag := range subnet.Tags {
315+
if clusterResourceTagKey == awssdk.StringValue(tag.Key) {
316+
return true
317+
}
318+
}
319+
return false
320+
}
321+
322+
// checkSubnetIsNotTaggedForOtherClusters checks whether the subnet is tagged for the current cluster
323+
// or it doesn't contain the cluster tag at all. If the subnet contains a tag for other clusters, then
324+
// this check returns false so that the subnet does not used for the load balancer.
325+
func (r *defaultSubnetsResolver) checkSubnetIsNotTaggedForOtherClusters(subnet *ec2sdk.Subnet) bool {
326+
clusterResourceTagPrefix := "kubernetes.io/cluster"
327+
clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName)
328+
hasClusterResourceTagPrefix := false
329+
for _, tag := range subnet.Tags {
330+
tagKey := awssdk.StringValue(tag.Key)
331+
if tagKey == clusterResourceTagKey {
332+
return true
333+
}
334+
if strings.HasPrefix(tagKey, clusterResourceTagPrefix) {
335+
// If the cluster tag is for a different cluster, keep track of it and exclude
336+
// the subnet if no matching tag found for the current cluster.
337+
hasClusterResourceTagPrefix = true
338+
}
339+
}
340+
if hasClusterResourceTagPrefix {
341+
return false
342+
}
343+
return true
344+
}

0 commit comments

Comments
 (0)