From cf3009a984b01eb90046c8c7513e531b7ab63762 Mon Sep 17 00:00:00 2001 From: Pavel Bondar Date: Tue, 31 May 2016 15:12:00 +0300 Subject: [PATCH] Allow auto-addressed ips deletion on port update By default ips for auto-addressed subnets can not be removed from port using port_update workflow. But during subnet_delete it has to be done via update_port to make sure that ipam drivers received appropriate call to deallocate ip addresses prior to subnet deletion. 'fixed_ips' property is tweeked to allow deletion ips from auto-addressed subnet. 'delete_subnet' boolean is added to mark subnet that is going to be deleted. This flag is analysed in _get_changed_ips_for_port to skip re-adding slaac subnets for the port. Manually resolved conflicts: neutron/tests/unit/plugins/ml2/test_plugin.py Closes-Bug: #1564335 Change-Id: Iec171efe8b64f8a6dc6cb003b97c11667c5e0048 Cherry-picked-From: dc19411ebf2cab7075dd5abe809797fb7253757c --- neutron/db/ipam_backend_mixin.py | 25 +++++++---- neutron/plugins/ml2/plugin.py | 16 ++++--- .../tests/unit/db/test_ipam_backend_mixin.py | 42 ++++++++++++++++++- neutron/tests/unit/plugins/ml2/test_plugin.py | 19 +++++++++ 4 files changed, 87 insertions(+), 15 deletions(-) diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index d7470e818ba..10235011642 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -372,21 +372,28 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): def _get_changed_ips_for_port(self, context, original_ips, new_ips, device_owner): """Calculate changes in IPs for the port.""" + # Collect auto addressed subnet ids that has to be removed on update + delete_subnet_ids = set(ip['subnet_id'] for ip in new_ips + if ip.get('delete_subnet')) + ips = [ip for ip in new_ips + if ip.get('subnet_id') not in delete_subnet_ids] # the new_ips contain all of the fixed_ips that are to be updated - self._validate_max_ips_per_port(new_ips, device_owner) + self._validate_max_ips_per_port(ips, device_owner) add_ips = [] remove_ips = [] + ips_map = {ip['ip_address']: ip for ip in itertools.chain(new_ips, original_ips) if 'ip_address' in ip} new = set() for ip in new_ips: - if 'ip_address' in ip: - new.add(ip['ip_address']) - else: - add_ips.append(ip) + if ip.get('subnet_id') not in delete_subnet_ids: + if 'ip_address' in ip: + new.add(ip['ip_address']) + else: + add_ips.append(ip) # Convert original ip addresses to sets orig = set(ip['ip_address'] for ip in original_ips) @@ -401,11 +408,13 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): # Mark ip for removing if it is not found in new_ips # and subnet requires ip to be set manually. - # For auto addresses leave ip unchanged + # For auto addressed subnet leave ip unchanged + # unless it is explicitly marked for delete. for ip in remove: subnet_id = ips_map[ip]['subnet_id'] - if self._is_ip_required_by_subnet(context, subnet_id, - device_owner): + ip_required = self._is_ip_required_by_subnet(context, subnet_id, + device_owner) + if ip_required or subnet_id in delete_subnet_ids: remove_ips.append(ips_map[ip]) else: prev_ips.append(ips_map[ip]) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 548ee20990a..15e7fa6ff75 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -1023,11 +1023,17 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, if a.port: # calling update_port() for each allocation to remove the # IP from the port and call the MechanismDrivers - data = {attributes.PORT: - {'fixed_ips': [{'subnet_id': ip.subnet_id, - 'ip_address': ip.ip_address} - for ip in a.port.fixed_ips - if ip.subnet_id != id]}} + fixed_ips = [{'subnet_id': ip.subnet_id, + 'ip_address': ip.ip_address} + for ip in a.port.fixed_ips + if ip.subnet_id != id] + # By default auto-addressed ips are not removed from port + # on port update, so mark subnet with 'delete_subnet' flag + # to force ip deallocation on port update. + if is_auto_addr_subnet: + fixed_ips.append({'subnet_id': id, + 'delete_subnet': True}) + data = {attributes.PORT: {'fixed_ips': fixed_ips}} try: self.update_port(context, a.port_id, data) except exc.PortNotFound: diff --git a/neutron/tests/unit/db/test_ipam_backend_mixin.py b/neutron/tests/unit/db/test_ipam_backend_mixin.py index c25045a63a0..6f563e36038 100644 --- a/neutron/tests/unit/db/test_ipam_backend_mixin.py +++ b/neutron/tests/unit/db/test_ipam_backend_mixin.py @@ -34,8 +34,14 @@ class TestIpamBackendMixin(base.BaseTestCase): self.owner_router = constants.DEVICE_OWNER_ROUTER_INTF def _prepare_ips(self, ips): - return [{'ip_address': ip[1], - 'subnet_id': ip[0]} for ip in ips] + results = [] + for ip in ips: + ip_dict = {'ip_address': ip[1], + 'subnet_id': ip[0]} + if len(ip) > 2: + ip_dict['delete_subnet'] = ip[2] + results.append(ip_dict) + return results def _mock_slaac_subnet_on(self): slaac_subnet = {'ipv6_address_mode': constants.IPV6_SLAAC, @@ -47,6 +53,18 @@ class TestIpamBackendMixin(base.BaseTestCase): 'ipv6_ra_mode': None} self.mixin._get_subnet = mock.Mock(return_value=non_slaac_subnet) + def _mock_slaac_for_subnet_ids(self, subnet_ids): + """Mock incoming subnets as autoaddressed.""" + def _get_subnet(context, subnet_id): + if subnet_id in subnet_ids: + return {'ipv6_address_mode': constants.IPV6_SLAAC, + 'ipv6_ra_mode': constants.IPV6_SLAAC} + else: + return {'ipv6_address_mode': None, + 'ipv6_ra_mode': None} + + self.mixin._get_subnet = mock.Mock(side_effect=_get_subnet) + def _test_get_changed_ips_for_port(self, expected_change, original_ips, new_ips, owner): change = self.mixin._get_changed_ips_for_port(self.ctx, @@ -80,6 +98,26 @@ class TestIpamBackendMixin(base.BaseTestCase): self._test_get_changed_ips_for_port(expected_change, original_ips, new_ips, self.owner_non_router) + def test__get_changed_ips_for_port_remove_autoaddress(self): + new = (('id-5', '2000:1234:5678::12FF:FE34:5678', True), + ('id-1', '192.168.1.1')) + new_ips = self._prepare_ips(new) + reference_ips = [ip for ip in new_ips + if ip['subnet_id'] == 'id-1'] + + original = (('id-5', '2000:1234:5678::12FF:FE34:5678'),) + original_ips = self._prepare_ips(original) + + # mock ipv6 subnet as auto addressed and leave ipv4 as regular + self._mock_slaac_for_subnet_ids([new[0][0]]) + # Autoaddressed ip allocation has to be removed + # if it has 'delete_subnet' flag set to True + expected_change = self.mixin.Changes(add=reference_ips, + original=[], + remove=original_ips) + self._test_get_changed_ips_for_port(expected_change, original_ips, + new_ips, self.owner_non_router) + def test__get_changed_ips_for_port_autoaddress_ipv6_pd_enabled(self): owner_not_router = constants.DEVICE_OWNER_DHCP new_ips = self._prepare_ips(self.default_new_ips) diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 862a8bbee71..7d10de42a57 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -1888,6 +1888,25 @@ class TestML2PluggableIPAM(test_ipam.UseIpamMixin, TestMl2SubnetsV2): driver_mock().allocate_subnet.assert_called_with(mock.ANY) driver_mock().remove_subnet.assert_called_with(request.subnet_id) + def test_delete_subnet_deallocates_slaac_correctly(self): + driver = 'neutron.ipam.drivers.neutrondb_ipam.driver.NeutronDbPool' + with self.network() as network: + with self.subnet(network=network, + cidr='2001:100::0/64', + ip_version=6, + ipv6_ra_mode=constants.IPV6_SLAAC) as subnet: + with self.port(subnet=subnet) as port: + with mock.patch(driver) as driver_mock: + # Validate that deletion of SLAAC allocation happens + # via IPAM interface, i.e. ipam_subnet.deallocate is + # called prior to subnet deletiong from db. + self._delete('subnets', subnet['subnet']['id']) + dealloc = driver_mock().get_subnet().deallocate + dealloc.assert_called_with( + port['port']['fixed_ips'][0]['ip_address']) + driver_mock().remove_subnet.assert_called_with( + subnet['subnet']['id']) + class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase): def setUp(self):