Differential D745

minion: remove old taskdir before execution
ClosedPublic
Actions

Authored by kparal on Feb 15 2016, 1:50 PM.

Details

Reviewers
lbrabec
tflink
mkrizek
jskladan
Commits
rLTRN0522bba4c730: minion: remote old taskdir before execution
Summary

We were keeping the directory intact, so on persistent minions, files
from old tasks were still kept around together with the freshly copied
files from the current task.

Also escape directories properly when creating them using mkdir.

Test Plan

tested on persistent minion, old files are now deleted before new files are copied over. Also tested on a disposable minion that it does not produce any issues.

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 minion: remote old taskdir before execution.Feb 15 2016, 1:50 PM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added reviewers: mkrizek, jskladan, tflink, lbrabec.
tflink accepted this revision.Feb 15 2016, 3:01 PM
This revision is now accepted and ready to land.Feb 15 2016, 3:01 PM
Closed by commit rLTRN0522bba4c730: minion: remote old taskdir before execution (authored by kparal). · Explain WhyFeb 15 2016, 3:58 PM
This revision was automatically updated to reflect the committed changes.
kparal retitled this revision from minion: remote old taskdir before execution to minion: remove old taskdir before execution.Feb 15 2016, 6:23 PM

Revision Contents

Diff 1916

View Options

libtaskotron/minion.py

Show First 20 Lines • Show All 70 Lines • ▼ Show 20 Line(s)49 def _prepare_task(self):
71 # libtaskotron-fedora should be installed as a check dep 71​ # libtaskotron-fedora should be installed as a check dep
72 self.ssh.install_pkgs(['libtaskotron-core', 'libtaskotron-fedora']) 72​ self.ssh.install_pkgs(['libtaskotron-core', 'libtaskotron-fedora'])
73 73​
74 # Copy config files. Go through CONF_DIRS in reversed order to preserve priority. 74​ # Copy config files. Go through CONF_DIRS in reversed order to preserve priority.
75 for config_dir in reversed(config.CONF_DIRS): 75​ for config_dir in reversed(config.CONF_DIRS):
76 if os.path.exists(config_dir): 76​ if os.path.exists(config_dir):
77 self.ssh.put_dir(config_dir, '/etc/taskotron/') 77​ self.ssh.put_dir(config_dir, '/etc/taskotron/')
78 78​
79 # create needed dirs 79​ # create needed dirs. remove existing content first.
80​ self.ssh.cmd('rm -rf %s' % pipes.quote(self.taskdir))
80 self.ssh.mkdir(self.taskdir) 81​ self.ssh.mkdir(self.taskdir)
81 82​
82 # patch remote libtaskotron if needed (for dev, shouldn't be used in production) 83​ # patch remote libtaskotron if needed (for dev, shouldn't be used in production)
83 if self.arg_data['patch'] is not None: 84​ if self.arg_data['patch'] is not None:
84 self.ssh.put_file(os.path.abspath(self.arg_data['patch']), '%s/%s' % ( 85​ self.ssh.put_file(os.path.abspath(self.arg_data['patch']), '%s/%s' % (
85 self.taskdir, self.arg_data['patch'])) 86​ self.taskdir, self.arg_data['patch']))
86 self.ssh.install_pkgs(['patch']) 87​ self.ssh.install_pkgs(['patch'])
87 self.ssh.cmd('patch -d /usr/lib/python2.7/site-packages/ -p1 -i %s/%s' % ( 88​ self.ssh.cmd('patch -d /usr/lib/python2.7/site-packages/ -p1 -i %s/%s' % (
88 self.taskdir, self.arg_data['patch'])) 89​ pipes.quote(self.taskdir), pipes.quote(self.arg_data['patch'])))
89 90​
90 # put files needed for execution (task, input files, etc) 91​ # put files needed for execution (task, input files, etc)
91 self.ssh.put_dir(os.path.dirname(os.path.abspath(self.arg_data['taskfile'])), self.taskdir) 92​ self.ssh.put_dir(os.path.dirname(os.path.abspath(self.arg_data['taskfile'])), self.taskdir)
92 93​
93 # need to have default_flow_style false to get valid yaml w/ nested dicts 94​ # need to have default_flow_style false to get valid yaml w/ nested dicts
94 self.ssh.write_file(os.path.join(self.taskdir, 95​ self.ssh.write_file(os.path.join(self.taskdir,
95 os.path.basename(self.arg_data['taskfile'])), 96​ os.path.basename(self.arg_data['taskfile'])),
96 yaml.dump(self.task_data, default_flow_style=False)) 97​ yaml.dump(self.task_data, default_flow_style=False))
▲ Show 20 Lines • Show All 104 Lines • Show Last 20 Lines
View Options

libtaskotron/remote_exec.py

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Line(s)97 def close(self):
99 99​
100 self.outstream.close() 100​ self.outstream.close()
101 self.sftp.close() 101​ self.sftp.close()
102 self.ssh.close() 102​ self.ssh.close()
103 103​
104 def cmd(self, cmd): 104​ def cmd(self, cmd):
105 '''Execute a command. 105​ '''Execute a command.
106 106​
107 :param str cmd: A command to be executed 107​ :param str cmd: A command to be executed. Make sure you escape it properly to prevent shell
108​ expansion, in case it is not desired.
108 :returns: returncode of the command 109​ :returns: returncode of the command
109 :raise TaskotronRemoteProcessError: If the command has non-zero return code and it isn't a 110​ :raise TaskotronRemoteProcessError: If the command has non-zero return code and it isn't a
110 code of the exitcode directive. 111​ code of the exitcode directive.
111 :raise TaskotronRemoteError: If the remote hasn't sent any output for :attr:`TIMEOUT`. 112​ :raise TaskotronRemoteError: If the remote hasn't sent any output for :attr:`TIMEOUT`.
112 ''' 113​ '''
113 114​
114 log.debug('Running command on remote host: %s', cmd) 115​ log.debug('Running command on remote host: %s', cmd)
115 116​
▲ Show 20 Lines • Show All 142 Lines • ▼ Show 20 Line(s)257 def mkdir(self, name, mode=777):
258 259​
259 :param str name: A name of the directory 260​ :param str name: A name of the directory
260 :param int mode: Permissions (posix-style, decimal) of the directory, 261​ :param int mode: Permissions (posix-style, decimal) of the directory,
261 default is 777 262​ default is 777
262 263​
263 :raise TaskotronRemoteError: If the directory could not be created 264​ :raise TaskotronRemoteError: If the directory could not be created
264 ''' 265​ '''
265 266​
266 self.cmd('mkdir -p -m %d %s' % (mode, name)) 267​ self.cmd('mkdir -p -m %d %s' % (mode, pipes.quote(name)))
267 268​
268 def put_dir(self, local_path, remote_path, overwrite=True): 269​ def put_dir(self, local_path, remote_path, overwrite=True):
269 '''Copy a directory to a remote path. 270​ '''Copy a directory to a remote path.
270 271​
271 :param str remote_path: A path to the remote directory 272​ :param str remote_path: A path to the remote directory
272 :param str local_path: A path to the local directory 273​ :param str local_path: A path to the local directory
273 :param bool overwrite: Whether to overwrite remote path. Default is True. 274​ :param bool overwrite: Whether to overwrite remote path. Default is True.
274 275​
▲ Show 20 Lines • Show All 50 Lines • Show Last 20 Lines
View Options

testing/test_remote_exec.py

1# -*- coding: utf-8 -*- 1​# -*- coding: utf-8 -*-
2# Copyright 2009-2015, Red Hat, Inc. 2​# Copyright 2009-2015, Red Hat, Inc.
3# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> 3​# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
4# See the LICENSE file for more details on Licensing 4​# See the LICENSE file for more details on Licensing
5 5​
6'''Unit tests for libtaskotron/remote_exec.py''' 6​'''Unit tests for libtaskotron/remote_exec.py'''
7 7​
8import errno 8​import errno
9import socket 9​import socket
10 10​
11import pytest 11​import pytest
12import paramiko 12​import paramiko
13from munch import Munch 13​from munch import Munch
14from dingus import Dingus 14​from dingus import Dingus
15import mock 15​import mock
16​import shlex
16 17​
17from libtaskotron import remote_exec 18​from libtaskotron import remote_exec
18from libtaskotron.exceptions import TaskotronRemoteError, TaskotronRemoteProcessError 19​from libtaskotron.exceptions import TaskotronRemoteError, TaskotronRemoteProcessError
19 20​
20 21​
21STUB_DIR = 'dirname' 22​STUB_DIR = 'dirname'
22STUB_FILE = 'filename' 23​STUB_FILE = 'filename'
23STUB_NONEXISTING_FILE = '' 24​STUB_NONEXISTING_FILE = ''
▲ Show 20 Lines • Show All 245 Lines • ▼ Show 20 Line(s)266 def test_mkdir(self):
269 assert self.remote.cmd.calls()[0][1][0] == 'mkdir -p -m 777 %s' % ref_dirname 270​ assert self.remote.cmd.calls()[0][1][0] == 'mkdir -p -m 777 %s' % ref_dirname
270 271​
271 def test_mkdir_mode(self): 272​ def test_mkdir_mode(self):
272 ref_dirname = 'dirname' 273​ ref_dirname = 'dirname'
273 ref_mode = 644 274​ ref_mode = 644
274 self.remote.cmd = Dingus() 275​ self.remote.cmd = Dingus()
275 self.remote.mkdir(ref_dirname, ref_mode) 276​ self.remote.mkdir(ref_dirname, ref_mode)
276 assert self.remote.cmd.calls()[0][1][0] == 'mkdir -p -m %d %s' % (ref_mode, ref_dirname) 277​ assert self.remote.cmd.calls()[0][1][0] == 'mkdir -p -m %d %s' % (ref_mode, ref_dirname)
278​
279​ def test_mkdir_escape(self):
280​ ref_dirname = 'dirname with spaces & other ch*r*cters; (how sneaky!)'
281​ self.remote.cmd = Dingus()
282​ self.remote.mkdir(ref_dirname)
283​ cmdline = shlex.split(self.remote.cmd.calls()[0][1][0])
284​ assert cmdline[-1] == ref_dirname