[Freeipa-devel] [python-pytest-multihost]-Ticket-6 run_command produces exit code 1 on windows
Niranjan
mrniranjan at fedoraproject.org
Thu Apr 21 05:25:20 UTC 2016
Petr Viktorin wrote:
> On 04/20/2016 06:11 AM, Niranjan wrote:
> > Petr Viktorin wrote:
> >> On 04/18/2016 12:39 PM, Niranjan wrote:
> >>> Niranjan wrote:
> >>>> Niranjan wrote:
> >>>>> Petr Viktorin wrote:
> >>>>>> On 04/06/2016 10:55 AM, Niranjan wrote:
> >>>>>>> Greetings,
> >>>>>>>
> >>>>>>> For https://fedorahosted.org/python-pytest-multihost/ticket/6 , i have proposed
> >>>>>>> a patch, I think this patch is more of a workaround , than a solution. I would
> >>>>>>> like to get more inputs on how to use pytest-multihost to execute commands
> >>>>>>> on Windows. The method i am using requires cygwin with openssh/bash to be
> >>>>>>> installed.
> >>>>>>
> >>>>>> Wow. I'm surprised the only problem on Windows is the "set -e".
> >>>>>>
> >>>>>> Regarding the patch:
> >>>>>>
> >>>>>> - Why is get_winhost_class needed? I don't see it called anywhere.
> >>>>>> - Similarly, why is host_type needed? Your patch only sets it.
> >>>>>>
> >>>>>> If run_command of WinHost and Unix hosts are this similar, I would put
> >>>>>> the 'set -e\n' for Unix (and an empty string for Windows) in a class
> >>>>>> attribute named e.g. "command_prelude", use it in run_command, and move
> >>>>>> run_command from Host to BaseHost.
> >>>>>> Would that work, or am I missing something?
> >>>>> How do we detect the host is windows/unix then , we still need host_type
> >>>>> to know what the type of host is (unix/windows)?
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> Petr Viktorin
> >>>> Please review the attached patch.
> >>
> >> Sorry for the delay.
> >>
> >> The information about whether the host is Unix or Windows is available:
> >> the class is either Host or WinHost, so I don't think an extra host_type
> >> is necessary.
> >> I'd be open to adding host_type as *configuration* (and corresponding
> >> attribute) if it also selected the actual Host class. Currently to use
> >> WinHost you need to write custom code.
> > I would like to have host_type available. We would like to add more
> > functions in classes that override Host/WinHost class , which
> > can be then called depending upon the host_type.
>
> Ah, I see; you're not using the WinHost subclass right now.
>
> I added a host_type; it is used to select the Host class.
> You can define a "host_classes" dict in your Domain class to list Host
> classes, and the particular Host class will be selected according to the
> host_type.
>
> The usage would be like this:
>
> ---------
>
> class QeHost(QeBaseHost):
> """Linux host class"""
> @classmethod
> def gethostname(self):
> """ Return system hostname """
> cmd = self.run_command(['hostname'], raiseonerr=False)
> return cmd.stdout_text.strip()
> ...
>
> class QeWinHost(QeBaseHost, pytest_multihost.host.WinHost):
> """Windows host class"""
>
> @classmethod
> def gethostname(self):
> """ Return system hostname """
> cmd = self.run_command(['hostname', '-A'], set_env=False,
> raiseonerr=False)
> return cmd.stdout_text.strip()
> ...
>
>
> class QeDomain:
> ...
> host_classes = {'default': QeHost, 'windows': QeWinHost}
>
> # remove your existing "get_host_class" method when host_classes is
> defined
> ...
>
> ---------
>
> And in the config, define host_type to 'windows'.
>
>
> Would it work for you like this?
Most of the things worked by doing this:
class QeBaseHost(pytest_multihost.host.BaseHost):
""" QeBaseHost subclass of multhost plugin BaseHost class """
transport_class = transport.SSHTransport
class QeHost(QeBaseHost):
"""
QeHost subclass of multihost plugin host class. This extends functionality
of the host class for SSSD QE purposes. Here we add support functions that
will be very widely used across tests and must be run on any or all hosts
in the environment.
"""
def gethostname(self):
""" Return system hostname """
cmd = self.run_command(['hostname'], raiseonerr=False)
return cmd.stdout_text.strip()
class QeWinHost(QeBaseHost, pytest_multihost.host.WinHost):
""" Windows Host class """
@classmethod
def gethostname(cls):
""" Return System hostname """
cmd = cls.run_command(['hostname', '-A'], set_env=False, raiseonerr=False)
return cmd.stdout_text.strip()
class QeDomain(pytest_multihost.config.Domain):
""" QeDomain subclass of multihost plugin domain class. """
def __init__(self, config, name, domain_type):
"""
Subclass of pytest_multihost.config.Domain
:param obj config: config config
:param str name: Name
:param str domain_type:
:return None:
"""
self.type = str(domain_type)
self.config = config
self.name = str(name)
self.hosts = []
host_classes = {'default': QeHost, 'windows': QeWinHost}
Specifically i see that you have "transport_class = transport.SSHTransport" added to Host Class
but not in WinHost class. So initially it failed transport class not available, but after i added below lines
it worked:
class QeBaseHost(pytest_multihost.host.BaseHost):
""" QeBaseHost subclass of multhost plugin BaseHost class """
transport_class = transport.SSHTransport
Apart from the above minor thing, Everything else works. Again the above 3 lines solves the minor issue,
just wanted to know if the above lines are to be added and is by design.
>
> --
> Petr Viktorin
> From b010bfca67a9aa985728f7d60ada713db9cf1b1e Mon Sep 17 00:00:00 2001
> From: Niranjan MR <mrniranjan at fedoraproject.org>
> Date: Tue, 12 Apr 2016 17:18:10 +0530
> Subject: [PATCH] Add 'host_type', make it possible to use Windows hosts
>
> Add global parameter windows_test_dir to specify test directory
> for Windows
> Windows hosts (WinHost) use this directory by default.
> test_dir can now be set individually for each host.
> Move run_command from Host class to BaseHost class
> Add a "host_type" configuration option and Host attribute. This
> is used to select the Host subclass. By default it can
> be 'default' or 'windows' (but Config subclasses can define more).
>
> Signed-off-by: Petr Viktorin <pviktroi at redhat.com>
> ---
> pytest_multihost/config.py | 15 +++++++--
> pytest_multihost/host.py | 56 ++++++++++++++++++++-------------
> test_pytestmultihost/test_testconfig.py | 12 ++++++-
> 3 files changed, 58 insertions(+), 25 deletions(-)
>
> diff --git a/pytest_multihost/config.py b/pytest_multihost/config.py
> index 31045c2..197d7ad 100644
> --- a/pytest_multihost/config.py
> +++ b/pytest_multihost/config.py
> @@ -45,6 +45,7 @@ class Config(object):
> self.ssh_password = kwargs.get('ssh_password')
> self.ssh_username = kwargs.get('ssh_username', 'root')
> self.ipv6 = bool(kwargs.get('ipv6', False))
> + self.windows_test_dir = kwargs.get('windows_test_dir', '/home/Administrator')
>
> if not self.ssh_password and not self.ssh_key_filename:
> self.ssh_key_filename = '~/.ssh/id_rsa'
> @@ -80,6 +81,8 @@ class Config(object):
> dct['ssh_key_filename'] = dct.pop('root_ssh_key_filename')
> if 'root_password' in dct:
> dct['ssh_password'] = dct.pop('root_password')
> + if 'windows_test_dir' in dct:
> + dct['windows_test_dir'] = dct.pop('windows_test_dir')
>
> all_init_args = set(init_args) | set(cls.extra_init_args)
> extra_args = set(dct) - all_init_args
> @@ -179,8 +182,16 @@ class Domain(object):
> self.hosts = []
>
> def get_host_class(self, host_dict):
> - from pytest_multihost.host import Host
> - return Host
> + host_type = host_dict.get('host_type', 'default')
> + return self.host_classes[host_type]
> +
> + @property
> + def host_classes(self):
> + from pytest_multihost.host import Host, WinHost
> + return {
> + 'default': Host,
> + 'windows': WinHost,
> + }
>
> @property
> def roles(self):
> diff --git a/pytest_multihost/host.py b/pytest_multihost/host.py
> index e6c0db5..9cc5e29 100644
> --- a/pytest_multihost/host.py
> +++ b/pytest_multihost/host.py
> @@ -25,9 +25,12 @@ class BaseHost(object):
> See README for an overview of the core classes.
> """
> transport_class = None
> + command_prelude = ''
>
> def __init__(self, domain, hostname, role, ip=None,
> - external_hostname=None, username=None, password=None):
> + external_hostname=None, username=None, password=None,
> + test_dir=None, host_type=None):
> + self.host_type = host_type
> self.domain = domain
> self.role = str(role)
> if username is None:
> @@ -40,6 +43,10 @@ class BaseHost(object):
> else:
> self.ssh_key_filename = None
> self.ssh_password = password
> + if test_dir is None:
> + self.test_dir = domain.config.test_dir
> + else:
> + self.test_dir = test_dir
>
> shortname, dot, ext_domain = hostname.partition('.')
> self.shortname = shortname
> @@ -78,7 +85,7 @@ class BaseHost(object):
> self.host_key = None
> self.ssh_port = 22
>
> - self.env_sh_path = os.path.join(domain.config.test_dir, 'env.sh')
> + self.env_sh_path = os.path.join(self.test_dir, 'env.sh')
>
> self.log_collectors = []
>
> @@ -118,20 +125,28 @@ class BaseHost(object):
>
> username = dct.pop('username', None)
> password = dct.pop('password', None)
> + host_type = dct.pop('host_type', 'default')
>
> check_config_dict_empty(dct, 'host %s' % hostname)
>
> - return cls(domain, hostname, role, ip, external_hostname,
> - username, password)
> + return cls(domain, hostname, role,
> + ip=ip,
> + external_hostname=external_hostname,
> + username=username,
> + password=password,
> + host_type=host_type)
>
> def to_dict(self):
> """Export info about this Host to a dict"""
> - return {
> + result = {
> 'name': str(self.hostname),
> 'ip': self.ip,
> 'role': self.role,
> 'external_hostname': self.external_hostname,
> }
> + if self.host_type != 'default':
> + result['host_type'] = self.host_type
> + return result
>
> @property
> def config(self):
> @@ -204,28 +219,18 @@ class BaseHost(object):
> does not exit with return code 0
> :param cwd: The working directory for the command
> """
> - raise NotImplementedError()
> -
> -
> -class Host(BaseHost):
> - """A Unix host"""
> - transport_class = transport.SSHTransport
> -
> - def run_command(self, argv, set_env=True, stdin_text=None,
> - log_stdout=True, raiseonerr=True,
> - cwd=None):
> - # This will give us a Bash shell
> command = self.transport.start_shell(argv, log_stdout=log_stdout)
> -
> # Set working directory
> if cwd is None:
> - cwd = self.config.test_dir
> + cwd = self.test_dir
> command.stdin.write('cd %s\n' % shell_quote(cwd))
>
> # Set the environment
> if set_env:
> command.stdin.write('. %s\n' % shell_quote(self.env_sh_path))
> - command.stdin.write('set -e\n')
> +
> + if self.command_prelude:
> + command.stdin.write(self.command_prelude)
>
> if isinstance(argv, basestring):
> # Run a shell command given as a string
> @@ -247,11 +252,18 @@ class Host(BaseHost):
> return command
>
>
> +class Host(BaseHost):
> + """A Unix host"""
> + transport_class = transport.SSHTransport
> + command_prelude = 'set -e\n'
> +
> +
> class WinHost(BaseHost):
> """
> Representation of a remote Windows host.
> -
> - This is a stub; Windows hosts can't currently be interacted with.
> """
>
> - pass
> + def __init__(self, domain, hostname, role, **kwargs):
> + # Set test_dir to the Windows directory, if not given explicitly
> + kwargs.setdefault('test_dir', domain.config.windows_test_dir)
> + super(WinHost, self).__init__(domain, hostname, role, **kwargs)
> diff --git a/test_pytestmultihost/test_testconfig.py b/test_pytestmultihost/test_testconfig.py
> index 8239a2c..b32fd38 100644
> --- a/test_pytestmultihost/test_testconfig.py
> +++ b/test_pytestmultihost/test_testconfig.py
> @@ -116,7 +116,8 @@ class TestComplexConfig(CheckConfig):
> dict(name='srv', ip='192.0.2.33', role='srv'),
> ]),
> dict(name='adomain2.test', hosts=[
> - dict(name='master.adomain2.test', ip='192.0.2.65'),
> + dict(name='master.adomain2.test', ip='192.0.2.65',
> + host_type='windows'),
> ]),
> ],
> )
> @@ -197,6 +198,7 @@ class TestComplexConfig(CheckConfig):
> ip="192.0.2.65",
> external_hostname="master.adomain2.test",
> role="master",
> + host_type='windows',
> ),
> ],
> ),
> @@ -228,3 +230,11 @@ class TestComplexConfig(CheckConfig):
> ad_dom = conf.domains[1]
> assert ad_dom.roles == ['srv']
> assert ad_dom.extra_roles == ['srv']
> +
> + assert conf.test_dir != conf.windows_test_dir
> +
> + assert conf.domains[0].hosts[0].host_type == 'default'
> + assert conf.domains[0].hosts[0].test_dir == conf.test_dir
> +
> + assert conf.domains[2].hosts[0].host_type == 'windows'
> + assert conf.domains[2].hosts[0].test_dir == conf.windows_test_dir
> --
> 2.5.5
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 328 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160421/f9739c0e/attachment.sig>
More information about the Freeipa-devel
mailing list