From 5d8868a4d58122dce6fdc81fa0ed17ae7fc55672 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Mon, 4 Feb 2013 12:12:43 -0500 Subject: [PATCH] Refactor server password metadata to avoid direct db usage This refactors the set_password() method in the server_password API extension to merely format the metadata and return it, leaving the job of updating the database to the caller. In the API extension case, the update can be done immediately against the database as before. In the case of the metadata API handler and the xenapi virt driver making the call, conductor can be used to update the instance's system_metadata, thereby avoiding a direct database access. Related to blueprint no-db-compute Change-Id: I0563feaa97d768a96f950c148b1dbf51c356c4ac --- nova/api/metadata/password.py | 18 +++++++++++------- .../compute/contrib/server_password.py | 5 ++++- .../compute/contrib/test_server_password.py | 5 +++-- nova/tests/test_metadata.py | 13 +++++++++++-- nova/virt/xenapi/agent.py | 11 ++++++++--- nova/virt/xenapi/vmops.py | 3 ++- 6 files changed, 39 insertions(+), 16 deletions(-) diff --git a/nova/api/metadata/password.py b/nova/api/metadata/password.py index b2bb83b15014..f3453e945e54 100644 --- a/nova/api/metadata/password.py +++ b/nova/api/metadata/password.py @@ -15,8 +15,9 @@ from webob import exc +from nova import conductor from nova import context -from nova import db +from nova import utils CHUNKS = 4 @@ -33,7 +34,7 @@ def extract_password(instance): return result or None -def set_password(context, instance_uuid, password): +def convert_password(context, password): """Stores password as system_metadata items. Password is stored with the keys 'password_0' -> 'password_3'. @@ -43,10 +44,7 @@ def set_password(context, instance_uuid, password): for i in xrange(CHUNKS): meta['password_%d' % i] = password[:CHUNK_LENGTH] password = password[CHUNK_LENGTH:] - db.instance_system_metadata_update(context, - instance_uuid, - meta, - False) + return meta def handle_password(req, meta_data): @@ -63,6 +61,12 @@ def handle_password(req, meta_data): if (req.content_length > MAX_SIZE or len(req.body) > MAX_SIZE): msg = _("Request is too large.") raise exc.HTTPBadRequest(explanation=msg) - set_password(ctxt, meta_data.uuid, req.body) + + conductor_api = conductor.API() + instance = conductor_api.instance_get_by_uuid(ctxt, meta_data.uuid) + sys_meta = utils.metadata_to_dict(instance['system_metadata']) + sys_meta.update(convert_password(ctxt, req.body)) + conductor_api.instance_update(ctxt, meta_data.uuid, + system_metadata=sys_meta) else: raise exc.HTTPBadRequest() diff --git a/nova/api/openstack/compute/contrib/server_password.py b/nova/api/openstack/compute/contrib/server_password.py index 0fd620fb8851..9436d354fee2 100644 --- a/nova/api/openstack/compute/contrib/server_password.py +++ b/nova/api/openstack/compute/contrib/server_password.py @@ -24,6 +24,7 @@ from nova.api.openstack import extensions from nova.api.openstack import wsgi from nova.api.openstack import xmlutil from nova import compute +from nova import db from nova import exception @@ -62,7 +63,9 @@ class ServerPasswordController(object): context = req.environ['nova.context'] authorize(context) instance = self._get_instance(context, server_id) - password.set_password(context, instance['uuid'], None) + meta = password.convert_password(context, None) + db.instance_system_metadata_update(context, instance['uuid'], + meta, False) class Server_password(extensions.ExtensionDescriptor): diff --git a/nova/tests/api/openstack/compute/contrib/test_server_password.py b/nova/tests/api/openstack/compute/contrib/test_server_password.py index 600c4eda4031..87da90efe385 100644 --- a/nova/tests/api/openstack/compute/contrib/test_server_password.py +++ b/nova/tests/api/openstack/compute/contrib/test_server_password.py @@ -40,11 +40,12 @@ class ServerPasswordTest(test.TestCase): def fake_extract_password(instance): return self.password - def fake_set_password(context, instance_uuid, password): + def fake_convert_password(context, password): self.password = password + return {} self.stubs.Set(password, 'extract_password', fake_extract_password) - self.stubs.Set(password, 'set_password', fake_set_password) + self.stubs.Set(password, 'convert_password', fake_convert_password) self.flags( osapi_compute_extension=[ 'nova.api.openstack.compute.contrib.select_extensions'], diff --git a/nova/tests/test_metadata.py b/nova/tests/test_metadata.py index f610cd6fcab7..827bfb398654 100644 --- a/nova/tests/test_metadata.py +++ b/nova/tests/test_metadata.py @@ -549,6 +549,7 @@ class MetadataPasswordTestCase(test.TestCase): self.instance = copy.copy(INSTANCES[0]) self.mdinst = fake_InstanceMetadata(self.stubs, self.instance, address=None, sgroups=None) + self.flags(use_local=True, group='conductor') def test_get_password(self): request = webob.Request.blank('') @@ -566,8 +567,16 @@ class MetadataPasswordTestCase(test.TestCase): request = webob.Request.blank('') request.method = 'POST' request.body = val - self.stubs.Set(db, 'instance_system_metadata_update', - lambda *a, **kw: None) + self.stubs.Set(db, 'instance_get_by_uuid', + lambda *a, **kw: {'system_metadata': []}) + + def fake_instance_update(context, uuid, updates): + self.assertIn('system_metadata', updates) + self.assertIn('password_0', updates['system_metadata']) + return self.instance, self.instance + + self.stubs.Set(db, 'instance_update_and_get_original', + fake_instance_update) password.handle_password(request, self.mdinst) def test_set_password(self): diff --git a/nova/virt/xenapi/agent.py b/nova/virt/xenapi/agent.py index e8a81f552c65..8220fb67bc3c 100644 --- a/nova/virt/xenapi/agent.py +++ b/nova/virt/xenapi/agent.py @@ -123,8 +123,9 @@ def _get_agent_version(session, instance, vm_ref): class XenAPIBasedAgent(object): - def __init__(self, session, instance, vm_ref): + def __init__(self, session, virtapi, instance, vm_ref): self.session = session + self.virtapi = virtapi self.instance = instance self.vm_ref = vm_ref @@ -212,9 +213,13 @@ class XenAPIBasedAgent(object): sshkey = self.instance.get('key_data') if sshkey: + ctxt = context.get_admin_context() enc = crypto.ssh_encrypt_text(sshkey, new_pass) - password.set_password(context.get_admin_context(), - self.instance['uuid'], base64.b64encode(enc)) + sys_meta = utils.metadata_to_dict(self.instance['system_metadata']) + sys_meta.update(password.convert_password(ctxt, + base64.b64encode(enc))) + self.virtapi.instance_update(ctxt, self.instance['uuid'], + {'system_metadata': sys_meta}) return resp['message'] diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 8a76f33680cc..9124b4dbe6e5 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -176,7 +176,8 @@ class VMOps(object): def _get_agent(self, instance, vm_ref): if self.agent_enabled: - return xapi_agent.XenAPIBasedAgent(self._session, instance, vm_ref) + return xapi_agent.XenAPIBasedAgent(self._session, self._virtapi, + instance, vm_ref) raise exception.NovaException(_("Error: Agent is disabled")) def list_instances(self):