From e33f4042c1292fc630fae6740331ccfb8c074717 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Thu, 17 Oct 2019 21:29:46 +0100 Subject: [PATCH] Functional test for UnexpectedDeletingTaskStateError Adds a regression-style test for two cleanup bugs when 'UnexpectedDeletingTaskStateError' is raised during build. Modified: nova/tests/functional/regressions/test_bug_1831771.py NOTE(stephenfin): Modifications are necessary since we don't have change Iea283322124cb35fc0bc6d25f35548621e8c8c2f, which moved the 'ProviderUsageBaseTestCase' base test class from 'test_servers.py' to 'integrated_helpers.py'. Change-Id: Ief1dfbb6cc9d67b73dfab4c7b63358e76e12866b Related-Bug: #1848666 Related-Bug: #1831771 (cherry picked from commit 10434bd229973b37647741e58aff3ac90b3a0a6c) (cherry picked from commit 24600430a2e4c67a4584d1ee466d3376aae96f25) (cherry picked from commit ff2101654376993f248abca03e123d7233af4562) (cherry picked from commit 5a17dd1e34caa44ce8a1406aea9377d8f04421ed) --- nova/tests/functional/integrated_helpers.py | 2 +- .../regressions/test_bug_1831771.py | 104 ++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 nova/tests/functional/regressions/test_bug_1831771.py diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 941a4015e8d3..3c831fd1d6f3 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -270,7 +270,7 @@ class InstanceHelperMixin(object): return server def _wait_until_deleted(self, server): - initially_in_error = (server['status'] == 'ERROR') + initially_in_error = server.get('status') == 'ERROR' try: for i in range(40): server = self.api.get_server(server['id']) diff --git a/nova/tests/functional/regressions/test_bug_1831771.py b/nova/tests/functional/regressions/test_bug_1831771.py new file mode 100644 index 000000000000..447c595f48c6 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_1831771.py @@ -0,0 +1,104 @@ +# Copyright 2019, Red Hat, Inc. All Rights Reserved. +# +# 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. + +import collections + +import mock + +from nova.compute import task_states +from nova.compute import vm_states +from nova import objects +from nova import test +from nova.tests.functional import test_servers +from nova.tests.unit.image import fake as fake_image + + +class TestDelete(test_servers.ProviderUsageBaseTestCase): + compute_driver = 'fake.MediumFakeDriver' + + def test_delete_during_create(self): + compute = self._start_compute('compute1') + + def delete_race(instance): + self.api.delete_server(instance.uuid) + self._wait_for_server_parameter( + self.api, + {'id': instance.uuid}, + {'OS-EXT-STS:task_state': task_states.DELETING}, + ) + + orig_save = objects.Instance.save + + # an in-memory record of the current instance task state as persisted + # to the db. + db_task_states = collections.defaultdict(str) + active_after_deleting_error = [False] + + # A wrapper round instance.save() which allows us to inject a race + # under specific conditions before calling the original instance.save() + def wrap_save(instance, *wrap_args, **wrap_kwargs): + # We're looking to inject the race before: + # instance.save(expected_task_state=task_states.SPAWNING) + # towards the end of _build_and_run_instance. + # + # At this point the driver has finished creating the instance, but + # we're still on the compute host and still holding the compute + # host instance lock. + # + # This is just a convenient interception point. In order to race + # the delete could have happened at any point prior to this since + # the previous instance.save() + expected_task_state = wrap_kwargs.get('expected_task_state') + if ( + expected_task_state == task_states.SPAWNING + ): + delete_race(instance) + + orig_save(instance, *wrap_args, **wrap_kwargs) + + if ( + db_task_states[instance.uuid] == task_states.DELETING and + instance.vm_state == vm_states.ACTIVE and + instance.task_state is None + ): + # the instance was in the DELETING task state in the db, and we + # overwrote that to set it to ACTIVE with no task state. + # Bug 1848666. + active_after_deleting_error[0] = True + + db_task_states[instance.uuid] = instance.task_state + + with test.nested( + mock.patch('nova.objects.Instance.save', wrap_save), + mock.patch.object(compute.driver, 'spawn'), + mock.patch.object(compute.driver, 'unplug_vifs'), + ) as (_, mock_spawn, mock_unplug_vifs): + # the compute manager doesn't set the ERROR state in cleanup since + # it might race with delete, therefore we'll be left in BUILDING + server_req = self._build_minimal_create_server_request( + self.api, 'sample-server', + image_uuid=fake_image.get_valid_image_id(), + networks='none', + ) + created_server = self.api.post_server({'server': server_req}) + self._wait_until_deleted(created_server) + + # assert that we spawned the instance, and unplugged vifs on + # cleanup + mock_spawn.assert_called() + # FIXME(mdbooth): We should have called unplug_vifs in cleanup, but + # we didn't due to bug 1831771 + mock_unplug_vifs.assert_not_called() + # FIXME(mdbooth): Bug 1848666 + self.assertTrue(active_after_deleting_error[0])