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.


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

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