From 5c5a983a19408ab8a17c28721eb250149de530fd Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Thu, 31 Jul 2014 21:54:27 +0300 Subject: [PATCH] Adds Hyper-V soft shutdown implementation This commit gives the Hyper-V guest VMs a chance to shutdown gracefully within a time limit, doing a hard shutdown if they fail to do so. Partially-Implements: blueprint user-defined-shutdown Change-Id: I68361ab43d8bba96af063b2ec53f89833a30d02a --- nova/tests/virt/hyperv/db_fakes.py | 1 + nova/tests/virt/hyperv/test_migrationops.py | 32 ++++++++++ nova/tests/virt/hyperv/test_vmops.py | 65 ++++++++++++++++++++- nova/virt/hyperv/driver.py | 8 +-- nova/virt/hyperv/migrationops.py | 5 +- nova/virt/hyperv/vmops.py | 41 +++++++++---- 6 files changed, 132 insertions(+), 20 deletions(-) diff --git a/nova/tests/virt/hyperv/db_fakes.py b/nova/tests/virt/hyperv/db_fakes.py index 53607352bd..0713abc024 100644 --- a/nova/tests/virt/hyperv/db_fakes.py +++ b/nova/tests/virt/hyperv/db_fakes.py @@ -147,6 +147,7 @@ def stub_out_db_instance_api(stubs): 'vcpus': flavor['vcpus'], 'mac_addresses': [{'address': values['mac_address']}], 'root_gb': flavor['root_gb'], + 'system_metadata': {'image_shutdown_timeout': 0}, } return FakeModel(base_options) diff --git a/nova/tests/virt/hyperv/test_migrationops.py b/nova/tests/virt/hyperv/test_migrationops.py index 0af56d55fe..15dd6e17f4 100644 --- a/nova/tests/virt/hyperv/test_migrationops.py +++ b/nova/tests/virt/hyperv/test_migrationops.py @@ -23,6 +23,9 @@ from nova.virt.hyperv import vmutils class MigrationOpsTestCase(test.NoDBTestCase): """Unit tests for the Hyper-V MigrationOps class.""" + _FAKE_TIMEOUT = 10 + _FAKE_RETRY_INTERVAL = 5 + def setUp(self): super(MigrationOpsTestCase, self).setUp() self.context = 'fake-context' @@ -35,6 +38,8 @@ class MigrationOpsTestCase(test.NoDBTestCase): self.addCleanup(patched_func.stop) self._migrationops = migrationops.MigrationOps() + self._migrationops._vmops = mock.MagicMock() + self._migrationops._vmutils = mock.MagicMock() def test_check_and_attach_config_drive_unknown_path(self): instance = fake_instance.fake_instance_obj(self.context) @@ -44,3 +49,30 @@ class MigrationOpsTestCase(test.NoDBTestCase): self.assertRaises(vmutils.HyperVException, self._migrationops._check_and_attach_config_drive, instance) + + @mock.patch.object(migrationops.MigrationOps, '_migrate_disk_files') + @mock.patch.object(migrationops.MigrationOps, '_check_target_flavor') + def test_migrate_disk_and_power_off(self, mock_check_flavor, + mock_migrate_disk_files): + instance = fake_instance.fake_instance_obj(self.context) + flavor = mock.MagicMock() + network_info = mock.MagicMock() + + disk_files = [mock.MagicMock()] + volume_drives = [mock.MagicMock()] + + mock_get_vm_st_path = self._migrationops._vmutils.get_vm_storage_paths + mock_get_vm_st_path.return_value = (disk_files, volume_drives) + + self._migrationops.migrate_disk_and_power_off( + self.context, instance, mock.sentinel.FAKE_DEST, flavor, + network_info, None, self._FAKE_TIMEOUT, self._FAKE_RETRY_INTERVAL) + + mock_check_flavor.assert_called_once_with(instance, flavor) + self._migrationops._vmops.power_off.assert_called_once_with( + instance, self._FAKE_TIMEOUT, self._FAKE_RETRY_INTERVAL) + mock_get_vm_st_path.assert_called_once_with(instance.name) + mock_migrate_disk_files.assert_called_once_with( + instance.name, disk_files, mock.sentinel.FAKE_DEST) + self._migrationops._vmops.destroy.assert_called_once_with( + instance, destroy_disks=False) diff --git a/nova/tests/virt/hyperv/test_vmops.py b/nova/tests/virt/hyperv/test_vmops.py index a4a37bbf17..6c4b565c4a 100644 --- a/nova/tests/virt/hyperv/test_vmops.py +++ b/nova/tests/virt/hyperv/test_vmops.py @@ -27,7 +27,7 @@ from nova.virt.hyperv import vmutils class VMOpsTestCase(test.NoDBTestCase): """Unit tests for the Hyper-V VMOps class.""" - _FAKE_TIMEOUT = 0 + _FAKE_TIMEOUT = 2 def __init__(self, test_case_name): super(VMOpsTestCase, self).__init__(test_case_name) @@ -100,8 +100,9 @@ class VMOpsTestCase(test.NoDBTestCase): self.assertTrue(result) + @mock.patch("time.sleep") @mock.patch("nova.virt.hyperv.vmutils.VMUtils.soft_shutdown_vm") - def test_soft_shutdown_failed(self, mock_shutdown_vm): + def test_soft_shutdown_failed(self, mock_shutdown_vm, mock_sleep): instance = fake_instance.fake_instance_obj(self.context) mock_shutdown_vm.side_effect = vmutils.HyperVException( @@ -112,6 +113,66 @@ class VMOpsTestCase(test.NoDBTestCase): mock_shutdown_vm.assert_called_once_with(instance.name) self.assertFalse(result) + @mock.patch("nova.virt.hyperv.vmutils.VMUtils.soft_shutdown_vm") + @mock.patch("nova.virt.hyperv.vmops.VMOps._wait_for_power_off") + def test_soft_shutdown_wait(self, mock_wait_for_power_off, + mock_shutdown_vm): + instance = fake_instance.fake_instance_obj(self.context) + mock_wait_for_power_off.side_effect = [False, True] + + result = self._vmops._soft_shutdown(instance, self._FAKE_TIMEOUT, 1) + + calls = [mock.call(instance.name, 1), + mock.call(instance.name, self._FAKE_TIMEOUT - 1)] + mock_shutdown_vm.assert_called_with(instance.name) + mock_wait_for_power_off.assert_has_calls(calls) + + self.assertTrue(result) + + @mock.patch("nova.virt.hyperv.vmutils.VMUtils.soft_shutdown_vm") + @mock.patch("nova.virt.hyperv.vmops.VMOps._wait_for_power_off") + def test_soft_shutdown_wait_timeout(self, mock_wait_for_power_off, + mock_shutdown_vm): + instance = fake_instance.fake_instance_obj(self.context) + mock_wait_for_power_off.return_value = False + + result = self._vmops._soft_shutdown(instance, self._FAKE_TIMEOUT, 1.5) + + calls = [mock.call(instance.name, 1.5), + mock.call(instance.name, self._FAKE_TIMEOUT - 1.5)] + mock_shutdown_vm.assert_called_with(instance.name) + mock_wait_for_power_off.assert_has_calls(calls) + + self.assertFalse(result) + + def _test_power_off(self, timeout): + instance = fake_instance.fake_instance_obj(self.context) + with mock.patch.object(self._vmops, '_set_vm_state') as mock_set_state: + self._vmops.power_off(instance, timeout) + + mock_set_state.assert_called_once_with( + instance, constants.HYPERV_VM_STATE_DISABLED) + + def test_power_off_hard(self): + self._test_power_off(timeout=0) + + @mock.patch("nova.virt.hyperv.vmops.VMOps._soft_shutdown") + def test_power_off_exception(self, mock_soft_shutdown): + mock_soft_shutdown.return_value = False + self._test_power_off(timeout=1) + + @mock.patch("nova.virt.hyperv.vmops.VMOps._set_vm_state") + @mock.patch("nova.virt.hyperv.vmops.VMOps._soft_shutdown") + def test_power_off_soft(self, mock_soft_shutdown, mock_set_state): + instance = fake_instance.fake_instance_obj(self.context) + mock_soft_shutdown.return_value = True + + self._vmops.power_off(instance, 1, 0) + + mock_soft_shutdown.assert_called_once_with( + instance, 1, vmops.SHUTDOWN_TIME_INCREMENT) + self.assertFalse(mock_set_state.called) + def test_get_vm_state(self): summary_info = {'EnabledState': constants.HYPERV_VM_STATE_DISABLED} diff --git a/nova/virt/hyperv/driver.py b/nova/virt/hyperv/driver.py index f06c77e2bc..7d4f209452 100644 --- a/nova/virt/hyperv/driver.py +++ b/nova/virt/hyperv/driver.py @@ -110,8 +110,7 @@ class HyperVDriver(driver.ComputeDriver): self._vmops.resume(instance) def power_off(self, instance, timeout=0, retry_interval=0): - # TODO(PhilDay): Add support for timeout (clean shutdown) - self._vmops.power_off(instance) + self._vmops.power_off(instance, timeout, retry_interval) def power_on(self, context, instance, network_info, block_device_info=None): @@ -189,12 +188,13 @@ class HyperVDriver(driver.ComputeDriver): flavor, network_info, block_device_info=None, timeout=0, retry_interval=0): - # TODO(PhilDay): Add support for timeout (clean shutdown) return self._migrationops.migrate_disk_and_power_off(context, instance, dest, flavor, network_info, - block_device_info) + block_device_info, + timeout, + retry_interval) def confirm_migration(self, migration, instance, network_info): self._migrationops.confirm_migration(migration, instance, network_info) diff --git a/nova/virt/hyperv/migrationops.py b/nova/virt/hyperv/migrationops.py index a803d4f797..09270999d5 100644 --- a/nova/virt/hyperv/migrationops.py +++ b/nova/virt/hyperv/migrationops.py @@ -113,12 +113,13 @@ class MigrationOps(object): def migrate_disk_and_power_off(self, context, instance, dest, flavor, network_info, - block_device_info=None): + block_device_info=None, timeout=0, + retry_interval=0): LOG.debug("migrate_disk_and_power_off called", instance=instance) self._check_target_flavor(instance, flavor) - self._vmops.power_off(instance) + self._vmops.power_off(instance, timeout, retry_interval) instance_name = instance["name"] diff --git a/nova/virt/hyperv/vmops.py b/nova/virt/hyperv/vmops.py index dbff5dec26..f0e6e52a73 100644 --- a/nova/virt/hyperv/vmops.py +++ b/nova/virt/hyperv/vmops.py @@ -19,6 +19,7 @@ Management class for basic VM operations. """ import functools import os +import time from eventlet import timeout as etimeout from oslo.config import cfg @@ -421,7 +422,8 @@ class VMOps(object): constants.HYPERV_VM_STATE_REBOOT) def _soft_shutdown(self, instance, - timeout=CONF.hyperv.wait_soft_reboot_seconds): + timeout=CONF.hyperv.wait_soft_reboot_seconds, + retry_interval=SHUTDOWN_TIME_INCREMENT): """Perform a soft shutdown on the VM. :return: True if the instance was shutdown within time limit, @@ -429,16 +431,26 @@ class VMOps(object): """ LOG.debug("Performing Soft shutdown on instance", instance=instance) - try: - self._vmutils.soft_shutdown_vm(instance.name) - if self._wait_for_power_off(instance.name, timeout): - LOG.info(_LI("Soft shutdown succeded."), instance=instance) - return True - except vmutils.HyperVException as e: - # Exception is raised when trying to shutdown the instance - # while it is still booting. - LOG.warning(_LW("Soft shutdown failed: %s"), e, instance=instance) - return False + while timeout > 0: + # Perform a soft shutdown on the instance. + # Wait maximum timeout for the instance to be shutdown. + # If it was not shutdown, retry until it succeded or a maximum of + # time waited is equal to timeout. + wait_time = min(retry_interval, timeout) + try: + LOG.debug("Soft shutdown instance, timeout remaining: %d", + timeout, instance=instance) + self._vmutils.soft_shutdown_vm(instance.name) + if self._wait_for_power_off(instance.name, wait_time): + LOG.info(_LI("Soft shutdown succeded."), instance=instance) + return True + except vmutils.HyperVException as e: + # Exception is raised when trying to shutdown the instance + # while it is still booting. + LOG.debug("Soft shutdown failed: %s", e, instance=instance) + time.sleep(wait_time) + + timeout -= retry_interval LOG.warning(_LW("Timed out while waiting for soft shutdown."), instance=instance) @@ -468,9 +480,14 @@ class VMOps(object): self._set_vm_state(instance, constants.HYPERV_VM_STATE_ENABLED) - def power_off(self, instance): + def power_off(self, instance, timeout=0, retry_interval=0): """Power off the specified instance.""" LOG.debug("Power off instance", instance=instance) + if retry_interval <= 0: + retry_interval = SHUTDOWN_TIME_INCREMENT + if timeout and self._soft_shutdown(instance, timeout, retry_interval): + return + self._set_vm_state(instance, constants.HYPERV_VM_STATE_DISABLED)