[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [test-API PATCH 5/6] Cleanup and fix of domain/define test



On 03/25/2012 01:42 AM, Martin Kletzander wrote:
I added support for 'uri' parameter and moved some functions into
util/Python/utils.py in order not to lose them from the code and keep
them accessible for other tests.
---
  repos/domain/define.py |  132 ++++++++----------------------------------------
  utils/Python/utils.py  |   45 ++++++++++++++++-
  2 files changed, 65 insertions(+), 112 deletions(-)

diff --git a/repos/domain/define.py b/repos/domain/define.py
index 8f0095a..25630c5 100644
--- a/repos/domain/define.py
+++ b/repos/domain/define.py
@@ -19,7 +19,7 @@
  __author__ = 'Alex Jia: ajia redhat com'
  __date__ = 'Mon Jan 28, 2010'
  __version__ = '0.1.0'
-__credits__ = 'Copyright (C) 2009 Red Hat, Inc.'
+__credits__ = 'Copyright (C) 2009, 2012 Red Hat, Inc.'
  __all__ = ['usage', 'check_define_domain', 'define']

  import os
@@ -63,9 +63,6 @@ def usage():
                             macaddr
                             ifacetype
                             source
-                           target_machine
-                           username
-                           password
            '''

  def check_params(params):
@@ -107,58 +104,17 @@ def ssh_keygen(logger):

      return 0

-def ssh_tunnel(hostname, username, password, logger):
-    """setup a tunnel to a give host"""
-    logger.info("setup ssh tunnel with host %s" % hostname)
-    user_host = "%s %s" % (username, hostname)
-    child = pexpect.spawn(SSH_COPY_ID, [ user_host])
-    while True:
-        index = child.expect(['yes\/no', 'password: ',
-                               pexpect.EOF,
-                               pexpect.TIMEOUT])
-        if index == 0:
-            child.sendline("yes")
-        elif index == 1:
-            child.sendline(password)
-        elif index == 2:
-            logger.debug(string.strip(child.before))
-            child.close()
-            return 0
-        elif index == 3:
-            logger.error("setup tunnel timeout")
-            logger.debug(string.strip(child.before))
-            child.close()
-            return 1
-
-    return 0
-
-def check_define_domain(guestname, guesttype, target_machine, username, \
-                        password, util, logger):
-    """Check define domain result, if define domain is successful,
-       guestname.xml will exist under /etc/libvirt/qemu/
-       and can use virt-xml-validate tool to check the file validity
+def check_define_domain(conn, guestname, logger):
+    """Check define domain result. To make this work on all
+    hypervisors and with all configuration posibilities, use the
+    default way through libvirt to check if the guest was defined
      """
-    if "kvm" in guesttype:
-        path = "/etc/libvirt/qemu/%s.xml" % guestname
-    elif "xen" in guesttype:
-        path = "/etc/xen/%s" % guestname
-    else:
-        logger.error("unknown guest type")
-
-    if target_machine:
-        cmd = "ls %s" % path
-        ret, output = util.remote_exec_pexpect(target_machine, username, \
-                                               password, cmd)
-        if ret:
-            logger.error("guest %s xml file doesn't exsits" % guestname)
-            return False
-        else:
-            return True
-    else:
-        if os.access(path, os.R_OK):
-            return True
-        else:
-            return False
+    try:
+        conn.lookupByName(guestname + 'asdf')

             the testing code?

+        return True
+    except libvirtError, e:
+        logger.error(e.message())
+        return False

It's better not to use libvirt API to check the result of one another API. We should use native approach to do the checking. So I insist on the original checking.


  def define(params):
      """Define a domain from xml"""
@@ -169,41 +125,10 @@ def define(params):
      logger = params['logger']
      guestname = params['guestname']
      guesttype = params['guesttype']
+    uri = params['uri']

If uri is not None, we need to get the IP or hostname of target machine from the uri
             Use that IP or hostname to perform ssh operation.

      test_result = False

-    if params.has_key('target_machine'):
-        logger.info("define domain on remote host")
-        target_machine = params['target_machine']
-        username = params['username']
-        password = params['password']
-    else:
-        logger.info("define domain on local host")
-        target_machine = None
-        username = None
-        password = None
-
      # Connect to hypervisor connection URI
-    util = utils.Utils()
-    if target_machine:
-        uri = util.get_uri(target_machine)
-
-        #generate ssh key pair
-        ret = ssh_keygen(logger)
-        if ret:
-            logger.error("failed to generate RSA key")
-            return 1
-        #setup ssh tunnel with target machine
-        ret = ssh_tunnel(target_machine, username, password, logger)
-        if ret:
-            logger.error("faild to setup ssh tunnel with target machine %s" % \
-                          target_machine)
-            return 1
-
-        commands.getstatusoutput("ssh-add")
-
-    else:
-        uri = util.get_uri('127.0.0.1')
-
      conn = connectAPI.ConnectAPI()
      virconn = conn.open(uri)

@@ -222,35 +147,20 @@ def define(params):

      # Define domain from xml
      try:
-        try:
-            dom_obj.define(dom_xml)
-            if check_define_domain(guestname, guesttype, target_machine, \
-                                   username, password, util, logger):
-                logger.info("define a domain form xml is successful")
-                test_result = True
-            else:
-                logger.error("fail to check define domain")
-                test_result = False
-        except LibvirtAPI, e:
-            logger.error("fail to define a domain from xml")
+        dom_obj.define(dom_xml)
+        if check_define_domain(virconn, guestname, logger):
+            logger.info("define a domain form xml is successful")
+            test_result = True
+        else:
+            logger.error("failed to check define domain")
              test_result = False
+    except LibvirtAPI, e:
+        logger.error("failed to define a domain from xml")
+        test_result = False
      finally:
          conn.close()
          logger.info("closed hypervisor connection")

           try ... except ...finally is new syntax since 2.5,
           But we need to support 2.4, so should use
           try:
                try:
                except:
           finally



-    if target_machine:
-        REMOVE_SSH = "ssh %s \"rm -rf /root/.ssh/*\"" % (target_machine)
-        logger.info("remove ssh key on remote machine")
-        status, ret = util.exec_cmd(REMOVE_SSH, shell=True)
-        if status:
-            logger.error("failed to remove ssh key")
-
-        REMOVE_LOCAL_SSH = "rm -rf /root/.ssh/*"

                  Never remove or change the local ssh directory like this.
                  The autotest have ssh configuration file stored here.


-        logger.info("remove local ssh key")
-        status, ret = util.exec_cmd(REMOVE_LOCAL_SSH, shell=True)
-        if status:
-            logger.error("failed to remove local ssh key")
-
      if test_result:
          return 0
      else:
diff --git a/utils/Python/utils.py b/utils/Python/utils.py
index 55c1cb5..7382abb 100644
--- a/utils/Python/utils.py
+++ b/utils/Python/utils.py
@@ -1,6 +1,6 @@
  #!/usr/bin/env python
  #
-# libvirt-test-API is copyright 2010 Red Hat, Inc.
+# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
  #
  # libvirt-test-API is free software: you can redistribute it and/or modify it
  # under the terms of the GNU General Public License as published by
@@ -433,3 +433,46 @@ class Utils(object):
                  return 1

          return 0
+
+
+    def ssh_keygen(logger):

          I agree with this, the migrate.py use this too.
          could you help clean migrate.py along with this together?
If we put this in utils.py, It's better not to accept "logger" argument just like other utilities do

If we create a ssh-keygen and ssh_tunnel as a standalone testcase. we will use the connect for all of following testcases rather than setup a ssh connection in each case. This will be good.

+        """using pexpect to generate RSA"""
+        SSH_KEYGEN = "ssh-keygen -t rsa"
+        SSH_COPY_ID = "ssh-copy-id"
+
+        logger.info("generate ssh RSA \"%s\"" % SSH_KEYGEN)
+        child = pexpect.spawn(SSH_KEYGEN)
+        while True:
+            index = child.expect(['Enter file in which to save the key ',
+                                  'Enter passphrase ',
+                                  'Enter same passphrase again: ',
+                                   pexpect.EOF,
+                                   pexpect.TIMEOUT])
+            if index == 0:
+                child.sendline("\r")
+            elif index == 1:
+                child.sendline("\r")
+            elif index == 2:
+                child.sendline("\r")
+            elif index == 3:
+                logger.debug(string.strip(child.before))
+                child.close()
+                return 0
+            elif index == 4:
+                logger.error("ssh_keygen timeout")
+                logger.debug(string.strip(child.before))
+                child.close()
+                return 1
+
+        return 0
+
+    def ssh_remove_keys(host, logger):

            same as above

+        """remove ssh keys on remote machine"""
+        REMOVE_SSH = "ssh %s \"rm -rf /root/.ssh/*\"" % (target_machine)
+        logger.info("remove ssh key on remote machine")
+        ret, dummy = self.exec_cmd(REMOVE_SSH)
+        if ret:
+            logger.error("failed to remove ssh key")
+            return 1
+
+        return 0


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]