diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index d887ee95b9df..3a50186d7906 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -1174,7 +1174,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 @@ -1218,6 +1219,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( @@ -1255,11 +1258,10 @@ class ComputeTaskManager(base.Base): instance_uuids, return_alternates=True) 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 = {} @@ -1283,12 +1285,11 @@ class ComputeTaskManager(base.Base): LOG.error('No host-to-cell mapping found for selected ' 'host %(host)s. Setup is incomplete.', {'host': host.service_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 718e8ae37ad9..ce187d19e4d8 100644 --- a/nova/tests/functional/api/client.py +++ b/nova/tests/functional/api/client.py @@ -442,6 +442,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 000000000000..9c03b3c5b3ed --- /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 d2acb9be9f15..f17bbac83147 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1985,7 +1985,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)) @@ -1993,6 +1994,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 = [[fake_selection1]] @@ -2209,6 +2212,28 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): 'image'): self.assertIn(key, request_spec_dict) + @mock.patch('nova.compute.utils.notify_about_compute_task_error') + @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_notify): + 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