openstack
/
nova
Code Issues Proposed changes

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
(cherry picked from commit aba3f64932)
This commit is contained in:
Maciej Józefczyk 2017-08-08 15:37:26 +02:00 committed by Maciej Jozefczyk
parent 6c67eee6f4
commit b03f548765
3 changed files with 16 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nova/compute/manager.py
View File

@ -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}
@ -6711,8 +6714,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)]

15
nova/tests/unit/compute/test_compute.py
View File

@ -6568,8 +6568,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')
@ -6584,8 +6583,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)])
@ -6604,8 +6602,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)])
@ -6625,8 +6622,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 +6645,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)

8
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -1385,6 +1385,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):
@ -1403,7 +1406,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')
@ -1437,6 +1440,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)]