Merge "neutron: handle binding:profile=None during migration"

nova/network/neutronv2/api.py

@@ -74,6 +74,19 @@ def _load_auth_plugin(conf):
     raise neutron_client_exc.Unauthorized(message=err_msg)
 
 
+def _get_binding_profile(port):
+    """Convenience method to get the binding:profile from the port
+
+    The binding:profile in the port is undefined in the networking service
+    API and is dependent on backend configuration. This means it could be
+    an empty dict, None, or have some values.
+
+    :param port: dict port response body from the networking service API
+    :returns: The port binding:profile dict; empty if not set on the port
+    """
+    return port.get(BINDING_PROFILE, {}) or {}
+
+
 @profiler.trace_cls("neutron_api")
 class ClientWrapper(clientv20.Client):
     """A Neutron client wrapper class.
@@ -260,7 +273,7 @@ class API(base_api.NetworkAPI):
             # If the port already has a migration profile and if
             # it is to be torn down, then we need to clean up
             # the migration profile.
-            port_profile = p.get(BINDING_PROFILE)
+            port_profile = _get_binding_profile(p)
             if not port_profile:
                 continue
             if MIGRATING_ATTR in port_profile:
@@ -281,7 +294,7 @@ class API(base_api.NetworkAPI):
             # the given 'host'.
             host_id = p.get(BINDING_HOST_ID)
             if host_id != host:
-                port_profile = p.get(BINDING_PROFILE, {})
+                port_profile = _get_binding_profile(p)
                 port_profile[MIGRATING_ATTR] = host
                 self._update_port_with_migration_profile(
                     instance, p['id'], port_profile, admin_client)
@@ -2204,7 +2217,7 @@ class API(base_api.NetworkAPI):
             mtu=network_mtu
             )
         network['subnets'] = subnets
-        port_profile = port.get(BINDING_PROFILE)
+        port_profile = _get_binding_profile(port)
         if port_profile:
             physical_network = port_profile.get('physical_network')
             if physical_network:
@@ -2271,7 +2284,7 @@ class API(base_api.NetworkAPI):
             vnic_type=current_neutron_port.get('binding:vnic_type',
                                                network_model.VNIC_TYPE_NORMAL),
             type=current_neutron_port.get('binding:vif_type'),
-            profile=current_neutron_port.get(BINDING_PROFILE),
+            profile=_get_binding_profile(current_neutron_port),
             details=current_neutron_port.get('binding:vif_details'),
             ovs_interfaceid=ovs_interfaceid,
             devname=devname,
@@ -2491,7 +2504,7 @@ class API(base_api.NetworkAPI):
         ports = data['ports']
         for p in ports:
             updates = {}
-            binding_profile = p.get(BINDING_PROFILE, {})
+            binding_profile = _get_binding_profile(p)
 
             # If the host hasn't changed, like in the case of resizing to the
             # same host, there is nothing to do.

nova/tests/unit/network/test_neutronv2.py

@@ -2964,8 +2964,11 @@ class TestNeutronv2(TestNeutronv2Base):
             self.assertEqual(requested_ports[index].get('binding:vif_details'),
                              nw_info.get('details'))
             self.assertEqual(
-                    requested_ports[index].get(neutronapi.BINDING_PROFILE),
+                # If the requested port does not define a binding:profile, or
-                    nw_info.get('profile'))
+                # has it set to None, we default to an empty dict to avoid
+                # NoneType errors.
+                requested_ports[index].get(neutronapi.BINDING_PROFILE) or {},
+                nw_info.get('profile'))
             index += 1
 
         self.assertFalse(nw_infos[0]['active'])
@@ -3882,6 +3885,31 @@ class TestNeutronv2WithMock(test.TestCase):
                                      neutronapi.BINDING_PROFILE: {
                                          'fake_profile': 'fake_data'}}})
 
+    @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
+    def test_update_port_bindings_for_instance_binding_profile_none(
+        self, get_client_mock):
+        """Tests _update_port_binding_for_instance when the binding:profile
+        value is None.
+        """
+        instance = fake_instance.fake_instance_obj(self.context)
+        self.api._has_port_binding_extension = mock.Mock(return_value=True)
+
+        fake_ports = {'ports': [
+                        {'id': uuids.portid,
+                         neutronapi.BINDING_PROFILE: None,
+                         neutronapi.BINDING_HOST_ID: instance.host}]}
+        list_ports_mock = mock.Mock(return_value=fake_ports)
+        get_client_mock.return_value.list_ports = list_ports_mock
+        update_port_mock = mock.Mock()
+        get_client_mock.return_value.update_port = update_port_mock
+
+        self.api._update_port_binding_for_instance(self.context, instance,
+                                                   'my-host')
+        # Assert that update_port was called on the port with a
+        # different host but with no binding profile.
+        update_port_mock.assert_called_once_with(
+            uuids.portid, {'port': {neutronapi.BINDING_HOST_ID: 'my-host'}})
+
     @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
     def test_update_port_bindings_for_instance_same_host(self,
                                                          get_client_mock):
@@ -4135,6 +4163,38 @@ class TestNeutronv2WithMock(test.TestCase):
             uuids.port_id,
             port_data)
 
+    @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
+    def test_update_port_profile_for_migration_teardown_false_none_profile(
+        self, get_client_mock):
+        """Tests setup_networks_on_host when migrating the port to the
+        destination host and the binding:profile is None in the port.
+        """
+        instance = fake_instance.fake_instance_obj(self.context)
+        self.api._has_port_binding_extension = mock.Mock(return_value=True)
+        # We test with an instance host and destination_host where the
+        # port will be moving but with binding:profile set to None.
+        get_ports = {
+            'ports': [
+                {'id': uuids.port_id,
+                 neutronapi.BINDING_HOST_ID: instance.host,
+                 neutronapi.BINDING_PROFILE: None}
+            ]
+        }
+        self.api.list_ports = mock.Mock(return_value=get_ports)
+        update_port_mock = mock.Mock()
+        get_client_mock.return_value.update_port = update_port_mock
+        migrate_profile = {neutronapi.MIGRATING_ATTR: 'my-new-host'}
+        port_data = {
+            'port': {
+                neutronapi.BINDING_PROFILE: migrate_profile
+            }
+        }
+        self.api.setup_networks_on_host(
+            self.context, instance, host='my-new-host', teardown=False)
+        update_port_mock.assert_called_once_with(
+            uuids.port_id,
+            port_data)
+
     @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
     def test__setup_migration_port_profile_called_on_teardown_false(
         self, get_client_mock):