diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 13db9301a5b..c782c91832b 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -2467,8 +2467,15 @@ func (c *Cloud) findELBSubnets(internalELB bool) ([]string, error) { continue } - // TODO: Should this be an error? - glog.Warningf("Found multiple subnets in AZ %q; making arbitrary choice between subnets %q and %q", az, *existing.SubnetId, *subnet.SubnetId) + // If we have two subnets for the same AZ we arbitrarily choose the one that is first lexicographically. + // TODO: Should this be an error. + if strings.Compare(*existing.SubnetId, *subnet.SubnetId) > 0 { + glog.Warningf("Found multiple subnets in AZ %q; choosing %q between subnets %q and %q", az, *subnet.SubnetId, *existing.SubnetId, *subnet.SubnetId) + subnetsByAZ[az] = subnet + continue + } + + glog.Warningf("Found multiple subnets in AZ %q; choosing %q between subnets %q and %q", az, *existing.SubnetId, *existing.SubnetId, *subnet.SubnetId) continue } diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index b1302510e63..ee2712b5205 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -792,12 +792,18 @@ func TestSubnetIDsinVPC(t *testing.T) { } } - // test with 4 subnets from 3 different AZs - // add duplicate az subnet + // Test with 5 subnets from 3 different AZs. + // Add 2 duplicate AZ subnets lexicographically chosen one is the middle element in array to + // check that we both choose the correct entry when it comes after and before another element + // in the same AZ. subnets[3] = make(map[string]string) - subnets[3]["id"] = "subnet-c0000002" + subnets[3]["id"] = "subnet-c0000000" subnets[3]["az"] = "af-south-1c" + subnets[4] = make(map[string]string) + subnets[4]["id"] = "subnet-c0000002" + subnets[4]["az"] = "af-south-1c" awsServices.ec2.Subnets = constructSubnets(subnets) + routeTables["subnet-c0000000"] = true routeTables["subnet-c0000002"] = true awsServices.ec2.RouteTables = constructRouteTables(routeTables) @@ -812,6 +818,16 @@ func TestSubnetIDsinVPC(t *testing.T) { return } + expected := []*string{aws.String("subnet-a0000001"), aws.String("subnet-b0000001"), aws.String("subnet-c0000000")} + for _, s := range result { + if !contains(expected, s) { + t.Errorf("Unexpected subnet '%s' found", s) + return + } + } + + delete(routeTables, "subnet-c0000002") + // test with 6 subnets from 3 different AZs // with 3 private subnets subnets[4] = make(map[string]string) @@ -825,7 +841,7 @@ func TestSubnetIDsinVPC(t *testing.T) { routeTables["subnet-a0000001"] = false routeTables["subnet-b0000001"] = false routeTables["subnet-c0000001"] = false - routeTables["subnet-c0000002"] = true + routeTables["subnet-c0000000"] = true routeTables["subnet-d0000001"] = true routeTables["subnet-d0000002"] = true awsServices.ec2.RouteTables = constructRouteTables(routeTables) @@ -840,7 +856,7 @@ func TestSubnetIDsinVPC(t *testing.T) { return } - expected := []*string{aws.String("subnet-c0000002"), aws.String("subnet-d0000001"), aws.String("subnet-d0000002")} + expected = []*string{aws.String("subnet-c0000000"), aws.String("subnet-d0000001"), aws.String("subnet-d0000002")} for _, s := range result { if !contains(expected, s) { t.Errorf("Unexpected subnet '%s' found", s)