From 6ae32f0ac37f8210e8e2ae2183a2a5950a78016f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89douard=20Thuleau?= Date: Thu, 8 Nov 2012 16:14:37 +0100 Subject: [PATCH] Compute doesn't set the 'host' field in instance When an instance starts, compute ask the resource manager to check the compute capabilities. If the compute resources are sufficient, the resource manager updates the 'host' instance field in the database. But the local variable 'instance' use by compute manger isn't updated. So, when compute manager asks the network manager to allocate a network for the instance, the 'host' instance field is null. Some compute tests doesn't update the local variable instance when they change some parameter on instance and failed with this patch. Fixes LP bug #1073600 Change-Id: I842d4814b9eabc6222c68118d8a244b20bb68164 (cherry picked from commit d32d5b252fc6c71a19b5372defb468fee9c57d3a) --- nova/compute/manager.py | 5 ++ nova/compute/resource_tracker.py | 20 ++--- nova/tests/compute/test_compute.py | 93 ++++++++++++++------- nova/tests/compute/test_resource_tracker.py | 1 + 4 files changed, 79 insertions(+), 40 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index c875da642016..5e720a304358 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1540,6 +1540,11 @@ class ComputeManager(manager.SchedulerDependentManager): self._notify_about_instance_usage( context, instance, "resize.prep.start") + if not instance['host']: + self._set_instance_error_state(context, instance['uuid']) + msg = _('Instance has no source host') + raise exception.MigrationError(msg) + same_host = instance['host'] == self.host if same_host and not FLAGS.allow_resize_to_same_host: self._set_instance_error_state(context, instance['uuid']) diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index fe56b1dad188..48dfeffe7cbc 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -155,14 +155,13 @@ class ResourceTracker(object): if self.disabled: # compute_driver doesn't support resource tracking, just # set the 'host' field and continue the build: - instance_ref = self._set_instance_host(context, - instance_ref['uuid']) + self._set_instance_host(context, instance_ref) return # sanity check: if instance_ref['host']: - LOG.warning(_("Host field should be not be set on the instance " - "until resources have been claimed."), + LOG.warning(_("Host field should not be set on the instance until " + "resources have been claimed."), instance=instance_ref) if not limits: @@ -195,7 +194,7 @@ class ResourceTracker(object): if not self._can_claim_cpu(vcpus, vcpu_limit): return - instance_ref = self._set_instance_host(context, instance_ref['uuid']) + self._set_instance_host(context, instance_ref) # keep track of this claim until we know whether the compute operation # was successful/completed: @@ -209,16 +208,17 @@ class ResourceTracker(object): self._update(context, self.compute_node) return claim - def _set_instance_host(self, context, instance_uuid): + def _set_instance_host(self, context, instance_ref): """Tag the instance as belonging to this host. This should be done while the COMPUTE_RESOURCES_SEMPAHORE is being held so the resource claim will not be lost if the audit process starts. """ values = {'host': self.host, 'launched_on': self.host} - (old_ref, instance_ref) = db.instance_update_and_get_original(context, - instance_uuid, values) - notifications.send_update(context, old_ref, instance_ref) - return instance_ref + (old_ref, new_ref) = db.instance_update_and_get_original(context, + instance_ref['uuid'], values) + notifications.send_update(context, old_ref, new_ref) + instance_ref['host'] = self.host + instance_ref['launched_on'] = self.host def _can_claim_memory(self, memory_mb, memory_mb_limit): """Test if memory needed for a claim can be safely allocated""" diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 8281f437d30a..674f531ae3e6 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -1650,15 +1650,16 @@ class ComputeTestCase(BaseTestCase): new_type_id = new_type['id'] self.compute.run_instance(self.context, instance=instance) - db.instance_update(self.context, instance['uuid'], {'host': 'foo'}) - db.instance_update(self.context, instance["uuid"], + new_instance = db.instance_update(self.context, instance['uuid'], + {'host': 'foo'}) + new_instance = jsonutils.to_primitive(new_instance) + db.instance_update(self.context, new_instance["uuid"], {"task_state": task_states.RESIZE_PREP}) - self.compute.prep_resize(context, instance=instance, + self.compute.prep_resize(context, instance=new_instance, instance_type=new_type, image={}) migration_ref = db.migration_get_by_instance_and_status(context, - instance['uuid'], - 'pre-migrating') - self.compute.resize_instance(context, instance=instance, + new_instance['uuid'], 'pre-migrating') + self.compute.resize_instance(context, instance=new_instance, migration_id=migration_ref['id'], image={}) timeutils.set_time_override(cur_time) @@ -1666,7 +1667,7 @@ class ComputeTestCase(BaseTestCase): self.compute.finish_resize(context, migration_id=int(migration_ref['id']), disk_info={}, image={}, - instance=instance) + instance=new_instance) self.assertEquals(len(test_notifier.NOTIFICATIONS), 2) msg = test_notifier.NOTIFICATIONS[0] @@ -1679,7 +1680,7 @@ class ComputeTestCase(BaseTestCase): payload = msg['payload'] self.assertEquals(payload['tenant_id'], self.project_id) self.assertEquals(payload['user_id'], self.user_id) - self.assertEquals(payload['instance_id'], instance['uuid']) + self.assertEquals(payload['instance_id'], new_instance['uuid']) self.assertEquals(payload['instance_type'], 'm1.small') self.assertEquals(str(payload['instance_type_id']), str(new_type_id)) self.assertTrue('display_name' in payload) @@ -1689,7 +1690,7 @@ class ComputeTestCase(BaseTestCase): image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF) self.assertEquals(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(context, - instance=jsonutils.to_primitive(instance)) + instance=jsonutils.to_primitive(new_instance)) def test_resize_instance_notification(self): """Ensure notifications on instance migrate/resize""" @@ -1703,12 +1704,14 @@ class ComputeTestCase(BaseTestCase): timeutils.set_time_override(cur_time) test_notifier.NOTIFICATIONS = [] - db.instance_update(self.context, instance['uuid'], {'host': 'foo'}) + new_instance = db.instance_update(self.context, instance['uuid'], + {'host': 'foo'}) + new_instance = jsonutils.to_primitive(new_instance) instance_type = instance_types.get_default_instance_type() - self.compute.prep_resize(context, instance=instance, + self.compute.prep_resize(context, instance=new_instance, instance_type=instance_type, image={}) db.migration_get_by_instance_and_status(context, - instance['uuid'], + new_instance['uuid'], 'pre-migrating') self.assertEquals(len(test_notifier.NOTIFICATIONS), 3) @@ -1725,7 +1728,7 @@ class ComputeTestCase(BaseTestCase): payload = msg['payload'] self.assertEquals(payload['tenant_id'], self.project_id) self.assertEquals(payload['user_id'], self.user_id) - self.assertEquals(payload['instance_id'], instance['uuid']) + self.assertEquals(payload['instance_id'], new_instance['uuid']) self.assertEquals(payload['instance_type'], 'm1.tiny') type_id = instance_types.get_instance_type_by_name('m1.tiny')['id'] self.assertEquals(str(payload['instance_type_id']), str(type_id)) @@ -1734,10 +1737,12 @@ class ComputeTestCase(BaseTestCase): self.assertTrue('launched_at' in payload) image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF) self.assertEquals(payload['image_ref_url'], image_ref_url) - self.compute.terminate_instance(context, instance=instance) + self.compute.terminate_instance(context, instance=new_instance) - def test_prep_resize_instance_migration_error(self): - """Ensure prep_resize raise a migration error""" + def test_prep_resize_instance_migration_error_on_same_host(self): + """Ensure prep_resize raise a migration error if destination is set on + the same source host and allow_resize_to_same_host is false + """ self.flags(host="foo", allow_resize_to_same_host=False) instance = jsonutils.to_primitive(self._create_fake_instance()) @@ -1757,6 +1762,26 @@ class ComputeTestCase(BaseTestCase): reservations=reservations) self.compute.terminate_instance(context, instance=new_instance) + def test_prep_resize_instance_migration_error_on_none_host(self): + """Ensure prep_resize raise a migration error if destination host is + not defined + """ + instance = jsonutils.to_primitive(self._create_fake_instance()) + + reservations = self._ensure_quota_reservations_rolledback() + + self.compute.run_instance(self.context, instance=instance) + new_instance = db.instance_update(self.context, instance['uuid'], + {'host': None}) + new_instance = jsonutils.to_primitive(new_instance) + instance_type = instance_types.get_default_instance_type() + + self.assertRaises(exception.MigrationError, self.compute.prep_resize, + self.context, instance=new_instance, + instance_type=instance_type, image={}, + reservations=reservations) + self.compute.terminate_instance(self.context, instance=new_instance) + def test_resize_instance_driver_error(self): """Ensure instance status set to Error on resize error""" @@ -1773,21 +1798,23 @@ class ComputeTestCase(BaseTestCase): reservations = self._ensure_quota_reservations_rolledback() self.compute.run_instance(self.context, instance=instance) - db.instance_update(self.context, instance['uuid'], {'host': 'foo'}) - self.compute.prep_resize(context, instance=instance, + new_instance = db.instance_update(self.context, instance['uuid'], + {'host': 'foo'}) + new_instance = jsonutils.to_primitive(new_instance) + self.compute.prep_resize(context, instance=new_instance, instance_type=instance_type, image={}, reservations=reservations) migration_ref = db.migration_get_by_instance_and_status(context, - instance['uuid'], 'pre-migrating') + new_instance['uuid'], 'pre-migrating') - db.instance_update(self.context, instance['uuid'], + db.instance_update(self.context, new_instance['uuid'], {"task_state": task_states.RESIZE_PREP}) #verify self.assertRaises(test.TestingException, self.compute.resize_instance, - context, instance=instance, + context, instance=new_instance, migration_id=migration_ref['id'], image={}, reservations=reservations) - instance = db.instance_get_by_uuid(context, instance['uuid']) + instance = db.instance_get_by_uuid(context, new_instance['uuid']) self.assertEqual(instance['vm_state'], vm_states.ERROR) self.compute.terminate_instance(context, @@ -1800,19 +1827,23 @@ class ComputeTestCase(BaseTestCase): context = self.context.elevated() self.compute.run_instance(self.context, instance=instance) - db.instance_update(self.context, instance['uuid'], + new_instance = db.instance_update(self.context, instance['uuid'], {'host': 'foo'}) - self.compute.prep_resize(context, instance=instance, + new_instance = jsonutils.to_primitive(new_instance) + self.compute.prep_resize(context, instance=new_instance, instance_type=instance_type, image={}) migration_ref = db.migration_get_by_instance_and_status(context, - instance['uuid'], 'pre-migrating') - db.instance_update(self.context, instance['uuid'], + new_instance['uuid'], 'pre-migrating') + db.instance_update(self.context, new_instance['uuid'], {"task_state": task_states.RESIZE_PREP}) - self.compute.resize_instance(context, instance=instance, + self.compute.resize_instance(context, instance=new_instance, migration_id=migration_ref['id'], image={}) - self.compute.terminate_instance(context, - instance=jsonutils.to_primitive(instance)) + inst = db.instance_get_by_uuid(self.context, new_instance['uuid']) + # NOTE(vish): removed check of dest_compute because it doesn't + # happen until finish_resize in folsom + self.compute.terminate_instance(self.context, + instance=jsonutils.to_primitive(inst)) def test_finish_revert_resize(self): """Ensure that the flavor is reverted to the original on revert""" @@ -1922,7 +1953,9 @@ class ComputeTestCase(BaseTestCase): context = self.context.elevated() self.compute.run_instance(self.context, instance=inst_ref) - db.instance_update(self.context, inst_ref['uuid'], {'host': 'foo'}) + inst_ref = db.instance_update(self.context, inst_ref['uuid'], + {'host': 'foo'}) + inst_ref = jsonutils.to_primitive(inst_ref) self.compute.prep_resize(context, instance=inst_ref, instance_type=instance_type, image={}, reservations=reservations) diff --git a/nova/tests/compute/test_resource_tracker.py b/nova/tests/compute/test_resource_tracker.py index 0dccb0d15d52..32d38c2a0662 100644 --- a/nova/tests/compute/test_resource_tracker.py +++ b/nova/tests/compute/test_resource_tracker.py @@ -130,6 +130,7 @@ class BaseTestCase(test.TestCase): 'project_id': '123456', 'vcpus': 1, 'host': None, + 'launched_on': None, } instance.update(kwargs)