Refactor update_port in db_base_plugin_v2

This commit is a preparation step for using pluggable IPAM.
- moved validations into _validate_port_for_update;
- updating ip addresses for port is backend specific, so
  moved into _update_port_with_ips in ipam_non_pluggable_backend;
- writing port changes to db is common for both backends, so
  moved into _update_db_port in ipam_backend_mixin;
- updated to use namedtuple to track add/original/remove ips;
- added _make_fixed_ip_dict to exclude keys other than
  ip_address and subnet_id;

Partially-Implements: blueprint neutron-ipam

Change-Id: I1110e88f372b1d0cc7ec72049ba69a6d548da867

neutron/db/db_base_plugin_common.py

@@ -245,6 +245,12 @@ class DbBasePluginCommon(common_db_mixin.CommonDbMixin):
                 args['ipv6_address_mode'] = subnet['ipv6_address_mode']
         return args
 
+    def _make_fixed_ip_dict(self, ips):
+        # Excludes from dict all keys except subnet_id and ip_address
+        return [{'subnet_id': ip["subnet_id"],
+                 'ip_address': ip["ip_address"]}
+                for ip in ips]
+
     def _gateway_ip_str(self, subnet, cidr_net):
         if subnet.get('gateway_ip') is attributes.ATTR_NOT_SPECIFIED:
                 return str(cidr_net.network + 1)

neutron/db/db_base_plugin_v2.py

@@ -1139,54 +1139,36 @@ class NeutronDbPluginV2(ipam_non_pluggable_backend.IpamNonPluggableBackend,
 
         return self._make_port_dict(db_port, process_extensions=False)
 
-    def update_port(self, context, id, port):
-        p = port['port']
+    def _validate_port_for_update(self, context, db_port, new_port, new_mac):
+        changed_owner = 'device_owner' in new_port
+        current_owner = (new_port.get('device_owner') or
+                         db_port['device_owner'])
+        changed_device_id = new_port.get('device_id') != db_port['device_id']
+        current_device_id = new_port.get('device_id') or db_port['device_id']
+
+        if current_owner and changed_device_id or changed_owner:
+            self._enforce_device_owner_not_router_intf_or_device_id(
+                context, current_owner, current_device_id,
+                db_port['tenant_id'])
+
+        if new_mac and new_mac != db_port['mac_address']:
+            self._check_mac_addr_update(context, db_port,
+                                        new_mac, current_owner)
+
+    def update_port(self, context, id, port):
+        new_port = port['port']
 
-        changed_ips = False
         with context.session.begin(subtransactions=True):
             port = self._get_port(context, id)
-            changed_owner = 'device_owner' in p
-            current_owner = p.get('device_owner') or port['device_owner']
-            changed_device_id = p.get('device_id') != port['device_id']
-            current_device_id = p.get('device_id') or port['device_id']
-
-            if current_owner and changed_device_id or changed_owner:
-                self._enforce_device_owner_not_router_intf_or_device_id(
-                    context, current_owner, current_device_id,
-                    port['tenant_id'])
-
-            new_mac = p.get('mac_address')
-            if new_mac and new_mac != port['mac_address']:
-                self._check_mac_addr_update(
-                    context, port, new_mac, current_owner)
-
-            # Check if the IPs need to be updated
-            network_id = port['network_id']
-            if 'fixed_ips' in p:
-                changed_ips = True
-                original = self._make_port_dict(port, process_extensions=False)
-                changes = self._update_ips_for_port(
-                    context, network_id,
-                    original["fixed_ips"], p['fixed_ips'],
-                    original['mac_address'], port['device_owner'])
-
-                # Update ips if necessary
-                for ip in changes.add:
-                    NeutronDbPluginV2._store_ip_allocation(
-                        context, ip['ip_address'], network_id,
-                        ip['subnet_id'], port.id)
-                # Remove all attributes in p which are not in the port DB model
-                # and then update the port
-            try:
-                port.update(self._filter_non_model_columns(p, models_v2.Port))
-                context.session.flush()
-            except db_exc.DBDuplicateEntry:
-                raise n_exc.MacAddressInUse(net_id=network_id, mac=new_mac)
-
+            new_mac = new_port.get('mac_address')
+            self._validate_port_for_update(context, port, new_port, new_mac)
+            changes = self._update_port_with_ips(context, port,
+                                                 new_port, new_mac)
         result = self._make_port_dict(port)
         # Keep up with fields that changed
-        if changed_ips:
-            result['fixed_ips'] = changes.original + changes.add
+        if changes.original or changes.add or changes.remove:
+            result['fixed_ips'] = self._make_fixed_ip_dict(
+                changes.original + changes.add)
         return result
 
     def delete_port(self, context, id):

neutron/db/ipam_backend_mixin.py

@@ -17,6 +17,7 @@ import collections
 
 import netaddr
 from oslo_config import cfg
+from oslo_db import exception as db_exc
 from oslo_log import log as logging
 
 from neutron.common import constants
@@ -36,6 +37,16 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
     # Tracks changes in ip allocation for port using namedtuple
     Changes = collections.namedtuple('Changes', 'add original remove')
 
+    def _update_db_port(self, context, db_port, new_port, network_id, new_mac):
+        # Remove all attributes in new_port which are not in the port DB model
+        # and then update the port
+        try:
+            db_port.update(self._filter_non_model_columns(new_port,
+                                                          models_v2.Port))
+            context.session.flush()
+        except db_exc.DBDuplicateEntry:
+            raise n_exc.MacAddressInUse(net_id=network_id, mac=new_mac)
+
     def _update_subnet_host_routes(self, context, id, s):
 
         def _combine(ht):

neutron/db/ipam_non_pluggable_backend.py

@@ -182,6 +182,25 @@ class IpamNonPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
             return True
         return False
 
+    def _update_port_with_ips(self, context, db_port, new_port, new_mac):
+        changes = self.Changes(add=[], original=[], remove=[])
+        # Check if the IPs need to be updated
+        network_id = db_port['network_id']
+        if 'fixed_ips' in new_port:
+            original = self._make_port_dict(db_port, process_extensions=False)
+            changes = self._update_ips_for_port(
+                context, network_id,
+                original["fixed_ips"], new_port['fixed_ips'],
+                original['mac_address'], db_port['device_owner'])
+
+            # Update ips if necessary
+            for ip in changes.add:
+                IpamNonPluggableBackend._store_ip_allocation(
+                    context, ip['ip_address'], network_id,
+                    ip['subnet_id'], db_port.id)
+        self._update_db_port(context, db_port, new_port, network_id, new_mac)
+        return changes
+
     def _test_fixed_ips_for_port(self, context, network_id, fixed_ips,
                                  device_owner):
         """Test fixed IPs for port.