Merge "refactors cold migration / resize"

This commit is contained in:
Jenkins 2017-02-07 12:30:54 +00:00 committed by Gerrit Code Review
commit 571d36e2e9
6 changed files with 144 additions and 194 deletions

View File

@ -48,7 +48,7 @@ class HyperVClusterDriver(driver.HyperVDriver):
block_device_info=None,
timeout=0, retry_interval=0):
self._clops.remove_from_cluster(instance)
super(HyperVClusterDriver, self).migrate_disk_and_power_off(
return super(HyperVClusterDriver, self).migrate_disk_and_power_off(
context, instance, dest, flavor, network_info,
block_device_info, timeout, retry_interval)

View File

@ -25,7 +25,7 @@ from oslo_log import log as logging
from oslo_utils import excutils
from oslo_utils import units
from hyperv.i18n import _, _LW, _LE
from hyperv.i18n import _, _LW
from hyperv.nova import block_device_manager
from hyperv.nova import constants
from hyperv.nova import imagecache
@ -46,58 +46,17 @@ class MigrationOps(object):
self._imagecache = imagecache.ImageCache()
self._block_dev_man = block_device_manager.BlockDeviceInfoManager()
def _migrate_disk_files(self, instance_name, disk_files, dest):
# TODO(mikal): it would be nice if this method took a full instance,
# because it could then be passed to the log messages below.
instance_path = self._pathutils.get_instance_dir(instance_name)
dest_path = self._pathutils.get_instance_dir(instance_name, dest)
def _move_vm_files(self, instance):
instance_path = self._pathutils.get_instance_dir(instance.name)
revert_path = self._pathutils.get_instance_migr_revert_dir(
instance_name, remove_dir=True, create_dir=True)
instance.name, remove_dir=True, create_dir=True)
shared_storage = (self._pathutils.exists(dest_path) and
self._pathutils.check_dirs_shared_storage(
instance_path, dest_path))
# copy the given instance's files to a _revert folder, as backup.
LOG.debug("Moving instance files to a revert path: %s",
revert_path, instance=instance)
self._pathutils.move_folder_files(instance_path, revert_path)
try:
if shared_storage:
# Since source and target are the same, we copy the files to
# a temporary location before moving them into place.
# This applies when the migration target is the source host or
# when shared storage is used for the instance files.
dest_path = '%s_tmp' % instance_path
self._pathutils.check_remove_dir(dest_path)
self._pathutils.makedirs(dest_path)
for disk_file in disk_files:
LOG.debug('Copying disk "%(disk_file)s" to '
'"%(dest_path)s"',
{'disk_file': disk_file, 'dest_path': dest_path})
self._pathutils.copy(disk_file, dest_path)
self._pathutils.move_folder_files(instance_path, revert_path)
if shared_storage:
self._pathutils.move_folder_files(dest_path, instance_path)
self._pathutils.rmtree(dest_path)
except Exception:
with excutils.save_and_reraise_exception():
self._cleanup_failed_disk_migration(instance_path, revert_path,
dest_path)
def _cleanup_failed_disk_migration(self, instance_path,
revert_path, dest_path):
try:
if dest_path and self._pathutils.exists(dest_path):
self._pathutils.rmtree(dest_path)
if self._pathutils.exists(revert_path):
self._pathutils.move_folder_files(revert_path, instance_path)
self._pathutils.rmtree(revert_path)
except Exception as ex:
# Log and ignore this exception
LOG.exception(ex)
LOG.error(_LE("Cannot cleanup migration files"))
return revert_path
def _check_target_flavor(self, instance, flavor):
new_root_gb = flavor.root_gb
@ -121,17 +80,12 @@ class MigrationOps(object):
self._check_target_flavor(instance, flavor)
self._vmops.power_off(instance, timeout, retry_interval)
instance_path = self._move_vm_files(instance)
(disk_files,
volume_drives) = self._vmutils.get_vm_storage_paths(instance.name)
self._vmops.destroy(instance, destroy_disks=True)
if disk_files:
self._migrate_disk_files(instance.name, disk_files, dest)
self._vmops.destroy(instance, destroy_disks=False)
# disk_info is not used
return ""
# return the instance's path location.
return instance_path
def confirm_migration(self, context, migration, instance, network_info):
LOG.debug("confirm_migration called", instance=instance)
@ -245,10 +199,21 @@ class MigrationOps(object):
self._vhdutils.reconnect_parent_vhd(diff_vhd_path,
base_vhd_path)
def _migrate_disks_from_source(self, migration, instance,
source_inst_dir):
source_inst_dir = self._pathutils.get_remote_path(
migration.source_compute, source_inst_dir)
inst_dir = self._pathutils.get_instance_dir(
instance.name, create_dir=True, remove_dir=True)
self._pathutils.copy_folder_files(source_inst_dir, inst_dir)
def finish_migration(self, context, migration, instance, disk_info,
network_info, image_meta, resize_instance=False,
block_device_info=None, power_on=True):
LOG.debug("finish_migration called", instance=instance)
self._migrate_disks_from_source(migration, instance, disk_info)
vm_gen = self._vmops.get_image_vm_generation(instance.uuid, image_meta)
self._check_and_update_disks(context, instance, vm_gen, image_meta,
block_device_info,

View File

@ -45,6 +45,23 @@ class PathUtils(pathutils.PathUtils):
super(PathUtils, self).__init__()
self._vmutils = utilsfactory.get_vmutils()
def copy_folder_files(self, src_dir, dest_dir):
"""Copies the files of the given src_dir to dest_dir.
It will ignore any nested folders.
:param src_dir: Given folder from which to copy files.
:param dest_dir: Folder to which to copy files.
"""
# NOTE(claudiub): this will have to be moved to os-win.
for fname in os.listdir(src_dir):
src = os.path.join(src_dir, fname)
# ignore subdirs.
if os.path.isfile(src):
self.copy(src, os.path.join(dest_dir, fname))
def get_instances_dir(self, remote_server=None):
local_instance_path = os.path.normpath(CONF.instances_path)
@ -55,11 +72,11 @@ class PathUtils(pathutils.PathUtils):
# In this case, we expect the instance dir to have the same
# location on the remote server.
path = local_instance_path
return self._get_remote_unc_path(remote_server, path)
return self.get_remote_path(remote_server, path)
else:
return local_instance_path
def _get_remote_unc_path(self, remote_server, remote_path):
def get_remote_path(self, remote_server, remote_path):
if remote_path.startswith('\\\\'):
remote_unc_path = remote_path
else:
@ -114,8 +131,8 @@ class PathUtils(pathutils.PathUtils):
instance_dir = vmutils.get_vm_config_root_dir(
instance_name)
if remote_server:
instance_dir = self._get_remote_unc_path(remote_server,
instance_dir)
instance_dir = self.get_remote_path(remote_server,
instance_dir)
LOG.info(_LI("Found instance dir at non-default location: %s"),
instance_dir)
except os_win_exc.HyperVVMNotFoundException:

View File

@ -67,14 +67,17 @@ class HyperVClusterTestCase(test_base.HyperVBaseTestCase):
@mock.patch.object(base_driver.HyperVDriver, 'migrate_disk_and_power_off')
def test_migrate_disk_and_power_off(self, mock_superclass_migrate):
self.driver.migrate_disk_and_power_off(self.context,
mock.sentinel.fake_instance,
mock.sentinel.destination,
mock.sentinel.flavor,
mock.sentinel.network_info,
mock.sentinel.block_dev_info,
mock.sentinel.timeout,
mock.sentinel.retry_interval)
disk_info = self.driver.migrate_disk_and_power_off(
self.context,
mock.sentinel.fake_instance,
mock.sentinel.destination,
mock.sentinel.flavor,
mock.sentinel.network_info,
mock.sentinel.block_dev_info,
mock.sentinel.timeout,
mock.sentinel.retry_interval)
self.assertEqual(mock_superclass_migrate.return_value, disk_info)
self.driver._clops.remove_from_cluster.assert_called_once_with(
mock.sentinel.fake_instance)
mock_superclass_migrate.assert_called_once_with(

View File

@ -57,109 +57,22 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
self._imagecache = self._migrationops._imagecache
self._block_dev_man = self._migrationops._block_dev_man
def _check_migrate_disk_files(self, shared_storage=False):
instance_path = 'fake/instance/path'
dest_instance_path = 'remote/instance/path'
self._migrationops._pathutils.get_instance_dir.side_effect = (
instance_path, dest_instance_path)
get_revert_dir = (
def test_move_vm_files(self):
mock_instance = fake_instance.fake_instance_obj(self.context)
vm_files_path = self._migrationops._move_vm_files(mock_instance)
mock_get_inst_dir = self._migrationops._pathutils.get_instance_dir
mock_get_inst_dir.assert_called_once_with(mock_instance.name)
mock_get_revert_dir = (
self._migrationops._pathutils.get_instance_migr_revert_dir)
check_shared_storage = (
self._migrationops._pathutils.check_dirs_shared_storage)
check_shared_storage.return_value = shared_storage
self._migrationops._pathutils.exists.return_value = True
mock_get_revert_dir.assert_called_once_with(
mock_instance.name, remove_dir=True, create_dir=True)
fake_disk_files = [os.path.join(instance_path, disk_name)
for disk_name in
['root.vhd', 'configdrive.vhd', 'configdrive.iso',
'eph0.vhd', 'eph1.vhdx']]
expected_get_dir = [mock.call(mock.sentinel.instance_name),
mock.call(mock.sentinel.instance_name,
mock.sentinel.dest_path)]
expected_move_calls = [mock.call(instance_path,
get_revert_dir.return_value)]
self._migrationops._migrate_disk_files(
instance_name=mock.sentinel.instance_name,
disk_files=fake_disk_files,
dest=mock.sentinel.dest_path)
self._migrationops._pathutils.exists.assert_called_once_with(
dest_instance_path)
check_shared_storage.assert_called_once_with(
instance_path, dest_instance_path)
get_revert_dir.assert_called_with(mock.sentinel.instance_name,
remove_dir=True, create_dir=True)
if shared_storage:
fake_dest_path = '%s_tmp' % instance_path
expected_move_calls.append(mock.call(fake_dest_path,
instance_path))
self._migrationops._pathutils.rmtree.assert_called_once_with(
fake_dest_path)
else:
fake_dest_path = dest_instance_path
self._migrationops._pathutils.makedirs.assert_called_once_with(
fake_dest_path)
check_remove_dir = self._migrationops._pathutils.check_remove_dir
check_remove_dir.assert_called_once_with(fake_dest_path)
self._migrationops._pathutils.get_instance_dir.assert_has_calls(
expected_get_dir)
self._migrationops._pathutils.copy.assert_has_calls(
mock.call(fake_disk_file, fake_dest_path)
for fake_disk_file in fake_disk_files)
self.assertEqual(len(fake_disk_files),
self._migrationops._pathutils.copy.call_count)
self._migrationops._pathutils.move_folder_files.assert_has_calls(
expected_move_calls)
def test_migrate_disk_files(self):
self._check_migrate_disk_files()
def test_migrate_disk_files_same_host(self):
self._check_migrate_disk_files(shared_storage=True)
@mock.patch.object(migrationops.MigrationOps,
'_cleanup_failed_disk_migration')
def test_migrate_disk_files_exception(self, mock_cleanup):
instance_path = 'fake/instance/path'
fake_dest_path = '%s_tmp' % instance_path
self._migrationops._pathutils.get_instance_dir.return_value = (
instance_path)
get_revert_dir = (
self._migrationops._pathutils.get_instance_migr_revert_dir)
self._migrationops._hostutils.get_local_ips.return_value = [
mock.sentinel.dest_path]
self._migrationops._pathutils.copy.side_effect = IOError(
"Expected exception.")
self.assertRaises(IOError, self._migrationops._migrate_disk_files,
instance_name=mock.sentinel.instance_name,
disk_files=[self._FAKE_DISK],
dest=mock.sentinel.dest_path)
mock_cleanup.assert_called_once_with(instance_path,
get_revert_dir.return_value,
fake_dest_path)
def test_cleanup_failed_disk_migration(self):
self._migrationops._pathutils.exists.return_value = True
self._migrationops._cleanup_failed_disk_migration(
instance_path=mock.sentinel.instance_path,
revert_path=mock.sentinel.revert_path,
dest_path=mock.sentinel.dest_path)
expected = [mock.call(mock.sentinel.dest_path),
mock.call(mock.sentinel.revert_path)]
self._migrationops._pathutils.exists.assert_has_calls(expected)
move_folder_files = self._migrationops._pathutils.move_folder_files
move_folder_files.assert_called_once_with(
mock.sentinel.revert_path, mock.sentinel.instance_path)
self._migrationops._pathutils.rmtree.assert_has_calls([
mock.call(mock.sentinel.dest_path),
mock.call(mock.sentinel.revert_path)])
mock_move = self._migrationops._pathutils.move_folder_files
mock_move.assert_called_once_with(mock_get_inst_dir.return_value,
mock_get_revert_dir.return_value)
self.assertEqual(mock_get_revert_dir.return_value, vm_files_path)
def test_check_target_flavor(self):
mock_instance = fake_instance.fake_instance_obj(self.context)
@ -193,32 +106,25 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
instance,
mock.sentinel.FAKE_VM_GEN)
@mock.patch.object(migrationops.MigrationOps, '_migrate_disk_files')
@mock.patch.object(migrationops.MigrationOps, '_move_vm_files')
@mock.patch.object(migrationops.MigrationOps, '_check_target_flavor')
def test_migrate_disk_and_power_off(self, mock_check_flavor,
mock_migrate_disk_files):
mock_move_vm_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(
disk_info = 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)
self.assertEqual(mock_move_vm_files.return_value, disk_info)
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)
mock_move_vm_files.assert_called_once_with(instance)
self._migrationops._vmops.destroy.assert_called_once_with(
instance, destroy_disks=False)
instance, destroy_disks=True)
def test_confirm_migration(self):
mock_instance = fake_instance.fake_instance_obj(self.context)
@ -226,10 +132,11 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
context=self.context,
migration=mock.sentinel.migration, instance=mock_instance,
network_info=mock.sentinel.network_info)
get_instance_migr_revert_dir = (
self._migrationops._pathutils.get_instance_migr_revert_dir)
get_instance_migr_revert_dir.assert_called_with(mock_instance.name,
remove_dir=True)
get_instance_migr_revert_dir.assert_called_with(
mock_instance.name, remove_dir=True)
def test_revert_migration_files(self):
instance_path = (
@ -366,11 +273,30 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
recon_parent_vhd.assert_called_once_with(
mock.sentinel.diff_vhd_path, fake_base_vhd)
def test_migrate_disks_from_source(self):
mock_migration = mock.MagicMock()
mock_instance = fake_instance.fake_instance_obj(self.context)
self._migrationops._migrate_disks_from_source(
mock_migration, mock_instance, mock.sentinel.source_dir)
mock_get_remote_path = self._migrationops._pathutils.get_remote_path
mock_get_remote_path.assert_called_once_with(
mock_migration.source_compute, mock.sentinel.source_dir)
mock_get_inst_dir = self._migrationops._pathutils.get_instance_dir
mock_get_inst_dir.assert_called_once_with(
mock_instance.name, create_dir=True, remove_dir=True)
mock_copy = self._migrationops._pathutils.copy_folder_files
mock_copy.assert_called_once_with(mock_get_remote_path.return_value,
mock_get_inst_dir.return_value)
@mock.patch.object(migrationops.MigrationOps,
'_check_and_attach_config_drive')
@mock.patch.object(migrationops.MigrationOps, '_check_and_update_disks')
def test_finish_migration(self, mock_check_and_update_disks,
mock_check_attach_config_drive):
@mock.patch.object(migrationops.MigrationOps, '_migrate_disks_from_source')
def test_finish_migration(self, mock_migrate_disks_from_source,
mock_check_and_update_disks,
mock_check_attach_config_drive):
mock_instance = fake_instance.fake_instance_obj(self.context)
get_image_vm_gen = self._vmops.get_image_vm_generation
@ -381,6 +307,8 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
image_meta=mock.sentinel.image_meta, resize_instance=True,
block_device_info=mock.sentinel.block_device_info)
mock_migrate_disks_from_source.assert_called_once_with(
mock.sentinel.migration, mock_instance, mock.sentinel.disk_info)
get_image_vm_gen.assert_called_once_with(mock_instance.uuid,
mock.sentinel.image_meta)
mock_check_and_update_disks.assert_called_once_with(

View File

@ -37,6 +37,24 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
self._pathutils = pathutils.PathUtils()
@mock.patch.object(pathutils.PathUtils, 'copy')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
def test_copy_folder_files(self, mock_listdir, mock_isfile, mock_copy):
src_dir = 'src'
dest_dir = 'dest'
fname = 'tmp_file.txt'
subdir = 'tmp_folder'
src_fname = os.path.join(src_dir, fname)
dest_fname = os.path.join(dest_dir, fname)
# making sure src_subdir is not copied.
mock_listdir.return_value = [fname, subdir]
mock_isfile.side_effect = [True, False]
self._pathutils.copy_folder_files(src_dir, dest_dir)
mock_copy.assert_called_once_with(src_fname, dest_fname)
@ddt.data({'conf_instances_path': r'c:\inst_dir',
'expected_dir': r'c:\inst_dir'},
{'conf_instances_path': r'c:\inst_dir',
@ -60,6 +78,25 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
self.assertEqual(expected_dir, instances_dir)
def test_get_remote_path_share(self):
fake_remote_path = '\\\\fake_path'
actual_path = self._pathutils.get_remote_path(mock.sentinel.server,
fake_remote_path)
self.assertEqual(fake_remote_path, actual_path)
def test_get_remote_path_normal(self):
fake_server = 'fake_server'
fake_remote_path = 'C:\\fake_path'
actual_path = self._pathutils.get_remote_path(fake_server,
fake_remote_path)
expected_path = ('\\\\%(remote_server)s\\%(path)s' %
dict(remote_server=fake_server,
path=fake_remote_path.replace(':', '$')))
self.assertEqual(expected_path, actual_path)
@mock.patch.object(pathutils.PathUtils, 'get_instances_dir')
@mock.patch.object(pathutils.PathUtils, '_check_dir')
def test_get_instances_sub_dir(self, mock_check_dir,
@ -128,14 +165,14 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
'remote_server': mock.sentinel.remote_server})
@ddt.unpack
@mock.patch.object(pathutils.PathUtils, '_get_instances_sub_dir')
@mock.patch.object(pathutils.PathUtils, '_get_remote_unc_path')
@mock.patch.object(pathutils.PathUtils, 'get_remote_path')
@mock.patch.object(pathutils.PathUtils, '_check_dir')
@mock.patch.object(pathutils.os.path, 'exists')
@mock.patch('os_win.utilsfactory.get_vmutils')
def test_get_instance_dir(self, mock_get_vmutils,
mock_exists,
mock_check_dir,
mock_get_remote_unc_path,
mock_get_remote_path,
mock_get_instances_sub_dir,
configured_dir_exists=False,
remote_server=None, vm_exists=False):
@ -152,7 +189,7 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
else os_win_exc.HyperVVMNotFoundException(
vm_name=mock.sentinel.instance_name))
mock_get_remote_unc_path.return_value = mock.sentinel.remote_root_dir
mock_get_remote_path.return_value = mock.sentinel.remote_root_dir
instance_dir = self._pathutils.get_instance_dir(
mock.sentinel.instance_name,
@ -170,7 +207,7 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
if remote_server:
expected_instance_dir = mock.sentinel.remote_root_dir
mock_get_remote_unc_path.assert_called_once_with(
mock_get_remote_path.assert_called_once_with(
mock.sentinel.remote_server,
mock.sentinel.config_root_dir)
else: