Clean up remote.py and misc.py changes.

Fixed method names to be non-redundant (remote_mktemp in remote is
now just mktemp, for example), and made some parameters be more
self descriptive.  Added some docstrings.  Fixed sudo setting in
get_file method.

Made chmod independent of the actual sftp file getting.
Do not do extra file copy if non-sudo read is needed.
Fixed some parameter names. Made sure temp files are removed.

diff --git a/teuthology/misc.py b/teuthology/misc.py
index 27e9b8b2e20c02de40381ae8b6f2051c7bcaafeb..a28e80a537fc4d4bd41e89f46eda5f88edf8f2bb 100644 (file)
--- a/teuthology/misc.py
+++ b/teuthology/misc.py
@@ -19,7 +19,6 @@ import yaml
 import json
 import re
 import tempfile
-import paramiko
 
 from teuthology import safepath
 from .orchestra import run
@@ -556,7 +555,7 @@ def remove_lines_from_file(remote, path, line_is_valid_test,
     # get a temp file path on the remote host to write to,
     # we don't want to blow away the remote file and then have the
     # network drop out
-    temp_file_path = remote.remote_mktemp()
+    temp_file_path = remote.mktemp()
 
     # write out the data to a temp file
     write_file(remote, temp_file_path, out_data)
@@ -572,7 +571,7 @@ def append_lines_to_file(remote, path, lines, sudo=False):
     Remove_lines_from_list.
     """
 
-    temp_file_path = remote.remote_mktemp()
+    temp_file_path = remote.mktemp()
 
     data = get_file(remote, path, sudo)
 
@@ -612,35 +611,11 @@ def create_file(remote, path, data="", permissions=str(644), sudo=False):
         append_lines_to_file(remote, path, data, sudo)
 
 
-def do_remote_sftp(remote, tempf, sudo=False):
-    """
-    Make sure file is aways readble if root, and use SFTPClient to
-    copy across data.
-    """
-    if sudo:
-        args = []
-        args.extend([
-            'sudo',
-            'chmod',
-            '0666',
-            tempf,
-            ])
-        remote.run(
-            args=args,
-            stdout=StringIO(),
-            )
-    conn = remote.connect()
-    transport = conn.get_transport()
-    sftp = paramiko.SFTPClient.from_transport(transport)
-    with sftp.open(tempf, 'rb') as file_sftp:
-        result = file_sftp.read()
-    return result
-
 def get_file(remote, path, sudo=False):
     """
     Copy_remote wrapper.
     """
-    return remote.copy_remote(path, sudo)
+    return remote.get_file(path, sudo)
 
 def pull_directory(remote, remotedir, localdir):
     """
@@ -650,7 +625,7 @@ def pull_directory(remote, remotedir, localdir):
               remote.shortname, remotedir, localdir)
     if not os.path.exists(localdir):
         os.mkdir(localdir)
-    result = remote.tar_remote(remotedir, sudo=True)
+    result = remote.get_tar(remotedir, sudo=True)
     _, local_tarfile = tempfile.mkstemp()
     with open(local_tarfile, 'r+') as fb1:
         fb1.write(result)
@@ -688,7 +663,7 @@ def pull_directory_tarball(remote, remotedir, localfile):
     """
     log.debug('Transferring archived files from %s:%s to %s',
               remote.shortname, remotedir, localfile)
-    tardata = remote.tar_remote(remotedir, zip_flag=True, sudo=True)
+    tardata = remote.get_tar(remotedir, zip_flag=True, sudo=True)
     with open(localfile, 'w') as out:
         out.write(tardata)
 

diff --git a/teuthology/orchestra/remote.py b/teuthology/orchestra/remote.py
index 905fc6f88ab4de29c0bb931fa25000ecfa2a70c2..e24d865e40b240c4fd5230974e33dd7afcdaefe4 100644 (file)
--- a/teuthology/orchestra/remote.py
+++ b/teuthology/orchestra/remote.py
@@ -109,18 +109,18 @@ class Remote(object):
         r.remote = self
         return r
 
-    def remote_mktemp(self, sudo=False):
+    def mktemp(self):
         """
         Make a remote temporary file 
+        
+        Returns: the name of the temp file created using
+                 tempfile.mkstemp
         """
-        args = []
-        if sudo:
-            args.append('sudo')
-        args.extend([
+        args = [
             'python',
             '-c',
             'import os; import tempfile; import sys; (fd,fname) = tempfile.mkstemp(); os.close(fd); sys.stdout.write(fname.rstrip()); sys.stdout.flush()'
-            ])
+            ]
         proc = self.run(
             args=args,
             stdout=StringIO(),
@@ -128,62 +128,76 @@ class Remote(object):
         data = proc.stdout.getvalue()
         return data
 
-    def _set_remote_perms(self, tempf, perms):
-        args = []
-        args.extend([
+    def chmod(self, file_path, permissions):
+        """
+        As super-user, set permissions on the remote file specified.
+        """
+        args = [
             'sudo',
             'chmod',
-            perms,
-            tempf,
-            ])
+            permissions,
+            file_path,
+            ]
         self.run(
             args=args,
-            stdout=StringIO(),
             )
 
-    def _do_sftp_cmd(self, args, tempf, sudo=False):
-        self.run(
-            args=args,
-            stdout=StringIO(),
-            )
-        if sudo:
-            self._set_remote_perms(tempf, '0666')
+    def _sftp_get_file(self, file_path):
+        """
+        Use the Paramiko SFTPClient to copy the data from the remote
+        file.  Returns the file's content.
+        """
         conn = self.connect()
         transport = conn.get_transport()
         sftp = paramiko.SFTPClient.from_transport(transport)
-        with sftp.open(tempf, 'rb') as file_sftp:
+        with sftp.open(file_path, 'rb') as file_sftp:
             result = file_sftp.read()
         return result
 
-    def copy_remote(self, path, sudo=False):
+    def remove(self, path):
+        self.run(args=['rm', '-fr', path]) 
+
+    def get_file(self, path, sudo=False): 
         """
         Read a file from the remote host into memory.
         """
-        tempf = self.remote_mktemp()
+        if not sudo:
+            return self._sftp_get_file(path)
+        temp_file_path = self.mktemp()
+        self.chmod(temp_file_path, '0666')
         args = [
             'sudo',
             'cp',
             path,
-            tempf,
+            temp_file_path,
             ]
-        return self._do_sftp_cmd(args, tempf, sudo)
+        self.run(args=args)
+        ret = self._sftp_get_file(temp_file_path) 
+        self.remove(temp_file_path)
+        return ret
 
-    def tar_remote(self, path, sudo=False, zip_flag=False):
+    def get_tar(self, path, sudo=False, zip_flag=False):
         """
         Tar a remote file.
         """
         zip_fld = lambda x: 'cz' if x else 'c'
-        tempf = self.remote_mktemp()
-        args = [
-            'sudo',
+        temp_file_path = self.mktemp()
+        self.chmod(temp_file_path, '0666')
+        args = []
+        if sudo:
+            args.append('sudo')
+        args.extend([
             'tar',
             zip_fld(zip_flag),
-            '-f', tempf,
+            '-f', temp_file_path,
             '-C', path,
             '--',
             '.',
-            ]
-        return self._do_sftp_cmd(args, tempf, sudo)
+            ])
+        self.run(args=args)
+        ret = self._sftp_get_file(temp_file_path)
+        self.remove(temp_file_path)
+        return ret
                 
 
 def getShortName(name):

diff --git a/teuthology/task/kernel.py b/teuthology/task/kernel.py
index d54e79ed9da153f4a2e9a87dbde82a3ac4fcd5c4..2011eb8d22e0d39ebb163ffce81d9c15c4410761 100644 (file)
--- a/teuthology/task/kernel.py
+++ b/teuthology/task/kernel.py
@@ -649,7 +649,7 @@ def update_grub_rpm(remote, newversion):
         newgrub = generate_legacy_grub_entry(remote, newversion)
         for line in newgrub:
             data += line + '\n'
-        temp_file_path = remote.remote_mktemp()
+        temp_file_path = remote.mktemp()
         teuthology.sudo_write_file(remote, temp_file_path, StringIO(data), '755')
         teuthology.move_file(remote, temp_file_path, '/boot/grub/grub.conf', True)
     else: