Merge "Find instance in another cell during floating IP re-association"

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

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'])
+
+    @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
 
-        # purge cached nw info for the original instance
-        base_api.update_instance_cache_with_nw_info(self, context,
-                                                    orig_instance)
+            # 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."""

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):