From 641feb62a33e211a3ed1f9a2309ee16d827f5fe2 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Mon, 8 Jul 2019 18:35:11 +0000 Subject: [PATCH] make config drives sticky bug 1835822 This change reorders the call in _update_instance_after_spawn so that we call configdrive.update_instance before we set instance.launched_at. This ensures that if the vm is booted with a config drive because the host had force_config_drive=true the instance will keep its config drive across reboots. This change fixes a regression introduced as part of fixing bug #1827492 which addressed failing to boot vms after changing force_config_drive=false to force_config_drive=true. Change-Id: I9194423f5f95e9799bd891548e24756131d65e76 Related-Bug: #1827492 Closes-Bug: #1835822 --- nova/compute/manager.py | 9 +- .../regressions/test_bug_1835822.py | 191 ++++++++++++++++++ 2 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 nova/tests/functional/regressions/test_bug_1835822.py diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 1d69ee604c58..c42382b19969 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1750,8 +1750,15 @@ class ComputeManager(manager.Manager): instance.power_state = self._get_power_state(context, instance) instance.vm_state = vm_states.ACTIVE instance.task_state = None - instance.launched_at = timeutils.utcnow() + # NOTE(sean-k-mooney): configdrive.update_instance checks + # instance.launched_at to determine if it is the first or + # subsequent spawn of an instance. We need to call update_instance + # first before setting instance.launched_at or instance.config_drive + # will never be set to true based on the value of force_config_drive. + # As a result the config drive will be lost on a hard reboot of the + # instance even when force_config_drive=true. see bug #1835822. configdrive.update_instance(instance) + instance.launched_at = timeutils.utcnow() def _update_scheduler_instance_info(self, context, instance): """Sends an InstanceList with created or updated Instance objects to diff --git a/nova/tests/functional/regressions/test_bug_1835822.py b/nova/tests/functional/regressions/test_bug_1835822.py new file mode 100644 index 000000000000..f2ff3e63ce02 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_1835822.py @@ -0,0 +1,191 @@ +# 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 fixtures as func_fixtures +from nova.tests.functional import integrated_helpers +from nova.tests.unit import fake_notifier +from nova.tests.unit.image import fake as fake_image +from nova.tests.unit import policy_fixture + + +class RegressionTest1835822( + test.TestCase, integrated_helpers.InstanceHelperMixin): + # ---------------------------- setup ---------------------------- + + def setUp(self): + super(RegressionTest1835822, self).setUp() + # Use the standard fixtures. + self.useFixture(policy_fixture.RealPolicyFixture()) + self.useFixture(nova_fixtures.NeutronFixture(self)) + self.useFixture(func_fixtures.PlacementFixture()) + self.api = self.useFixture(nova_fixtures.OSAPIFixture( + api_version='v2.1')).api + self.start_service('conductor') + self.start_service('scheduler') + self.start_service('compute') + # the image fake backend needed for image discovery + fake_image.stub_out_image_service(self) + self.addCleanup(fake_image.FakeImageService_reset) + images = self.api.get_images() + self.image_ref_0 = images[0]['id'] + self.image_ref_1 = images[1]['id'] + + fake_notifier.stub_notifier(self) + self.addCleanup(fake_notifier.reset) + + # ---------------------------- helpers ---------------------------- + def _create_active_server(self, server_args=None): + basic_server = { + 'flavorRef': 1, + 'name': 'RegressionTest1835822', + 'networks': [{ + 'uuid': nova_fixtures.NeutronFixture.network_1['id'] + }], + 'imageRef': self.image_ref_0 + } + if server_args: + basic_server.update(server_args) + server = self.api.post_server({'server': basic_server}) + return self._wait_for_state_change(self.api, server, 'ACTIVE') + + def _hard_reboot_server(self, active_server): + args = {"reboot": {"type": "HARD"}} + self.api.api_post('servers/%s/action' % + active_server['id'], args) + fake_notifier.wait_for_versioned_notifications('instance.reboot.end') + return self._wait_for_state_change(self.api, active_server, 'ACTIVE') + + def _rebuild_server(self, active_server): + args = {"rebuild": {"imageRef": self.image_ref_1}} + self.api.api_post('servers/%s/action' % + active_server['id'], args) + fake_notifier.wait_for_versioned_notifications('instance.rebuild.end') + return self._wait_for_state_change(self.api, active_server, 'ACTIVE') + + def _shelve_server(self, active_server): + self.api.post_server_action(active_server['id'], {'shelve': {}}) + return self._wait_for_state_change( + self.api, active_server, 'SHELVED_OFFLOADED') + + def _unshelve_server(self, shelved_server): + self.api.post_server_action(shelved_server['id'], {'unshelve': {}}) + return self._wait_for_state_change(self.api, shelved_server, 'ACTIVE') + + # ---------------------------- tests ---------------------------- + def test_create_server_with_config_drive(self): + """Verify that we can create a server with a config drive. + """ + active_server = self._create_active_server( + server_args={'config_drive': True}) + self.assertTrue(active_server['config_drive']) + + def test_create_server_without_config_drive(self): + """Verify that we can create a server without + a config drive. + """ + self.flags(force_config_drive=False) + active_server = self._create_active_server() + self.assertEqual('', active_server['config_drive']) + + def test_create_server_with_forced_config_drive(self): + """Verify that we can create a server with a forced + config drive. + """ + self.flags(force_config_drive=True) + active_server = self._create_active_server() + self.assertTrue(active_server['config_drive']) + + def test_create_server_with_forced_config_drive_reboot(self): + """Verify that we can create a server with a forced + config drive and it survives reboot. + """ + self.flags(force_config_drive=True) + active_server = self._create_active_server() + self.assertTrue(active_server['config_drive']) + active_server = self._hard_reboot_server(active_server) + self.assertTrue(active_server['config_drive']) + + def test_create_server_config_drive_reboot_after_conf_change(self): + """Verify that we can create a server with or without a forced + config drive it does not change across a reboot. + """ + # NOTE(sean-k-mooney): we do not need to restart the compute + # service because of the way self.flags overrides the config + # values. + self.flags(force_config_drive=True) + with_config_drive = self._create_active_server() + self.assertTrue(with_config_drive['config_drive']) + self.flags(force_config_drive=False) + without_config_drive = self._create_active_server() + self.assertEqual('', without_config_drive['config_drive']) + + # this server was created with force_config_drive=true + # so assert now that force_config_drive is false it does + # not override the value it was booted with. + with_config_drive = self._hard_reboot_server(with_config_drive) + self.assertTrue(with_config_drive['config_drive']) + + # this server was booted with force_config_drive=False so + # assert that it's config drive setting is not overridden + self.flags(force_config_drive=True) + without_config_drive = self._hard_reboot_server(without_config_drive) + self.assertEqual('', without_config_drive['config_drive']) + + def test_create_server_config_drive_rebuild_after_conf_change(self): + """Verify that we can create a server with or without a forced + config drive it does not change across a rebuild. + """ + self.flags(force_config_drive=True) + with_config_drive = self._create_active_server() + self.assertTrue(with_config_drive['config_drive']) + self.flags(force_config_drive=False) + without_config_drive = self._create_active_server() + self.assertEqual('', without_config_drive['config_drive']) + + # this server was created with force_config_drive=true + # so assert now that force_config_drive is false it does + # not override the value it was booted with. + with_config_drive = self._rebuild_server(with_config_drive) + self.assertTrue(with_config_drive['config_drive']) + + # this server was booted with force_config_drive=False so + # assert that it's config drive setting is not overridden + self.flags(force_config_drive=True) + without_config_drive = self._rebuild_server(without_config_drive) + self.assertEqual('', without_config_drive['config_drive']) + + def test_create_server_config_drive_shelve_unshelve_conf_change(self): + """Verify that we can create a server with or without a forced + config drive it does not change across a shelve and unshelve. + """ + self.flags(force_config_drive=True) + with_config_drive = self._create_active_server() + self.assertTrue(with_config_drive['config_drive']) + self.flags(force_config_drive=False) + without_config_drive = self._create_active_server() + self.assertEqual('', without_config_drive['config_drive']) + + # this server was created with force_config_drive=true + # so assert now that force_config_drive is false it does + # not override the value it was booted with. + with_config_drive = self._shelve_server(with_config_drive) + with_config_drive = self._unshelve_server(with_config_drive) + self.assertTrue(with_config_drive['config_drive']) + + # this server was booted with force_config_drive=False so + # assert that it's config drive setting is not overridden + self.flags(force_config_drive=True) + without_config_drive = self._shelve_server(without_config_drive) + without_config_drive = self._unshelve_server(without_config_drive) + self.assertEqual('', without_config_drive['config_drive'])