Handle tags in _bury_in_cell0

We allowed add instance tags when booting in
Iac54b9627cb4398e45f1f15a2f4e7d6f86242093, conductor
will create tags for us in the selected cell, but
we forgot to consider the case that no valid host
was found.

So we will be unable to filter those instances in
cell0 with tags.

Conflicts:
      nova/conductor/manager.py

NOTE(mriedem): The conflict is due to not having change
I70b11dd489d222be3d70733355bfe7966df556aa in Pike.

Change-Id: Idfe37d356b349024dfe963c934728111e1a1d314
closes-bug: #1806515
(cherry picked from commit 3cd439ad8a99f122566f0b5d0264ff5c8d34f998)
(cherry picked from commit cbb9eb5b056b96ea507d65fea0e32c27bbf6cdfa)
(cherry picked from commit fa178ed4e5ab925d39f7176d851d3c735e328189)

4 files changed, 109 insertions, 8 deletions

diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index bbfc15746e..d1615c629d 100644
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@@ -990,7 +990,8 @@ class ComputeTaskManager(base.Base):
 
     def _bury_in_cell0(self, context, request_spec, exc,
                        build_requests=None, instances=None,
-                       block_device_mapping=None):
+                       block_device_mapping=None,
+                       tags=None):
         """Ensure all provided build_requests and instances end up in cell0.
 
         Cell0 is the fake cell we schedule dead instances to when we can't
@@ -1035,6 +1036,8 @@ class ComputeTaskManager(base.Base):
                        cell0, instance.flavor, instance.uuid,
                        block_device_mapping)
 
+                self._create_tags(cctxt, instance.uuid, tags)
+
                 # Use the context targeted to cell0 here since the instance is
                 # now in cell0.
                 self._set_vm_state_and_notify(
@@ -1072,11 +1075,10 @@ class ComputeTaskManager(base.Base):
                                              instance_uuids)
         except Exception as exc:
             LOG.exception('Failed to schedule instances')
-            # FIXME(mriedem): If the tags are not persisted with the instance
-            # in cell0 then the API will not show them.
             self._bury_in_cell0(context, request_specs[0], exc,
                                 build_requests=build_requests,
-                                block_device_mapping=block_device_mapping)
+                                block_device_mapping=block_device_mapping,
+                                tags=tags)
             return
 
         host_mapping_cache = {}
@@ -1096,12 +1098,11 @@ class ComputeTaskManager(base.Base):
                     LOG.error('No host-to-cell mapping found for selected '
                               'host %(host)s. Setup is incomplete.',
                               {'host': host['host']})
-                    # FIXME(mriedem): If the tags are not persisted with the
-                    # instance in cell0 then the API will not show them.
                     self._bury_in_cell0(
                         context, request_spec, exc,
                         build_requests=[build_request], instances=[instance],
-                        block_device_mapping=block_device_mapping)
+                        block_device_mapping=block_device_mapping,
+                        tags=tags)
                     # This is a placeholder in case the quota recheck fails.
                     instances.append(None)
                     continue
diff --git a/nova/tests/functional/api/client.py b/nova/tests/functional/api/client.py
index 73fcc3ac76..3e76bf4f20 100644
--- a/nova/tests/functional/api/client.py
+++ b/nova/tests/functional/api/client.py
@@ -437,6 +437,14 @@ class TestOpenStackClient(object):
     def get_limits(self):
         return self.api_get('/limits').body['limits']
 
+    def get_server_tags(self, server_id):
+        """Get the tags on the given server.
+
+        :param server_id: The server uuid
+        :return: The list of tags from the response
+        """
+        return self.api_get('/servers/%s/tags' % server_id).body['tags']
+
     def put_server_tags(self, server_id, tags):
         """Put (or replace) a list of tags on the given server.
 
diff --git a/nova/tests/functional/regressions/test_bug_1806515.py b/nova/tests/functional/regressions/test_bug_1806515.py
new file mode 100644
index 0000000000..9c03b3c5b3
--- /dev/null
+++ b/nova/tests/functional/regressions/test_bug_1806515.py
@@ -0,0 +1,69 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from nova import test
+from nova.tests import fixtures as nova_fixtures
+from nova.tests.functional import integrated_helpers
+from nova.tests.unit import fake_notifier
+import nova.tests.unit.image.fake
+
+
+class ShowErrorServerWithTags(test.TestCase,
+                              integrated_helpers.InstanceHelperMixin):
+    """Test list of an instance in error state that has tags.
+
+    This test boots a server with tag which will fail to be scheduled,
+    ending up in ERROR state with no host assigned and then show the server.
+    """
+
+    def setUp(self):
+        super(ShowErrorServerWithTags, self).setUp()
+
+        api_fixture = self.useFixture(nova_fixtures.OSAPIFixture(
+            api_version='v2.1'))
+        self.api = api_fixture.admin_api
+
+        self.useFixture(nova_fixtures.NeutronFixture(self))
+
+        self.start_service('conductor')
+        self.start_service('scheduler')
+
+        # the image fake backend needed for image discovery
+        nova.tests.unit.image.fake.stub_out_image_service(self)
+        self.addCleanup(nova.tests.unit.image.fake.FakeImageService_reset)
+
+        self.image_id = self.api.get_images()[0]['id']
+
+        self.api.microversion = 'latest'
+
+        self.addCleanup(fake_notifier.reset)
+
+    def _create_error_server(self):
+        server = self.api.post_server({
+            'server': {
+                'flavorRef': '1',
+                'name': 'show-server-with-tag-in-error-status',
+                'networks': 'none',
+                'tags': ['tag1'],
+                'imageRef': self.image_id
+            }
+        })
+        return self._wait_for_state_change(self.api, server, 'ERROR')
+
+    def test_show_server_tag_in_error(self):
+        # Create a server which should go to ERROR state because we don't
+        # have any active computes.
+        server = self._create_error_server()
+        server_id = server['id']
+
+        tags = self.api.get_server_tags(server_id)
+        self.assertIn('tag1', tags)
diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py
index 0b3f568eda..26fd651375 100644
--- a/nova/tests/unit/conductor/test_conductor.py
+++ b/nova/tests/unit/conductor/test_conductor.py
@@ -1810,7 +1810,8 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
                                                                build_and_run):
         def _fake_bury(ctxt, request_spec, exc,
                        build_requests=None, instances=None,
-                       block_device_mapping=None):
+                       block_device_mapping=None,
+                       tags=None):
             self.assertIn('not mapped to any cell', str(exc))
             self.assertEqual(1, len(build_requests))
             self.assertEqual(1, len(instances))
@@ -1818,6 +1819,8 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
                              instances[0].uuid)
             self.assertEqual(self.params['block_device_mapping'],
                              block_device_mapping)
+            self.assertEqual(self.params['tags'],
+                             tags)
 
         bury.side_effect = _fake_bury
         select_dest.return_value = [{'host': 'missing-host',
@@ -1985,6 +1988,26 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
             self.cell_mappings['cell0'], inst.flavor, inst.uuid,
             self.params['block_device_mapping'])
 
+    @mock.patch.object(objects.CellMapping, 'get_by_uuid')
+    @mock.patch.object(conductor_manager.ComputeTaskManager,
+                       '_create_tags')
+    def test_bury_in_cell0_with_tags(self, mock_create_tags, mock_get_cell):
+        mock_get_cell.return_value = self.cell_mappings['cell0']
+
+        inst_br = fake_build_request.fake_req_obj(self.ctxt)
+        del inst_br.instance.id
+        inst_br.create()
+        inst = inst_br.get_new_instance(self.ctxt)
+
+        self.conductor._bury_in_cell0(
+            self.ctxt, self.params['request_specs'][0], Exception('Foo'),
+            build_requests=[inst_br], instances=[inst],
+            tags=self.params['tags'])
+
+        mock_create_tags.assert_called_once_with(
+            test.MatchType(context.RequestContext), inst.uuid,
+            self.params['tags'])
+
     def test_reset(self):
         with mock.patch('nova.compute.rpcapi.ComputeAPI') as mock_rpc:
             old_rpcapi = self.conductor_manager.compute_rpcapi