diff --git a/aws/adapter.go b/aws/adapter.go index 19df8977..7326c337 100644 --- a/aws/adapter.go +++ b/aws/adapter.go @@ -3,6 +3,7 @@ package aws import ( "errors" "fmt" + "sort" "strings" "time" @@ -915,47 +916,42 @@ func (a *Adapter) FindLBSubnets(scheme string) []string { internal = true } - subnetsByAZ := make(map[string]*subnetDetails) + subnetsByAZ := make(map[string][]*subnetDetails) for _, subnet := range a.manifest.subnets { // ignore private subnet for public LB if !internal && !subnet.public { continue } + subnetsByAZ[subnet.availabilityZone] = append(subnetsByAZ[subnet.availabilityZone], subnet) + } - existing, ok := subnetsByAZ[subnet.availabilityZone] - if !ok { - subnetsByAZ[subnet.availabilityZone] = subnet - continue - } + subnetIDs := make([]string, 0, len(subnetsByAZ)) + for az, subnets := range subnetsByAZ { + if len(subnets) > 1 { + sort.Slice(subnets, func(a, b int) bool { + subnetA, subnetB := subnets[a], subnets[b] - // prefer subnet with an elb role tag - var tagName string - if internal { - tagName = internalELBRoleTagName - } else { - tagName = elbRoleTagName - } + // prefer subnet with an elb role tag + tagName := elbRoleTagName + if internal { + tagName = internalELBRoleTagName + } - _, existingHasTag := existing.tags[tagName] - _, subnetHasTag := subnet.tags[tagName] + _, subnetAHasTag := subnetA.tags[tagName] + _, subnetBHasTag := subnetB.tags[tagName] - if existingHasTag != subnetHasTag { - if subnetHasTag { - subnetsByAZ[subnet.availabilityZone] = subnet - } - continue - } + if subnetAHasTag != subnetBHasTag { + return subnetAHasTag + } - // If we have two subnets for the same AZ we arbitrarily choose - // the one that is first lexicographically. - if strings.Compare(existing.id, subnet.id) > 0 { - subnetsByAZ[subnet.availabilityZone] = subnet + // prefer subnet id that is first lexicographically + return subnetA.id < subnetB.id + }) + + log.Warnf("Found multiple subnets %v for availability zone %s, using %s", subnets, az, subnets[0].id) } - } - subnetIDs := make([]string, 0, len(subnetsByAZ)) - for _, subnet := range subnetsByAZ { - subnetIDs = append(subnetIDs, subnet.id) + subnetIDs = append(subnetIDs, subnets[0].id) } return subnetIDs diff --git a/aws/adapter_test.go b/aws/adapter_test.go index 007ce1ce..c32060ca 100644 --- a/aws/adapter_test.go +++ b/aws/adapter_test.go @@ -704,6 +704,23 @@ func TestFindLBSubnets(tt *testing.T) { scheme: elbv2.LoadBalancerSchemeEnumInternetFacing, expectedSubnets: []string{"1"}, }, + { + name: "should select first lexicographically subnet when two match a single zone (regardless of details order)", + subnets: []*subnetDetails{ + { + availabilityZone: "a", + public: true, + id: "1", + }, + { + availabilityZone: "a", + public: true, + id: "2", + }, + }, + scheme: elbv2.LoadBalancerSchemeEnumInternetFacing, + expectedSubnets: []string{"1"}, + }, { name: "should not use internal subnets for public LB", subnets: []*subnetDetails{ @@ -736,6 +753,34 @@ func TestFindLBSubnets(tt *testing.T) { scheme: elbv2.LoadBalancerSchemeEnumInternetFacing, expectedSubnets: []string{"2"}, }, + { + name: "should prefer tagged subnet selected first lexicographically", + subnets: []*subnetDetails{ + { + availabilityZone: "a", + public: true, + id: "1", + }, + { + availabilityZone: "a", + public: true, + id: "2", + tags: map[string]string{ + elbRoleTagName: "", + }, + }, + { + availabilityZone: "a", + public: true, + id: "3", + tags: map[string]string{ + elbRoleTagName: "", + }, + }, + }, + scheme: elbv2.LoadBalancerSchemeEnumInternetFacing, + expectedSubnets: []string{"2"}, + }, { name: "should prefer tagged subnet (internal)", subnets: []*subnetDetails{