Host class improvements

- Use the external hostname when connecting to remote hosts
- Make it possible to specify working directory for remote commands
- Move kinit calls to installation code
  This allows tests where installation is done later
- Log at error level when a remote command fails unexpectedly
- Clean up test directory before testing
- Break infinite recursion in mkdir_recursive if dir can't be created

3 files changed, 34 insertions, 15 deletions

diff --git a/ipatests/test_integration/base.py b/ipatests/test_integration/base.py
index 934dce056..8b243f937 100644
--- a/ipatests/test_integration/base.py
+++ b/ipatests/test_integration/base.py
@@ -62,7 +62,6 @@ class IntegrationTest(object):
 
         try:
             cls.install()
-            cls.kinit_all()
         except:
             cls.uninstall()
             raise
@@ -92,12 +91,6 @@ class IntegrationTest(object):
             raise ValueError('Unknown topology %s' % cls.topology)
 
     @classmethod
-    def kinit_all(cls):
-        for host in cls.get_all_hosts():
-            host.run_command(['kinit', 'admin'],
-                             stdin_text=host.config.admin_password)
-
-    @classmethod
     def teardown_class(cls):
         try:
             cls.uninstall()
@@ -117,7 +110,7 @@ class IntegrationTest(object):
     @classmethod
     def collect_log(cls, host, filename):
         cls.log.info('Adding %s:%s to list of logs to collect' %
-                     (host.hostname, filename))
+                     (host.external_hostname, filename))
         cls.logs_to_collect.setdefault(host, []).append(filename)
 
 IntegrationTest.log = log_mgr.get_logger(IntegrationTest())
diff --git a/ipatests/test_integration/host.py b/ipatests/test_integration/host.py
index 404e5eec8..8109f51aa 100644
--- a/ipatests/test_integration/host.py
+++ b/ipatests/test_integration/host.py
@@ -95,9 +95,11 @@ class RemoteCommand(object):
 
         self._done = True
 
-        self.log.info('Exit code: %s', self.returncode)
         if raiseonerr and self.returncode:
+            self.log.error('Exit code: %s', self.returncode)
             raise subprocess.CalledProcessError(self.returncode, self.argv)
+        else:
+            self.log.info('Exit code: %s', self.returncode)
         return self.returncode
 
     def _start_pipe_thread(self, result_list, stream, name, do_log=True):
@@ -187,7 +189,8 @@ class Host(object):
         return env
 
     def run_command(self, argv, set_env=True, stdin_text=None,
-                    log_stdout=True, raiseonerr=True):
+                    log_stdout=True, raiseonerr=True,
+                    cwd=None):
         """Run the given command on this host
 
         Returns a RemoteCommand instance. The command will have already run
@@ -208,8 +211,13 @@ class Host(object):
                                 log_stdout=log_stdout)
         self._command_index += 1
 
+        if cwd is None:
+            cwd = self.config.test_dir
+        command.stdin.write('cd %s\n' % ipautil.shell_quote(cwd))
+
         if set_env:
-            command.stdin.write('. %s\n' % self.env_sh_path)
+            command.stdin.write('. %s\n' %
+                                ipautil.shell_quote(self.env_sh_path))
         command.stdin.write('set -e\n')
 
         if isinstance(argv, basestring):
@@ -234,7 +242,8 @@ class Host(object):
         try:
             return self._transport
         except AttributeError:
-            sock = socket.create_connection((self.hostname, self.ssh_port))
+            sock = socket.create_connection((self.external_hostname,
+                                             self.ssh_port))
             self._transport = transport = paramiko.Transport(sock)
             transport.connect(hostkey=self.host_key)
             if self.root_ssh_key_filename:
@@ -249,6 +258,8 @@ class Host(object):
             else:
                 self.log.critical('No SSH credentials configured')
                 raise RuntimeError('No SSH credentials configured')
+            # Clean up the test directory
+            self.run_command(['rm', '-rvf', self.config.test_dir])
             return transport
 
     @property
@@ -264,8 +275,10 @@ class Host(object):
     def mkdir_recursive(self, path):
         """`mkdir -p` on the remote host"""
         try:
-            self.sftp.chdir(path)
-        except IOError:
+            self.sftp.chdir(path or '/')
+        except IOError as e:
+            if not path or path == '/':
+                raise
             self.mkdir_recursive(os.path.dirname(path))
             self.sftp.mkdir(path)
             self.sftp.chdir(path)
@@ -280,7 +293,7 @@ class Host(object):
         """Write the given string to the named remote file"""
         self.log.info('WRITE %s', filename)
         with self.sftp.open(filename, 'w') as f:
-            return f.write(contents)
+            f.write(contents)
 
     def file_exists(self, filename):
         """Return true if the named remote file exists"""
@@ -293,3 +306,11 @@ class Host(object):
             else:
                 raise
         return True
+
+    def get_file(self, remotepath, localpath):
+        self.log.info('GET %s', remotepath)
+        self.sftp.get(remotepath, localpath)
+
+    def put_file(self, localpath, remotepath):
+        self.log.info('PUT %s', remotepath)
+        self.sftp.put(localpath, remotepath)
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 5e4606276..d131e4df6 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -148,6 +148,8 @@ def install_master(host, collect_log=None):
 
     enable_replication_debugging(host)
 
+    host.run_command(['kinit', 'admin'],
+                      stdin_text=host.config.admin_password)
 
 def install_replica(master, replica, collect_log=None):
     if collect_log:
@@ -173,6 +175,9 @@ def install_replica(master, replica, collect_log=None):
 
     enable_replication_debugging(replica)
 
+    replica.run_command(['kinit', 'admin'],
+                        stdin_text=replica.config.admin_password)
+
 
 def connect_replica(master, replica=None):
     if replica is None: