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
(cherry picked from commit cf3009a984b01eb90046c8c7513e531b7ab63762)

4 files changed, 87 insertions, 15 deletions

diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py
index a0dac3a477..c0c8e1427f 100644
--- a/neutron/db/ipam_backend_mixin.py
+++ b/neutron/db/ipam_backend_mixin.py
@@ -370,21 +370,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)
@@ -399,11 +406,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 1e36cfe805..fe8d52f30a 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -966,11 +966,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 c25045a63a..6f563e3603 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 619855597a..ee33e208f9 100644
--- a/neutron/tests/unit/plugins/ml2/test_plugin.py
+++ b/neutron/tests/unit/plugins/ml2/test_plugin.py
@@ -1843,6 +1843,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):