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

Guannan Ren gren at redhat.com
Mon Mar 26 13:02:58 UTC 2012


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 at 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




More information about the libvir-list mailing list