diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index 1aacb643ce9..601b67eb64e 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -398,21 +398,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) @@ -427,11 +434,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 e68eb14a3c3..a5c934fb877 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -1094,11 +1094,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: # NOTE Don't inline port_id; needed for PortNotFound. port_id = a.port_id diff --git a/neutron/tests/unit/db/test_ipam_backend_mixin.py b/neutron/tests/unit/db/test_ipam_backend_mixin.py index 66113dd459d..880112fb718 100644 --- a/neutron/tests/unit/db/test_ipam_backend_mixin.py +++ b/neutron/tests/unit/db/test_ipam_backend_mixin.py @@ -41,8 +41,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': n_const.IPV6_SLAAC, @@ -54,6 +60,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': n_const.IPV6_SLAAC, + 'ipv6_ra_mode': n_const.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, @@ -87,6 +105,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 6a9aa22a1d6..fccc33eea43 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -32,6 +32,7 @@ from neutron._i18n import _ from neutron.callbacks import events from neutron.callbacks import registry from neutron.callbacks import resources +from neutron.common import constants as n_const from neutron.common import utils from neutron import context from neutron.db import agents_db @@ -2150,6 +2151,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=n_const.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):