Differential D697

remote_exec: make install_pkgs() accept list only, improve messaging
ClosedPublic
Actions

Authored by kparal on Dec 16 2015, 4:31 PM.

Details

Reviewers
mkrizek
Commits
rLTRN148c7c49ee33: remote_exec: make install_pkgs() accept list only, improve messaging
Summary

This is just a few trivial improvements I made when looking at T661.

install_pkgs() now uses python structures instead of string parsing (no
current code needs that, more reliable). Improved debug messages when
command fails. When installing commands, we now inform about it on INFO
level (the full command line executed including all packages is still
visible on DEBUG level).

Test Plan

test suite works

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal retitled this revision from to remote_exec: make install_pkgs() accept list only, improve messaging.Dec 16 2015, 4:31 PM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added a reviewer: mkrizek.
mkrizek accepted this revision.Dec 16 2015, 4:36 PM
This revision is now accepted and ready to land.Dec 16 2015, 4:36 PM
Closed by commit rLTRN148c7c49ee33: remote_exec: make install_pkgs() accept list only, improve messaging (authored by kparal). · Explain WhyDec 16 2015, 4:39 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 1801

View Options

libtaskotron/minion.py

Show First 20 Lines • Show All 46 Lines • ▼ Show 20 Line(s)29 def __init__(self, task_data, arg_data):
47 self.ssh = None 47​ self.ssh = None
48 48​
49 def _prepare_task(self): 49​ def _prepare_task(self):
50 '''Prepare the environment to run the task.''' 50​ '''Prepare the environment to run the task.'''
51 51​
52 self.taskdir = config.get_config().client_taskdir 52​ self.taskdir = config.get_config().client_taskdir
53 53​
54 # add additional repos to minion 54​ # add additional repos to minion
55 repos = [' --add-repo %s' % repo for repo in config.get_config().minion_repos] 55​ repos = ['--add-repo %s' % repo for repo in config.get_config().minion_repos]
56 if repos: 56​ if repos:
57 self.ssh.cmd('dnf config-manager%s' % ''.join(repos)) 57​ self.ssh.cmd('dnf config-manager %s' % ' '.join(repos))
58 58​
59 # FIXME this should only need libtaskotron-core T651 59​ # FIXME this should only need libtaskotron-core T651
60 # libtaskotron-fedora should be installed as a check dep 60​ # libtaskotron-fedora should be installed as a check dep
61 self.ssh.install_pkgs('libtaskotron-core, libtaskotron-fedora') 61​ self.ssh.install_pkgs(['libtaskotron-core', 'libtaskotron-fedora'])
62 62​
63 # Copy config files. Go through CONF_DIRS in reversed order to preserve priority. 63​ # Copy config files. Go through CONF_DIRS in reversed order to preserve priority.
64 for config_dir in reversed(config.CONF_DIRS): 64​ for config_dir in reversed(config.CONF_DIRS):
65 if os.path.exists(config_dir): 65​ if os.path.exists(config_dir):
66 self.ssh.put_dir(config_dir, '/etc/taskotron/') 66​ self.ssh.put_dir(config_dir, '/etc/taskotron/')
67 67​
68 # create needed dirs 68​ # create needed dirs
69 self.ssh.mkdir(self.taskdir) 69​ self.ssh.mkdir(self.taskdir)
70 70​
71 # patch remote libtaskotron if needed (for dev, shouldn't be used in production) 71​ # patch remote libtaskotron if needed (for dev, shouldn't be used in production)
72 if self.arg_data['patch'] is not None: 72​ if self.arg_data['patch'] is not None:
73 self.ssh.put_file(os.path.abspath(self.arg_data['patch']), '%s/%s' % ( 73​ self.ssh.put_file(os.path.abspath(self.arg_data['patch']), '%s/%s' % (
74 self.taskdir, self.arg_data['patch'])) 74​ self.taskdir, self.arg_data['patch']))
75 self.ssh.install_pkgs('patch') 75​ self.ssh.install_pkgs(['patch'])
76 self.ssh.cmd('patch -d /usr/lib/python2.7/site-packages/ -p1 -i %s/%s' % ( 76​ self.ssh.cmd('patch -d /usr/lib/python2.7/site-packages/ -p1 -i %s/%s' % (
77 self.taskdir, self.arg_data['patch'])) 77​ self.taskdir, self.arg_data['patch']))
78 78​
79 # put files needed for execution (task, input files, etc) 79​ # put files needed for execution (task, input files, etc)
80 self.ssh.put_dir(os.path.dirname(os.path.abspath(self.arg_data['taskfile'])), self.taskdir) 80​ self.ssh.put_dir(os.path.dirname(os.path.abspath(self.arg_data['taskfile'])), self.taskdir)
81 81​
82 # need to have default_flow_style false to get valid yaml w/ nested dicts 82​ # need to have default_flow_style false to get valid yaml w/ nested dicts
83 self.ssh.write_file(os.path.join(self.taskdir, 83​ self.ssh.write_file(os.path.join(self.taskdir,
▲ Show 20 Lines • Show All 104 Lines • Show Last 20 Lines
View Options

libtaskotron/remote_exec.py

Show First 20 Lines • Show All 129 Lines • ▼ Show 20 Line(s)129 if channel.recv_stderr_ready():
130 data = channel.recv_stderr(1024) 130​ data = channel.recv_stderr(1024)
131 self.outstream.write(data) 131​ self.outstream.write(data)
132 alive_counter = 0 132​ alive_counter = 0
133 133​
134 sleep(0.1) 134​ sleep(0.1)
135 alive_counter += 0.1 135​ alive_counter += 0.1
136 136​
137 if alive_counter >= self.TIMEOUT: 137​ if alive_counter >= self.TIMEOUT:
138 raise exc.TaskotronRemoteError('Has not received any output from the remote ' 138​ raise exc.TaskotronRemoteError('No output received from remote %s@%s:%s for %d '
139 '%s@%s:%s for %d seconds' % 139​ 'seconds while running: %s' %
140 (self.username, self.hostname, self.port, 140​ (self.username, self.hostname, self.port,
141 self.TIMEOUT)) 141​ self.TIMEOUT, cmd))
142 142​
143 retcode = channel.recv_exit_status() 143​ retcode = channel.recv_exit_status()
144 144​
145 # read the rest of data that was left in the buffers 145​ # read the rest of data that was left in the buffers
146 self.outstream.write(stdout.read()) 146​ self.outstream.write(stdout.read())
147 self.outstream.write(stderr.read()) 147​ self.outstream.write(stderr.read())
148 148​
149 if retcode != 0 and retcode != exitcode_directive.FAILURE: 149​ if retcode != 0 and retcode != exitcode_directive.FAILURE:
150 raise exc.TaskotronRemoteError('Command "%s" on %s@%s exited with code %s' % 150​ raise exc.TaskotronRemoteError('Command "%s" on %s@%s exited with code %s' %
151 (cmd, self.username, self.hostname, retcode)) 151​ (cmd, self.username, self.hostname, retcode))
152 return retcode 152​ return retcode
153 153​
154 def install_pkgs(self, pkgs): 154​ def install_pkgs(self, pkgs):
155 '''Install packages via dnf. 155​ '''Install packages via dnf.
156 156​
157 :param list pkgs: A list or a string separated by commas of packages to be installed 157​ :param list pkgs: A list of packages to be installed
158 158​
159 :raise TaskotronRemoteError: If the command has non-zero return code 159​ :raise TaskotronRemoteError: If the command has non-zero return code
160 ''' 160​ '''
161 161​
162 if isinstance(pkgs, basestring): 162​ log.info('Installing %d packages on remote host...', len(pkgs))
163 pkgs = pkgs.split(',') ​
164 ​
165 log.debug('Installing packages on remote host: %s', ','.join(pkgs)) ​
166 self.cmd('dnf -y install %s' % ' '.join(pkgs)) 163​ self.cmd('dnf -y install %s' % ' '.join(pkgs))
167 164​
168 def write_file(self, remote_path, data, overwrite=True): 165​ def write_file(self, remote_path, data, overwrite=True):
169 '''Write data to a remote file. 166​ '''Write data to a remote file.
170 167​
171 :param str remote_path: A path to the remote file 168​ :param str remote_path: A path to the remote file
172 :param str data: Data to be written 169​ :param str data: Data to be written
173 :param bool overwrite: Whether to overwrite remote path. Default is True. 170​ :param bool overwrite: Whether to overwrite remote path. Default is True.
▲ Show 20 Lines • Show All 143 Lines • Show Last 20 Lines
View Options

testing/test_remote_exec.py

Show First 20 Lines • Show All 226 Lines • ▼ Show 20 Line(s)144class TestRemoteExec(object):
227 227​
228 def test_cmd_success(self): 228​ def test_cmd_success(self):
229 assert self.remote.cmd(STUB_CMD_TRUE) == STUB_CMD_TRUE 229​ assert self.remote.cmd(STUB_CMD_TRUE) == STUB_CMD_TRUE
230 230​
231 def test_cmd_failure(self): 231​ def test_cmd_failure(self):
232 with pytest.raises(TaskotronRemoteError): 232​ with pytest.raises(TaskotronRemoteError):
233 self.remote.cmd(STUB_CMD_FALSE) 233​ self.remote.cmd(STUB_CMD_FALSE)
234 234​
235 def test_install_pkgs_str(self): 235​ def test_install_pkgs(self):
236 self.remote.cmd = Dingus() ​
237 self.remote.install_pkgs('libtaskotron,rpmlint') ​
238 assert self.remote.cmd.calls()[0][1][0] == 'dnf -y install libtaskotron rpmlint' ​
239 ​
240 def test_install_pkgs_list(self): ​
241 self.remote.cmd = Dingus() 236​ self.remote.cmd = Dingus()
242 self.remote.install_pkgs(['libtaskotron', 'rpmlint']) 237​ self.remote.install_pkgs(['libtaskotron', 'rpmlint'])
243 assert self.remote.cmd.calls()[0][1][0] == 'dnf -y install libtaskotron rpmlint' 238​ assert self.remote.cmd.calls()[0][1][0] == 'dnf -y install libtaskotron rpmlint'
244 239​
245 def test_mkdir(self): 240​ def test_mkdir(self):
246 ref_dirname = 'dirname' 241​ ref_dirname = 'dirname'
247 self.remote.cmd = Dingus() 242​ self.remote.cmd = Dingus()
248 self.remote.mkdir(ref_dirname) 243​ self.remote.mkdir(ref_dirname)
249 assert self.remote.cmd.calls()[0][1][0] == 'mkdir -p -m 777 %s' % ref_dirname 244​ assert self.remote.cmd.calls()[0][1][0] == 'mkdir -p -m 777 %s' % ref_dirname
250 245​
251 def test_mkdir_mode(self): 246​ def test_mkdir_mode(self):
252 ref_dirname = 'dirname' 247​ ref_dirname = 'dirname'
253 ref_mode = 644 248​ ref_mode = 644
254 self.remote.cmd = Dingus() 249​ self.remote.cmd = Dingus()
255 self.remote.mkdir(ref_dirname, ref_mode) 250​ self.remote.mkdir(ref_dirname, ref_mode)
256 assert self.remote.cmd.calls()[0][1][0] == 'mkdir -p -m %d %s' % (ref_mode, ref_dirname) 251​ assert self.remote.cmd.calls()[0][1][0] == 'mkdir -p -m %d %s' % (ref_mode, ref_dirname)