xenapi: refactor: Agent class

Related to blueprint xenapi-config-drive

As config drive and agent are doing similar operations, part of the
config drive introduction, we would like to make the agent optional.
This refactor is introducing an agent class holding together the agent
operations, as a first step.

The steps were:
Introduce XenAPIBasedAgent
Added constructor parameters to XenAPIBasedAgent
get rid of parameters on get_agent_version
get rid of parameters on agent_update
get rid of parameters on set_admin_password
get rid of parameters on inject_file
get rid of parameters on resetnetwork
Correct newlines
add factory method for getting an agent instance

For the small changes, see:
https://github.com/citrix-openstack/nova/commits/agent2class

Change-Id: I3928c0776ff98c830c19487eb8ff7820e41ca025

2 files changed, 130 insertions, 118 deletions

diff --git a/nova/virt/xenapi/agent.py b/nova/virt/xenapi/agent.py
index e3288a47b..0c17dccff 100644
--- a/nova/virt/xenapi/agent.py
+++ b/nova/virt/xenapi/agent.py
@@ -115,121 +115,128 @@ def _get_agent_version(session, instance, vm_ref):
     return resp['message'].replace('\\r\\n', '')
 
 
-def get_agent_version(session, instance, vm_ref):
-    """Get the version of the agent running on the VM instance."""
+class XenAPIBasedAgent(object):
+    def __init__(self, session, instance, vm_ref):
+        self.session = session
+        self.instance = instance
+        self.vm_ref = vm_ref
 
-    LOG.debug(_('Querying agent version'), instance=instance)
+    def get_agent_version(self):
+        """Get the version of the agent running on the VM instance."""
 
-    # The agent can be slow to start for a variety of reasons. On Windows,
-    # it will generally perform a setup process on first boot that can
-    # take a couple of minutes and then reboot. On Linux, the system can
-    # also take a while to boot. So we need to be more patient than
-    # normal as well as watch for domid changes
+        LOG.debug(_('Querying agent version'), instance=self.instance)
 
-    expiration = time.time() + FLAGS.agent_version_timeout
-    while time.time() < expiration:
-        ret = _get_agent_version(session, instance, vm_ref)
-        if ret:
-            return ret
+        # The agent can be slow to start for a variety of reasons. On Windows,
+        # it will generally perform a setup process on first boot that can
+        # take a couple of minutes and then reboot. On Linux, the system can
+        # also take a while to boot. So we need to be more patient than
+        # normal as well as watch for domid changes
 
-    LOG.info(_('Reached maximum time attempting to query agent version'),
-             instance=instance)
+        expiration = time.time() + FLAGS.agent_version_timeout
+        while time.time() < expiration:
+            ret = _get_agent_version(self.session, self.instance, self.vm_ref)
+            if ret:
+                return ret
 
-    return None
+        LOG.info(_('Reached maximum time attempting to query agent version'),
+                 instance=self.instance)
 
-
-def agent_update(session, instance, vm_ref, agent_build):
-    """Update agent on the VM instance."""
-
-    LOG.info(_('Updating agent to %s'), agent_build['version'],
-             instance=instance)
-
-    # Send the encrypted password
-    args = {'url': agent_build['url'], 'md5sum': agent_build['md5hash']}
-    resp = _call_agent(session, instance, vm_ref, 'agentupdate', args)
-    if resp['returncode'] != '0':
-        LOG.error(_('Failed to update agent: %(resp)r'), locals(),
-                  instance=instance)
-        return None
-    return resp['message']
-
-
-def set_admin_password(session, instance, vm_ref, new_pass):
-    """Set the root/admin password on the VM instance.
-
-    This is done via an agent running on the VM. Communication between nova
-    and the agent is done via writing xenstore records. Since communication
-    is done over the XenAPI RPC calls, we need to encrypt the password.
-    We're using a simple Diffie-Hellman class instead of a more advanced
-    library (such as M2Crypto) for compatibility with the agent code.
-    """
-    LOG.debug(_('Setting admin password'), instance=instance)
-
-    dh = SimpleDH()
-
-    # Exchange keys
-    args = {'pub': str(dh.get_public())}
-    resp = _call_agent(session, instance, vm_ref, 'key_init', args)
-
-    # Successful return code from key_init is 'D0'
-    if resp['returncode'] != 'D0':
-        msg = _('Failed to exchange keys: %(resp)r') % locals()
-        LOG.error(msg, instance=instance)
-        raise Exception(msg)
-
-    # Some old versions of the Windows agent have a trailing \\r\\n
-    # (ie CRLF escaped) for some reason. Strip that off.
-    agent_pub = int(resp['message'].replace('\\r\\n', ''))
-    dh.compute_shared(agent_pub)
-
-    # Some old versions of Linux and Windows agent expect trailing \n
-    # on password to work correctly.
-    enc_pass = dh.encrypt(new_pass + '\n')
-
-    # Send the encrypted password
-    args = {'enc_pass': enc_pass}
-    resp = _call_agent(session, instance, vm_ref, 'password', args)
-
-    # Successful return code from password is '0'
-    if resp['returncode'] != '0':
-        msg = _('Failed to update password: %(resp)r') % locals()
-        LOG.error(msg, instance=instance)
-        raise Exception(msg)
-
-    return resp['message']
-
-
-def inject_file(session, instance, vm_ref, path, contents):
-    LOG.debug(_('Injecting file path: %r'), path, instance=instance)
-
-    # Files/paths must be base64-encoded for transmission to agent
-    b64_path = base64.b64encode(path)
-    b64_contents = base64.b64encode(contents)
-
-    args = {'b64_path': b64_path, 'b64_contents': b64_contents}
-
-    # If the agent doesn't support file injection, a NotImplementedError
-    # will be raised with the appropriate message.
-    resp = _call_agent(session, instance, vm_ref, 'inject_file', args)
-    if resp['returncode'] != '0':
-        LOG.error(_('Failed to inject file: %(resp)r'), locals(),
-                  instance=instance)
-        return None
-
-    return resp['message']
-
-
-def resetnetwork(session, instance, vm_ref):
-    LOG.debug(_('Resetting network'), instance=instance)
-
-    resp = _call_agent(session, instance, vm_ref, 'resetnetwork',
-                       timeout=FLAGS.agent_resetnetwork_timeout)
-    if resp['returncode'] != '0':
-        LOG.error(_('Failed to reset network: %(resp)r'), locals(),
-                  instance=instance)
         return None
 
-    return resp['message']
+    def agent_update(self, agent_build):
+        """Update agent on the VM instance."""
+
+        LOG.info(_('Updating agent to %s'), agent_build['version'],
+                 instance=self.instance)
+
+        # Send the encrypted password
+        args = {'url': agent_build['url'], 'md5sum': agent_build['md5hash']}
+        resp = _call_agent(
+            self.session, self.instance, self.vm_ref, 'agentupdate', args)
+        if resp['returncode'] != '0':
+            LOG.error(_('Failed to update agent: %(resp)r'), locals(),
+                      instance=self.instance)
+            return None
+        return resp['message']
+
+    def set_admin_password(self, new_pass):
+        """Set the root/admin password on the VM instance.
+
+        This is done via an agent running on the VM. Communication between nova
+        and the agent is done via writing xenstore records. Since communication
+        is done over the XenAPI RPC calls, we need to encrypt the password.
+        We're using a simple Diffie-Hellman class instead of a more advanced
+        library (such as M2Crypto) for compatibility with the agent code.
+        """
+        LOG.debug(_('Setting admin password'), instance=self.instance)
+
+        dh = SimpleDH()
+
+        # Exchange keys
+        args = {'pub': str(dh.get_public())}
+        resp = _call_agent(
+            self.session, self.instance, self.vm_ref, 'key_init', args)
+
+        # Successful return code from key_init is 'D0'
+        if resp['returncode'] != 'D0':
+            msg = _('Failed to exchange keys: %(resp)r') % locals()
+            LOG.error(msg, instance=self.instance)
+            raise Exception(msg)
+
+        # Some old versions of the Windows agent have a trailing \\r\\n
+        # (ie CRLF escaped) for some reason. Strip that off.
+        agent_pub = int(resp['message'].replace('\\r\\n', ''))
+        dh.compute_shared(agent_pub)
+
+        # Some old versions of Linux and Windows agent expect trailing \n
+        # on password to work correctly.
+        enc_pass = dh.encrypt(new_pass + '\n')
+
+        # Send the encrypted password
+        args = {'enc_pass': enc_pass}
+        resp = _call_agent(
+            self.session, self.instance, self.vm_ref, 'password', args)
+
+        # Successful return code from password is '0'
+        if resp['returncode'] != '0':
+            msg = _('Failed to update password: %(resp)r') % locals()
+            LOG.error(msg, instance=self.instance)
+            raise Exception(msg)
+
+        return resp['message']
+
+    def inject_file(self, path, contents):
+        LOG.debug(_('Injecting file path: %r'), path, instance=self.instance)
+
+        # Files/paths must be base64-encoded for transmission to agent
+        b64_path = base64.b64encode(path)
+        b64_contents = base64.b64encode(contents)
+
+        args = {'b64_path': b64_path, 'b64_contents': b64_contents}
+
+        # If the agent doesn't support file injection, a NotImplementedError
+        # will be raised with the appropriate message.
+        resp = _call_agent(
+            self.session, self.instance, self.vm_ref, 'inject_file', args)
+        if resp['returncode'] != '0':
+            LOG.error(_('Failed to inject file: %(resp)r'), locals(),
+                      instance=self.instance)
+            return None
+
+        return resp['message']
+
+    def resetnetwork(self):
+        LOG.debug(_('Resetting network'), instance=self.instance)
+
+        resp = _call_agent(
+            self.session, self.instance, self.vm_ref, 'resetnetwork',
+            timeout=FLAGS.agent_resetnetwork_timeout)
+        if resp['returncode'] != '0':
+            LOG.error(_('Failed to reset network: %(resp)r'), locals(),
+                      instance=self.instance)
+            return None
+
+        return resp['message']
 
 
 def find_guest_agent(base_dir):
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index 469892ae8..ac00be6d7 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -42,7 +42,7 @@ from nova.openstack.common import log as logging
 from nova.openstack.common import timeutils
 from nova import utils
 from nova.virt import firewall
-from nova.virt.xenapi import agent
+from nova.virt.xenapi import agent as xapi_agent
 from nova.virt.xenapi import pool_states
 from nova.virt.xenapi import vm_utils
 from nova.virt.xenapi import volume_utils
@@ -156,6 +156,9 @@ class VMOps(object):
         self.vif_driver = vif_impl(xenapi_session=self._session)
         self.default_root_dev = '/dev/sda'
 
+    def _get_agent(self, instance, vm_ref):
+        return xapi_agent.XenAPIBasedAgent(self._session, instance, vm_ref)
+
     def list_instances(self):
         """List VM instances."""
         # TODO(justinsb): Should we just always use the details method?
@@ -514,14 +517,15 @@ class VMOps(object):
 
         # Update agent, if necessary
         # This also waits until the agent starts
-        version = agent.get_agent_version(self._session, instance, vm_ref)
+        agent = self._get_agent(instance, vm_ref)
+        version = agent.get_agent_version()
         if version:
             LOG.info(_('Instance agent version: %s'), version,
                      instance=instance)
 
         if (version and agent_build and
             cmp_version(version, agent_build['version']) < 0):
-            agent.agent_update(self._session, instance, vm_ref, agent_build)
+            agent.agent_update(agent_build)
 
         # if the guest agent is not available, configure the
         # instance, but skip the admin password configuration
@@ -531,16 +535,14 @@ class VMOps(object):
         if injected_files:
             # Inject any files, if specified
             for path, contents in injected_files:
-                agent.inject_file(self._session, instance, vm_ref,
-                                  path, contents)
+                agent.inject_file(path, contents)
 
         # Set admin password, if necessary
         if admin_password and not no_agent:
-            agent.set_admin_password(self._session, instance, vm_ref,
-                                     admin_password)
+            agent.set_admin_password(admin_password)
 
         # Reset network config
-        agent.resetnetwork(self._session, instance, vm_ref)
+        agent.resetnetwork()
 
         # Set VCPU weight
         inst_type = db.instance_type_get(ctx, instance['instance_type_id'])
@@ -834,12 +836,14 @@ class VMOps(object):
     def set_admin_password(self, instance, new_pass):
         """Set the root/admin password on the VM instance."""
         vm_ref = self._get_vm_opaque_ref(instance)
-        agent.set_admin_password(self._session, instance, vm_ref, new_pass)
+        agent = self._get_agent(instance, vm_ref)
+        agent.set_admin_password(new_pass)
 
     def inject_file(self, instance, path, contents):
         """Write a file to the VM instance."""
         vm_ref = self._get_vm_opaque_ref(instance)
-        agent.inject_file(self._session, instance, vm_ref, path, contents)
+        agent = self._get_agent(instance, vm_ref)
+        agent.inject_file(path, contents)
 
     @staticmethod
     def _sanitize_xenstore_key(key):
@@ -1384,7 +1388,8 @@ class VMOps(object):
     def reset_network(self, instance):
         """Calls resetnetwork method in agent."""
         vm_ref = self._get_vm_opaque_ref(instance)
-        agent.resetnetwork(self._session, instance, vm_ref)
+        agent = self._get_agent(instance, vm_ref)
+        agent.resetnetwork()
 
     def inject_hostname(self, instance, vm_ref, hostname):
         """Inject the hostname of the instance into the xenstore."""