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

@@ -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'],
+            alloc_pools = subnet.get('allocation_pools')
-                                         subnet_id,
+            alloc_pools = (alloc_pools if validators.is_attr_set(alloc_pools)
-                                         cidr,
+                           else None)
-                                         subnet.get('gateway_ip'),
+            if not cidr and gateway_ip:
-                                         subnet.get('allocation_pools'))
+                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)

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']

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):
+                       subnetpool_id=None, ip_version=None, gateway_ip=None):
-        ip_version = 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))

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)

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))

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.