From 01c4230c3b821bf1b5c46c0157974ae84d66d6e2 Mon Sep 17 00:00:00 2001 From: Shoham Peller Date: Tue, 27 Sep 2016 20:25:05 +0300 Subject: [PATCH] Handle spawning error on unshelving If spawning fails when unshelving, terminate the volumes' connections with the node, and remove the node reference from the instance entry. Co-Authored-By: Matt Riedemann Conflicts: nova/compute/manager.py nova/tests/unit/compute/test_shelve.py NOTE(mriedem): Conflicts are due to the following changes not in Ocata: I2394b0588bc7210efd16456af2fc12dc7071681c Id2c7b7b3b4abda8a3b878fdee6806bcfe096e12e I3a3caa4c566ecc132aa2699f8c7e5987bbcc863a Closes-Bug: 1627694 Change-Id: I8cfb2280d956d452ccad1fc711bd814b7258147f (cherry picked from commit dcdd2c9832c7c60fe9163cd744ca2b5acfe16bcc) (cherry picked from commit 73c3e4969c108f2eefc77b57021d9b3e17afdc8e) (cherry picked from commit 06946b7f8ad310e01eff895362ad0f3a164636f5) --- nova/compute/manager.py | 5 ++ nova/tests/unit/compute/test_shelve.py | 86 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 2e83f07ae67d..8c83167d4af6 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4494,6 +4494,11 @@ class ComputeManager(manager.Manager): with excutils.save_and_reraise_exception(): LOG.exception(_LE('Instance failed to spawn'), instance=instance) + # FIXME: Umm, shouldn't we be rolling back port bindings too? + self._terminate_volume_connections(context, instance, bdms) + # The reverts_task_state decorator on unshelve_instance will + # eventually save these updates. + self._nil_out_instance_obj_host_and_node(instance) if image: instance.image_ref = shelved_image_ref diff --git a/nova/tests/unit/compute/test_shelve.py b/nova/tests/unit/compute/test_shelve.py index 2d21f092f8fc..8c74faec2c00 100644 --- a/nova/tests/unit/compute/test_shelve.py +++ b/nova/tests/unit/compute/test_shelve.py @@ -397,6 +397,92 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): self.compute.unshelve_instance(self.context, instance, image=None, filter_properties=filter_properties, node=node) + @mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid') + @mock.patch('nova.compute.utils.notify_about_instance_action') + @mock.patch.object(nova.compute.resource_tracker.ResourceTracker, + 'instance_claim') + @mock.patch('nova.network.neutronv2.api.API.' + 'setup_instance_network_on_host') + @mock.patch.object(nova.virt.fake.SmallFakeDriver, 'spawn', + side_effect=test.TestingException('oops!')) + @mock.patch.object(nova.compute.manager.ComputeManager, + '_prep_block_device', return_value='fake_bdm') + @mock.patch.object(nova.compute.manager.ComputeManager, + '_notify_about_instance_usage') + @mock.patch('nova.utils.get_image_from_system_metadata') + @mock.patch.object(nova.compute.manager.ComputeManager, + '_terminate_volume_connections') + def test_unshelve_spawn_fails_cleanup_volume_connections( + self, mock_terminate_volume_connections, mock_image_meta, + mock_notify_instance_usage, mock_prep_block_device, mock_spawn, + mock_setup_network, mock_instance_claim, + mock_notify_instance_action, mock_get_bdms): + """Tests error handling when a instance fails to unshelve and makes + sure that volume connections are cleaned up from the host + and that the host/node values are unset on the instance. + """ + mock_bdms = mock.Mock() + mock_get_bdms.return_value = mock_bdms + instance = self._create_fake_instance_obj() + node = test_compute.NODENAME + limits = {} + filter_properties = {'limits': limits} + instance.task_state = task_states.UNSHELVING + instance.save() + image_meta = {'properties': {'base_image_ref': uuids.image_id}} + mock_image_meta.return_value = image_meta + + tracking = {'last_state': instance.task_state} + + def fake_claim(context, instance, node, limits): + instance.host = self.compute.host + instance.node = node + requests = objects.InstancePCIRequests(requests=[]) + return claims.Claim(context, instance, node, + self.rt, _fake_resources(), + requests, limits=limits) + mock_instance_claim.side_effect = fake_claim + + def check_save(expected_task_state=None): + if tracking['last_state'] == task_states.UNSHELVING: + # This is before we've failed. + self.assertEqual(task_states.SPAWNING, instance.task_state) + tracking['last_state'] = instance.task_state + elif tracking['last_state'] == task_states.SPAWNING: + # This is after we've failed. + self.assertIsNone(instance.host) + self.assertIsNone(instance.node) + self.assertIsNone(instance.task_state) + tracking['last_state'] = instance.task_state + else: + self.fail('Unexpected save!') + + with mock.patch.object(instance, 'save') as mock_save: + mock_save.side_effect = check_save + self.assertRaises(test.TestingException, + self.compute.unshelve_instance, + self.context, instance, image=None, + filter_properties=filter_properties, node=node) + + mock_notify_instance_action.assert_called_once_with( + self.context, instance, 'fake-mini', action='unshelve', + phase='start') + mock_notify_instance_usage.assert_called_once_with( + self.context, instance, 'unshelve.start') + mock_prep_block_device.assert_called_once_with( + self.context, instance, mock_bdms, do_check_attach=False) + mock_setup_network.assert_called_once_with(self.context, instance, + self.compute.host) + mock_instance_claim.assert_called_once_with(self.context, instance, + test_compute.NODENAME, + limits) + mock_spawn.assert_called_once_with( + self.context, instance, test.MatchType(objects.ImageMeta), + injected_files=[], admin_password=None, + network_info=[], block_device_info='fake_bdm') + mock_terminate_volume_connections.assert_called_once_with( + self.context, instance, mock_bdms) + @mock.patch.object(objects.InstanceList, 'get_by_filters') def test_shelved_poll_none_offloaded(self, mock_get_by_filters): # Test instances are not offloaded when shelved_offload_time is -1