From aba3f649323d328f8122d372dea38256dfed6a52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20J=C3=B3zefczyk?= Date: Tue, 8 Aug 2017 15:37:26 +0200 Subject: [PATCH] Remove host filter for _cleanup_running_deleted_instances periodic task Periodic task _cleanup_running_deleted_instances() looks for orphaned and running instances on hypervisor that should be deleted. The problem is it checks if running instance has the same hypervisor defined as it is in nova database. In bug #1285000 it has been found that removing instance during its migration could lead to abandon instance files on destination host. This change removes host filter in _running_deleted_instances() to find also orphaned instances that are running on 'post migration' destination host. Change-Id: Idd1b58b85329b8e021eba4bc27f577af1b3338f4 Partial-Bug: #1285000 --- nova/compute/manager.py | 6 ++++-- nova/tests/unit/compute/test_compute.py | 15 +++++---------- nova/tests/unit/compute/test_compute_mgr.py | 8 +++++++- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index a5ead22a1a..501cb3a615 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -610,6 +610,9 @@ class ComputeManager(manager.Manager): # The driver doesn't support uuids listing, so we'll have # to brute force. driver_instances = self.driver.list_instances() + # NOTE(mjozefcz): In this case we need to apply host filter. + # Without this all instance data would be fetched from db. + filters['host'] = self.host instances = objects.InstanceList.get_by_filters(context, filters, use_slave=True) name_map = {instance.name: instance for instance in instances} @@ -6719,8 +6722,7 @@ class ComputeManager(manager.Manager): """ timeout = CONF.running_deleted_instance_timeout filters = {'deleted': True, - 'soft_deleted': False, - 'host': self.host} + 'soft_deleted': False} instances = self._get_instances_on_driver(context, filters) return [i for i in instances if self._deleted_old_enough(i, timeout)] diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index aebfebef38..8d4387130f 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -6613,8 +6613,7 @@ class ComputeTestCase(BaseTestCase, mock.call(ctxt, inst2.uuid, use_slave=True)]) mock_get_inst.assert_called_once_with(ctxt, {'deleted': True, - 'soft_deleted': False, - 'host': self.compute.host}) + 'soft_deleted': False}) @mock.patch.object(compute_manager.ComputeManager, '_get_instances_on_driver') @@ -6629,8 +6628,7 @@ class ComputeTestCase(BaseTestCase, mock_get.assert_called_once_with(ctxt, {'deleted': True, - 'soft_deleted': False, - 'host': self.compute.host}) + 'soft_deleted': False}) mock_power.assert_has_calls([mock.call(inst1), mock.call(inst2)]) mock_set.assert_has_calls([mock.call(inst1, False), mock.call(inst2, False)]) @@ -6649,8 +6647,7 @@ class ComputeTestCase(BaseTestCase, mock_get.assert_called_once_with(ctxt, {'deleted': True, - 'soft_deleted': False, - 'host': self.compute.host}) + 'soft_deleted': False}) mock_set.assert_has_calls([mock.call(inst1, False), mock.call(inst2, False)]) mock_power.assert_has_calls([mock.call(inst1), mock.call(inst2)]) @@ -6670,8 +6667,7 @@ class ComputeTestCase(BaseTestCase, mock_get.assert_called_once_with(ctxt, {'deleted': True, - 'soft_deleted': False, - 'host': self.compute.host}) + 'soft_deleted': False}) mock_power.assert_has_calls([mock.call(inst1), mock.call(inst2)]) mock_set.assert_has_calls([mock.call(inst1, False), mock.call(inst2, False)]) @@ -6694,8 +6690,7 @@ class ComputeTestCase(BaseTestCase, self.assertEqual(val, [instance]) mock_get.assert_called_once_with( admin_context, {'deleted': True, - 'soft_deleted': False, - 'host': self.compute.host}) + 'soft_deleted': False}) mock_is_older.assert_called_once_with(now, CONF.running_deleted_instance_timeout) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 6c79400979..7231d5bc66 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -1428,6 +1428,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.assertEqual([x['uuid'] for x in driver_instances], [x['uuid'] for x in result]) + expected_filters = {'uuid': driver_uuids} + mock_instance_list.assert_called_with(self.context, expected_filters, + use_slave=True) @mock.patch('nova.objects.InstanceList.get_by_filters') def test_get_instances_on_driver_empty(self, mock_instance_list): @@ -1446,7 +1449,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): # Test getting instances when driver doesn't support # 'list_instance_uuids' self.compute.host = 'host' - filters = {'host': self.compute.host} + filters = {} self.flags(instance_name_template='inst-%i') @@ -1480,6 +1483,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.assertEqual([x['uuid'] for x in driver_instances], [x['uuid'] for x in result]) + expected_filters = {'host': self.compute.host} + mock_instance_list.assert_called_with(self.context, expected_filters, + use_slave=True) def test_instance_usage_audit(self): instances = [objects.Instance(uuid=uuids.instance)]