From e64c559f47fcc6160d424bc1ab6530c0d59d809e Mon Sep 17 00:00:00 2001 From: Simon Westphahl Date: Thu, 17 Apr 2025 14:16:50 +0200 Subject: [PATCH] Allow a list of subnet IDs for AWS label Allow configuring a list of subnet IDs for an AWS provider label instead of just a single one. With this change we can avoid the need to create multiple providers that only differ on the subnet. In addition we allow the AWS fleet service to make better decisions when all possible subnets are included. For AWS fleet requests we will provide launcht template overrides for each subnet. For normal run-instance requests - which only support requests with a single subnet - we will select a random subnet id from the list. Change-Id: Ib6700c54920981271daad944a642741ed6187811 --- tests/fixtures/layouts/aws/spot.yaml | 2 +- tests/unit/test_aws_driver.py | 43 +++++++++++++++++++++------- zuul/driver/aws/awsendpoint.py | 19 ++++++++---- zuul/driver/aws/awsprovider.py | 2 +- 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/tests/fixtures/layouts/aws/spot.yaml b/tests/fixtures/layouts/aws/spot.yaml index 52cb43e1ce..00856941b7 100644 --- a/tests/fixtures/layouts/aws/spot.yaml +++ b/tests/fixtures/layouts/aws/spot.yaml @@ -68,7 +68,7 @@ region: us-east-1 az: us-east-1b security-group-id: testgroup - subnet-id: {subnet_id} + subnet-ids: {subnet_ids} public-ipv6: True userdata: testuserdata iam-instance-profile: diff --git a/tests/unit/test_aws_driver.py b/tests/unit/test_aws_driver.py index 12ac85e5ca..30999a7db0 100644 --- a/tests/unit/test_aws_driver.py +++ b/tests/unit/test_aws_driver.py @@ -15,6 +15,7 @@ import base64 import concurrent.futures import contextlib +import ipaddress import time from unittest import mock import urllib.parse @@ -39,6 +40,12 @@ from tests.unit.test_launcher import ImageMocksFixture from tests.unit.test_cloud_driver import BaseCloudDriverTest +def _make_ipv6_subnets(cidr_block): + network = ipaddress.IPv6Network(cidr_block) + # AWS only supports /64 prefix length + return [str(sn) for sn in network.subnets(new_prefix=64)] + + class TestAwsDriver(BaseCloudDriverTest): config_file = 'zuul-connections-nodepool.conf' cloud_test_image_format = 'raw' @@ -95,6 +102,7 @@ class TestAwsDriver(BaseCloudDriverTest): self.register_image_calls = [] # TEST-NET-3 + self.subnet_ids = [] ipv6 = False if ipv6: # This is currently unused, but if moto gains IPv6 support @@ -104,17 +112,32 @@ class TestAwsDriver(BaseCloudDriverTest): AmazonProvidedIpv6CidrBlock=True) ipv6_cidr = self.vpc['Vpc'][ 'Ipv6CidrBlockAssociationSet'][0]['Ipv6CidrBlock'] - ipv6_cidr = ipv6_cidr.split('/')[0] + '/64' - self.subnet = self.ec2_client.create_subnet( - CidrBlock='203.0.113.128/25', - Ipv6CidrBlock=ipv6_cidr, + ipv6_subnets = _make_ipv6_subnets(ipv6_cidr) + + subnet1 = self.ec2_client.create_subnet( + AvailabilityZone='us-east-1a', + CidrBlock='203.0.113.64/26', + Ipv6CidrBlock=ipv6_subnets[0], VpcId=self.vpc['Vpc']['VpcId']) - self.subnet_id = self.subnet['Subnet']['SubnetId'] + self.subnet_ids.append(subnet1['Subnet']['SubnetId']) + subnet2 = self.ec2_client.create_subnet( + AvailabilityZone='us-east-1b', + CidrBlock='203.0.113.128/26', + Ipv6CidrBlock=ipv6_subnets[1], + VpcId=self.vpc['Vpc']['VpcId']) + self.subnet_ids.append(subnet2['Subnet']['SubnetId']) else: self.vpc = self.ec2_client.create_vpc(CidrBlock='203.0.113.0/24') - self.subnet = self.ec2_client.create_subnet( - CidrBlock='203.0.113.128/25', VpcId=self.vpc['Vpc']['VpcId']) - self.subnet_id = self.subnet['Subnet']['SubnetId'] + subnet1 = self.ec2_client.create_subnet( + AvailabilityZone='us-east-1a', + CidrBlock='203.0.113.64/26', + VpcId=self.vpc['Vpc']['VpcId']) + self.subnet_ids.append(subnet1['Subnet']['SubnetId']) + subnet2 = self.ec2_client.create_subnet( + AvailabilityZone='us-east-1b', + CidrBlock='203.0.113.128/26', + VpcId=self.vpc['Vpc']['VpcId']) + self.subnet_ids.append(subnet2['Subnet']['SubnetId']) self.security_group = self.ec2_client.create_security_group( GroupName='zuul-nodes', VpcId=self.vpc['Vpc']['VpcId'], @@ -201,7 +224,7 @@ class TestAwsDriver(BaseCloudDriverTest): @simple_layout('layouts/aws/spot.yaml', enable_nodepool=True, replace=lambda test: { - 'subnet_id': test.subnet_id, + 'subnet_ids': test.subnet_ids, 'iam_profile_name': test.profile.name, }) @driver_config('aws', node_checks=check_spot_node_attrs) @@ -238,7 +261,7 @@ class TestAwsDriver(BaseCloudDriverTest): self.commitConfigUpdate( 'org/common-config', 'layouts/aws/spot.yaml', replace=lambda test: { - 'subnet_id': test.subnet_id, + 'subnet_ids': test.subnet_ids, 'iam_profile_name': test.profile.name, }) diff --git a/zuul/driver/aws/awsendpoint.py b/zuul/driver/aws/awsendpoint.py index 278ded81fb..829791d574 100644 --- a/zuul/driver/aws/awsendpoint.py +++ b/zuul/driver/aws/awsendpoint.py @@ -23,6 +23,7 @@ import json import logging import math import queue +import random import re import threading import time @@ -1569,13 +1570,20 @@ class AwsProviderEndpoint(BaseProviderEndpoint): }, ], } - if label.subnet_id: - override_dict['SubnetId'] = label.subnet_id if flavor.fleet['allocation-strategy'] in [ 'prioritized', 'capacity-optimized-prioritized']: override_dict['Priority'] = priority priority += 1 - overrides.append(override_dict) + + # Duplicate overrides for each subnet + if label.subnet_ids: + for subnet_id in label.subnet_ids: + overrides.append(dict( + **override_dict, + SubnetId=subnet_id, + )) + else: + overrides.append(override_dict) if flavor.market_type == 'spot': capacity_type_option = { @@ -1678,8 +1686,9 @@ class AwsProviderEndpoint(BaseProviderEndpoint): label.security_group_id ] - if label.subnet_id: - args['NetworkInterfaces'][0]['SubnetId'] = label.subnet_id + if label.subnet_ids: + args['NetworkInterfaces'][0]['SubnetId'] = random.choice( + label.subnet_ids) if flavor.public_ipv6: args['NetworkInterfaces'][0]['Ipv6AddressCount'] = 1 diff --git a/zuul/driver/aws/awsprovider.py b/zuul/driver/aws/awsprovider.py index 5d30e9f11b..222c8b66b9 100644 --- a/zuul/driver/aws/awsprovider.py +++ b/zuul/driver/aws/awsprovider.py @@ -172,7 +172,7 @@ class AwsProviderLabel(BaseProviderLabel): # TODO: aws accepts a list everywhere we use this; should this # be as_list? Optional('security-group-id'): Nullable(str), - Optional('subnet-id'): Nullable(str), + Optional('subnet-ids', default=[]): [str], # TODO: as_list? Optional('iam-instance-profile'): Nullable(aws_iam_schema), })