openstack/nova
Code Issues Proposed changes

Compute doesn't set the 'host' field in instance

Browse Source 
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 d32d5b252f)
This commit is contained in:
Édouard Thuleau 2012-11-08 16:14:37 +01:00 committed by Vishvananda Ishaya
parent 284f6ea64b
commit 6ae32f0ac3
4 changed files with 79 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
nova/compute/manager.py
View File

@ -1540,6 +1540,11 @@ class ComputeManager(manager.SchedulerDependentManager):
self._notify_about_instance_usage( self._notify_about_instance_usage(
context, instance, "resize.prep.start") 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 same_host = instance['host'] == self.host
if same_host and not FLAGS.allow_resize_to_same_host: if same_host and not FLAGS.allow_resize_to_same_host:
self._set_instance_error_state(context, instance['uuid']) self._set_instance_error_state(context, instance['uuid'])

20
nova/compute/resource_tracker.py
View File

@ -155,14 +155,13 @@ class ResourceTracker(object):
if self.disabled: if self.disabled:
# compute_driver doesn't support resource tracking, just # compute_driver doesn't support resource tracking, just
# set the 'host' field and continue the build: # set the 'host' field and continue the build:
instance_ref = self._set_instance_host(context, self._set_instance_host(context, instance_ref)
instance_ref['uuid'])
return return
# sanity check: # sanity check:
if instance_ref['host']: if instance_ref['host']:
LOG.warning(_("Host field should be not be set on the instance " LOG.warning(_("Host field should not be set on the instance until "
"until resources have been claimed."), "resources have been claimed."),
instance=instance_ref) instance=instance_ref)
if not limits: if not limits:
@ -195,7 +194,7 @@ class ResourceTracker(object):
if not self._can_claim_cpu(vcpus, vcpu_limit): if not self._can_claim_cpu(vcpus, vcpu_limit):
return 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 # keep track of this claim until we know whether the compute operation
# was successful/completed: # was successful/completed:
@ -209,16 +208,17 @@ class ResourceTracker(object):
self._update(context, self.compute_node) self._update(context, self.compute_node)
return claim 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 """Tag the instance as belonging to this host. This should be done
while the COMPUTE_RESOURCES_SEMPAHORE is being held so the resource while the COMPUTE_RESOURCES_SEMPAHORE is being held so the resource
claim will not be lost if the audit process starts. claim will not be lost if the audit process starts.
""" """
values = {'host': self.host, 'launched_on': self.host} values = {'host': self.host, 'launched_on': self.host}
(old_ref, instance_ref) = db.instance_update_and_get_original(context, (old_ref, new_ref) = db.instance_update_and_get_original(context,
instance_uuid, values) instance_ref['uuid'], values)
notifications.send_update(context, old_ref, instance_ref) notifications.send_update(context, old_ref, new_ref)
return instance_ref instance_ref['host'] = self.host
instance_ref['launched_on'] = self.host
def _can_claim_memory(self, memory_mb, memory_mb_limit): def _can_claim_memory(self, memory_mb, memory_mb_limit):
"""Test if memory needed for a claim can be safely allocated""" """Test if memory needed for a claim can be safely allocated"""

93
nova/tests/compute/test_compute.py
View File

@ -1650,15 +1650,16 @@ class ComputeTestCase(BaseTestCase):
new_type_id = new_type['id'] new_type_id = new_type['id']
self.compute.run_instance(self.context, instance=instance) self.compute.run_instance(self.context, instance=instance)
db.instance_update(self.context, instance['uuid'], {'host': 'foo'}) new_instance = db.instance_update(self.context, instance['uuid'],
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}) {"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={}) instance_type=new_type, image={})
migration_ref = db.migration_get_by_instance_and_status(context, migration_ref = db.migration_get_by_instance_and_status(context,
instance['uuid'], new_instance['uuid'], 'pre-migrating')
'pre-migrating') self.compute.resize_instance(context, instance=new_instance,
self.compute.resize_instance(context, instance=instance,
migration_id=migration_ref['id'], migration_id=migration_ref['id'],
image={}) image={})
timeutils.set_time_override(cur_time) timeutils.set_time_override(cur_time)
@ -1666,7 +1667,7 @@ class ComputeTestCase(BaseTestCase):
self.compute.finish_resize(context, self.compute.finish_resize(context,
migration_id=int(migration_ref['id']), disk_info={}, image={}, migration_id=int(migration_ref['id']), disk_info={}, image={},
instance=instance) instance=new_instance)
self.assertEquals(len(test_notifier.NOTIFICATIONS), 2) self.assertEquals(len(test_notifier.NOTIFICATIONS), 2)
msg = test_notifier.NOTIFICATIONS[0] msg = test_notifier.NOTIFICATIONS[0]
@ -1679,7 +1680,7 @@ class ComputeTestCase(BaseTestCase):
payload = msg['payload'] payload = msg['payload']
self.assertEquals(payload['tenant_id'], self.project_id) self.assertEquals(payload['tenant_id'], self.project_id)
self.assertEquals(payload['user_id'], self.user_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(payload['instance_type'], 'm1.small')
self.assertEquals(str(payload['instance_type_id']), str(new_type_id)) self.assertEquals(str(payload['instance_type_id']), str(new_type_id))
self.assertTrue('display_name' in payload) self.assertTrue('display_name' in payload)
@ -1689,7 +1690,7 @@ class ComputeTestCase(BaseTestCase):
image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF) image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF)
self.assertEquals(payload['image_ref_url'], image_ref_url) self.assertEquals(payload['image_ref_url'], image_ref_url)
self.compute.terminate_instance(context, self.compute.terminate_instance(context,
instance=jsonutils.to_primitive(instance)) instance=jsonutils.to_primitive(new_instance))
def test_resize_instance_notification(self): def test_resize_instance_notification(self):
"""Ensure notifications on instance migrate/resize""" """Ensure notifications on instance migrate/resize"""
@ -1703,12 +1704,14 @@ class ComputeTestCase(BaseTestCase):
timeutils.set_time_override(cur_time) timeutils.set_time_override(cur_time)
test_notifier.NOTIFICATIONS = [] 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() 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={}) instance_type=instance_type, image={})
db.migration_get_by_instance_and_status(context, db.migration_get_by_instance_and_status(context,
instance['uuid'], new_instance['uuid'],
'pre-migrating') 'pre-migrating')
self.assertEquals(len(test_notifier.NOTIFICATIONS), 3) self.assertEquals(len(test_notifier.NOTIFICATIONS), 3)
@ -1725,7 +1728,7 @@ class ComputeTestCase(BaseTestCase):
payload = msg['payload'] payload = msg['payload']
self.assertEquals(payload['tenant_id'], self.project_id) self.assertEquals(payload['tenant_id'], self.project_id)
self.assertEquals(payload['user_id'], self.user_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') self.assertEquals(payload['instance_type'], 'm1.tiny')
type_id = instance_types.get_instance_type_by_name('m1.tiny')['id'] type_id = instance_types.get_instance_type_by_name('m1.tiny')['id']
self.assertEquals(str(payload['instance_type_id']), str(type_id)) self.assertEquals(str(payload['instance_type_id']), str(type_id))
@ -1734,10 +1737,12 @@ class ComputeTestCase(BaseTestCase):
self.assertTrue('launched_at' in payload) self.assertTrue('launched_at' in payload)
image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF) image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF)
self.assertEquals(payload['image_ref_url'], image_ref_url) 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): def test_prep_resize_instance_migration_error_on_same_host(self):
"""Ensure prep_resize raise a migration error""" """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) self.flags(host="foo", allow_resize_to_same_host=False)
instance = jsonutils.to_primitive(self._create_fake_instance()) instance = jsonutils.to_primitive(self._create_fake_instance())
@ -1757,6 +1762,26 @@ class ComputeTestCase(BaseTestCase):
reservations=reservations) reservations=reservations)
self.compute.terminate_instance(context, instance=new_instance) 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): def test_resize_instance_driver_error(self):
"""Ensure instance status set to Error on resize error""" """Ensure instance status set to Error on resize error"""
@ -1773,21 +1798,23 @@ class ComputeTestCase(BaseTestCase):
reservations = self._ensure_quota_reservations_rolledback() reservations = self._ensure_quota_reservations_rolledback()
self.compute.run_instance(self.context, instance=instance) self.compute.run_instance(self.context, instance=instance)
db.instance_update(self.context, instance['uuid'], {'host': 'foo'}) new_instance = db.instance_update(self.context, instance['uuid'],
self.compute.prep_resize(context, instance=instance, {'host': 'foo'})
new_instance = jsonutils.to_primitive(new_instance)
self.compute.prep_resize(context, instance=new_instance,
instance_type=instance_type, image={}, instance_type=instance_type, image={},
reservations=reservations) reservations=reservations)
migration_ref = db.migration_get_by_instance_and_status(context, 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}) {"task_state": task_states.RESIZE_PREP})
#verify #verify
self.assertRaises(test.TestingException, self.compute.resize_instance, self.assertRaises(test.TestingException, self.compute.resize_instance,
context, instance=instance, context, instance=new_instance,
migration_id=migration_ref['id'], image={}, migration_id=migration_ref['id'], image={},
reservations=reservations) 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.assertEqual(instance['vm_state'], vm_states.ERROR)
self.compute.terminate_instance(context, self.compute.terminate_instance(context,
@ -1800,19 +1827,23 @@ class ComputeTestCase(BaseTestCase):
context = self.context.elevated() context = self.context.elevated()
self.compute.run_instance(self.context, instance=instance) 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'}) {'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={}) instance_type=instance_type, image={})
migration_ref = db.migration_get_by_instance_and_status(context, 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}) {"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'], migration_id=migration_ref['id'],
image={}) image={})
self.compute.terminate_instance(context, inst = db.instance_get_by_uuid(self.context, new_instance['uuid'])
instance=jsonutils.to_primitive(instance)) # 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): def test_finish_revert_resize(self):
"""Ensure that the flavor is reverted to the original on revert""" """Ensure that the flavor is reverted to the original on revert"""
@ -1922,7 +1953,9 @@ class ComputeTestCase(BaseTestCase):
context = self.context.elevated() context = self.context.elevated()
self.compute.run_instance(self.context, instance=inst_ref) 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, self.compute.prep_resize(context, instance=inst_ref,
instance_type=instance_type, instance_type=instance_type,
image={}, reservations=reservations) image={}, reservations=reservations)

1
nova/tests/compute/test_resource_tracker.py
View File

@ -130,6 +130,7 @@ class BaseTestCase(test.TestCase):
'project_id': '123456', 'project_id': '123456',
'vcpus': 1, 'vcpus': 1,
'host': None, 'host': None,
'launched_on': None,
} }
instance.update(kwargs) instance.update(kwargs)