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

@@ -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()

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):

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'],

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):

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']
 

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):