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.
I will test and let you know asap
> 
> 
> -- 
> Petr Viktorin

> From 7eb346c0bf29b1fddfd962675258a3895fbc8300 Mon Sep 17 00:00:00 2001
> From: Niranjan MR <mrniran...@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 <pvikt...@redhat.com>
> ---
>  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
> 

Attachment: pgpF0qjSEah2r.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

Reply via email to