From 74eff43ee1a2b73159277370cfa9d194e42bf49c Mon Sep 17 00:00:00 2001 From: Warren Usui Date: Thu, 24 Apr 2014 15:56:41 -0700 Subject: [PATCH] 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. --- teuthology/misc.py | 35 +++------------ teuthology/orchestra/remote.py | 78 ++++++++++++++++++++-------------- teuthology/task/kernel.py | 2 +- 3 files changed, 52 insertions(+), 63 deletions(-) diff --git a/teuthology/misc.py b/teuthology/misc.py index 27e9b8b2..a28e80a5 100644 --- 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 905fc6f8..e24d865e 100644 --- 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 d54e79ed..2011eb8d 100644 --- 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: -- 2.47.3