Skip to content

Commit d52bd30

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4504 from nelsen129/subnet-mapping-order
Fix: preserve requested order for subnets when using aws-load-balancer-subnets annotation
2 parents f5e4e8e + defc1c1 commit d52bd30

File tree

2 files changed

+175
-3
lines changed

2 files changed

+175
-3
lines changed

‎pkg/networking/subnet_resolver.go‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ func (r *defaultSubnetsResolver) ResolveViaNameOrIDSlice(ctx context.Context, su
240240
}
241241

242242
// listSubnetsByNameOrIDs lists subnets within vpc matching given ID or name.
243+
// The returned subnets will be in the same order as the input subnetNameOrIDs slice.
243244
func (r *defaultSubnetsResolver) listSubnetsByNameOrIDs(ctx context.Context, subnetNameOrIDs []string) ([]ec2types.Subnet, error) {
244245
var subnetIDs []string
245246
var subnetNames []string
@@ -250,21 +251,57 @@ func (r *defaultSubnetsResolver) listSubnetsByNameOrIDs(ctx context.Context, sub
250251
subnetNames = append(subnetNames, nameOrID)
251252
}
252253
}
253-
var resolvedSubnets []ec2types.Subnet
254+
255+
subnetByID := make(map[string]ec2types.Subnet)
256+
subnetByName := make(map[string]ec2types.Subnet)
257+
254258
if len(subnetIDs) > 0 {
255259
subnets, err := r.listSubnetsByIDs(ctx, subnetIDs)
256260
if err != nil {
257261
return nil, err
258262
}
259-
resolvedSubnets = append(resolvedSubnets, subnets...)
263+
for _, subnet := range subnets {
264+
subnetByID[awssdk.ToString(subnet.SubnetId)] = subnet
265+
}
260266
}
261267
if len(subnetNames) > 0 {
262268
subnets, err := r.listSubnetsByNames(ctx, subnetNames)
263269
if err != nil {
264270
return nil, err
265271
}
266-
resolvedSubnets = append(resolvedSubnets, subnets...)
272+
for _, subnet := range subnets {
273+
// Extract the Name tag value for mapping
274+
var subnetName string
275+
for _, tag := range subnet.Tags {
276+
if awssdk.ToString(tag.Key) == "Name" {
277+
subnetName = awssdk.ToString(tag.Value)
278+
break
279+
}
280+
}
281+
if subnetName != "" {
282+
subnetByName[subnetName] = subnet
283+
}
284+
}
285+
}
286+
287+
// Reconstruct the subnet list in the original requested order
288+
resolvedSubnets := make([]ec2types.Subnet, 0, len(subnetNameOrIDs))
289+
for _, nameOrID := range subnetNameOrIDs {
290+
if strings.HasPrefix(nameOrID, "subnet-") {
291+
if subnet, ok := subnetByID[nameOrID]; ok {
292+
resolvedSubnets = append(resolvedSubnets, subnet)
293+
} else {
294+
return nil, fmt.Errorf("subnet ID not found: %s", nameOrID)
295+
}
296+
} else {
297+
if subnet, ok := subnetByName[nameOrID]; ok {
298+
resolvedSubnets = append(resolvedSubnets, subnet)
299+
} else {
300+
return nil, fmt.Errorf("subnet with Name tag not found: %s", nameOrID)
301+
}
302+
}
267303
}
304+
268305
return resolvedSubnets, nil
269306
}
270307

‎pkg/networking/subnet_resolver_test.go‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2246,13 +2246,25 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) {
22462246
AvailabilityZoneId: awssdk.String("usw2-az1"),
22472247
AvailableIpAddressCount: awssdk.Int32(8),
22482248
VpcId: awssdk.String("vpc-dummy"),
2249+
Tags: []ec2types.Tag{
2250+
{
2251+
Key: awssdk.String("Name"),
2252+
Value: awssdk.String("name-1"),
2253+
},
2254+
},
22492255
},
22502256
{
22512257
SubnetId: awssdk.String("subnet-2"),
22522258
AvailabilityZone: awssdk.String("us-west-2b"),
22532259
AvailabilityZoneId: awssdk.String("usw2-az2"),
22542260
AvailableIpAddressCount: awssdk.Int32(8),
22552261
VpcId: awssdk.String("vpc-dummy"),
2262+
Tags: []ec2types.Tag{
2263+
{
2264+
Key: awssdk.String("Name"),
2265+
Value: awssdk.String("name-2"),
2266+
},
2267+
},
22562268
},
22572269
},
22582270
},
@@ -2292,13 +2304,25 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) {
22922304
AvailabilityZoneId: awssdk.String("usw2-az1"),
22932305
AvailableIpAddressCount: awssdk.Int32(8),
22942306
VpcId: awssdk.String("vpc-dummy"),
2307+
Tags: []ec2types.Tag{
2308+
{
2309+
Key: awssdk.String("Name"),
2310+
Value: awssdk.String("name-1"),
2311+
},
2312+
},
22952313
},
22962314
{
22972315
SubnetId: awssdk.String("subnet-2"),
22982316
AvailabilityZone: awssdk.String("us-west-2b"),
22992317
AvailabilityZoneId: awssdk.String("usw2-az2"),
23002318
AvailableIpAddressCount: awssdk.Int32(8),
23012319
VpcId: awssdk.String("vpc-dummy"),
2320+
Tags: []ec2types.Tag{
2321+
{
2322+
Key: awssdk.String("Name"),
2323+
Value: awssdk.String("name-2"),
2324+
},
2325+
},
23022326
},
23032327
},
23042328
},
@@ -2343,6 +2367,12 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) {
23432367
AvailabilityZoneId: awssdk.String("usw2-az2"),
23442368
AvailableIpAddressCount: awssdk.Int32(8),
23452369
VpcId: awssdk.String("vpc-dummy"),
2370+
Tags: []ec2types.Tag{
2371+
{
2372+
Key: awssdk.String("Name"),
2373+
Value: awssdk.String("name-2"),
2374+
},
2375+
},
23462376
},
23472377
},
23482378
},
@@ -2376,6 +2406,111 @@ func Test_defaultSubnetsResolver_ResolveViaNameOrIDSlice(t *testing.T) {
23762406
},
23772407
},
23782408
want: []ec2types.Subnet{
2409+
{
2410+
SubnetId: awssdk.String("subnet-1"),
2411+
AvailabilityZone: awssdk.String("us-west-2a"),
2412+
AvailabilityZoneId: awssdk.String("usw2-az1"),
2413+
AvailableIpAddressCount: awssdk.Int32(8),
2414+
VpcId: awssdk.String("vpc-dummy"),
2415+
},
2416+
{
2417+
SubnetId: awssdk.String("subnet-2"),
2418+
AvailabilityZone: awssdk.String("us-west-2b"),
2419+
AvailabilityZoneId: awssdk.String("usw2-az2"),
2420+
AvailableIpAddressCount: awssdk.Int32(8),
2421+
VpcId: awssdk.String("vpc-dummy"),
2422+
Tags: []ec2types.Tag{
2423+
{
2424+
Key: awssdk.String("Name"),
2425+
Value: awssdk.String("name-2"),
2426+
},
2427+
},
2428+
},
2429+
},
2430+
},
2431+
{
2432+
name: "order is preserved when AWS returns subnets in different order",
2433+
fields: fields{
2434+
clusterTagCheckEnabled: true,
2435+
albSingleSubnetEnabled: false,
2436+
discoveryByReachabilityEnabled: true,
2437+
describeSubnetsAsListCalls: []describeSubnetsAsListCall{
2438+
{
2439+
input: &ec2sdk.DescribeSubnetsInput{
2440+
SubnetIds: []string{"subnet-3", "subnet-1", "subnet-2"},
2441+
},
2442+
// AWS returns in different order than requested
2443+
output: []ec2types.Subnet{
2444+
{
2445+
SubnetId: awssdk.String("subnet-1"),
2446+
AvailabilityZone: awssdk.String("us-west-2a"),
2447+
AvailabilityZoneId: awssdk.String("usw2-az1"),
2448+
AvailableIpAddressCount: awssdk.Int32(8),
2449+
VpcId: awssdk.String("vpc-dummy"),
2450+
},
2451+
{
2452+
SubnetId: awssdk.String("subnet-2"),
2453+
AvailabilityZone: awssdk.String("us-west-2b"),
2454+
AvailabilityZoneId: awssdk.String("usw2-az2"),
2455+
AvailableIpAddressCount: awssdk.Int32(8),
2456+
VpcId: awssdk.String("vpc-dummy"),
2457+
},
2458+
{
2459+
SubnetId: awssdk.String("subnet-3"),
2460+
AvailabilityZone: awssdk.String("us-west-2c"),
2461+
AvailabilityZoneId: awssdk.String("usw2-az3"),
2462+
AvailableIpAddressCount: awssdk.Int32(8),
2463+
VpcId: awssdk.String("vpc-dummy"),
2464+
},
2465+
},
2466+
},
2467+
},
2468+
fetchAZInfosCalls: []fetchAZInfosCall{
2469+
{
2470+
availabilityZoneIDs: []string{"usw2-az3"},
2471+
azInfoByAZID: map[string]ec2types.AvailabilityZone{
2472+
"usw2-az3": {
2473+
ZoneId: awssdk.String("usw2-az3"),
2474+
ZoneType: awssdk.String("availability-zone"),
2475+
},
2476+
},
2477+
},
2478+
{
2479+
availabilityZoneIDs: []string{"usw2-az1"},
2480+
azInfoByAZID: map[string]ec2types.AvailabilityZone{
2481+
"usw2-az1": {
2482+
ZoneId: awssdk.String("usw2-az1"),
2483+
ZoneType: awssdk.String("availability-zone"),
2484+
},
2485+
},
2486+
},
2487+
{
2488+
availabilityZoneIDs: []string{"usw2-az2"},
2489+
azInfoByAZID: map[string]ec2types.AvailabilityZone{
2490+
"usw2-az2": {
2491+
ZoneId: awssdk.String("usw2-az2"),
2492+
ZoneType: awssdk.String("availability-zone"),
2493+
},
2494+
},
2495+
},
2496+
},
2497+
},
2498+
args: args{
2499+
nameOrIDs: []string{"subnet-3", "subnet-1", "subnet-2"},
2500+
opts: []SubnetsResolveOption{
2501+
WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication),
2502+
WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing),
2503+
},
2504+
},
2505+
// Expected result must be in the requested order, not AWS's order
2506+
want: []ec2types.Subnet{
2507+
{
2508+
SubnetId: awssdk.String("subnet-3"),
2509+
AvailabilityZone: awssdk.String("us-west-2c"),
2510+
AvailabilityZoneId: awssdk.String("usw2-az3"),
2511+
AvailableIpAddressCount: awssdk.Int32(8),
2512+
VpcId: awssdk.String("vpc-dummy"),
2513+
},
23792514
{
23802515
SubnetId: awssdk.String("subnet-1"),
23812516
AvailabilityZone: awssdk.String("us-west-2a"),

0 commit comments

Comments
 (0)