From 70f406e85c6163beed7ca02de1d03ddbb6f7f620 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Mon, 2 Jan 2017 15:52:09 +0200 Subject: [PATCH] NSX-V3| IPAM support for subnet update Support changing the subnet gateway or allocation pools Change-Id: Ie326b5a5c110bb655e90c2b99bf78efcd32dd0ce --- vmware_nsx/services/ipam/common/driver.py | 62 ++++++++++++++----- vmware_nsx/services/ipam/nsx_v/driver.py | 6 ++ vmware_nsx/services/ipam/nsx_v3/driver.py | 40 +++++++++++- .../unit/services/ipam/test_nsxv3_driver.py | 49 ++++++++------- 4 files changed, 117 insertions(+), 40 deletions(-) diff --git a/vmware_nsx/services/ipam/common/driver.py b/vmware_nsx/services/ipam/common/driver.py index cef01ddf83..020ba48f4a 100644 --- a/vmware_nsx/services/ipam/common/driver.py +++ b/vmware_nsx/services/ipam/common/driver.py @@ -67,6 +67,12 @@ class NsxAbstractIpamDriver(subnet_alloc.SubnetAllocator, NsxIpamBase): # the neutron internal driver will be used self.default_ipam = neutron_driver.NeutronDbPool(subnetpool, context) + # Mark which updates to the pool are supported + # (The NSX-v backend does not support changing the ip pool cidr + # or gateway) + self.support_update_gateway = False + self.support_update_pools = False + def _is_supported_net(self, subnet_request): """By default - all networks are supported""" return True @@ -123,6 +129,10 @@ class NsxAbstractIpamDriver(subnet_alloc.SubnetAllocator, NsxIpamBase): self._context, tenant_id=subnet_request.tenant_id) + @abc.abstractmethod + def update_backend_pool(self, nsx_pool_id, subnet_request): + pass + def _raise_update_not_supported(self): msg = _('Changing the subnet range or gateway is not supported') raise ipam_exc.IpamValueInvalid(message=msg) @@ -130,9 +140,8 @@ class NsxAbstractIpamDriver(subnet_alloc.SubnetAllocator, NsxIpamBase): def update_subnet(self, subnet_request): """Update subnet info in the IPAM driver. - The NSX backend does not support changing the ip pool cidr or gateway + Do the update only if the specific change is supported by the backend """ - #TODO(asarfaty): the nsx-v3 backend does support update nsx_pool_id = nsx_db.get_nsx_ipam_pool_for_subnet( self._context.session, subnet_request.subnet_id) if not nsx_pool_id: @@ -145,24 +154,49 @@ class NsxAbstractIpamDriver(subnet_alloc.SubnetAllocator, NsxIpamBase): subnet_request.subnet_id, nsx_pool_id, self._context, tenant_id=subnet_request.tenant_id).get_details() - # check that the gateway / cidr / pools did not change - if (subnet_request.gateway_ip and - str(subnet_request.gateway_ip) != str(curr_subnet.gateway_ip)): - self._raise_update_not_supported() + # check if the gateway changed + gateway_changed = False + if (str(subnet_request.gateway_ip) != str(curr_subnet.gateway_ip)): + if not self.support_update_gateway: + self._raise_update_not_supported() + gateway_changed = True + # check that the prefix / cidr / pools changed + pools_changed = False if subnet_request.prefixlen != curr_subnet.prefixlen: - self._raise_update_not_supported() + if not self.support_update_pools: + self._raise_update_not_supported() + pools_changed = True + + if subnet_request.subnet_cidr[0] != curr_subnet.subnet_cidr[0]: + if not self.support_update_pools: + self._raise_update_not_supported() + pools_changed = True if (len(subnet_request.allocation_pools) != len(curr_subnet.allocation_pools)): - self._raise_update_not_supported() - - for pool_ind in range(len(subnet_request.allocation_pools)): - pool_req = subnet_request.allocation_pools[pool_ind] - curr_pool = curr_subnet.allocation_pools[pool_ind] - if (pool_req.first != curr_pool.first or - pool_req.last != curr_pool.last): + if not self.support_update_pools: self._raise_update_not_supported() + pools_changed = True + + if (len(subnet_request.allocation_pools) != + len(curr_subnet.allocation_pools)): + if not self.support_update_pools: + self._raise_update_not_supported() + pools_changed = True + else: + for pool_ind in range(len(subnet_request.allocation_pools)): + pool_req = subnet_request.allocation_pools[pool_ind] + curr_pool = curr_subnet.allocation_pools[pool_ind] + if (pool_req.first != curr_pool.first or + pool_req.last != curr_pool.last): + if not self.support_update_pools: + self._raise_update_not_supported() + pools_changed = True + + # update the relevant attributes at the backend pool + if gateway_changed or pools_changed: + self.update_backend_pool(nsx_pool_id, subnet_request) @abc.abstractmethod def delete_backend_pool(self, nsx_pool_id): diff --git a/vmware_nsx/services/ipam/nsx_v/driver.py b/vmware_nsx/services/ipam/nsx_v/driver.py index 270b84897b..7defee57e3 100644 --- a/vmware_nsx/services/ipam/nsx_v/driver.py +++ b/vmware_nsx/services/ipam/nsx_v/driver.py @@ -116,6 +116,12 @@ class NsxvIpamDriver(common.NsxAbstractIpamDriver, NsxVIpamBase): LOG.error(_LE("Failed to delete IPAM from backend: %s"), e) # Continue anyway, since this subnet was already removed + def update_backend_pool(self, subnet_request): + # The NSX-v backend does not support changing the ip pool cidr + # or gateway. + # If this function is called - there is no need to update the backend + pass + class NsxvIpamSubnet(common.NsxAbstractIpamSubnet, NsxVIpamBase): """Manage IP addresses for the NSX-V IPAM driver.""" diff --git a/vmware_nsx/services/ipam/nsx_v3/driver.py b/vmware_nsx/services/ipam/nsx_v3/driver.py index fffb0ba191..a4a0f51ca8 100644 --- a/vmware_nsx/services/ipam/nsx_v3/driver.py +++ b/vmware_nsx/services/ipam/nsx_v3/driver.py @@ -38,6 +38,10 @@ class Nsxv3IpamDriver(common.NsxAbstractIpamDriver): self.nsxlib_ipam = resources.IpPool( self.get_core_plugin().nsxlib.client) + # Mark which updates to the pool are supported + self.support_update_gateway = True + self.support_update_pools = True + @property def _subnet_class(self): return Nsxv3IpamSubnet @@ -46,22 +50,25 @@ class Nsxv3IpamDriver(common.NsxAbstractIpamDriver): return "%s/%s" % (subnet_request.subnet_cidr[0], subnet_request.prefixlen) - def allocate_backend_pool(self, subnet_request): - """Create a pool on the NSX backend and return its ID""" + def _get_ranges_from_request(self, subnet_request): if subnet_request.allocation_pools: ranges = [ {'start': str(pool[0]), 'end': str(pool[-1])} for pool in subnet_request.allocation_pools] else: ranges = [] + return ranges + def allocate_backend_pool(self, subnet_request): + """Create a pool on the NSX backend and return its ID""" # name/description length on backend is long, so there is no problem name = 'subnet_' + subnet_request.subnet_id description = 'OS IP pool for subnet ' + subnet_request.subnet_id try: response = self.nsxlib_ipam.create( self._get_cidr_from_request(subnet_request), - ranges=ranges, + allocation_ranges=self._get_ranges_from_request( + subnet_request), display_name=name, description=description, gateway_ip=subnet_request.gateway_ip) @@ -94,6 +101,33 @@ class Nsxv3IpamDriver(common.NsxAbstractIpamDriver): LOG.error(_LE("Failed to delete IPAM from backend: %s"), e) # Continue anyway, since this subnet was already removed + def update_backend_pool(self, nsx_pool_id, subnet_request): + update_args = { + 'cidr': self._get_cidr_from_request(subnet_request), + 'allocation_ranges': self._get_ranges_from_request(subnet_request), + 'gateway_ip': subnet_request.gateway_ip} + try: + self.nsxlib_ipam.update( + nsx_pool_id, **update_args) + except nsx_lib_exc.ManagerError as e: + LOG.error(_LE("NSX IPAM failed to update pool %(id)s: " + " %(e)s; code %(code)s"), + {'e': e, + 'id': nsx_pool_id, + 'code': e.error_code}) + if (e.error_code == error.ERR_CODE_IPAM_RANGE_MODIFY or + e.error_code == error.ERR_CODE_IPAM_RANGE_DELETE or + e.error_code == error.ERR_CODE_IPAM_RANGE_SHRUNK): + # The change is not allowed: already allocated IPs out of + # the new range + raise ipam_exc.InvalidSubnetRequest( + reason=_("Already allocated IPs outside of the updated " + "pools")) + except Exception as e: + # unexpected error + msg = _('Failed to update subnet IPAM: %s') % e + raise ipam_exc.IpamValueInvalid(message=msg) + class Nsxv3IpamSubnet(common.NsxAbstractIpamSubnet): """Manage IP addresses for the NSX V3 IPAM driver.""" diff --git a/vmware_nsx/tests/unit/services/ipam/test_nsxv3_driver.py b/vmware_nsx/tests/unit/services/ipam/test_nsxv3_driver.py index 9bcd723df3..284d4921c8 100644 --- a/vmware_nsx/tests/unit/services/ipam/test_nsxv3_driver.py +++ b/vmware_nsx/tests/unit/services/ipam/test_nsxv3_driver.py @@ -33,7 +33,7 @@ class MockIPPools(object): gateway_ip = None if kwargs.get('gateway_ip'): gateway_ip = str(kwargs['gateway_ip']) - subnet = {"allocation_ranges": kwargs.get('ranges'), + subnet = {"allocation_ranges": kwargs.get('allocation_ranges'), "gateway_ip": gateway_ip, "cidr": args[0]} pool = {'id': pool_id, @@ -41,6 +41,21 @@ class MockIPPools(object): self.nsx_pools[pool_id] = {'pool': pool, 'allocated': []} return {'id': pool_id} + def _update_pool(pool_id, **kwargs): + pool = self.nsx_pools[pool_id]['pool'] + subnet = pool['subnets'][0] + if 'gateway_ip' in kwargs: + if kwargs['gateway_ip']: + subnet["gateway_ip"] = str(kwargs['gateway_ip']) + else: + del subnet["gateway_ip"] + + if 'allocation_ranges' in kwargs: + if kwargs['allocation_ranges']: + subnet["allocation_ranges"] = kwargs['allocation_ranges'] + else: + del subnet["allocation_ranges"] + def _delete_pool(pool_id): del self.nsx_pools[pool_id] @@ -76,6 +91,9 @@ class MockIPPools(object): mock.patch( "vmware_nsxlib.v3.resources.IpPool.create", side_effect=_create_pool).start() + mock.patch( + "vmware_nsxlib.v3.resources.IpPool.update", + side_effect=_update_pool).start() mock.patch( "vmware_nsxlib.v3.resources.IpPool.delete", side_effect=_delete_pool).start() @@ -95,29 +113,8 @@ class TestNsxv3IpamSubnets(test_plugin.TestSubnetsV2, MockIPPools): super(TestNsxv3IpamSubnets, self).setUp() self.patch_nsxlib_ipam() - def test_update_subnet_from_gw_to_new_gw(self): - self.skipTest('Update ipam subnet is not supported') - - def test_update_subnet_gw_outside_cidr_returns_200(self): - self.skipTest('Update ipam subnet is not supported') - - def test_update_subnet_from_gw_to_no_gw(self): - self.skipTest('Update ipam subnet is not supported') - - def test_update_subnet_allocation_pools(self): - self.skipTest('Update ipam subnet is not supported') - - def test_update_subnet_allocation_pools_and_gateway_ip(self): - self.skipTest('Update ipam subnet is not supported') - def test_update_subnet_gw_ip_in_use_by_router_returns_409(self): - self.skipTest('Update ipam subnet is not supported') - - def test_update_subnet_from_no_gw_to_no_gw(self): - self.skipTest('Update ipam subnet is not supported') - - def _test_subnet_update_ipv4_and_ipv6_pd_subnets(self, ra_addr_mode): - self.skipTest('Update ipam subnet is not supported') + self.skipTest('Allocating a specific IP is not supported') def test_subnet_with_allocation_range(self): self.skipTest('Allocating a specific IP is not supported') @@ -140,6 +137,12 @@ class TestNsxv3IpamSubnets(test_plugin.TestSubnetsV2, MockIPPools): def test_create_subnet_ipv6_slaac_with_db_reference_error(self): self.skipTest('Allocating a specific IP is not supported') + def test_subnet_update_ipv4_and_ipv6_pd_slaac_subnets(self): + self.skipTest('Allocating a specific IP is not supported') + + def test_subnet_update_ipv4_and_ipv6_pd_v6stateless_subnets(self): + self.skipTest('Allocating a specific IP is not supported') + class TestNsxv3IpamPorts(test_plugin.TestPortsV2, MockIPPools): """Run the nsxv3 plugin ports tests with the ipam driver."""