Petr Viktorin wrote: > On 04/21/2016 07:25 AM, Niranjan wrote: > > 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 [...] > > > > 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. > > Right, since SSHTransport works for Windows hosts as well, I've added it > to BaseHost. With this version of patch it shouldn't be necessary to add > it in your code. > > If this works for you, I'll commit it and release.
With this latest patch it worked after removing the line "transport_class = transport.SSHTransport" from QeBaseHost class. Please go ahead and commit it. > > > -- > Petr Viktorin > From 7eb346c0bf29b1fddfd962675258a3895fbc8300 Mon Sep 17 00:00:00 2001 > From: Niranjan MR <[email protected]> > 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 <[email protected]> > --- > pytest_multihost/config.py | 15 +++++++-- > pytest_multihost/host.py | 57 > ++++++++++++++++++++------------- > test_pytestmultihost/test_testconfig.py | 12 ++++++- > 3 files changed, 58 insertions(+), 26 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..826372d 100644 > --- a/pytest_multihost/host.py > +++ b/pytest_multihost/host.py > @@ -24,10 +24,13 @@ class BaseHost(object): > > See README for an overview of the core classes. > """ > - transport_class = None > + transport_class = transport.SSHTransport > + 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,17 @@ class Host(BaseHost): > return command > > > +class Host(BaseHost): > + """A Unix host""" > + 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 >
pgpo0MrIbZOua.pgp
Description: PGP signature
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
