From 6b065dad8015ea4fdacb0aedc763b38c5c8a9130 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 15 Oct 2018 18:16:40 -0400 Subject: [PATCH] Move live_migration.pre.start to the start of the method This simply moves the pre_live_migration start notification back to the start of the method before creating new volume attachments. While in here, the list of bdms are passed to the notify_about_instance_action method so if [notifications]/bdms_in_notifications is True we don't have to pull them out of the database again for each versioned notification. Conflicts in nova/compute/manager.py due to absence of comment from f0b9d5204f3. Change-Id: I372eddb9e356b02328f937917a856bc631691f53 Partial-Bug: #1763051 (cherry picked from commit 7a2228142b8f8f015a554e2b25198ab37811a058) (cherry picked from commit ae2da62955e5c856fec71eebb4f3458fb4c7fb79) --- nova/compute/manager.py | 20 ++++++++++---------- nova/tests/unit/compute/test_compute.py | 11 ++++++++--- nova/tests/unit/compute/test_compute_mgr.py | 11 +++++++---- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 5f1ba0d84a40..a23343708393 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -5929,6 +5929,15 @@ class ComputeManager(manager.Manager): migrate_data.old_vol_attachment_ids = {} bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( context, instance.uuid) + network_info = self.network_api.get_instance_nw_info(context, instance) + self._notify_about_instance_usage( + context, instance, "live_migration.pre.start", + network_info=network_info) + compute_utils.notify_about_instance_action( + context, instance, self.host, + action=fields.NotificationAction.LIVE_MIGRATION_PRE, + phase=fields.NotificationPhase.START, bdms=bdms) + try: connector = self.driver.get_volume_connector(instance) for bdm in bdms: @@ -5973,15 +5982,6 @@ class ComputeManager(manager.Manager): context, instance, refresh_conn_info=True, bdms=bdms) - network_info = self.network_api.get_instance_nw_info(context, instance) - self._notify_about_instance_usage( - context, instance, "live_migration.pre.start", - network_info=network_info) - compute_utils.notify_about_instance_action( - context, instance, self.host, - action=fields.NotificationAction.LIVE_MIGRATION_PRE, - phase=fields.NotificationPhase.START) - migrate_data = self.driver.pre_live_migration(context, instance, block_device_info, @@ -6016,7 +6016,7 @@ class ComputeManager(manager.Manager): compute_utils.notify_about_instance_action( context, instance, self.host, action=fields.NotificationAction.LIVE_MIGRATION_PRE, - phase=fields.NotificationPhase.END) + phase=fields.NotificationPhase.END, bdms=bdms) LOG.debug('pre_live_migration result data is %s', migrate_data) return migrate_data diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 2045cc62b4a8..eb0ab8cb2e31 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -6098,7 +6098,10 @@ class ComputeTestCase(BaseTestCase, @mock.patch.object(fake.FakeDriver, 'ensure_filtering_rules_for_instance') @mock.patch.object(fake.FakeDriver, 'pre_live_migration') @mock.patch('nova.compute.utils.notify_about_instance_action') - def test_pre_live_migration_works_correctly(self, mock_notify, + @mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid', + return_value=objects.BlockDeviceMappingList()) + def test_pre_live_migration_works_correctly(self, mock_get_bdms, + mock_notify, mock_pre, mock_ensure): # Confirm setup_compute_volume is called when volume is mounted. def stupid(*args, **kwargs): @@ -6136,9 +6139,11 @@ class ComputeTestCase(BaseTestCase, mock_notify.assert_has_calls([ mock.call(c, instance, 'fake-mini', - action='live_migration_pre', phase='start'), + action='live_migration_pre', phase='start', + bdms=mock_get_bdms.return_value), mock.call(c, instance, 'fake-mini', - action='live_migration_pre', phase='end')]) + action='live_migration_pre', phase='end', + bdms=mock_get_bdms.return_value)]) mock_pre.assert_called_once_with( test.MatchType(nova.context.RequestContext), diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 4dd951b7da88..a736bcfdc418 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -6933,9 +6933,11 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): mock_notify_about_inst.assert_has_calls([ mock.call(self.context, instance, 'fake-mini', - action='live_migration_pre', phase='start'), + action='live_migration_pre', phase='start', + bdms=mock_get_bdms.return_value), mock.call(self.context, instance, 'fake-mini', - action='live_migration_pre', phase='end')]) + action='live_migration_pre', phase='end', + bdms=mock_get_bdms.return_value)]) self.assertIsInstance(r, migrate_data_obj.LiveMigrateData) self.assertIsInstance(mock_plm.call_args_list[0][0][5], migrate_data_obj.LiveMigrateData) @@ -6985,6 +6987,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') @mock.patch.object(vol1_bdm, 'save') @mock.patch.object(compute, '_notify_about_instance_usage') + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch.object(compute, 'network_api') @mock.patch.object(compute.driver, 'pre_live_migration') @mock.patch.object(compute, '_get_instance_block_device_info') @@ -6994,8 +6997,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): @mock.patch.object(compute.volume_api, 'attachment_delete') @mock.patch.object(compute.volume_api, 'attachment_create') def _test(mock_attach_create, mock_attach_delete, mock_get_bdms, - mock_ivbi, mock_gibdi, mock_plm, mock_nwapi, mock_notify, - mock_bdm_save, mock_exception): + mock_ivbi, mock_gibdi, mock_plm, mock_nwapi, mock_ver_notify, + mock_notify, mock_bdm_save, mock_exception): new_attachment_id = uuids.attachment3 mock_attach_create.side_effect = [{'id': new_attachment_id}, test.TestingException]