diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 1ccee1f330f9..0a1162d16153 100644 --- a/nova/network/neutronv2/api.py +++ b/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. diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index 6286ba7a2f9f..309b4160a81b 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/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), - nw_info.get('profile')) + # If the requested port does not define a binding:profile, or + # 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):