Skip to content

Relax cluster tag requirement on subnets during auto-discovery #1773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 50 additions & 6 deletions pkg/networking/subnet_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,7 @@ func (r *defaultSubnetsResolver) ResolveViaDiscovery(ctx context.Context, opts .
case elbv2model.LoadBalancerSchemeInternetFacing:
subnetRoleTagKey = TagKeySubnetPublicELB
}
clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName)
req := &ec2sdk.DescribeSubnetsInput{Filters: []*ec2sdk.Filter{
{
Name: awssdk.String("tag:" + clusterResourceTagKey),
Values: awssdk.StringSlice([]string{"owned", "shared"}),
},
{
Name: awssdk.String("tag:" + subnetRoleTagKey),
Values: awssdk.StringSlice([]string{"", "1"}),
Expand All @@ -128,17 +123,31 @@ func (r *defaultSubnetsResolver) ResolveViaDiscovery(ctx context.Context, opts .
},
}}

subnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req)
allSubnets, err := r.ec2Client.DescribeSubnetsAsList(ctx, req)
if err != nil {
return nil, err
}
var subnets []*ec2sdk.Subnet
for _, subnet := range allSubnets {
if r.checkSubnetIsNotTaggedForOtherClusters(subnet) {
subnets = append(subnets, subnet)
}
}
subnetsByAZ := mapSDKSubnetsByAZ(subnets)
chosenSubnets := make([]*ec2sdk.Subnet, 0, len(subnetsByAZ))
for az, subnets := range subnetsByAZ {
if len(subnets) == 1 {
chosenSubnets = append(chosenSubnets, subnets[0])
} else if len(subnets) > 1 {
sort.Slice(subnets, func(i, j int) bool {
clusterTagI := r.checkSubnetHasClusterTag(subnets[i])
clusterTagJ := r.checkSubnetHasClusterTag(subnets[j])
if clusterTagI != clusterTagJ {
if clusterTagI {
return true
}
return false
}
return awssdk.StringValue(subnets[i].SubnetId) < awssdk.StringValue(subnets[j].SubnetId)
})
r.logger.Info("multiple subnet in the same AvailabilityZone", "AvailabilityZone", az,
Expand Down Expand Up @@ -298,3 +307,38 @@ func sortSubnetsByID(subnets []*ec2sdk.Subnet) {
return awssdk.StringValue(subnets[i].SubnetId) < awssdk.StringValue(subnets[j].SubnetId)
})
}

// checkSubnetHasClusterTag checks if the subnet is tagged for the current cluster
func (r *defaultSubnetsResolver) checkSubnetHasClusterTag(subnet *ec2sdk.Subnet) bool {
clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName)
for _, tag := range subnet.Tags {
if clusterResourceTagKey == awssdk.StringValue(tag.Key) {
return true
}
}
return false
}

// checkSubnetIsNotTaggedForOtherClusters checks whether the subnet is tagged for the current cluster
// or it doesn't contain the cluster tag at all. If the subnet contains a tag for other clusters, then
// this check returns false so that the subnet does not used for the load balancer.
func (r *defaultSubnetsResolver) checkSubnetIsNotTaggedForOtherClusters(subnet *ec2sdk.Subnet) bool {
clusterResourceTagPrefix := "kubernetes.io/cluster"
clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName)
hasClusterResourceTagPrefix := false
for _, tag := range subnet.Tags {
tagKey := awssdk.StringValue(tag.Key)
if tagKey == clusterResourceTagKey {
return true
}
if strings.HasPrefix(tagKey, clusterResourceTagPrefix) {
// If the cluster tag is for a different cluster, keep track of it and exclude
// the subnet if no matching tag found for the current cluster.
hasClusterResourceTagPrefix = true
}
}
if hasClusterResourceTagPrefix {
return false
}
return true
}
Loading