From 41b0d3c8fff9a534ce15ee9791b89e32a387a3c7 Mon Sep 17 00:00:00 2001
From: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Date: Mon, 20 Nov 2023 16:41:22 +0100
Subject: [PATCH] aws: log when multiple subnets found for the same
 availability zone

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
---
 aws/adapter.go      | 57 ++++++++++++++++++++++++---------------------
 aws/adapter_test.go | 17 ++++++++++++++
 2 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/aws/adapter.go b/aws/adapter.go
index 19df8977..fd8a3dd2 100644
--- a/aws/adapter.go
+++ b/aws/adapter.go
@@ -3,6 +3,7 @@ package aws
 import (
 	"errors"
 	"fmt"
+	"sort"
 	"strings"
 	"time"
 
@@ -915,47 +916,49 @@ 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
-		}
+	for _, subnets := range subnetsByAZ {
+		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]
+			_, sunbetBHasTag := subnetB.tags[tagName]
 
-		if existingHasTag != subnetHasTag {
-			if subnetHasTag {
-				subnetsByAZ[subnet.availabilityZone] = subnet
+			if subnetAHasTag != sunbetBHasTag {
+				if subnetAHasTag {
+					return true
+				}
 			}
-			continue
-		}
 
-		// 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
-		}
+			// If we have two subnets for the same AZ we arbitrarily choose
+			// the one that is first lexicographically.
+			if subnetA.id < subnetB.id {
+				return true
+			}
+			return false
+		})
 	}
 
 	subnetIDs := make([]string, 0, len(subnetsByAZ))
-	for _, subnet := range subnetsByAZ {
-		subnetIDs = append(subnetIDs, subnet.id)
+	for az, subnets := range subnetsByAZ {
+		if len(subnets) > 1 {
+			log.Warnf("Found multiple subnets %v for availability zone %s, using %s", subnets, az, subnets[0].id)
+		}
+		subnetIDs = append(subnetIDs, subnets[0].id)
 	}
 
 	return subnetIDs
diff --git a/aws/adapter_test.go b/aws/adapter_test.go
index 007ce1ce..70def4ba 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{