From 36dc58791482d44d63d63e9780451f9499619f05 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Mon, 30 Jul 2012 15:20:56 -0400 Subject: Send a full instance in rollback_live_migration_at_destination. Change the rollback_live_migration_at_destination method of the compute rpc API to take a full instance over rpc instead of just the instance ID. This cuts down on database access needed by nova-compute. This was the last method of this API that was taking an instance ID as an argument. There still some left that take the instance UUID, but I can see the light at the end of the tunnel. Part of blueprint no-db-messaging. Change-Id: Id581d23d36fee5c6fc06d2a655e999fb9aae5ae3 --- nova/compute/manager.py | 20 ++++++++++++-------- nova/compute/rpcapi.py | 8 ++++++-- nova/tests/compute/test_compute.py | 4 ++-- nova/tests/compute/test_rpcapi.py | 9 ++++----- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index f1de42d61..426577d5c 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -272,7 +272,7 @@ def _get_image_meta(context, image_ref): class ComputeManager(manager.SchedulerDependentManager): """Manages the running instances from creation to destruction.""" - RPC_API_VERSION = '1.31' + RPC_API_VERSION = '1.32' def __init__(self, compute_driver=None, *args, **kwargs): """Load configuration options and connect to the hypervisor.""" @@ -2407,24 +2407,28 @@ class ComputeManager(manager.SchedulerDependentManager): self.compute_rpcapi.rollback_live_migration_at_destination(context, instance_ref, dest) - def rollback_live_migration_at_destination(self, context, instance_id): + def rollback_live_migration_at_destination(self, context, instance=None, + instance_id=None): """ Cleaning up image directory that is created pre_live_migration. :param context: security context - :param instance_id: nova.db.sqlalchemy.models.Instance.Id + :param instance_id: (deprecated) nova.db.sqlalchemy.models.Instance.Id + :param instance: an Instance dict sent over rpc """ - instance_ref = self.db.instance_get(context, instance_id) - network_info = self._get_instance_nw_info(context, instance_ref) + if not instance: + instance = self.db.instance_get(context, instance_id) + + network_info = self._get_instance_nw_info(context, instance) # NOTE(tr3buchet): tear down networks on destination host - self.network_api.setup_networks_on_host(context, instance_ref, + self.network_api.setup_networks_on_host(context, instance, self.host, teardown=True) # NOTE(vish): The mapping is passed in so the driver can disconnect # from remote volumes if necessary block_device_info = self._get_instance_volume_block_device_info( - context, instance_id) - self.driver.destroy(instance_ref, self._legacy_nw_info(network_info), + context, instance['id']) + self.driver.destroy(instance, self._legacy_nw_info(network_info), block_device_info) @manager.periodic_task diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index 107bf4d8d..c3b7f068d 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -102,6 +102,8 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy): 1.29 - Remove instance_uuid, add instance argument to resize_instance() 1.30 - Remove instance_uuid, add instance argument to resume_instance() 1.31 - Remove instance_uuid, add instance argument to revert_resize() + 1.32 - Remove instance_id, add instance argument to + rollback_live_migration_at_destination() ''' BASE_RPC_API_VERSION = '1.0' @@ -386,9 +388,11 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy): version='1.31') def rollback_live_migration_at_destination(self, ctxt, instance, host): + instance_p = jsonutils.to_primitive(instance) self.cast(ctxt, self.make_msg('rollback_live_migration_at_destination', - instance_id=instance['id']), - topic=_compute_topic(self.topic, ctxt, host, None)) + instance=instance_p), + topic=_compute_topic(self.topic, ctxt, host, None), + version='1.32') def set_admin_password(self, ctxt, instance, new_pass): self.cast(ctxt, self.make_msg('set_admin_password', diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 36d001966..030314dc5 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -1625,8 +1625,8 @@ class ComputeTestCase(BaseTestCase): "version": "1.26"}, None) rpc.cast(c, topic, {"method": "rollback_live_migration_at_destination", - "args": {'instance_id': inst_id}, - "version": "1.0"}) + "args": {'instance': rpcinst}, + "version": "1.32"}) # start test self.mox.ReplayAll() diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py index 7a1195fe0..9b55bbb97 100644 --- a/nova/tests/compute/test_rpcapi.py +++ b/nova/tests/compute/test_rpcapi.py @@ -60,6 +60,7 @@ class ComputeRpcAPITestCase(test.TestCase): 'rebuild_instance', 'remove_fixed_ip_from_instance', 'remove_volume_connection', 'rescue_instance', 'reset_network', 'resize_instance', 'resume_instance', 'revert_resize', + 'rollback_live_migration_at_destination', 'start_instance', 'stop_instance', 'suspend_instance', 'unpause_instance' ] @@ -86,10 +87,7 @@ class ComputeRpcAPITestCase(test.TestCase): methods_with_instance): instance = expected_msg['args']['instance'] del expected_msg['args']['instance'] - if method in ['rollback_live_migration_at_destination']: - expected_msg['args']['instance_id'] = instance['id'] - else: - expected_msg['args']['instance_uuid'] = instance['uuid'] + expected_msg['args']['instance_uuid'] = instance['uuid'] expected_msg['version'] = expected_version cast_and_call = ['confirm_resize', 'stop_instance'] @@ -291,7 +289,8 @@ class ComputeRpcAPITestCase(test.TestCase): def test_rollback_live_migration_at_destination(self): self._test_compute_api('rollback_live_migration_at_destination', - 'cast', instance=self.fake_instance, host='host') + 'cast', instance=self.fake_instance, host='host', + version='1.32') def test_set_admin_password(self): self._test_compute_api('set_admin_password', 'cast', -- cgit From 99f6c32bf8a5204da0e07137486c61cdba5318a3 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Mon, 30 Jul 2012 15:39:18 -0400 Subject: Send a full instance in set_admin_password. Change the set_admin_password method of the compute rpc API to take a full instance over rpc instead of just the instance UUID. This cuts down on database access needed by nova-compute. Part of blueprint no-db-messaging. Change-Id: Iddcc7cb068090faa98f0bb87307e09d5b0ebf0c2 --- nova/compute/manager.py | 30 ++++++++++++++++-------------- nova/compute/rpcapi.py | 8 ++++++-- nova/tests/compute/test_compute.py | 12 +++++++----- nova/tests/compute/test_rpcapi.py | 4 ++-- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 426577d5c..6d8a52d16 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -272,7 +272,7 @@ def _get_image_meta(context, image_ref): class ComputeManager(manager.SchedulerDependentManager): """Manages the running instances from creation to destruction.""" - RPC_API_VERSION = '1.32' + RPC_API_VERSION = '1.33' def __init__(self, compute_driver=None, *args, **kwargs): """Load configuration options and connect to the hypervisor.""" @@ -1221,7 +1221,8 @@ class ComputeManager(manager.SchedulerDependentManager): @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) @checks_instance_lock @wrap_instance_fault - def set_admin_password(self, context, instance_uuid, new_pass=None): + def set_admin_password(self, context, instance=None, instance_uuid=None, + new_pass=None): """Set the root/admin password for an instance on this host. This is generally only called by API password resets after an @@ -1234,43 +1235,44 @@ class ComputeManager(manager.SchedulerDependentManager): # Generate a random password new_pass = utils.generate_password(FLAGS.password_length) + if not instance: + instance = self.db.instance_get_by_uuid(context, instance_uuid) + max_tries = 10 for i in xrange(max_tries): - instance_ref = self.db.instance_get_by_uuid(context, instance_uuid) - - current_power_state = self._get_power_state(context, instance_ref) + current_power_state = self._get_power_state(context, instance) expected_state = power_state.RUNNING if current_power_state != expected_state: - self._instance_update(context, instance_ref['uuid'], + self._instance_update(context, instance['uuid'], task_state=None) _msg = _('Failed to set admin password. Instance %s is not' - ' running') % instance_ref["uuid"] + ' running') % instance["uuid"] raise exception.Invalid(_msg) else: try: - self.driver.set_admin_password(instance_ref, new_pass) - LOG.audit(_("Root password set"), instance=instance_ref) + self.driver.set_admin_password(instance, new_pass) + LOG.audit(_("Root password set"), instance=instance) self._instance_update(context, - instance_ref['uuid'], + instance['uuid'], task_state=None) break except NotImplementedError: # NOTE(dprince): if the driver doesn't implement # set_admin_password we break to avoid a loop LOG.warn(_('set_admin_password is not implemented ' - 'by this driver.'), instance=instance_ref) + 'by this driver.'), instance=instance) self._instance_update(context, - instance_ref['uuid'], + instance['uuid'], task_state=None) break except Exception, e: # Catch all here because this could be anything. - LOG.exception(e, instance=instance_ref) + LOG.exception(e, instance=instance) if i == max_tries - 1: self._set_instance_error_state(context, - instance_ref['uuid']) + instance['uuid']) # We create a new exception here so that we won't # potentially reveal password information to the # API caller. The real exception is logged above diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index c3b7f068d..15d1db81a 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -104,6 +104,8 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy): 1.31 - Remove instance_uuid, add instance argument to revert_resize() 1.32 - Remove instance_id, add instance argument to rollback_live_migration_at_destination() + 1.33 - Remove instance_uuid, add instance argument to + set_admin_password() ''' BASE_RPC_API_VERSION = '1.0' @@ -395,9 +397,11 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy): version='1.32') def set_admin_password(self, ctxt, instance, new_pass): + instance_p = jsonutils.to_primitive(instance) self.cast(ctxt, self.make_msg('set_admin_password', - instance_uuid=instance['uuid'], new_pass=new_pass), - topic=_compute_topic(self.topic, ctxt, None, instance)) + instance=instance_p, new_pass=new_pass), + topic=_compute_topic(self.topic, ctxt, None, instance), + version='1.33') def set_host_enabled(self, ctxt, enabled, host): topic = _compute_topic(self.topic, ctxt, host, None) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 030314dc5..4dbcd3522 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -589,7 +589,7 @@ class ComputeTestCase(BaseTestCase): def test_set_admin_password(self): """Ensure instance can have its admin password set""" - instance = self._create_fake_instance() + instance = jsonutils.to_primitive(self._create_fake_instance()) self.compute.run_instance(self.context, instance['uuid']) db.instance_update(self.context, instance['uuid'], {'task_state': task_states.UPDATING_PASSWORD}) @@ -598,7 +598,7 @@ class ComputeTestCase(BaseTestCase): self.assertEqual(inst_ref['vm_state'], vm_states.ACTIVE) self.assertEqual(inst_ref['task_state'], task_states.UPDATING_PASSWORD) - self.compute.set_admin_password(self.context, instance['uuid']) + self.compute.set_admin_password(self.context, instance=instance) inst_ref = db.instance_get_by_uuid(self.context, instance['uuid']) self.assertEqual(inst_ref['vm_state'], vm_states.ACTIVE) @@ -613,7 +613,8 @@ class ComputeTestCase(BaseTestCase): db.instance_update(self.context, instance['uuid'], { "power_state": power_state.NOSTATE, }) - instance = db.instance_get_by_uuid(self.context, instance['uuid']) + instance = jsonutils.to_primitive(db.instance_get_by_uuid( + self.context, instance['uuid'])) self.assertEqual(instance['power_state'], power_state.NOSTATE) @@ -630,7 +631,7 @@ class ComputeTestCase(BaseTestCase): self.assertRaises(exception.Invalid, self.compute.set_admin_password, self.context, - instance['uuid']) + instance=instance) self.compute.terminate_instance(self.context, instance['uuid']) def test_set_admin_password_driver_error(self): @@ -660,7 +661,8 @@ class ComputeTestCase(BaseTestCase): #so a new error is raised self.assertRaises(exception.NovaException, self.compute.set_admin_password, - self.context, instance['uuid']) + self.context, + instance=jsonutils.to_primitive(inst_ref)) inst_ref = db.instance_get_by_uuid(self.context, instance['uuid']) self.assertEqual(inst_ref['vm_state'], vm_states.ERROR) diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py index 9b55bbb97..e726606d4 100644 --- a/nova/tests/compute/test_rpcapi.py +++ b/nova/tests/compute/test_rpcapi.py @@ -60,7 +60,7 @@ class ComputeRpcAPITestCase(test.TestCase): 'rebuild_instance', 'remove_fixed_ip_from_instance', 'remove_volume_connection', 'rescue_instance', 'reset_network', 'resize_instance', 'resume_instance', 'revert_resize', - 'rollback_live_migration_at_destination', + 'rollback_live_migration_at_destination', 'set_admin_password', 'start_instance', 'stop_instance', 'suspend_instance', 'unpause_instance' ] @@ -294,7 +294,7 @@ class ComputeRpcAPITestCase(test.TestCase): def test_set_admin_password(self): self._test_compute_api('set_admin_password', 'cast', - instance=self.fake_instance, new_pass='pw') + instance=self.fake_instance, new_pass='pw', version='1.33') def test_set_host_enabled(self): self._test_compute_api('set_host_enabled', 'call', -- cgit From 773d1d61a72700ad5170c259d599891f1dd0a609 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Mon, 30 Jul 2012 16:08:50 -0400 Subject: Send a full instance in snapshot_instance. Change the snapshot_instance method of the compute rpc API to take a full instance over rpc instead of just the instance UUID. This cuts down on database access needed by nova-compute. Part of blueprint no-db-messaging. Change-Id: I401beb932e329683c3434ecd643db345aa217492 --- nova/compute/manager.py | 29 ++++++++++++++++------------- nova/compute/rpcapi.py | 8 ++++++-- nova/tests/compute/test_compute.py | 8 ++++---- nova/tests/compute/test_rpcapi.py | 7 ++++--- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 6d8a52d16..65bd43f21 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -272,7 +272,7 @@ def _get_image_meta(context, image_ref): class ComputeManager(manager.SchedulerDependentManager): """Manages the running instances from creation to destruction.""" - RPC_API_VERSION = '1.33' + RPC_API_VERSION = '1.34' def __init__(self, compute_driver=None, *args, **kwargs): """Load configuration options and connect to the hypervisor.""" @@ -1116,13 +1116,14 @@ class ComputeManager(manager.SchedulerDependentManager): @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) @wrap_instance_fault - def snapshot_instance(self, context, instance_uuid, image_id, + def snapshot_instance(self, context, image_id, image_type='snapshot', backup_type=None, - rotation=None): + rotation=None, instance=None, instance_uuid=None): """Snapshot an instance on this host. :param context: security context - :param instance_uuid: nova.db.sqlalchemy.models.Instance.Uuid + :param instance_uuid: (deprecated) db.sqlalchemy.models.Instance.Uuid + :param instance: an Instance dict :param image_id: glance.db.sqlalchemy.models.Image.Id :param image_type: snapshot | backup :param backup_type: daily | weekly @@ -1130,18 +1131,20 @@ class ComputeManager(manager.SchedulerDependentManager): None if rotation shouldn't be used (as in the case of snapshots) """ context = context.elevated() - instance_ref = self.db.instance_get_by_uuid(context, instance_uuid) - current_power_state = self._get_power_state(context, instance_ref) + if not instance: + instance = self.db.instance_get_by_uuid(context, instance_uuid) + + current_power_state = self._get_power_state(context, instance) self._instance_update(context, - instance_ref['uuid'], + instance['uuid'], power_state=current_power_state) LOG.audit(_('instance snapshotting'), context=context, instance_uuid=instance_uuid) - if instance_ref['power_state'] != power_state.RUNNING: - state = instance_ref['power_state'] + if instance['power_state'] != power_state.RUNNING: + state = instance['power_state'] running = power_state.RUNNING LOG.warn(_('trying to snapshot a non-running ' 'instance: (state: %(state)s ' @@ -1149,12 +1152,12 @@ class ComputeManager(manager.SchedulerDependentManager): instance_uuid=instance_uuid) self._notify_about_instance_usage( - context, instance_ref, "snapshot.start") + context, instance, "snapshot.start") try: - self.driver.snapshot(context, instance_ref, image_id) + self.driver.snapshot(context, instance, image_id) finally: - self._instance_update(context, instance_ref['uuid'], + self._instance_update(context, instance['uuid'], task_state=None) if image_type == 'snapshot' and rotation: @@ -1167,7 +1170,7 @@ class ComputeManager(manager.SchedulerDependentManager): raise exception.RotationRequiredForBackup() self._notify_about_instance_usage( - context, instance_ref, "snapshot.end") + context, instance, "snapshot.end") @wrap_instance_fault def rotate_backups(self, context, instance_uuid, backup_type, rotation): diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index 15d1db81a..153348374 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -106,6 +106,8 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy): rollback_live_migration_at_destination() 1.33 - Remove instance_uuid, add instance argument to set_admin_password() + 1.34 - Remove instance_uuid, add instance argument to + snapshot_instance() ''' BASE_RPC_API_VERSION = '1.0' @@ -415,11 +417,13 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy): def snapshot_instance(self, ctxt, instance, image_id, image_type, backup_type, rotation): + instance_p = jsonutils.to_primitive(instance) self.cast(ctxt, self.make_msg('snapshot_instance', - instance_uuid=instance['uuid'], image_id=image_id, + instance=instance_p, image_id=image_id, image_type=image_type, backup_type=backup_type, rotation=rotation), - topic=_compute_topic(self.topic, ctxt, None, instance)) + topic=_compute_topic(self.topic, ctxt, None, instance), + version='1.34') def start_instance(self, ctxt, instance): instance_p = jsonutils.to_primitive(instance) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 4dbcd3522..874804152 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -749,11 +749,11 @@ class ComputeTestCase(BaseTestCase): def test_snapshot(self): """Ensure instance can be snapshotted""" - instance = self._create_fake_instance() + instance = jsonutils.to_primitive(self._create_fake_instance()) instance_uuid = instance['uuid'] name = "myfakesnapshot" self.compute.run_instance(self.context, instance_uuid) - self.compute.snapshot_instance(self.context, instance_uuid, name) + self.compute.snapshot_instance(self.context, name, instance=instance) self.compute.terminate_instance(self.context, instance_uuid) def test_snapshot_fails(self): @@ -763,11 +763,11 @@ class ComputeTestCase(BaseTestCase): self.stubs.Set(self.compute.driver, 'snapshot', fake_snapshot) - instance = self._create_fake_instance() + instance = jsonutils.to_primitive(self._create_fake_instance()) self.compute.run_instance(self.context, instance['uuid']) self.assertRaises(test.TestingException, self.compute.snapshot_instance, - self.context, instance['uuid'], "failing_snapshot") + self.context, "failing_snapshot", instance=instance) self._assert_state({'task_state': None}) self.compute.terminate_instance(self.context, instance['uuid']) diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py index e726606d4..cafd7fd55 100644 --- a/nova/tests/compute/test_rpcapi.py +++ b/nova/tests/compute/test_rpcapi.py @@ -61,8 +61,8 @@ class ComputeRpcAPITestCase(test.TestCase): 'remove_volume_connection', 'rescue_instance', 'reset_network', 'resize_instance', 'resume_instance', 'revert_resize', 'rollback_live_migration_at_destination', 'set_admin_password', - 'start_instance', 'stop_instance', 'suspend_instance', - 'unpause_instance' + 'snapshot_instance', 'start_instance', 'stop_instance', + 'suspend_instance', 'unpause_instance' ] if 'rpcapi_class' in kwargs: @@ -307,7 +307,8 @@ class ComputeRpcAPITestCase(test.TestCase): def test_snapshot_instance(self): self._test_compute_api('snapshot_instance', 'cast', instance=self.fake_instance, image_id='id', image_type='type', - backup_type='type', rotation='rotation') + backup_type='type', rotation='rotation', + version='1.34') def test_start_instance(self): self._test_compute_api('start_instance', 'cast', -- cgit From e4dd27188e071778c0675b6e8e4d050fb13004f3 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Mon, 30 Jul 2012 20:16:26 -0400 Subject: Fix arg to get_instance_volume_block_device_info(). Michael Still caught this bug while reviewing an earlier patch in this patch series. This code was passing an instance ID to this function, but it expects an instance UUID. Not really related to, but tied up in the patch series for blueprint no-db-messaging. Change-Id: I3ec871deadd13a8117f16ad46c31b0a9c8a016db --- nova/compute/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 65bd43f21..78d69becc 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2432,7 +2432,7 @@ class ComputeManager(manager.SchedulerDependentManager): # NOTE(vish): The mapping is passed in so the driver can disconnect # from remote volumes if necessary block_device_info = self._get_instance_volume_block_device_info( - context, instance['id']) + context, instance['uuid']) self.driver.destroy(instance, self._legacy_nw_info(network_info), block_device_info) -- cgit