openstack
/
compute-hyperv
Code Issues Proposed changes

Merge "Prevent reverting resizes from leaking files"

This commit is contained in:
Zuul 2018-11-29 17:40:29 +00:00 committed by Gerrit Code Review
parent a9a82b2e9b 976d316eb7
commit 0937cd1073
2 changed files with 33 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
compute_hyperv/nova/vmops.py
View File

@ -24,6 +24,7 @@ import time
from eventlet import timeout as etimeout
from nova.api.metadata import base as instance_metadata
from nova.compute import task_states
from nova.compute import vm_states
from nova import exception
from nova import objects
@ -884,6 +885,15 @@ class VMOps(object):
instance_path = self._pathutils.get_instance_dir(instance.name,
create_dir=False)
# When reverting resizes, the manager will request instance files
# cleanup to be skipped if the hosts use shared storage, in which
# case we'd leak files if multiple CSVs are used. It's safe to
# cleanup the instance files when reverting resizes as long as we
# preserve the "*_revert" dir.
if instance.task_state == task_states.RESIZE_REVERTING:
destroy_disks = True
cleanup_migration_files = False
try:
if self._vmutils.vm_exists(instance_name):

32
compute_hyperv/tests/unit/test_vmops.py
View File

@ -17,6 +17,7 @@ import os
import ddt
from eventlet import timeout as etimeout
import mock
from nova.compute import task_states
from nova.compute import vm_states
from nova import exception
from nova import objects
@ -1317,9 +1318,11 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
self._pathutils.check_remove_dir.assert_has_calls(
exp_check_remove_dir_calls)
@ddt.data({"force_destroy": True},
@ddt.data({"force_destroy": True, "destroy_disks": False},
{'vm_exists': False, 'planned_vm_exists': False},
{'vm_exists': False, 'planned_vm_exists': True})
{'vm_exists': False, 'planned_vm_exists': True},
{'task_state': task_states.RESIZE_REVERTING},
{'cleanup_migr_files': False})
@ddt.unpack
@mock.patch('compute_hyperv.nova.vmops.VMOps._delete_disk_files')
@mock.patch('compute_hyperv.nova.vmops.VMOps.power_off')
@ -1327,10 +1330,14 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
def test_destroy(self, mock_unplug_vifs, mock_power_off,
mock_delete_disk_files, vm_exists=True,
planned_vm_exists=False,
force_destroy=False):
force_destroy=False,
task_state=task_states.DELETING,
destroy_disks=True,
cleanup_migr_files=True):
self.flags(force_destroy_instances=force_destroy, group="hyperv")
mock_instance = fake_instance.fake_instance_obj(self.context)
mock_instance = fake_instance.fake_instance_obj(
self.context, task_state=task_state)
self._vmops._vmutils.vm_exists.return_value = vm_exists
self._vmops._migrutils.planned_vm_exists.return_value = (
planned_vm_exists)
@ -1339,7 +1346,8 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
instance=mock_instance,
block_device_info=mock.sentinel.FAKE_BD_INFO,
network_info=mock.sentinel.fake_network_info,
cleanup_migration_files=mock.sentinel.cleanup_migr_files)
cleanup_migration_files=cleanup_migr_files,
destroy_disks=destroy_disks)
self._vmops._vmutils.vm_exists.assert_called_with(
mock_instance.name)
@ -1366,10 +1374,16 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
mock_instance, mock.sentinel.fake_network_info)
self._vmops._volumeops.disconnect_volumes.assert_called_once_with(
mock.sentinel.FAKE_BD_INFO)
mock_delete_disk_files.assert_called_once_with(
mock_instance,
self._pathutils.get_instance_dir.return_value,
mock.sentinel.cleanup_migr_files)
reverting_resize = task_state == task_states.RESIZE_REVERTING
exp_migr_files_cleanup = cleanup_migr_files and not reverting_resize
if destroy_disks or reverting_resize:
mock_delete_disk_files.assert_called_once_with(
mock_instance,
self._pathutils.get_instance_dir.return_value,
exp_migr_files_cleanup)
else:
mock_delete_disk_files.assert_not_called()
@mock.patch('compute_hyperv.nova.vmops.VMOps.power_off')
def test_destroy_exception(self, mock_power_off):