From 41a0f51341087e48eba16bbf6fd0bf779dd3b189 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 13 Jul 2017 14:57:15 +0200 Subject: [PATCH] use already loaded BDM in instance. (2) I8849ae0f54605e003d5b294ca3d66dcef89d7d27 made it possible for _get_instance_block_device_info to take a BDM parameter instead of loading the BDM from the db. This allow us to load the BDM a bit earlier in the call chain and pass that BDM to the notification calls too. The remaining calls of the notify_about_instance_action does not have the BDM loaded already. Change-Id: Icc3ffe4037a44f4f323bec2f80d99ca226742e22 Related-Bug: #1718226 --- nova/compute/manager.py | 39 +++++++++++++------- nova/tests/unit/compute/test_compute.py | 41 ++++++++++++++------- nova/tests/unit/compute/test_compute_mgr.py | 5 ++- 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 997956e44061..d392ef44f6bb 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -3024,8 +3024,10 @@ class ComputeManager(manager.Manager): context = context.elevated() LOG.info("Rebooting instance", instance=instance) - block_device_info = self._get_instance_block_device_info(context, - instance) + bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( + context, instance.uuid) + block_device_info = self._get_instance_block_device_info( + context, instance, bdms) network_info = self.network_api.get_instance_nw_info(context, instance) @@ -3033,7 +3035,8 @@ class ComputeManager(manager.Manager): compute_utils.notify_about_instance_action( context, instance, self.host, action=fields.NotificationAction.REBOOT, - phase=fields.NotificationPhase.START + phase=fields.NotificationPhase.START, + bdms=bdms ) instance.power_state = self._get_power_state(context, instance) @@ -3088,7 +3091,7 @@ class ComputeManager(manager.Manager): context, instance, self.host, action=fields.NotificationAction.REBOOT, phase=fields.NotificationPhase.ERROR, - exception=error + exception=error, bdms=bdms ) ctxt.reraise = False else: @@ -3111,7 +3114,8 @@ class ComputeManager(manager.Manager): compute_utils.notify_about_instance_action( context, instance, self.host, action=fields.NotificationAction.REBOOT, - phase=fields.NotificationPhase.END + phase=fields.NotificationPhase.END, + bdms=bdms ) @delete_image_on_error @@ -4071,7 +4075,7 @@ class ComputeManager(manager.Manager): instance.flavor = instance_type def _finish_resize(self, context, instance, migration, disk_info, - image_meta): + image_meta, bdms): resize_instance = False old_instance_type_id = migration['old_instance_type_id'] new_instance_type_id = migration['new_instance_type_id'] @@ -4111,10 +4115,10 @@ class ComputeManager(manager.Manager): network_info=network_info) compute_utils.notify_about_instance_action(context, instance, self.host, action=fields.NotificationAction.RESIZE_FINISH, - phase=fields.NotificationPhase.START) + phase=fields.NotificationPhase.START, bdms=bdms) block_device_info = self._get_instance_block_device_info( - context, instance, refresh_conn_info=True) + context, instance, refresh_conn_info=True, bdms=bdms) # NOTE(mriedem): If the original vm_state was STOPPED, we don't # automatically power on the instance after it's migrated @@ -4155,11 +4159,14 @@ class ComputeManager(manager.Manager): new host machine. """ + bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( + context, instance.uuid) + with self._error_out_instance_on_exception(context, instance), \ errors_out_migration_ctxt(migration): image_meta = objects.ImageMeta.from_dict(image) network_info = self._finish_resize(context, instance, migration, - disk_info, image_meta) + disk_info, image_meta, bdms) self._update_scheduler_instance_info(context, instance) self._notify_about_instance_usage( @@ -4167,7 +4174,7 @@ class ComputeManager(manager.Manager): network_info=network_info) compute_utils.notify_about_instance_action(context, instance, self.host, action=fields.NotificationAction.RESIZE_FINISH, - phase=fields.NotificationPhase.END) + phase=fields.NotificationPhase.END, bdms=bdms) @wrap_exception() @wrap_instance_fault @@ -4349,13 +4356,17 @@ class ComputeManager(manager.Manager): LOG.info('Resuming', instance=instance) self._notify_about_instance_usage(context, instance, 'resume.start') + + bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( + context, instance.uuid) + block_device_info = self._get_instance_block_device_info( + context, instance, bdms) + compute_utils.notify_about_instance_action(context, instance, self.host, action=fields.NotificationAction.RESUME, - phase=fields.NotificationPhase.START) + phase=fields.NotificationPhase.START, bdms=bdms) network_info = self.network_api.get_instance_nw_info(context, instance) - block_device_info = self._get_instance_block_device_info( - context, instance) with self._error_out_instance_on_exception(context, instance, instance_state=instance.vm_state): @@ -4373,7 +4384,7 @@ class ComputeManager(manager.Manager): self._notify_about_instance_usage(context, instance, 'resume.end') compute_utils.notify_about_instance_action(context, instance, self.host, action=fields.NotificationAction.RESUME, - phase=fields.NotificationPhase.END) + phase=fields.NotificationPhase.END, bdms=bdms) @wrap_exception() @reverts_task_state diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 49aa1f5052b4..e0e3a620b06b 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -2658,13 +2658,17 @@ class ComputeTestCase(BaseTestCase, self.compute.terminate_instance(self.context, instance, [], []) + @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(nova.compute.utils, 'notify_about_instance_action') @mock.patch('nova.context.RequestContext.elevated') - def test_resume_notifications(self, mock_context, mock_notify): + def test_resume_notifications(self, mock_context, mock_notify, + mock_get_bdms): # ensure instance can be suspended and resumed. context = self.context mock_context.return_value = context instance = self._create_fake_instance_obj() + bdms = block_device_obj.block_device_make_list(self.context, []) + mock_get_bdms.return_value = bdms self.compute.build_and_run_instance(self.context, instance, {}, {}, {}, block_device_mapping=[]) instance.task_state = task_states.SUSPENDING @@ -2682,9 +2686,9 @@ class ComputeTestCase(BaseTestCase, 'compute.instance.resume.end') mock_notify.assert_has_calls([ mock.call(context, instance, 'fake-mini', - action='resume', phase='start'), + action='resume', phase='start', bdms=bdms), mock.call(context, instance, 'fake-mini', - action='resume', phase='end')]) + action='resume', phase='end', bdms=bdms)]) self.compute.terminate_instance(self.context, instance, [], []) def test_resume_no_old_state(self): @@ -2899,6 +2903,7 @@ class ComputeTestCase(BaseTestCase, on_shared_storage=False) self.compute.terminate_instance(self.context, instance, [], []) + @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(compute_manager.ComputeManager, '_get_instance_block_device_info') @mock.patch.object(compute_manager.ComputeManager, @@ -2909,7 +2914,7 @@ class ComputeTestCase(BaseTestCase, @mock.patch('nova.compute.utils.notify_about_instance_action') def _test_reboot(self, soft, mock_notify_action, mock_get_power, mock_get_orig, mock_update, mock_notify_usage, - mock_get_blk, test_delete=False, + mock_get_blk, mock_get_bdms, test_delete=False, test_unrescue=False, fail_reboot=False, fail_running=False): reboot_type = soft and 'SOFT' or 'HARD' @@ -2959,6 +2964,9 @@ class ComputeTestCase(BaseTestCase, task_state=expected_task, launched_at=timeutils.utcnow())) + bdms = block_device_obj.block_device_make_list(self.context, []) + mock_get_bdms.return_value = bdms + if test_unrescue: instance.vm_state = vm_states.RESCUED instance.obj_reset_changes() @@ -2985,7 +2993,7 @@ class ComputeTestCase(BaseTestCase, notify_call_list = [mock.call(econtext, instance, 'reboot.start')] notify_action_call_list = [ mock.call(econtext, instance, 'fake-mini', action='reboot', - phase='start')] + phase='start', bdms=bdms)] ps_call_list = [mock.call(econtext, instance)] db_call_list = [mock.call(econtext, instance['uuid'], @@ -3052,7 +3060,7 @@ class ComputeTestCase(BaseTestCase, 'reboot.end')) notify_action_call_list.append( mock.call(econtext, instance, 'fake-mini', - action='reboot', phase='end')) + action='reboot', phase='end', bdms=bdms)) elif fail_reboot and not fail_running: mock_get_orig.side_effect = chain(mock_get_orig.side_effect, [fault]) @@ -3074,12 +3082,13 @@ class ComputeTestCase(BaseTestCase, 'reboot.error', fault=fault)) notify_action_call_list.append( mock.call(econtext, instance, 'fake-mini', - action='reboot', phase='error', exception=fault)) + action='reboot', phase='error', exception=fault, + bdms=bdms)) notify_call_list.append(mock.call(econtext, instance, 'reboot.end')) notify_action_call_list.append( mock.call(econtext, instance, 'fake-mini', - action='reboot', phase='end')) + action='reboot', phase='end', bdms=bdms)) if not fail_reboot or fail_running: self.compute.reboot_instance(self.context, instance=instance, @@ -3093,7 +3102,7 @@ class ComputeTestCase(BaseTestCase, reboot_type=reboot_type) self.assertEqual(expected_call_info, reboot_call_info) - mock_get_blk.assert_called_once_with(econtext, instance) + mock_get_blk.assert_called_once_with(econtext, instance, bdms) mock_get_nw.assert_called_once_with(econtext, instance) mock_notify_usage.assert_has_calls(notify_call_list) mock_notify_action.assert_has_calls(notify_action_call_list) @@ -4589,6 +4598,9 @@ class ComputeTestCase(BaseTestCase, network_api = self.compute.network_api with test.nested( + mock.patch.object(objects.BlockDeviceMappingList, + 'get_by_instance_uuid', + return_value='fake_bdms'), mock.patch.object(network_api, 'setup_networks_on_host'), mock.patch.object(network_api, 'migrate_instance_finish'), mock.patch.object(self.compute.network_api, @@ -4599,7 +4611,7 @@ class ComputeTestCase(BaseTestCase, mock.patch.object(self.compute, '_get_instance_block_device_info'), mock.patch.object(migration, 'save'), mock.patch.object(instance, 'save'), - ) as (mock_setup, mock_net_mig, mock_get_nw, mock_notify, + ) as (mock_get_bdm, mock_setup, mock_net_mig, mock_get_nw, mock_notify, mock_notify_action, mock_virt_mig, mock_get_blk, mock_mig_save, mock_inst_save): def _mig_save(): @@ -4662,9 +4674,11 @@ class ComputeTestCase(BaseTestCase, network_info='fake-nwinfo1')]) mock_notify_action.assert_has_calls([ mock.call(self.context, instance, 'fake-mini', - action='resize_finish', phase='start'), + action='resize_finish', phase='start', + bdms='fake_bdms'), mock.call(self.context, instance, 'fake-mini', - action='resize_finish', phase='end')]) + action='resize_finish', phase='end', + bdms='fake_bdms')]) # nova.conf sets the default flavor to m1.small and the test # sets the default flavor to m1.tiny so they should be different # which makes this a resize @@ -4673,7 +4687,8 @@ class ComputeTestCase(BaseTestCase, test.MatchType(objects.ImageMeta), resize_instance, 'fake-bdminfo', power_on) mock_get_blk.assert_called_once_with(self.context, instance, - refresh_conn_info=True) + refresh_conn_info=True, + bdms='fake_bdms') mock_inst_save.assert_has_calls(inst_call_list) mock_mig_save.assert_called_once_with() diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 9a3844fca30a..26df2446ed7c 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -5497,7 +5497,10 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): mock.patch.object(db, 'instance_fault_create'), mock.patch.object(self.compute, '_update_resource_tracker'), mock.patch.object(self.instance, 'save'), - ) as (_finish_resize, fault_create, instance_update, instance_save): + mock.patch.object(objects.BlockDeviceMappingList, + 'get_by_instance_uuid') + ) as (_finish_resize, fault_create, instance_update, instance_save, + get_bdm): fault_create.return_value = ( test_instance_fault.fake_faults['fake-uuid'][0]) yield _finish_resize