From 303d24ab8a9f4e337cc76a837082888a976f99be Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 20 Jan 2021 11:15:14 +0000 Subject: [PATCH] Allow to manually define the gateway IP when using subnet pools Now is possible to define a gateway IP when creating a subnet using a subnet pool. The IPAM subnet generator retrieves the available IP ranges in the subnet pool and generates a list of candidate subnets with the prefix lenght defined. If the gateway IP can be allocated in one of those candidate subnets, the IPAM returns a valid IpamSubnet that will be used to create a Neutron subnet. Closes-Bug: #1904436 Change-Id: Ib1d1f591c4d0f59ebff3ddcb3be7b10b0b5e67dc --- neutron/ipam/requests.py | 40 ++++++++++-- neutron/tests/fullstack/resources/client.py | 18 +++++- neutron/tests/fullstack/test_subnet.py | 62 +++++++++++++++++-- .../tests/unit/db/test_db_base_plugin_v2.py | 3 +- neutron/tests/unit/ipam/test_requests.py | 36 ++++++++++- ...eway-and-subnetpools-559335807639b5b6.yaml | 7 +++ 6 files changed, 151 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/create-sunet-with-gateway-and-subnetpools-559335807639b5b6.yaml diff --git a/neutron/ipam/requests.py b/neutron/ipam/requests.py index 2b90da786d7..5152cb4b5b2 100644 --- a/neutron/ipam/requests.py +++ b/neutron/ipam/requests.py @@ -21,6 +21,7 @@ from oslo_utils import uuidutils from neutron._i18n import _ from neutron.common import utils as common_utils from neutron.ipam import exceptions as ipam_exc +from neutron.ipam import utils as ipam_utils class SubnetPool(object, metaclass=abc.ABCMeta): @@ -110,6 +111,17 @@ class SubnetRequest(object, metaclass=abc.ABCMeta): raise ipam_exc.IpamValueInvalid(_( "allocation_pools are not in the subnet")) + @staticmethod + def _validate_gateway_ip_in_subnet(subnet_cidr, gateway_ip): + if not gateway_ip: + return + + if ipam_utils.check_gateway_invalid_in_subnet(subnet_cidr, gateway_ip): + raise ipam_exc.IpamValueInvalid(_( + 'Gateway IP %(gateway_ip)s cannot be allocated in CIDR ' + '%(subnet_cidr)s' % {'gateway_ip': gateway_ip, + 'subnet_cidr': subnet_cidr})) + class AnySubnetRequest(SubnetRequest): """A template for allocating an unspecified subnet from IPAM @@ -171,6 +183,7 @@ class SpecificSubnetRequest(SubnetRequest): self._subnet_cidr = netaddr.IPNetwork(subnet_cidr) self._validate_with_subnet(self._subnet_cidr) + self._validate_gateway_ip_in_subnet(self._subnet_cidr, self.gateway_ip) @property def subnet_cidr(self): @@ -299,9 +312,12 @@ class SubnetRequestFactory(object): @classmethod def get_request(cls, context, subnet, subnetpool): cidr = subnet.get('cidr') + cidr = cidr if validators.is_attr_set(cidr) else None + gateway_ip = subnet.get('gateway_ip') + gateway_ip = gateway_ip if validators.is_attr_set(gateway_ip) else None subnet_id = subnet.get('id', uuidutils.generate_uuid()) - is_any_subnetpool_request = not validators.is_attr_set(cidr) + is_any_subnetpool_request = not (cidr or gateway_ip) if is_any_subnetpool_request: prefixlen = subnet['prefixlen'] if not validators.is_attr_set(prefixlen): @@ -313,8 +329,20 @@ class SubnetRequestFactory(object): common_utils.ip_version_from_int(subnetpool['ip_version']), prefixlen) else: - return SpecificSubnetRequest(subnet['tenant_id'], - subnet_id, - cidr, - subnet.get('gateway_ip'), - subnet.get('allocation_pools')) + alloc_pools = subnet.get('allocation_pools') + alloc_pools = (alloc_pools if validators.is_attr_set(alloc_pools) + else None) + if not cidr and gateway_ip: + prefixlen = subnet['prefixlen'] + if not validators.is_attr_set(prefixlen): + prefixlen = int(subnetpool['default_prefixlen']) + gw_ip_net = netaddr.IPNetwork('%s/%s' % + (gateway_ip, prefixlen)) + cidr = gw_ip_net.cidr + + return SpecificSubnetRequest( + subnet['tenant_id'], + subnet_id, + cidr, + gateway_ip=gateway_ip, + allocation_pools=alloc_pools) diff --git a/neutron/tests/fullstack/resources/client.py b/neutron/tests/fullstack/resources/client.py index 55198d9a3d3..ce1d1ca579f 100644 --- a/neutron/tests/fullstack/resources/client.py +++ b/neutron/tests/fullstack/resources/client.py @@ -118,7 +118,7 @@ class ClientFixture(fixtures.Fixture): return self._delete_resource('network', id) def create_subnet(self, tenant_id, network_id, - cidr, gateway_ip=None, name=None, enable_dhcp=True, + cidr=None, gateway_ip=None, name=None, enable_dhcp=True, ipv6_address_mode='slaac', ipv6_ra_mode='slaac', subnetpool_id=None, ip_version=None): resource_type = 'subnet' @@ -141,6 +141,22 @@ class ClientFixture(fixtures.Fixture): return self._create_resource(resource_type, spec) + def create_subnetpool(self, project_id, name=None, min_prefixlen=8, + max_prefixlen=24, default_prefixlen=24, + prefixes=None): + resource_type = 'subnetpool' + name = name or utils.get_rand_name(prefix=resource_type) + spec = {'project_id': project_id, + 'name': name, + 'min_prefixlen': min_prefixlen, + 'max_prefixlen': max_prefixlen, + 'default_prefixlen': default_prefixlen, + 'is_default': False, + 'shared': False, + 'prefixes': prefixes} + + return self._create_resource(resource_type, spec) + def list_subnets(self, retrieve_all=True, **kwargs): resp = self.client.list_subnets(retrieve_all=retrieve_all, **kwargs) return resp['subnets'] diff --git a/neutron/tests/fullstack/test_subnet.py b/neutron/tests/fullstack/test_subnet.py index 74bcb78cbbd..d65390ea6f2 100644 --- a/neutron/tests/fullstack/test_subnet.py +++ b/neutron/tests/fullstack/test_subnet.py @@ -14,6 +14,7 @@ import netaddr from neutron_lib import constants +from neutronclient.common import exceptions as nclient_exceptions from oslo_utils import uuidutils from neutron.tests.common.exclusive_resources import ip_network @@ -38,16 +39,23 @@ class TestSubnet(base.BaseFullStackTestCase): def _create_network(self, project_id, name='test_network'): return self.safe_client.create_network(project_id, name=name) - def _create_subnet(self, project_id, network_id, cidr, + def _create_subnetpool(self, project_id, min_prefixlen, max_prefixlen, + default_prefixlen, prefixes): + return self.safe_client.create_subnetpool( + project_id=project_id, min_prefixlen=min_prefixlen, + max_prefixlen=max_prefixlen, + default_prefixlen=default_prefixlen, prefixes=prefixes) + + def _create_subnet(self, project_id, network_id, cidr=None, ipv6_address_mode=None, ipv6_ra_mode=None, - subnetpool_id=None): - ip_version = None + subnetpool_id=None, ip_version=None, gateway_ip=None): if ipv6_address_mode or ipv6_ra_mode: ip_version = constants.IP_VERSION_6 return self.safe_client.create_subnet( - project_id, network_id, cidr, enable_dhcp=True, + project_id, network_id, cidr=cidr, enable_dhcp=True, ipv6_address_mode=ipv6_address_mode, ipv6_ra_mode=ipv6_ra_mode, - subnetpool_id=subnetpool_id, ip_version=ip_version) + subnetpool_id=subnetpool_id, ip_version=ip_version, + gateway_ip=gateway_ip) def _show_subnet(self, subnet_id): return self.client.show_subnet(subnet_id) @@ -80,3 +88,47 @@ class TestSubnet(base.BaseFullStackTestCase): subnetpool_id='prefix_delegation') subnet = self._show_subnet(subnet['id']) self.assertIsNone(subnet['subnet']['gateway_ip']) + + def test_create_subnet_ipv4_with_subnetpool(self): + subnetpool_cidr = self.useFixture( + ip_network.ExclusiveIPNetwork( + '240.0.0.0', '240.255.255.255', '16')).network + subnetpool = self._create_subnetpool(self._project_id, 8, 24, 24, + [subnetpool_cidr]) + subnets = list(subnetpool_cidr.subnet(24)) + + # Request from subnetpool. + subnet = self._create_subnet(self._project_id, self._network['id'], + subnetpool_id=subnetpool['id'], + ip_version=4) + subnet = self._show_subnet(subnet['id']) + self.assertEqual(subnet['subnet']['cidr'], str(subnets[0].cidr)) + self.assertEqual(subnet['subnet']['gateway_ip'], + str(subnets[0].network + 1)) + + # Request from subnetpool with gateway_ip. + gateway_ip = subnets[1].ip + 10 + subnet = self._create_subnet(self._project_id, self._network['id'], + subnetpool_id=subnetpool['id'], + ip_version=4, gateway_ip=gateway_ip) + subnet = self._show_subnet(subnet['id']) + self.assertEqual(subnet['subnet']['cidr'], str(subnets[1].cidr)) + self.assertEqual(subnet['subnet']['gateway_ip'], str(gateway_ip)) + + # Request from subnetpool with incorrect gateway_ip (cannot be the + # network broadcast IP). + gateway_ip = subnets[2].ip + self.assertRaises(nclient_exceptions.Conflict, + self._create_subnet, self._project_id, + self._network['id'], subnetpool_id=subnetpool['id'], + ip_version=4, gateway_ip=gateway_ip) + + # Request from subnetpool using a correct gateway_ip from the same + # CIDR; that means this subnet has not been allocated yet. + gateway_ip += 1 + subnet = self._create_subnet(self._project_id, self._network['id'], + subnetpool_id=subnetpool['id'], + ip_version=4, gateway_ip=gateway_ip) + subnet = self._show_subnet(subnet['id']) + self.assertEqual(subnet['subnet']['cidr'], str(subnets[2].cidr)) + self.assertEqual(subnet['subnet']['gateway_ip'], str(gateway_ip)) diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 5db9c53d31a..d30818f86f8 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -1690,7 +1690,8 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s res['port']['fixed_ips']) def test_no_more_port_exception(self): - with self.subnet(cidr='10.0.0.0/31', enable_dhcp=False) as subnet: + with self.subnet(cidr='10.0.0.0/31', enable_dhcp=False, + gateway_ip=None) as subnet: id = subnet['subnet']['network_id'] res = self._create_port(self.fmt, id) data = self.deserialize(self.fmt, res) diff --git a/neutron/tests/unit/ipam/test_requests.py b/neutron/tests/unit/ipam/test_requests.py index e4e5679209c..f9a30104f53 100644 --- a/neutron/tests/unit/ipam/test_requests.py +++ b/neutron/tests/unit/ipam/test_requests.py @@ -328,12 +328,13 @@ class TestAddressRequestFactory(base.BaseTestCase): class TestSubnetRequestFactory(IpamSubnetRequestTestCase): def _build_subnet_dict(self, id=None, cidr='192.168.1.0/24', - prefixlen=8, ip_version=constants.IP_VERSION_4): + prefixlen=8, ip_version=constants.IP_VERSION_4, + gateway_ip=None): subnet = {'cidr': cidr, 'prefixlen': prefixlen, 'ip_version': ip_version, 'tenant_id': self.tenant_id, - 'gateway_ip': None, + 'gateway_ip': gateway_ip, 'allocation_pools': None, 'id': id or self.subnet_id} subnetpool = {'ip_version': ip_version, @@ -354,6 +355,21 @@ class TestSubnetRequestFactory(IpamSubnetRequestTestCase): subnetpool), ipam_req.SpecificSubnetRequest) + def test_specific_gateway_request_is_loaded(self): + gw_prefixlen = [('10.12.0.15', 24), ('10.12.0.1', 8), + ('fffe::1', 64), ('fffe::', 64)] + for gateway_ip, prefixlen in gw_prefixlen: + subnet, subnetpool = self._build_subnet_dict( + cidr=None, gateway_ip=gateway_ip, prefixlen=prefixlen) + request = ipam_req.SubnetRequestFactory.get_request( + None, subnet, subnetpool) + + cidr = netaddr.IPNetwork(str(gateway_ip) + '/%s' % prefixlen).cidr + self.assertIsInstance(request, ipam_req.SpecificSubnetRequest) + self.assertEqual(cidr, request.subnet_cidr) + self.assertEqual(netaddr.IPAddress(gateway_ip), request.gateway_ip) + self.assertEqual(prefixlen, request.prefixlen) + def test_any_address_request_is_loaded_for_ipv4(self): subnet, subnetpool = self._build_subnet_dict( cidr=None, ip_version=constants.IP_VERSION_4) @@ -401,3 +417,19 @@ class TestGetRequestFactory(base.BaseTestCase): self.assertEqual( self.driver.get_address_request_factory(), ipam_req.AddressRequestFactory) + + +class TestSubnetRequestMetaclass(base.BaseTestCase): + + def test__validate_gateway_ip_in_subnet(self): + method = ipam_req.SubnetRequest._validate_gateway_ip_in_subnet + cidr4 = netaddr.IPNetwork('192.168.0.0/24') + self.assertIsNone(method(cidr4, cidr4.ip + 1)) + self.assertRaises(ipam_exc.IpamValueInvalid, method, cidr4, cidr4.ip) + self.assertRaises(ipam_exc.IpamValueInvalid, method, cidr4, + cidr4.broadcast) + + cidr6 = netaddr.IPNetwork('2001:db8::/64') + self.assertIsNone(method(cidr6, cidr6.ip + 1)) + self.assertIsNone(method(cidr6, cidr6.ip)) + self.assertIsNone(method(cidr6, cidr6.broadcast)) diff --git a/releasenotes/notes/create-sunet-with-gateway-and-subnetpools-559335807639b5b6.yaml b/releasenotes/notes/create-sunet-with-gateway-and-subnetpools-559335807639b5b6.yaml new file mode 100644 index 00000000000..080c692f01d --- /dev/null +++ b/releasenotes/notes/create-sunet-with-gateway-and-subnetpools-559335807639b5b6.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Now it is possible to define a gateway IP when creating a subnet using a + subnet pool. If the gateway IP can be allocated in one of the subnet + pool available subnets, this subnet is created; otherwise a ``Conflict`` + exception is raised.