From 3ef78478bc935b8beddc9deb36e9550a852b6b66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Ribaud?= Date: Fri, 26 Aug 2022 17:20:34 +0200 Subject: [PATCH] Support rebooting an instance with shares (compute manager part) Allow to reboot an instance with shares attached. Manila is the OpenStack Shared Filesystems service. These series of patches implement changes required in Nova to allow the shares provided by Manila to be associated with and attached to instances using virtiofs. Implements: blueprint libvirt-virtiofs-attach-manila-shares Change-Id: I4ee4bb6cfe4f2baee121c2bcd62a8532d4ed8d1c --- nova/compute/manager.py | 14 ++ nova/objects/share_mapping.py | 8 ++ nova/tests/unit/compute/test_compute.py | 166 +++++++++++++++++++++++- 3 files changed, 182 insertions(+), 6 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index c3ea07f744..ce4eb78edd 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4386,6 +4386,8 @@ class ComputeManager(manager.Manager): accel_info = self._get_accel_info(context, instance) + share_info = self._get_share_info(context, instance) + self._notify_about_instance_usage(context, instance, "reboot.start") compute_utils.notify_about_instance_action( context, instance, self.host, @@ -4423,12 +4425,24 @@ class ComputeManager(manager.Manager): instance.task_state = task_states.REBOOT_STARTED_HARD expected_state = task_states.REBOOT_PENDING_HARD instance.save(expected_task_state=expected_state) + + # Attempt to mount the shares again. + # Note: The API ref states that soft reboot can only be + # done if the instance is in ACTIVE state. If the instance + # is in ACTIVE state it cannot have a share_mapping in ERROR + # so it is safe to ignore the re-mounting of the share for + # soft reboot. + if reboot_type == "HARD": + self._mount_all_shares(context, instance, share_info) + self.driver.reboot(context, instance, network_info, reboot_type, block_device_info=block_device_info, accel_info=accel_info, + share_info=share_info, bad_volumes_callback=bad_volumes_callback) + share_info.activate_all() except Exception as error: with excutils.save_and_reraise_exception() as ctxt: diff --git a/nova/objects/share_mapping.py b/nova/objects/share_mapping.py index da246b658a..63f7a855f8 100644 --- a/nova/objects/share_mapping.py +++ b/nova/objects/share_mapping.py @@ -163,3 +163,11 @@ class ShareMappingList(base.ObjectListBase, base.NovaObject): def deactivate_all(self): for share in self: share.deactivate() + + def contains_error(self): + return any( + [ + share_mapping.status == fields.ShareMappingStatus.ERROR + for share_mapping in self + ] + ) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index d0ace523b6..b28a1d6665 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -3210,6 +3210,7 @@ class ComputeTestCase(BaseTestCase, @mock.patch.object(compute_manager.ComputeManager, '_delete_dangling_bdms') + @mock.patch('nova.compute.manager.ComputeManager._get_share_info') @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(compute_manager.ComputeManager, '_get_instance_block_device_info') @@ -3221,9 +3222,11 @@ 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, mock_get_bdms, mock_del_stale_bdms, - test_delete=False, test_unrescue=False, - fail_reboot=False, fail_running=False): + mock_get_blk, mock_get_bdms, mock_shares, + mock_del_stale_bdms, + test_delete=False, test_unrescue=False, fail_reboot=False, + fail_running=False): + mock_shares.return_value = objects.ShareMappingList() reboot_type = soft and 'SOFT' or 'HARD' task_pending = (soft and task_states.REBOOT_PENDING or task_states.REBOOT_PENDING_HARD) @@ -3324,7 +3327,8 @@ class ComputeTestCase(BaseTestCase, 'args': (econtext, instance, expected_nw_info, reboot_type), 'kwargs': {'block_device_info': fake_block_dev_info, - 'accel_info': []}} + 'accel_info': [], + 'share_info': mock_shares.return_value}} fault = exception.InstanceNotFound(instance_id='instance-0000') def fake_reboot(self, *args, **kwargs): @@ -3442,6 +3446,7 @@ class ComputeTestCase(BaseTestCase, @mock.patch.object(compute_manager.ComputeManager, '_delete_dangling_bdms') + @mock.patch('nova.compute.manager.ComputeManager._get_share_info') @mock.patch('nova.virt.fake.FakeDriver.reboot') @mock.patch('nova.objects.instance.Instance.save') @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @@ -3456,8 +3461,10 @@ class ComputeTestCase(BaseTestCase, def _test_reboot_with_accels(self, mock_notify_action, mock_get_power, mock_get_orig, mock_update, mock_notify_usage, mock_get_blk, mock_get_bdms, mock_inst_save, mock_reboot, - mock_del_stale_bdms, extra_specs=None, accel_info=None): + mock_shares, mock_del_stale_bdms, + extra_specs=None, accel_info=None): + mock_shares.return_value = objects.ShareMappingList() self.compute.network_api.get_instance_nw_info = mock.Mock() reboot_type = 'SOFT' @@ -3472,7 +3479,8 @@ class ComputeTestCase(BaseTestCase, mock.ANY, instance, mock.ANY, reboot_type, block_device_info=mock.ANY, bad_volumes_callback=mock.ANY, - accel_info=accel_info or [] + accel_info=accel_info or [], + share_info=mock_shares.return_value ) return instance['uuid'] @@ -3494,6 +3502,152 @@ class ComputeTestCase(BaseTestCase, self._test_reboot_with_accels(extra_specs=None, accel_info=None) mock_get_arqs.assert_not_called() + @mock.patch('nova.compute.manager.ComputeManager._mount_all_shares') + @mock.patch('nova.virt.fake.FakeDriver.reboot') + @mock.patch('nova.objects.instance.Instance.save') + @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, + '_notify_about_instance_usage') + @mock.patch.object(compute_manager.ComputeManager, '_instance_update') + @mock.patch.object(db, 'instance_update_and_get_original') + @mock.patch.object(compute_manager.ComputeManager, '_get_power_state') + @mock.patch('nova.compute.utils.notify_about_instance_action') + @mock.patch('nova.compute.manager.ComputeManager._get_share_info') + def test_soft_reboot_with_share_info( + self, + mock_shares, + mock_notify_action, + mock_get_power, + mock_get_orig, + mock_update, + mock_notify_usage, + mock_get_blk, + mock_get_bdms, + mock_inst_save, + mock_reboot, + mock_mount_all, + ): + reboot_type = 'SOFT' + instance = self._create_fake_instance_obj() + + share_info = self.fake_share_info() + mock_shares.return_value = share_info + + self.compute.reboot_instance(self.context, instance=instance, + block_device_info=None, reboot_type=reboot_type) + + mock_reboot.assert_called_once_with( + mock.ANY, instance, mock.ANY, reboot_type, + block_device_info=mock.ANY, + bad_volumes_callback=mock.ANY, + accel_info=mock.ANY, + share_info=share_info + ) + + mock_mount_all.assert_not_called() + + return instance['uuid'] + + @mock.patch('nova.compute.manager.ComputeManager._mount_all_shares') + @mock.patch('nova.virt.fake.FakeDriver.reboot') + @mock.patch('nova.objects.instance.Instance.save') + @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, + '_notify_about_instance_usage') + @mock.patch.object(compute_manager.ComputeManager, '_instance_update') + @mock.patch.object(db, 'instance_update_and_get_original') + @mock.patch.object(compute_manager.ComputeManager, '_get_power_state') + @mock.patch('nova.compute.utils.notify_about_instance_action') + @mock.patch('nova.compute.manager.ComputeManager._get_share_info') + def test_hard_reboot_with_share_info( + self, + mock_shares, + mock_notify_action, + mock_get_power, + mock_get_orig, + mock_update, + mock_notify_usage, + mock_get_blk, + mock_get_bdms, + mock_inst_save, + mock_reboot, + mock_mount_all, + ): + reboot_type = 'HARD' + instance = self._create_fake_instance_obj() + + share_info = self.fake_share_info() + mock_shares.return_value = share_info + + self.compute.reboot_instance(self.context, instance=instance, + block_device_info=None, reboot_type=reboot_type) + + mock_reboot.assert_called_once_with( + mock.ANY, instance, mock.ANY, reboot_type, + block_device_info=mock.ANY, + bad_volumes_callback=mock.ANY, + accel_info=mock.ANY, + share_info=share_info + ) + + mock_mount_all.assert_called_once_with(mock.ANY, instance, share_info) + + return instance['uuid'] + + @mock.patch('nova.compute.manager.ComputeManager._mount_all_shares') + @mock.patch('nova.virt.fake.FakeDriver.reboot') + @mock.patch('nova.objects.instance.Instance.save') + @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, + '_notify_about_instance_usage') + @mock.patch.object(compute_manager.ComputeManager, '_instance_update') + @mock.patch.object(db, 'instance_update_and_get_original') + @mock.patch.object(compute_manager.ComputeManager, '_get_power_state') + @mock.patch('nova.compute.utils.notify_about_instance_action') + @mock.patch('nova.compute.manager.ComputeManager._get_share_info') + def test_hard_reboot_with_share_info_error( + self, + mock_shares, + mock_notify_action, + mock_get_power, + mock_get_orig, + mock_update, + mock_notify_usage, + mock_get_blk, + mock_get_bdms, + mock_inst_save, + mock_reboot, + mock_mount_all, + ): + reboot_type = 'HARD' + instance = self._create_fake_instance_obj() + + share_info = self.fake_share_info() + share_info[0].status = 'error' + mock_shares.return_value = share_info + + self.compute.reboot_instance(self.context, instance=instance, + block_device_info=None, reboot_type=reboot_type) + + mock_reboot.assert_called_once_with( + mock.ANY, instance, mock.ANY, reboot_type, + block_device_info=mock.ANY, + bad_volumes_callback=mock.ANY, + accel_info=mock.ANY, + share_info=share_info + ) + + mock_mount_all.assert_called_once_with( + mock.ANY, instance, share_info) + + return instance['uuid'] + @mock.patch.object(jsonutils, 'to_primitive') def test_reboot_fail(self, mock_to_primitive): self._test_reboot(False, fail_reboot=True)