summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZuul <zuul@review.openstack.org>2019-02-12 16:37:01 +0000
committerGerrit Code Review <review@openstack.org>2019-02-12 16:37:01 +0000
commit2f4fc5cba2de26efce72361ad3255190ca5f3339 (patch)
treef60cb617898b33c97826cd5d9d40742ab2f70a62
parenteb120eb7c861c7332fc8ff561a4d7de80a1185d1 (diff)
parentfb61e864b98dde4955ab29f7a39c9935fbd5ffd0 (diff)
Merge "Fix InstanceNotFound during _destroy_evacuated_instances" into stable/pikestable/pike
-rw-r--r--nova/compute/manager.py32
-rw-r--r--nova/tests/functional/regressions/test_bug_1794996.py10
-rw-r--r--nova/tests/unit/compute/test_compute.py21
-rw-r--r--nova/tests/unit/compute/test_compute_mgr.py10
4 files changed, 48 insertions, 25 deletions
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 6001182..cb5f937 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -659,7 +659,14 @@ class ComputeManager(manager.Manager):
659 return 659 return
660 evacuations = {mig.instance_uuid: mig for mig in evacuations} 660 evacuations = {mig.instance_uuid: mig for mig in evacuations}
661 661
662 local_instances = self._get_instances_on_driver(context) 662 # The instances might be deleted in which case we need to avoid
663 # InstanceNotFound being raised from lazy-loading fields on the
664 # instances while cleaning up this host.
665 read_deleted_context = context.elevated(read_deleted='yes')
666 # TODO(mriedem): We could optimize by pre-loading the joined fields
667 # we know we'll use, like info_cache and flavor. We can also replace
668 # this with a generic solution: https://review.openstack.org/575190/
669 local_instances = self._get_instances_on_driver(read_deleted_context)
663 evacuated = [inst for inst in local_instances 670 evacuated = [inst for inst in local_instances
664 if inst.uuid in evacuations] 671 if inst.uuid in evacuations]
665 672
@@ -704,15 +711,20 @@ class ComputeManager(manager.Manager):
704 continue 711 continue
705 cn_uuid = compute_nodes[migration.source_node] 712 cn_uuid = compute_nodes[migration.source_node]
706 713
707 my_resources = scheduler_utils.resources_from_flavor( 714 # If the instance was deleted in the interim, assume its
708 instance, instance.flavor) 715 # allocations were properly cleaned up (either by its hosting
709 res = self.reportclient.remove_provider_from_instance_allocation( 716 # compute service or the API).
710 instance.uuid, cn_uuid, instance.user_id, 717 if not instance.deleted:
711 instance.project_id, my_resources) 718 my_resources = scheduler_utils.resources_from_flavor(
712 if not res: 719 instance, instance.flavor)
713 LOG.error("Failed to clean allocation of evacuated instance " 720 res = self.reportclient.\
714 "on the source node %s", 721 remove_provider_from_instance_allocation(
715 cn_uuid, instance=instance) 722 instance.uuid, cn_uuid, instance.user_id,
723 instance.project_id, my_resources)
724 if not res:
725 LOG.error("Failed to clean allocation of evacuated "
726 "instance on the source node %s",
727 cn_uuid, instance=instance)
716 728
717 migration.status = 'completed' 729 migration.status = 'completed'
718 migration.save() 730 migration.save()
diff --git a/nova/tests/functional/regressions/test_bug_1794996.py b/nova/tests/functional/regressions/test_bug_1794996.py
index f6b2069..f213373 100644
--- a/nova/tests/functional/regressions/test_bug_1794996.py
+++ b/nova/tests/functional/regressions/test_bug_1794996.py
@@ -12,7 +12,6 @@
12 12
13from oslo_log import log as logging 13from oslo_log import log as logging
14 14
15from nova import exception
16from nova.tests.functional import test_servers 15from nova.tests.functional import test_servers
17 16
18LOG = logging.getLogger(__name__) 17LOG = logging.getLogger(__name__)
@@ -149,7 +148,8 @@ class TestEvacuateDeleteServerRestartOriginalCompute(
149 instance. Before the bug is fixed, the original compute fails to start 148 instance. Before the bug is fixed, the original compute fails to start
150 because lazy-loading the instance.flavor on the deleted instance, 149 because lazy-loading the instance.flavor on the deleted instance,
151 which is needed to cleanup allocations from the source host, raises 150 which is needed to cleanup allocations from the source host, raises
152 InstanceNotFound. 151 InstanceNotFound. After the bug is fixed, the original source host
152 compute service starts up.
153 """ 153 """
154 source_hostname = self.compute1.host 154 source_hostname = self.compute1.host
155 dest_hostname = self.compute2.host 155 dest_hostname = self.compute2.host
@@ -197,8 +197,4 @@ class TestEvacuateDeleteServerRestartOriginalCompute(
197 self._delete_and_check_allocations(server) 197 self._delete_and_check_allocations(server)
198 198
199 # restart the source compute 199 # restart the source compute
200 # FIXME(mriedem): This is bug 1794996 where we try to lazy-load the 200 self.restart_compute_service(self.compute1)
201 # instance.flavor from the deleted instance which causes the compute
202 # startup to fail.
203 self.assertRaises(exception.InstanceNotFound,
204 self.restart_compute_service, self.compute1)
diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py
index 9f2b77a..f29d798 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -7170,6 +7170,7 @@ class ComputeTestCase(BaseTestCase,
7170 instance = self._create_fake_instance_obj({'host': 'someotherhost'}) 7170 instance = self._create_fake_instance_obj({'host': 'someotherhost'})
7171 self.compute._instance_update(self.context, instance, vcpus=4) 7171 self.compute._instance_update(self.context, instance, vcpus=4)
7172 7172
7173 @mock.patch('nova.context.RequestContext.elevated')
7173 @mock.patch.object(compute_manager.ComputeManager, 7174 @mock.patch.object(compute_manager.ComputeManager,
7174 '_get_instances_on_driver') 7175 '_get_instances_on_driver')
7175 @mock.patch.object(compute_manager.ComputeManager, 7176 @mock.patch.object(compute_manager.ComputeManager,
@@ -7181,8 +7182,10 @@ class ComputeTestCase(BaseTestCase,
7181 @mock.patch('nova.objects.Migration.save') 7182 @mock.patch('nova.objects.Migration.save')
7182 def test_destroy_evacuated_instance_on_shared_storage(self, mock_save, 7183 def test_destroy_evacuated_instance_on_shared_storage(self, mock_save,
7183 mock_get_filter, mock_destroy, mock_is_inst, mock_get_blk, 7184 mock_get_filter, mock_destroy, mock_is_inst, mock_get_blk,
7184 mock_get_inst): 7185 mock_get_inst, mock_elevated):
7185 fake_context = context.get_admin_context() 7186 fake_context = context.get_admin_context()
7187 read_deleted_context = fake_context.elevated(read_only='yes')
7188 mock_elevated.return_value = read_deleted_context
7186 7189
7187 # instances in central db 7190 # instances in central db
7188 instances = [ 7191 instances = [
@@ -7219,7 +7222,7 @@ class ComputeTestCase(BaseTestCase,
7219 'pre-migrating', 7222 'pre-migrating',
7220 'done'], 7223 'done'],
7221 'migration_type': 'evacuation'}) 7224 'migration_type': 'evacuation'})
7222 mock_get_inst.assert_called_once_with(fake_context) 7225 mock_get_inst.assert_called_once_with(read_deleted_context)
7223 mock_get_nw.assert_called_once_with(fake_context, evacuated_instance) 7226 mock_get_nw.assert_called_once_with(fake_context, evacuated_instance)
7224 mock_get_blk.assert_called_once_with(fake_context, evacuated_instance) 7227 mock_get_blk.assert_called_once_with(fake_context, evacuated_instance)
7225 mock_is_inst.assert_called_once_with(fake_context, evacuated_instance) 7228 mock_is_inst.assert_called_once_with(fake_context, evacuated_instance)
@@ -7227,6 +7230,7 @@ class ComputeTestCase(BaseTestCase,
7227 'fake_network_info', 7230 'fake_network_info',
7228 'fake_bdi', False) 7231 'fake_bdi', False)
7229 7232
7233 @mock.patch('nova.context.RequestContext.elevated')
7230 @mock.patch.object(compute_manager.ComputeManager, 7234 @mock.patch.object(compute_manager.ComputeManager,
7231 '_get_instances_on_driver') 7235 '_get_instances_on_driver')
7232 @mock.patch.object(compute_manager.ComputeManager, 7236 @mock.patch.object(compute_manager.ComputeManager,
@@ -7242,8 +7246,10 @@ class ComputeTestCase(BaseTestCase,
7242 @mock.patch('nova.objects.Migration.save') 7246 @mock.patch('nova.objects.Migration.save')
7243 def test_destroy_evacuated_instance_with_disks(self, mock_save, 7247 def test_destroy_evacuated_instance_with_disks(self, mock_save,
7244 mock_get_filter, mock_destroy, mock_check_clean, mock_check, 7248 mock_get_filter, mock_destroy, mock_check_clean, mock_check,
7245 mock_check_local, mock_get_blk, mock_get_drv): 7249 mock_check_local, mock_get_blk, mock_get_drv, mock_elevated):
7246 fake_context = context.get_admin_context() 7250 fake_context = context.get_admin_context()
7251 read_deleted_context = fake_context.elevated(read_deleted='yes')
7252 mock_elevated.return_value = read_deleted_context
7247 7253
7248 # instances in central db 7254 # instances in central db
7249 instances = [ 7255 instances = [
@@ -7274,7 +7280,7 @@ class ComputeTestCase(BaseTestCase,
7274 return_value='fake_network_info') as mock_get_nw: 7280 return_value='fake_network_info') as mock_get_nw:
7275 self.compute._destroy_evacuated_instances(fake_context) 7281 self.compute._destroy_evacuated_instances(fake_context)
7276 7282
7277 mock_get_drv.assert_called_once_with(fake_context) 7283 mock_get_drv.assert_called_once_with(read_deleted_context)
7278 mock_get_nw.assert_called_once_with(fake_context, evacuated_instance) 7284 mock_get_nw.assert_called_once_with(fake_context, evacuated_instance)
7279 mock_get_blk.assert_called_once_with(fake_context, evacuated_instance) 7285 mock_get_blk.assert_called_once_with(fake_context, evacuated_instance)
7280 mock_check_local.assert_called_once_with(fake_context, 7286 mock_check_local.assert_called_once_with(fake_context,
@@ -7288,6 +7294,7 @@ class ComputeTestCase(BaseTestCase,
7288 'fake_network_info', 'fake-bdi', 7294 'fake_network_info', 'fake-bdi',
7289 True) 7295 True)
7290 7296
7297 @mock.patch('nova.context.RequestContext.elevated')
7291 @mock.patch.object(compute_manager.ComputeManager, 7298 @mock.patch.object(compute_manager.ComputeManager,
7292 '_get_instances_on_driver') 7299 '_get_instances_on_driver')
7293 @mock.patch.object(compute_manager.ComputeManager, 7300 @mock.patch.object(compute_manager.ComputeManager,
@@ -7302,8 +7309,10 @@ class ComputeTestCase(BaseTestCase,
7302 @mock.patch('nova.objects.Migration.save') 7309 @mock.patch('nova.objects.Migration.save')
7303 def test_destroy_evacuated_instance_not_implemented(self, mock_save, 7310 def test_destroy_evacuated_instance_not_implemented(self, mock_save,
7304 mock_get_filter, mock_destroy, mock_check_clean, mock_check, 7311 mock_get_filter, mock_destroy, mock_check_clean, mock_check,
7305 mock_check_local, mock_get_blk, mock_get_inst): 7312 mock_check_local, mock_get_blk, mock_get_inst, mock_elevated):
7306 fake_context = context.get_admin_context() 7313 fake_context = context.get_admin_context()
7314 read_deleted_context = fake_context.elevated(read_deleted='yes')
7315 mock_elevated.return_value = read_deleted_context
7307 7316
7308 # instances in central db 7317 # instances in central db
7309 instances = [ 7318 instances = [
@@ -7333,7 +7342,7 @@ class ComputeTestCase(BaseTestCase,
7333 return_value='fake_network_info') as mock_get_nw: 7342 return_value='fake_network_info') as mock_get_nw:
7334 self.compute._destroy_evacuated_instances(fake_context) 7343 self.compute._destroy_evacuated_instances(fake_context)
7335 7344
7336 mock_get_inst.assert_called_once_with(fake_context) 7345 mock_get_inst.assert_called_once_with(read_deleted_context)
7337 mock_get_nw.assert_called_once_with(fake_context, evacuated_instance) 7346 mock_get_nw.assert_called_once_with(fake_context, evacuated_instance)
7338 mock_get_blk.assert_called_once_with(fake_context, evacuated_instance) 7347 mock_get_blk.assert_called_once_with(fake_context, evacuated_instance)
7339 mock_check_local.assert_called_once_with(fake_context, 7348 mock_check_local.assert_called_once_with(fake_context,
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index 4d29c51..1f77aae 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -656,6 +656,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
656 self.compute.init_virt_events() 656 self.compute.init_virt_events()
657 self.assertFalse(mock_register.called) 657 self.assertFalse(mock_register.called)
658 658
659 @mock.patch('nova.context.RequestContext.elevated')
659 @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') 660 @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
660 @mock.patch('nova.scheduler.utils.resources_from_flavor') 661 @mock.patch('nova.scheduler.utils.resources_from_flavor')
661 @mock.patch.object(manager.ComputeManager, '_get_instances_on_driver') 662 @mock.patch.object(manager.ComputeManager, '_get_instances_on_driver')
@@ -670,10 +671,12 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
670 def test_init_host_with_evacuated_instance(self, mock_save, mock_mig_get, 671 def test_init_host_with_evacuated_instance(self, mock_save, mock_mig_get,
671 mock_temp_mut, mock_init_host, mock_destroy, mock_host_get, 672 mock_temp_mut, mock_init_host, mock_destroy, mock_host_get,
672 mock_admin_ctxt, mock_init_virt, mock_get_inst, mock_resources, 673 mock_admin_ctxt, mock_init_virt, mock_get_inst, mock_resources,
673 mock_get_node): 674 mock_get_node, mock_elevated):
674 our_host = self.compute.host 675 our_host = self.compute.host
675 not_our_host = 'not-' + our_host 676 not_our_host = 'not-' + our_host
676 677
678 read_deleted_context = self.context.elevated(read_deleted='yes')
679 mock_elevated.return_value = read_deleted_context
677 deleted_instance = fake_instance.fake_instance_obj( 680 deleted_instance = fake_instance.fake_instance_obj(
678 self.context, host=not_our_host, uuid=uuids.deleted_instance) 681 self.context, host=not_our_host, uuid=uuids.deleted_instance)
679 migration = objects.Migration(instance_uuid=deleted_instance.uuid) 682 migration = objects.Migration(instance_uuid=deleted_instance.uuid)
@@ -709,7 +712,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
709 expected_attrs=['info_cache', 'metadata']) 712 expected_attrs=['info_cache', 'metadata'])
710 mock_init_virt.assert_called_once_with() 713 mock_init_virt.assert_called_once_with()
711 mock_temp_mut.assert_called_once_with(self.context, read_deleted='yes') 714 mock_temp_mut.assert_called_once_with(self.context, read_deleted='yes')
712 mock_get_inst.assert_called_once_with(self.context) 715 mock_get_inst.assert_called_once_with(read_deleted_context)
713 mock_get_net.assert_called_once_with(self.context, deleted_instance) 716 mock_get_net.assert_called_once_with(self.context, deleted_instance)
714 717
715 # ensure driver.destroy is called so that driver may 718 # ensure driver.destroy is called so that driver may
@@ -3359,6 +3362,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
3359 instance_2.host = 'not-' + our_host 3362 instance_2.host = 'not-' + our_host
3360 instance_2.user_id = uuids.user_id 3363 instance_2.user_id = uuids.user_id
3361 instance_2.project_id = uuids.project_id 3364 instance_2.project_id = uuids.project_id
3365 instance_2.deleted = False
3362 3366
3363 # Only instance 2 has a migration record 3367 # Only instance 2 has a migration record
3364 migration = objects.Migration(instance_uuid=instance_2.uuid) 3368 migration = objects.Migration(instance_uuid=instance_2.uuid)
@@ -3416,6 +3420,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
3416 instance_1.host = 'not-' + our_host 3420 instance_1.host = 'not-' + our_host
3417 instance_1.user_id = uuids.user_id 3421 instance_1.user_id = uuids.user_id
3418 instance_1.project_id = uuids.project_id 3422 instance_1.project_id = uuids.project_id
3423 instance_1.deleted = False
3419 instance_2 = objects.Instance(self.context, flavor=flavor) 3424 instance_2 = objects.Instance(self.context, flavor=flavor)
3420 instance_2.uuid = uuids.instance_2 3425 instance_2.uuid = uuids.instance_2
3421 instance_2.task_state = None 3426 instance_2.task_state = None
@@ -3423,6 +3428,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
3423 instance_2.host = 'not-' + our_host 3428 instance_2.host = 'not-' + our_host
3424 instance_2.user_id = uuids.user_id 3429 instance_2.user_id = uuids.user_id
3425 instance_2.project_id = uuids.project_id 3430 instance_2.project_id = uuids.project_id
3431 instance_2.deleted = False
3426 3432
3427 migration_1 = objects.Migration(instance_uuid=instance_1.uuid) 3433 migration_1 = objects.Migration(instance_uuid=instance_1.uuid)
3428 # Consider the migration successful but the node was deleted while the 3434 # Consider the migration successful but the node was deleted while the