diff options
| -rw-r--r-- | nova/compute/api.py | 2 | ||||
| -rw-r--r-- | nova/compute/manager.py | 28 | ||||
| -rw-r--r-- | nova/compute/utils.py | 3 | ||||
| -rw-r--r-- | nova/tests/api/openstack/compute/test_server_actions.py | 1 | ||||
| -rw-r--r-- | nova/tests/test_compute.py | 51 | ||||
| -rw-r--r-- | nova/utils.py | 5 |
6 files changed, 67 insertions, 23 deletions
diff --git a/nova/compute/api.py b/nova/compute/api.py index b54aa1b57..116bfb9af 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1345,7 +1345,6 @@ class API(BaseAPI): self.update(context, instance, - image_ref=image_href, vm_state=vm_states.REBUILDING, task_state=None, progress=0, @@ -1354,6 +1353,7 @@ class API(BaseAPI): rebuild_params = { "new_pass": admin_password, "injected_files": files_to_inject, + "image_ref": image_href, } self._cast_compute_message('rebuild_instance', context, instance, diff --git a/nova/compute/manager.py b/nova/compute/manager.py index aa94448d8..faccfef50 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -793,7 +793,7 @@ class ComputeManager(manager.SchedulerDependentManager): @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) @checks_instance_lock @wrap_instance_fault - def rebuild_instance(self, context, instance_uuid, **kwargs): + def rebuild_instance(self, context, instance_uuid, image_ref, **kwargs): """Destroy and re-make this instance. A 'rebuild' effectively purges all existing data from the system and @@ -805,7 +805,8 @@ class ComputeManager(manager.SchedulerDependentManager): :param new_pass: password to set on rebuilt instance """ try: - self._rebuild_instance(context, instance_uuid, kwargs) + self._rebuild_instance(context, instance_uuid, image_ref, + kwargs) except exception.ImageNotFound: LOG.error(_('Cannot rebuild instance because the given image does ' 'not exist.'), @@ -816,7 +817,7 @@ class ComputeManager(manager.SchedulerDependentManager): context=context, instance_uuid=instance_uuid) self._set_instance_error_state(context, instance_uuid) - def _rebuild_instance(self, context, instance_uuid, kwargs): + def _rebuild_instance(self, context, instance_uuid, image_ref, kwargs): context = context.elevated() LOG.audit(_("Rebuilding instance"), context=context, @@ -825,7 +826,17 @@ class ComputeManager(manager.SchedulerDependentManager): instance = self.db.instance_get_by_uuid(context, instance_uuid) compute_utils.notify_usage_exists( context, instance, current_period=True) - self._notify_about_instance_usage(context, instance, "rebuild.start") + + # NOTE(comstud): Since we've not updated the DB yet with the new + # image_ref, and rebuild.start should have image_ref_url pointing + # to the new image ref, we need to override it. + # Arguably, image_ref_url should still contain the old image at + # this point and we should pass a 'new_image_ref_url', instead. + image_ref_url = utils.generate_image_url(image_ref) + extra_usage_info = {'image_ref_url': image_ref_url} + self._notify_about_instance_usage(context, instance, + "rebuild.start", extra_usage_info=extra_usage_info) + current_power_state = self._get_power_state(context, instance) self._instance_update(context, instance_uuid, @@ -836,17 +847,20 @@ class ComputeManager(manager.SchedulerDependentManager): network_info = self._get_instance_nw_info(context, instance) self.driver.destroy(instance, self._legacy_nw_info(network_info)) - self._instance_update(context, + # NOTE(comstud): Now that we've shutdown the old instance, we + # can update the image_ref in the DB. + instance = self._instance_update(context, instance_uuid, vm_state=vm_states.REBUILDING, - task_state=task_states.BLOCK_DEVICE_MAPPING) + task_state=task_states.BLOCK_DEVICE_MAPPING, + image_ref=image_ref) instance.injected_files = kwargs.get('injected_files', []) network_info = self.network_api.get_instance_nw_info(context, instance) device_info = self._setup_block_device_mapping(context, instance) - self._instance_update(context, + instance = self._instance_update(context, instance_uuid, vm_state=vm_states.REBUILDING, task_state=task_states.SPAWNING) diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 5da501c8a..6c2b265dc 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -212,8 +212,7 @@ def _usage_from_instance(context, instance_ref, network_info=None, **kw): def null_safe_str(s): return str(s) if s else '' - image_ref_url = "%s/images/%s" % (utils.generate_glance_url(), - instance_ref['image_ref']) + image_ref_url = utils.generate_image_url(instance_ref['image_ref']) usage_info = dict( tenant_id=instance_ref['project_id'], diff --git a/nova/tests/api/openstack/compute/test_server_actions.py b/nova/tests/api/openstack/compute/test_server_actions.py index 774700361..43edafb4b 100644 --- a/nova/tests/api/openstack/compute/test_server_actions.py +++ b/nova/tests/api/openstack/compute/test_server_actions.py @@ -457,7 +457,6 @@ class ServerActionsControllerTest(test.TestCase): req = fakes.HTTPRequest.blank(self.url) context = req.environ['nova.context'] update(context, mox.IgnoreArg(), - image_ref=self._image_href, vm_state=vm_states.REBUILDING, task_state=None, progress=0, **attributes).AndReturn(None) self.mox.ReplayAll() diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index fad6ba5e1..e1ea40fcd 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -60,6 +60,7 @@ flags.DECLARE('live_migration_retry_count', 'nova.compute.manager') flags.DECLARE('additional_compute_capabilities', 'nova.compute.manager') +FAKE_IMAGE_REF = 'fake-image-ref' orig_rpc_call = rpc.call orig_rpc_cast = rpc.cast @@ -134,7 +135,7 @@ class BaseTestCase(test.TestCase): inst = {} inst['vm_state'] = vm_states.ACTIVE - inst['image_ref'] = 1 + inst['image_ref'] = FAKE_IMAGE_REF inst['reservation_id'] = 'r-fakeres' inst['launch_time'] = '10' inst['user_id'] = self.user_id @@ -487,8 +488,13 @@ class ComputeTestCase(BaseTestCase): instance = self._create_fake_instance() instance_uuid = instance['uuid'] + new_image_ref = instance['image_ref'] + '-new-image' + self.compute.run_instance(self.context, instance_uuid) - self.compute.rebuild_instance(self.context, instance_uuid) + self.compute.rebuild_instance(self.context, instance_uuid, + new_image_ref) + inst_ref = db.instance_get_by_uuid(self.context, instance_uuid) + self.assertEqual(inst_ref['image_ref'], new_image_ref) self.compute.terminate_instance(self.context, instance_uuid) def test_rebuild_launch_time(self): @@ -501,7 +507,8 @@ class ComputeTestCase(BaseTestCase): self.compute.run_instance(self.context, instance_uuid) utils.set_time_override(cur_time) - self.compute.rebuild_instance(self.context, instance_uuid) + self.compute.rebuild_instance(self.context, instance_uuid, + instance['image_ref']) instance = db.instance_get_by_uuid(self.context, instance_uuid) self.assertEquals(cur_time, instance['launched_at']) self.compute.terminate_instance(self.context, instance_uuid) @@ -867,7 +874,7 @@ class ComputeTestCase(BaseTestCase): self.assertTrue('created_at' in payload) self.assertTrue('launched_at' in payload) self.assertTrue(payload['launched_at']) - image_ref_url = "%s/images/1" % utils.generate_glance_url() + image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF) self.assertEquals(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(self.context, instance_uuid) @@ -907,7 +914,7 @@ class ComputeTestCase(BaseTestCase): self.assertTrue('launched_at' in payload) self.assertTrue('deleted_at' in payload) self.assertEqual(payload['deleted_at'], str(cur_time)) - image_ref_url = "%s/images/1" % utils.generate_glance_url() + image_ref_url = utils.generate_image_url(FAKE_IMAGE_REF) self.assertEquals(payload['image_ref_url'], image_ref_url) def test_run_instance_existing(self): @@ -1071,20 +1078,26 @@ class ComputeTestCase(BaseTestCase): instance = db.instance_get_by_uuid(self.context, instance_uuid) image_ref = instance["image_ref"] + new_image_ref = image_ref + '-new_image_ref' password = "new_password" + self.compute._rebuild_instance(self.context, instance_uuid, - dict(image_ref=image_ref, - new_pass=password)) + new_image_ref, dict(new_pass=password)) instance = db.instance_get_by_uuid(self.context, instance_uuid) + image_ref_url = utils.generate_image_url(image_ref) + new_image_ref_url = utils.generate_image_url(new_image_ref) + self.assertEquals(len(test_notifier.NOTIFICATIONS), 3) msg = test_notifier.NOTIFICATIONS[0] self.assertEquals(msg['event_type'], 'compute.instance.exists') + self.assertEquals(msg['payload']['image_ref_url'], image_ref_url) msg = test_notifier.NOTIFICATIONS[1] self.assertEquals(msg['event_type'], 'compute.instance.rebuild.start') + self.assertEquals(msg['payload']['image_ref_url'], new_image_ref_url) msg = test_notifier.NOTIFICATIONS[2] self.assertEquals(msg['event_type'], 'compute.instance.rebuild.end') @@ -1100,8 +1113,7 @@ class ComputeTestCase(BaseTestCase): self.assertTrue('created_at' in payload) self.assertTrue('launched_at' in payload) self.assertEqual(payload['launched_at'], str(cur_time)) - image_ref_url = "%s/images/1" % utils.generate_glance_url() - self.assertEquals(payload['image_ref_url'], image_ref_url) + self.assertEquals(payload['image_ref_url'], new_image_ref_url) self.compute.terminate_instance(self.context, instance_uuid) def test_finish_resize_instance_notification(self): @@ -1150,7 +1162,7 @@ class ComputeTestCase(BaseTestCase): self.assertTrue('created_at' in payload) self.assertTrue('launched_at' in payload) self.assertEqual(payload['launched_at'], str(cur_time)) - image_ref_url = "%s/images/1" % utils.generate_glance_url() + 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_uuid) @@ -1195,7 +1207,7 @@ class ComputeTestCase(BaseTestCase): self.assertTrue('display_name' in payload) self.assertTrue('created_at' in payload) self.assertTrue('launched_at' in payload) - image_ref_url = "%s/images/1" % utils.generate_glance_url() + 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_uuid) @@ -2205,9 +2217,24 @@ class ComputeAPITestCase(BaseTestCase): instance = db.instance_get_by_uuid(self.context, instance_uuid) self.assertEqual(instance['task_state'], None) - image_ref = instance["image_ref"] + # Make sure Compute API doesn't try to update the image_ref. + # It should be done on the manager side. We can't check the DB + # at the end of this test method because we've called the + # manager by that point.. + orig_update = self.compute_api.update + info = {'image_ref_updated': False} + + def update_wrapper(*args, **kwargs): + if 'image_ref' in kwargs: + info['image_ref_updated'] = True + return orig_update(*args, **kwargs) + + self.stubs.Set(self.compute_api, 'update', update_wrapper) + + image_ref = instance["image_ref"] + '-new_image_ref' password = "new_password" self.compute_api.rebuild(self.context, instance, image_ref, password) + self.assertFalse(info['image_ref_updated']) instance = db.instance_get_by_uuid(self.context, instance_uuid) self.assertEqual(instance['vm_state'], vm_states.REBUILDING) diff --git a/nova/utils.py b/nova/utils.py index 0c20dd34d..69011fb61 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -1259,6 +1259,11 @@ def generate_glance_url(): return "http://%s:%d" % (FLAGS.glance_host, FLAGS.glance_port) +def generate_image_url(image_ref): + """Generate a image URL from an image_ref.""" + return "%s/images/%s" % (generate_glance_url(), image_ref) + + @contextlib.contextmanager def logging_error(message): """Catches exception, write message to the log, re-raise. |