Prevent port update from binding a host where IPs won't work

In the context of a routed network, if a port has fixed ips already
then those fixed ips can only be used on the segment with that subnet
on it. If a port update tries to update the host binding to a
different segment, an exception will now be raised.

Change-Id: I8dc8890907d1e241dd12448fa184cea1b0620663
Partially-Implements: blueprint routed-networks

neutron/db/ipam_backend_mixin.py

@@ -615,10 +615,21 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
                                             new_port.get('mac_address'))
 
         fixed_ips_requested = validators.is_attr_set(new_port.get('fixed_ips'))
+        old_ips = old_port.get('fixed_ips')
         deferred_ip_allocation = (host and not old_host
-                                  and not old_port.get('fixed_ips')
+                                  and not old_ips
                                   and not fixed_ips_requested)
         if not deferred_ip_allocation:
+            # Check that any existing IPs are valid on the new segment
+            new_host_requested = host and host != old_host
+            if old_ips and new_host_requested and not fixed_ips_requested:
+                valid_subnets = self._ipam_get_subnets(
+                    context, old_port['network_id'], host)
+                valid_subnet_ids = {s['id'] for s in valid_subnets}
+                for fixed_ip in old_ips:
+                    if fixed_ip['subnet_id'] not in valid_subnet_ids:
+                        raise segment_exc.HostNotCompatibleWithFixedIps(
+                            host=host, port_id=old_port['id'])
             return changes
 
         # Allocate as if this were the port create.

neutron/services/segments/exceptions.py

@@ -48,3 +48,9 @@ class HostNotConnectedToAnySegment(exceptions.Conflict):
     message = _("Host %(host)s is not connected to any segments on routed "
                 "provider network '%(network_id)s'.  It should be connected "
                 "to one.")
+
+
+class HostNotCompatibleWithFixedIps(exceptions.Conflict):
+    message = _("Host %(host)s is not connected to a segment where the "
+                "existing fixed_ips on port %(port_id)s will function given "
+                "the routed network topology.")

neutron/tests/unit/extensions/test_segment.py

@@ -129,6 +129,13 @@ class SegmentTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
             context, port['port'], port_dict)
         return port_dict
 
+    def update_port(self, context, id, port):
+        port_dict = super(SegmentTestPlugin, self).update_port(
+            context, id, port)
+        self._process_portbindings_create_and_update(
+            context, port['port'], port_dict)
+        return port_dict
+
 
 class TestSegment(SegmentTestCase):
 
@@ -1036,6 +1043,55 @@ class TestSegmentAwareIpam(SegmentTestCase):
         self.assertEqual(n_exc.IpAddressGenerationFailure.__name__,
                          res['NeutronError']['type'])
 
+    def test_port_update_fails_if_host_on_wrong_segment(self):
+        """Update a port with existing IPs to a host where they don't work"""
+        network, segments, subnets = self._create_test_segments_with_subnets(2)
+
+        self._setup_host_mappings([(segments[0]['segment']['id'], 'fakehost2'),
+                                   (segments[1]['segment']['id'], 'fakehost')])
+
+        # Create a bound port with an IP address
+        response = self._create_port(self.fmt,
+                                     net_id=network['network']['id'],
+                                     tenant_id=network['network']['tenant_id'],
+                                     arg_list=(portbindings.HOST_ID,),
+                                     **{portbindings.HOST_ID: 'fakehost'})
+        self._assert_one_ip_in_subnet(response, subnets[1]['subnet']['cidr'])
+        port = self.deserialize(self.fmt, response)
+
+        # Now, try to update binding to a host on the other segment
+        data = {'port': {portbindings.HOST_ID: 'fakehost2'}}
+        port_req = self.new_update_request('ports', data, port['port']['id'])
+        response = port_req.get_response(self.api)
+
+        # It fails since the IP address isn't compatible with the new segment
+        self.assertEqual(webob.exc.HTTPConflict.code, response.status_int)
+
+    def test_port_update_fails_if_host_on_good_segment(self):
+        """Update a port with existing IPs to a host where they don't work"""
+        network, segments, subnets = self._create_test_segments_with_subnets(2)
+
+        self._setup_host_mappings([(segments[0]['segment']['id'], 'fakehost2'),
+                                   (segments[1]['segment']['id'], 'fakehost1'),
+                                   (segments[1]['segment']['id'], 'fakehost')])
+
+        # Create a bound port with an IP address
+        response = self._create_port(self.fmt,
+                                     net_id=network['network']['id'],
+                                     tenant_id=network['network']['tenant_id'],
+                                     arg_list=(portbindings.HOST_ID,),
+                                     **{portbindings.HOST_ID: 'fakehost'})
+        self._assert_one_ip_in_subnet(response, subnets[1]['subnet']['cidr'])
+        port = self.deserialize(self.fmt, response)
+
+        # Now, try to update binding to another host in same segment
+        data = {'port': {portbindings.HOST_ID: 'fakehost1'}}
+        port_req = self.new_update_request('ports', data, port['port']['id'])
+        response = port_req.get_response(self.api)
+
+        # Since the new host is in the same segment, it succeeds.
+        self.assertEqual(webob.exc.HTTPOk.code, response.status_int)
+
 
 class TestSegmentAwareIpamML2(TestSegmentAwareIpam):
     def setUp(self):