From c9ec8ab7936f669552c56c1fbdf44242d66c4aed Mon Sep 17 00:00:00 2001 From: Yang Yang Date: Sun, 30 Mar 2025 21:45:47 -0700 Subject: [PATCH] fix bug in subnet resolver --- pkg/networking/subnet_resolver.go | 20 ++-- pkg/networking/subnet_resolver_test.go | 154 +++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 10 deletions(-) diff --git a/pkg/networking/subnet_resolver.go b/pkg/networking/subnet_resolver.go index 3ea34846f0..c755846647 100644 --- a/pkg/networking/subnet_resolver.go +++ b/pkg/networking/subnet_resolver.go @@ -458,20 +458,20 @@ func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet) [ for az, azSubnets := range subnetsByAZ { if len(azSubnets) == 1 { chosenSubnets = append(chosenSubnets, azSubnets[0]) - } else if len(subnets) > 1 { - sort.Slice(subnets, func(i, j int) bool { - subnetIHasCurrentClusterTag := r.isSubnetContainsCurrentClusterTag(subnets[i]) - subnetJHasCurrentClusterTag := r.isSubnetContainsCurrentClusterTag(subnets[j]) + } else if len(azSubnets) > 1 { + sort.Slice(azSubnets, func(i, j int) bool { + subnetIHasCurrentClusterTag := r.isSubnetContainsCurrentClusterTag(azSubnets[i]) + subnetJHasCurrentClusterTag := r.isSubnetContainsCurrentClusterTag(azSubnets[j]) if subnetIHasCurrentClusterTag && (!subnetJHasCurrentClusterTag) { return true } else if (!subnetIHasCurrentClusterTag) && subnetJHasCurrentClusterTag { return false } - return awssdk.ToString(subnets[i].SubnetId) < awssdk.ToString(subnets[j].SubnetId) + return awssdk.ToString(azSubnets[i].SubnetId) < awssdk.ToString(azSubnets[j].SubnetId) }) r.logger.V(1).Info("multiple subnets in the same AvailabilityZone", "AvailabilityZone", az, - "chosen", subnets[0].SubnetId, "ignored", extractSubnetIDs(subnets[1:])) - chosenSubnets = append(chosenSubnets, subnets[0]) + "chosen", azSubnets[0].SubnetId, "ignored", extractSubnetIDs(azSubnets[1:])) + chosenSubnets = append(chosenSubnets, azSubnets[0]) } } sortSubnetsByID(chosenSubnets) @@ -528,9 +528,9 @@ func (r *defaultSubnetsResolver) isSubnetContainsSufficientIPAddresses(subnet ec // subnets passed-in must be non-empty func (r *defaultSubnetsResolver) validateSubnetsAZExclusivity(subnets []ec2types.Subnet) error { subnetsByAZ := mapSDKSubnetsByAZ(subnets) - for az, subnets := range subnetsByAZ { - if len(subnets) > 1 { - return fmt.Errorf("multiple subnets in same Availability Zone %v: %v", az, extractSubnetIDs(subnets)) + for az, azSubnets := range subnetsByAZ { + if len(azSubnets) > 1 { + return fmt.Errorf("multiple subnets in same Availability Zone %v: %v", az, extractSubnetIDs(azSubnets)) } } return nil diff --git a/pkg/networking/subnet_resolver_test.go b/pkg/networking/subnet_resolver_test.go index 1f30ab5b4d..ff3cc3177e 100644 --- a/pkg/networking/subnet_resolver_test.go +++ b/pkg/networking/subnet_resolver_test.go @@ -1024,6 +1024,146 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) { }, wantErr: errors.New("unable to resolve at least one subnet. Evaluated 2 subnets: 1 are tagged for other clusters, and 1 have insufficient available IP addresses"), }, + { + name: "multiple subnets found per AZ, pick one per az", + fields: fields{ + clusterTagCheckEnabled: true, + albSingleSubnetEnabled: false, + discoveryByReachabilityEnabled: true, + describeSubnetsAsListCalls: []describeSubnetsAsListCall{ + { + input: &ec2sdk.DescribeSubnetsInput{ + Filters: []ec2types.Filter{ + { + Name: awssdk.String("vpc-id"), + Values: []string{"vpc-dummy"}, + }, + { + Name: awssdk.String("tag:kubernetes.io/role/elb"), + Values: []string{"", "1"}, + }, + }, + }, + output: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-1"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/cluster-dummy"), + Value: awssdk.String("owned"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-4"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, + }, + }, + }, + }, + fetchAZInfosCalls: []fetchAZInfosCall{ + { + availabilityZoneIDs: []string{"usw2-az1"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az1": { + ZoneId: awssdk.String("usw2-az1"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, + { + availabilityZoneIDs: []string{"usw2-az2"}, + azInfoByAZID: map[string]ec2types.AvailabilityZone{ + "usw2-az2": { + ZoneId: awssdk.String("usw2-az2"), + ZoneType: awssdk.String("availability-zone"), + }, + }, + }, + }, + }, + args: args{ + opts: []SubnetsResolveOption{ + WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication), + WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing), + }, + }, + want: []ec2types.Subnet{ + { + SubnetId: awssdk.String("subnet-2"), + AvailabilityZone: awssdk.String("us-west-2a"), + AvailabilityZoneId: awssdk.String("usw2-az1"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + { + Key: awssdk.String("kubernetes.io/cluster/cluster-dummy"), + Value: awssdk.String("owned"), + }, + }, + }, + { + SubnetId: awssdk.String("subnet-3"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + Tags: []ec2types.Tag{ + { + Key: awssdk.String("kubernetes.io/role/elb"), + Value: awssdk.String("1"), + }, + }, + }, + }, + }, { name: "fallback to reachability were disabled", fields: fields{ @@ -2353,6 +2493,13 @@ func Test_defaultSubnetsResolver_chooseSubnetsPerAZ(t *testing.T) { AvailableIpAddressCount: awssdk.Int32(8), VpcId: awssdk.String("vpc-dummy"), }, + { + SubnetId: awssdk.String("subnet-4"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, }, want: []ec2types.Subnet{ { @@ -2401,6 +2548,13 @@ func Test_defaultSubnetsResolver_chooseSubnetsPerAZ(t *testing.T) { AvailableIpAddressCount: awssdk.Int32(8), VpcId: awssdk.String("vpc-dummy"), }, + { + SubnetId: awssdk.String("subnet-4"), + AvailabilityZone: awssdk.String("us-west-2b"), + AvailabilityZoneId: awssdk.String("usw2-az2"), + AvailableIpAddressCount: awssdk.Int32(8), + VpcId: awssdk.String("vpc-dummy"), + }, }, want: []ec2types.Subnet{ {