diff --git a/devstack/nova-multi-cell-blacklist.txt b/devstack/nova-multi-cell-blacklist.txt index 87f07be42603..afda00fae1b2 100644 --- a/devstack/nova-multi-cell-blacklist.txt +++ b/devstack/nova-multi-cell-blacklist.txt @@ -4,6 +4,3 @@ # Exclude tempest.scenario.test_network tests since they are slow and # only test advanced neutron features, unrelated to multi-cell testing. ^tempest.scenario.test_network - -# Disable associate floating IP tests until bug 1826472 is fixed. -tempest.api.compute.floating_ips.test_floating_ips_actions.FloatingIPsAssociationTestJSON diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index e803131021d8..54f0e5779127 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -29,6 +29,7 @@ import six from nova.compute import utils as compute_utils import nova.conf +from nova import context as nova_context from nova import exception from nova.i18n import _ from nova.network import base_api @@ -2366,12 +2367,59 @@ class API(base_api.NetworkAPI): LOG.info('re-assign floating IP %(address)s from ' 'instance %(instance_id)s', msg_dict, instance=instance) - orig_instance = objects.Instance.get_by_uuid(context, - orig_instance_uuid) + orig_instance = self._get_instance_by_uuid_using_api_db( + context, orig_instance_uuid) + if orig_instance: + # purge cached nw info for the original instance; pass the + # context from the instance in case we found it in another cell + base_api.update_instance_cache_with_nw_info( + self, orig_instance._context, orig_instance) + else: + # Leave a breadcrumb about not being able to refresh the + # the cache for the original instance. + LOG.info('Unable to refresh the network info cache for ' + 'instance %s after disassociating floating IP %s. ' + 'If the instance still exists, its info cache may ' + 'be healed automatically.', + orig_instance_uuid, fip['id']) - # purge cached nw info for the original instance - base_api.update_instance_cache_with_nw_info(self, context, - orig_instance) + @staticmethod + def _get_instance_by_uuid_using_api_db(context, instance_uuid): + """Look up the instance by UUID + + This method is meant to be used sparingly since it tries to find + the instance by UUID in the cell-targeted context. If the instance + is not found, this method will try to determine if it's not found + because it is deleted or if it is just in another cell. Therefore + it assumes to have access to the API database and should only be + called from methods that are used in the control plane services. + + :param context: cell-targeted nova auth RequestContext + :param instance_uuid: UUID of the instance to find + :returns: Instance object if the instance was found, else None. + """ + try: + return objects.Instance.get_by_uuid(context, instance_uuid) + except exception.InstanceNotFound: + # The instance could be deleted or it could be in another cell. + # To determine if its in another cell, check the instance + # mapping in the API DB. + try: + inst_map = objects.InstanceMapping.get_by_instance_uuid( + context, instance_uuid) + except exception.InstanceMappingNotFound: + # The instance is gone so just return. + return + + # We have the instance mapping, look up the instance in the + # cell the instance is in. + with nova_context.target_cell( + context, inst_map.cell_mapping) as cctxt: + try: + return objects.Instance.get_by_uuid(cctxt, instance_uuid) + except exception.InstanceNotFound: + # Alright it's really gone. + return def get_all(self, context): """Get all networks for client.""" diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index f5fe81a7d384..6c431bad22ae 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -5804,6 +5804,174 @@ class TestNeutronv2WithMock(TestNeutronv2Base): mock_update_cache.assert_called_once_with( # from @refresh_cache self.api, ctxt, instance, nw_info=None) + @mock.patch('nova.network.base_api.update_instance_cache_with_nw_info') + def test_update_inst_info_cache_for_disassociated_fip_other_cell( + self, mock_update_cache): + """Tests a scenario where a floating IP is associated to an instance + in another cell from the one in which it's currently associated + and the network info cache on the original instance is refreshed. + """ + ctxt = context.get_context() + mock_client = mock.Mock() + new_instance = fake_instance.fake_instance_obj(ctxt) + cctxt = context.get_context() + old_instance = fake_instance.fake_instance_obj(cctxt) + fip = {'id': uuids.floating_ip_id, + 'port_id': uuids.old_port_id, + 'floating_ip_address': '172.24.5.15'} + # Setup the mocks. + with test.nested( + mock.patch.object(self.api, '_show_port', + return_value={ + 'device_id': old_instance.uuid}), + mock.patch.object(self.api, '_get_instance_by_uuid_using_api_db', + return_value=old_instance) + ) as ( + _show_port, _get_instance_by_uuid_using_api_db + ): + # Run the code. + self.api._update_inst_info_cache_for_disassociated_fip( + ctxt, new_instance, mock_client, fip) + # Assert the calls. + _show_port.assert_called_once_with( + ctxt, uuids.old_port_id, neutron_client=mock_client) + _get_instance_by_uuid_using_api_db.assert_called_once_with( + ctxt, old_instance.uuid) + mock_update_cache.assert_called_once_with( + self.api, cctxt, old_instance) + + @mock.patch('nova.network.neutronv2.api.LOG.info') + @mock.patch('nova.network.base_api.update_instance_cache_with_nw_info') + def test_update_inst_info_cache_for_disassociated_fip_inst_not_found( + self, mock_update_cache, mock_log_info): + """Tests the case that a floating IP is re-associated to an instance + in another cell but the original instance cannot be found. + """ + ctxt = context.get_context() + mock_client = mock.Mock() + new_instance = fake_instance.fake_instance_obj(ctxt) + fip = {'id': uuids.floating_ip_id, + 'port_id': uuids.old_port_id, + 'floating_ip_address': '172.24.5.15'} + # Setup the mocks. + with test.nested( + mock.patch.object(self.api, '_show_port', + return_value={ + 'device_id': uuids.original_inst_uuid}), + mock.patch.object(self.api, + '_get_instance_by_uuid_using_api_db', + return_value=None) + ) as ( + _show_port, _get_instance_by_uuid_using_api_db + ): + # Run the code. + self.api._update_inst_info_cache_for_disassociated_fip( + ctxt, new_instance, mock_client, fip) + # Assert the calls. + _show_port.assert_called_once_with( + ctxt, uuids.old_port_id, neutron_client=mock_client) + _get_instance_by_uuid_using_api_db.assert_called_once_with( + ctxt, uuids.original_inst_uuid) + mock_update_cache.assert_not_called() + self.assertEqual(2, mock_log_info.call_count) + self.assertIn('If the instance still exists, its info cache may ' + 'be healed automatically.', + mock_log_info.call_args[0][0]) + + @mock.patch('nova.objects.Instance.get_by_uuid') + @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') + def test_get_instance_by_uuid_using_api_db_current_cell( + self, mock_get_map, mock_get_inst): + """Tests that _get_instance_by_uuid_using_api_db finds the + instance in the cell currently targeted by the context. + """ + ctxt = context.get_context() + instance = fake_instance.fake_instance_obj(ctxt) + mock_get_inst.return_value = instance + inst = self.api._get_instance_by_uuid_using_api_db(ctxt, instance.uuid) + self.assertIs(inst, instance) + mock_get_inst.assert_called_once_with(ctxt, instance.uuid) + mock_get_map.assert_not_called() + + @mock.patch('nova.objects.Instance.get_by_uuid') + @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') + def test_get_instance_by_uuid_using_api_db_other_cell( + self, mock_get_map, mock_get_inst): + """Tests that _get_instance_by_uuid_using_api_db finds the + instance in another cell different from the currently targeted context. + """ + ctxt = context.get_context() + instance = fake_instance.fake_instance_obj(ctxt) + mock_get_map.return_value = objects.InstanceMapping( + cell_mapping=objects.CellMapping( + uuid=uuids.cell_mapping_uuid, + database_connection= + self.cell_mappings['cell1'].database_connection, + transport_url='none://fake')) + # Mock get_by_uuid to not find the instance in the first call, but + # do find it in the second. Target the instance context as well so + # we can assert that we used a different context for the 2nd call. + + def stub_inst_get_by_uuid(_context, instance_uuid, *args, **kwargs): + if not mock_get_map.called: + # First call, raise InstanceNotFound. + self.assertIs(_context, ctxt) + raise exception.InstanceNotFound(instance_id=instance_uuid) + # Else return the instance with a newly targeted context. + self.assertIsNot(_context, ctxt) + instance._context = _context + return instance + mock_get_inst.side_effect = stub_inst_get_by_uuid + + inst = self.api._get_instance_by_uuid_using_api_db(ctxt, instance.uuid) + # The instance's internal context should still be targeted and not + # the original context. + self.assertIsNot(inst._context, ctxt) + self.assertIsNotNone(inst._context.db_connection) + mock_get_map.assert_called_once_with(ctxt, instance.uuid) + mock_get_inst.assert_has_calls([ + mock.call(ctxt, instance.uuid), + mock.call(inst._context, instance.uuid)]) + + @mock.patch('nova.objects.Instance.get_by_uuid', + side_effect=exception.InstanceNotFound(instance_id=uuids.inst)) + @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') + def test_get_instance_by_uuid_using_api_db_other_cell_never_found( + self, mock_get_map, mock_get_inst): + """Tests that _get_instance_by_uuid_using_api_db does not find the + instance in either the current cell or another cell. + """ + ctxt = context.get_context() + instance = fake_instance.fake_instance_obj(ctxt, uuid=uuids.inst) + mock_get_map.return_value = objects.InstanceMapping( + cell_mapping=objects.CellMapping( + uuid=uuids.cell_mapping_uuid, + database_connection= + self.cell_mappings['cell1'].database_connection, + transport_url='none://fake')) + self.assertIsNone( + self.api._get_instance_by_uuid_using_api_db(ctxt, instance.uuid)) + mock_get_map.assert_called_once_with(ctxt, instance.uuid) + mock_get_inst.assert_has_calls([ + mock.call(ctxt, instance.uuid), + mock.call(test.MatchType(context.RequestContext), instance.uuid)]) + + @mock.patch('nova.objects.Instance.get_by_uuid', + side_effect=exception.InstanceNotFound(instance_id=uuids.inst)) + @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid', + side_effect=exception.InstanceMappingNotFound(uuid=uuids.inst)) + def test_get_instance_by_uuid_using_api_db_other_cell_map_not_found( + self, mock_get_map, mock_get_inst): + """Tests that _get_instance_by_uuid_using_api_db does not find an + instance mapping for the instance. + """ + ctxt = context.get_context() + instance = fake_instance.fake_instance_obj(ctxt, uuid=uuids.inst) + self.assertIsNone( + self.api._get_instance_by_uuid_using_api_db(ctxt, instance.uuid)) + mock_get_inst.assert_called_once_with(ctxt, instance.uuid) + mock_get_map.assert_called_once_with(ctxt, instance.uuid) + @mock.patch('nova.network.neutronv2.api._get_ksa_client', new_callable=mock.NonCallableMock) # asserts not called def test_migrate_instance_start_no_binding_ext(self, get_client_mock):