Extend SubnetRequestFactory to access subnet dict

SubnetRequestFactory incorporates logic for preparing arguments and
building right request.
Subnet dict, context and subnetpool are used to make decision on
what kind of reqest should be created.
SubnetRequestFactory can be overriden on ipam driver level,
it allows to easily extend existent Request model.

Partially-Implements: blueprint neutron-ipam

Change-Id: I9de120381172657a527bb35990d25525c41939ab

diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py
index 09ebee3ee31390f7859a2a9712c54148611749ba..9ca165e5b0110d7059944dadfaf90f033f68a89a 100644 (file)
--- a/neutron/db/db_base_plugin_v2.py
+++ b/neutron/db/db_base_plugin_v2.py
@@ -31,7 +31,6 @@ from neutron.callbacks import resources
 from neutron.common import constants
 from neutron.common import exceptions as n_exc
 from neutron.common import ipv6_utils
-from neutron.common import utils
 from neutron import context as ctx
 from neutron.db import api as db_api
 from neutron.db import ipam_non_pluggable_backend
@@ -440,32 +439,11 @@ class NeutronDbPluginV2(ipam_non_pluggable_backend.IpamNonPluggableBackend,
                     external_gateway_info}}
                 l3plugin.update_router(context, id, info)
 
-    def _make_subnet_request(self, tenant_id, subnet, subnetpool):
-        cidr = subnet.get('cidr')
-        subnet_id = subnet.get('id', uuidutils.generate_uuid())
-        is_any_subnetpool_request = not attributes.is_attr_set(cidr)
-
-        if is_any_subnetpool_request:
-            prefixlen = subnet['prefixlen']
-            if not attributes.is_attr_set(prefixlen):
-                prefixlen = int(subnetpool['default_prefixlen'])
-
-            return ipam.AnySubnetRequest(
-                          tenant_id,
-                          subnet_id,
-                          utils.ip_version_from_int(subnetpool['ip_version']),
-                          prefixlen)
-        else:
-            return ipam.SpecificSubnetRequest(tenant_id,
-                                              subnet_id,
-                                              cidr)
-
     @oslo_db_api.wrap_db_retry(max_retries=db_api.MAX_RETRIES,
                                retry_on_request=True,
                                retry_on_deadlock=True)
     def _create_subnet_from_pool(self, context, subnet, subnetpool_id):
         s = subnet['subnet']
-        tenant_id = self._get_tenant_id_for_create(context, s)
         self._validate_pools_with_subnetpool(s)
 
         with context.session.begin(subtransactions=True):
@@ -474,7 +452,7 @@ class NeutronDbPluginV2(ipam_non_pluggable_backend.IpamNonPluggableBackend,
 
             network = self._get_network(context, s["network_id"])
             allocator = subnet_alloc.SubnetAllocator(subnetpool, context)
-            req = self._make_subnet_request(tenant_id, s, subnetpool)
+            req = ipam.SubnetRequestFactory.get_request(context, s, subnetpool)
 
             ipam_subnet = allocator.allocate_subnet(req)
             detail = ipam_subnet.get_details()
@@ -498,9 +476,8 @@ class NeutronDbPluginV2(ipam_non_pluggable_backend.IpamNonPluggableBackend,
     def _create_subnet_from_implicit_pool(self, context, subnet):
         s = subnet['subnet']
         self._validate_subnet(context, s)
-        tenant_id = self._get_tenant_id_for_create(context, s)
         id = s.get('id', uuidutils.generate_uuid())
-        detail = ipam.SpecificSubnetRequest(tenant_id,
+        detail = ipam.SpecificSubnetRequest(s['tenant_id'],
                                             id,
                                             s['cidr'])
         with context.session.begin(subtransactions=True):
@@ -571,6 +548,7 @@ class NeutronDbPluginV2(ipam_non_pluggable_backend.IpamNonPluggableBackend,
             net = netaddr.IPNetwork(s['cidr'])
             subnet['subnet']['cidr'] = '%s/%s' % (net.network, net.prefixlen)
 
+        s['tenant_id'] = self._get_tenant_id_for_create(context, s)
         subnetpool_id = self._get_subnetpool_id(s)
         if not subnetpool_id:
             if not has_cidr:

diff --git a/neutron/ipam/__init__.py b/neutron/ipam/__init__.py
index c756946bacbfa793a221127ba2d1ff1478ce7fee..6e1e438177d4354667a99ec0b86918427b29f3c1 100644 (file)
--- a/neutron/ipam/__init__.py
+++ b/neutron/ipam/__init__.py
@@ -14,10 +14,13 @@ import abc
 import netaddr
 
 from oslo_config import cfg
+from oslo_utils import uuidutils
 import six
 
+from neutron.api.v2 import attributes
 from neutron.common import constants
 from neutron.common import ipv6_utils
+from neutron.common import utils as common_utils
 from neutron.ipam import exceptions as ipam_exc
 
 
@@ -243,28 +246,42 @@ class RouterGatewayAddressRequest(AddressRequest):
     """Used to request allocating the special router gateway address."""
 
 
-class BaseRequestFactory(object):
-    """Factory class to create right request based on input"""
-    any_request = None
-    specific_request = None
-    address_index = 0
+class AddressRequestFactory(object):
+    """Builds request using ip info
+
+    Additional parameters(port and context) are not used in default
+    implementation, but planned to be used in sub-classes
+    provided by specific ipam driver,
+    """
 
     @classmethod
-    def get_request(cls, *args, **kwargs):
-        args_list = [a for a in args]
-        address = args_list.pop(cls.address_index)
-        if not address:
-            return cls.any_request(*args_list, **kwargs)
+    def get_request(cls, context, port, ip):
+        if not ip:
+            return AnyAddressRequest()
         else:
-            return cls.specific_request(*args, **kwargs)
-
+            return SpecificAddressRequest(ip)
 
-class AddressRequestFactory(BaseRequestFactory):
-    any_request = AnyAddressRequest
-    specific_request = SpecificAddressRequest
 
+class SubnetRequestFactory(object):
+    """Builds request using subnet info"""
 
-class SubnetRequestFactory(BaseRequestFactory):
-    any_request = AnySubnetRequest
-    specific_request = SpecificSubnetRequest
-    address_index = 2
+    @classmethod
+    def get_request(cls, context, subnet, subnetpool):
+        cidr = subnet.get('cidr')
+        subnet_id = subnet.get('id', uuidutils.generate_uuid())
+        is_any_subnetpool_request = not attributes.is_attr_set(cidr)
+
+        if is_any_subnetpool_request:
+            prefixlen = subnet['prefixlen']
+            if not attributes.is_attr_set(prefixlen):
+                prefixlen = int(subnetpool['default_prefixlen'])
+
+            return AnySubnetRequest(
+                subnet['tenant_id'],
+                subnet_id,
+                common_utils.ip_version_from_int(subnetpool['ip_version']),
+                prefixlen)
+        else:
+            return SpecificSubnetRequest(subnet['tenant_id'],
+                                         subnet_id,
+                                         cidr)

diff --git a/neutron/tests/unit/test_ipam.py b/neutron/tests/unit/test_ipam.py
index 1dc6e0df62c94a17f48fcd29b93ee242ac8a5ca0..932159432fad4d6cb0e2df19770f6b0bd8e2257c 100755 (executable)
--- a/neutron/tests/unit/test_ipam.py
+++ b/neutron/tests/unit/test_ipam.py
@@ -292,18 +292,33 @@ class TestAddressRequestFactory(base.BaseTestCase):
     def test_specific_address_request_is_loaded(self):
         for address in ('10.12.0.15', 'fffe::1'):
             self.assertIsInstance(
-                ipam.AddressRequestFactory.get_request(address),
+                ipam.AddressRequestFactory.get_request(None,
+                                                       None,
+                                                       address),
                 ipam.SpecificAddressRequest)
 
     def test_any_address_request_is_loaded(self):
         for addr in [None, '']:
             self.assertIsInstance(
-                ipam.AddressRequestFactory.get_request(addr),
+                ipam.AddressRequestFactory.get_request(None,
+                                                       None,
+                                                       addr),
                 ipam.AnyAddressRequest)
 
 
 class TestSubnetRequestFactory(IpamSubnetRequestTestCase):
 
+    def _build_subnet_dict(self, id=None, cidr='192.168.1.0/24',
+                           prefixlen=8, ip_version=4):
+        subnet = {'cidr': cidr,
+                  'prefixlen': prefixlen,
+                  'ip_version': ip_version,
+                  'tenant_id': self.tenant_id,
+                  'id': id or self.subnet_id}
+        subnetpool = {'ip_version': ip_version,
+                      'default_prefixlen': prefixlen}
+        return subnet, subnetpool
+
     def test_specific_subnet_request_is_loaded(self):
         addresses = [
             '10.12.0.15/24',
@@ -311,35 +326,34 @@ class TestSubnetRequestFactory(IpamSubnetRequestTestCase):
             'fffe::1/64',
             'fffe::/64']
         for address in addresses:
+            subnet, subnetpool = self._build_subnet_dict(cidr=address)
             self.assertIsInstance(
-                ipam.SubnetRequestFactory.get_request(self.tenant_id,
-                                                      self.subnet_id,
-                                                      address),
+                ipam.SubnetRequestFactory.get_request(None,
+                                                      subnet,
+                                                      subnetpool),
                 ipam.SpecificSubnetRequest)
 
     def test_any_address_request_is_loaded_for_ipv4(self):
+        subnet, subnetpool = self._build_subnet_dict(cidr=None, ip_version=4)
         self.assertIsInstance(
-            ipam.SubnetRequestFactory.get_request(self.tenant_id,
-                                                  self.subnet_id,
-                                                  None,
-                                                  constants.IPv4,
-                                                  8),
+            ipam.SubnetRequestFactory.get_request(None,
+                                                  subnet,
+                                                  subnetpool),
             ipam.AnySubnetRequest)
 
     def test_any_address_request_is_loaded_for_ipv6(self):
+        subnet, subnetpool = self._build_subnet_dict(cidr=None, ip_version=6)
         self.assertIsInstance(
-            ipam.SubnetRequestFactory.get_request(self.tenant_id,
-                                                  self.subnet_id,
-                                                  None,
-                                                  constants.IPv6,
-                                                  64),
+            ipam.SubnetRequestFactory.get_request(None,
+                                                  subnet,
+                                                  subnetpool),
             ipam.AnySubnetRequest)
 
     def test_args_are_passed_to_specific_request(self):
-        request = ipam.SubnetRequestFactory.get_request(
-            self.tenant_id,
-            self.subnet_id,
-            '192.168.1.0/24')
+        subnet, subnetpool = self._build_subnet_dict()
+        request = ipam.SubnetRequestFactory.get_request(None,
+                                                        subnet,
+                                                        subnetpool)
         self.assertIsInstance(request,
                               ipam.SpecificSubnetRequest)
         self.assertEqual(self.tenant_id, request.tenant_id)