From 5ffce2e33b8f57287946dca7ef17ce02e6424fe4 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 1/17] Basic functionality for save_failed_tree plugin --- diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py new file mode 100644 index 0000000..784bf20 --- /dev/null +++ b/plugins/builder/save_failed_tree.py @@ -0,0 +1,40 @@ +import fnmatch +import os +import tarfile +import koji.tasks as tasks +from __main__ import BuildRoot + +__all__ = ('SaveFailedTreeTask',) + +def omit_ccache(tarinfo): + if fnmatch.fnmatch(tarinfo.name, '*/tmp/krb5cc') or \ + fnmatch.fnmatch(tarinfo.name, '*/etc/*.keytab'): + return None + else: + return tarinfo + + + +class SaveFailedTreeTask(tasks.BaseTaskHandler): + Methods = ['saveFailedTree'] + _taskWeight = 3.0 + + + def handler(self, taskID, full=False): + self.logger.debug("Starting saving buildroots for task %d [full=%s]" % (taskID, full)) + tar_path = os.path.join(self.workdir, 'broots-task-%s.tar.gz' % taskID) + f = tarfile.open(tar_path, "w:gz") + for broot in self.session.listBuildroots(taskID=taskID): + broot = BuildRoot(self.session, self.options, broot['id']) + path = broot.rootdir() + if full: + self.logger.debug("Adding buildroot (full): %s" % path) + else: + path = os.path.join(path, 'builddir') + self.logger.debug("Adding buildroot: %s" % path) + f.add(path, filter=omit_ccache) + f.close() + self.logger.debug("Uploading %s to hub." % tar_path) + self.uploadFile(tar_path) + os.unlink(tar_path) + self.logger.debug("Finished saving buildroots for task %d" % taskID) diff --git a/plugins/hub/save_failed_tree.py b/plugins/hub/save_failed_tree.py new file mode 100644 index 0000000..9eccca6 --- /dev/null +++ b/plugins/hub/save_failed_tree.py @@ -0,0 +1,29 @@ +import koji +from koji.plugin import export + +import sys +sys.path.insert(0, '/usr/share/koji-hub/') +import kojihub + +__all__ = ('saveFailedTree',) + +@export +def saveFailedTree(taskID, full=False, **opts): + # let it raise errors + taskID = int(taskID) + full = bool(full) + + task_info = kojihub.Task(taskID).getInfo() + if task_info['state'] != koji.TASK_STATES['FAILED']: + return 'Task %s has not failed.' % taskID + elif task_info['method'] != 'buildArch': + # TODO: allowed tasks could be defined in plugin hub config + return 'Only buildArch tasks can upload buildroot (Task %(id)s is %(method)s).' % task_info + # owner? + # permissions? + + args = koji.encode_args(taskID, full, **opts) + taskopts = { + 'assign': task_info['host_id'], + } + return kojihub.make_task('saveFailedTree', args, **taskopts) From 86c1127009787961fcaa4b176cbc0752e6310d43 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 2/17] Basic docs for save_failed_tree plugin --- diff --git a/docs/source/index.rst b/docs/source/index.rst index 6de3165..9a6539e 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -31,6 +31,7 @@ Contents server_bootstrap server_howto using_the_koji_build_system + plugins writing_a_plugin writing_koji_code content_generators diff --git a/docs/source/plugins.rst b/docs/source/plugins.rst new file mode 100644 index 0000000..432941b --- /dev/null +++ b/docs/source/plugins.rst @@ -0,0 +1,56 @@ +======= +Plugins +======= + +Following plugins are available in default koji installation. + +Runroot +======= + +Plugin for running any command in buildroot. + +Save Failed Tree Plugin +======================= + +In some cases developers want to investigate exact environment in which their +build failed. Reconstructing this environment via mock needn't end with +exactly same structure (due to builder settings, etc.). In such case this +plugin can be used to retrieve tarball with complete mock tree. + +.. warning:: + For security reasons, currently all ``/tmp/krb5cc*`` and ``/etc/*.keytab`` + files are removed from tarball. If we found some other dangerous pieces, + they can be added to this blacklist. + +Special task method is created for achieving this which is called +``SaveFailedTree``. This task can be created via CLI: +``koji save-failed-tree ``. Additional options are: + +.. option:: --full + + directs koji to create tarball with complete tree. + +.. option:: --nowait + + exit immediately after creating task + +.. option:: --quiet + + don't print any information to output + +After task finishes, one can find the tarball on relevant task web page (URL +will be printed to stdout until ``--quiet`` is used. + +Currently plugin allow to save trees only for ``buildArch`` tasks and anybody +is allowed to create this type of task (and download tarball). + +.. warning:: + Don't forget that this type of task can generate huge amount of data, so use + it wisely. + +TODO +---- + * Make allowed task types configurable on hub + * Restricted access (original owner, special permission, ...) + * Separate volume/directory on hub + * garbage collector + policy for retaining generated tarballs From c3607435adacc7894452da3b350f0bc08d0a5a94 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 3/17] CLI for save_failed_tree --- diff --git a/cli/koji b/cli/koji index 6eb48f3..ca9a8a6 100755 --- a/cli/koji +++ b/cli/koji @@ -7202,6 +7202,52 @@ def handle_runroot(options, session, args): sys.exit(1) return +def handle_save_failed_tree(options, session, args): + "Create tarball with whole buildtree" + usage = _("usage: %prog save-failed-tree [options] taskID") + usage += _("\n(Specify the --help global option for a list of other help options)") + parser = OptionParser(usage=usage) + parser.disable_interspersed_args() + parser.add_option("-f", "--full", action="store_true", default=False, + help=_("Download whole tree, if not specified, only builddir will be downloaded")) + parser.add_option("--quiet", action="store_true", + help=_("Do not print the task information"), default=options.quiet) + parser.add_option("--nowait", action="store_true", + help=_("Don't wait on build")) + + (opts, args) = parser.parse_args(args) + + if len(args) != 1: + parser.error(_("List exactly one taskID")) + + try: + taskID = int(args[0]) + except ValueError: + parser.error(_("Task ID must be an integer.")) + + activate_session(session) + try: + task_id = session.saveFailedTree(taskID, opts.full) + except koji.GenericError as e: + if 'Invalid method' in str(e): + print "* The save_failed_tree plugin appears to not be installed" \ + " on the koji hub. Please contact the administrator." + raise + + if type(task_id) != int: + print 'Error: %s' % task_id + return + + if not opts.quiet: + print "Created task:", task_id + print "Task info: %s/taskinfo?taskID=%s" % (options.weburl, task_id) + + if opts.nowait: + return + else: + session.logout() + watch_tasks(session, [task_id], quiet=opts.quiet) + def handle_help(options, session, args): "[info] List available commands" diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py index 784bf20..223e821 100644 --- a/plugins/builder/save_failed_tree.py +++ b/plugins/builder/save_failed_tree.py @@ -6,6 +6,7 @@ from __main__ import BuildRoot __all__ = ('SaveFailedTreeTask',) + def omit_ccache(tarinfo): if fnmatch.fnmatch(tarinfo.name, '*/tmp/krb5cc') or \ fnmatch.fnmatch(tarinfo.name, '*/etc/*.keytab'): @@ -14,12 +15,10 @@ def omit_ccache(tarinfo): return tarinfo - class SaveFailedTreeTask(tasks.BaseTaskHandler): Methods = ['saveFailedTree'] _taskWeight = 3.0 - def handler(self, taskID, full=False): self.logger.debug("Starting saving buildroots for task %d [full=%s]" % (taskID, full)) tar_path = os.path.join(self.workdir, 'broots-task-%s.tar.gz' % taskID) diff --git a/plugins/hub/save_failed_tree.py b/plugins/hub/save_failed_tree.py index 9eccca6..7f98602 100644 --- a/plugins/hub/save_failed_tree.py +++ b/plugins/hub/save_failed_tree.py @@ -1,14 +1,19 @@ +import sys import koji from koji.plugin import export -import sys sys.path.insert(0, '/usr/share/koji-hub/') import kojihub __all__ = ('saveFailedTree',) + @export def saveFailedTree(taskID, full=False, **opts): + '''xmlrpc method for creating saveFailedTree task. If arguments are + invalid, error message is returned. Otherwise task id of newly created + task is returned.''' + # let it raise errors taskID = int(taskID) full = bool(full) diff --git a/tests/test_cli/data/list-commands.txt b/tests/test_cli/data/list-commands.txt index 7a4d8ce..6249a73 100644 --- a/tests/test_cli/data/list-commands.txt +++ b/tests/test_cli/data/list-commands.txt @@ -118,6 +118,7 @@ miscellaneous commands: call Execute an arbitrary XML-RPC call import-comps Import group/package information from a comps file moshimoshi Introduce yourself + save-failed-tree Create tarball with whole buildtree monitor commands: wait-repo Wait for a repo to be regenerated diff --git a/tests/test_cli/test_save_failed_tree.py b/tests/test_cli/test_save_failed_tree.py new file mode 100644 index 0000000..90488fe --- /dev/null +++ b/tests/test_cli/test_save_failed_tree.py @@ -0,0 +1,128 @@ +import StringIO +import unittest +import koji +import mock + +import loadcli +cli = loadcli.cli + + +class TestSaveFailedTree(unittest.TestCase): + def setUp(self): + self.options = mock.MagicMock() + self.session = mock.MagicMock() + self.args = mock.MagicMock() + self.original_parser = cli.OptionParser + cli.OptionParser = mock.MagicMock() + self.parser = cli.OptionParser.return_value + cli.options = self.options # globals!!! + + def tearDown(self): + cli.OptionParser = self.original_parser + + # Show long diffs in error output... + maxDiff = None + + @mock.patch('koji_cli.activate_session') + def test_handle_save_failed_tree_simple(self, activate_session_mock): + # koji save-failed-tree 123456 + task_id = 123456 + arguments = [task_id] + options = mock.MagicMock() + options.full = False + options.nowait = True + self.parser.parse_args.return_value = [options, arguments] + self.session.getAPIVersion.return_value = koji.API_VERSION + + # Mock out the xmlrpc server + self.session.saveFailedTree.return_value = 123 + + # Run it and check immediate output + cli.handle_save_failed_tree(self.options, self.session, self.args) + + # Finally, assert that things were called as we expected. + activate_session_mock.assert_called_once_with(self.session) + self.session.saveFailedTree.assert_called_once_with(task_id, options.full) + + @mock.patch('koji_cli.activate_session') + def test_handle_save_failed_tree_full(self, activate_session_mock): + # koji save-failed-tree 123456 --full + task_id = 123456 + arguments = [task_id] + options = mock.MagicMock() + options.full = True + options.nowait = True + self.parser.parse_args.return_value = [options, arguments] + self.session.getAPIVersion.return_value = koji.API_VERSION + + # Mock out the xmlrpc server + self.session.saveFailedTree.return_value = 123 + + # Run it and check immediate output + cli.handle_save_failed_tree(self.options, self.session, self.args) + + # Finally, assert that things were called as we expected. + activate_session_mock.assert_called_once_with(self.session) + self.session.saveFailedTree.assert_called_once_with(task_id, options.full) + + @mock.patch('koji_cli.activate_session') + @mock.patch('koji_cli.watch_tasks') + def test_handle_save_failed_tree_wait(self, watch_tasks_mock, activate_session_mock): + # koji save-failed-tree 123456 --full + task_id = 123456 + arguments = [task_id] + options = mock.MagicMock() + options.full = True + options.nowait = False + options.quiet = False + self.parser.parse_args.return_value = [options, arguments] + self.session.getAPIVersion.return_value = koji.API_VERSION + + # Mock out the xmlrpc server + spawned_id = 123 + self.session.saveFailedTree.return_value = spawned_id + + # Run it and check immediate output + cli.handle_save_failed_tree(self.options, self.session, self.args) + + # Finally, assert that things were called as we expected. + self.session.saveFailedTree.assert_called_once_with(task_id, options.full) + activate_session_mock.assert_called_once_with(self.session) + self.session.logout.assert_called_once_with() + watch_tasks_mock.assert_called_once_with(self.session, [spawned_id], + quiet=options.quiet) + + @mock.patch('sys.stdout', new_callable=StringIO.StringIO) + @mock.patch('koji_cli.activate_session') + @mock.patch('koji_cli.watch_tasks') + def test_handle_save_failed_tree_errors(self, watch_tasks_mock, activate_session_mock, stdout): + # koji save-failed-tree 123 456 + arguments = [123, 456] + options = mock.MagicMock() + self.parser.parse_args.return_value = [options, arguments] + self.parser.error.side_effect = Exception() + self.session.getAPIVersion.return_value = koji.API_VERSION + + self.assertRaises(Exception, cli.handle_save_failed_tree, + self.options, self.session, self.args) + + arguments = ["text"] + self.parser.parse_args.return_value = [options, arguments] + self.assertRaises(Exception, cli.handle_save_failed_tree, self.options, + self.session, self.args) + + # plugin not installed + arguments = [123] + self.parser.parse_args.return_value = [options, arguments] + self.session.saveFailedTree.side_effect = koji.GenericError("Invalid method") + self.assertRaises(koji.GenericError, cli.handle_save_failed_tree, + self.options, self.session, self.args) + + # something wrong happened in task + stdout.seek(0) + stdout.truncate() + self.session.saveFailedTree.return_value = 'xyz' + self.session.saveFailedTree.side_effect = None + cli.handle_save_failed_tree(self.options, self.session, self.args) + actual = stdout.getvalue() + self.assertEqual(actual, 'Error: xyz\n') From 54bcd8cfa248b889285b62b32325d99915443724 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 4/17] Define tasks via config file --- diff --git a/docs/source/plugins.rst b/docs/source/plugins.rst index 432941b..b3d98ad 100644 --- a/docs/source/plugins.rst +++ b/docs/source/plugins.rst @@ -41,7 +41,10 @@ Special task method is created for achieving this which is called After task finishes, one can find the tarball on relevant task web page (URL will be printed to stdout until ``--quiet`` is used. -Currently plugin allow to save trees only for ``buildArch`` tasks and anybody +Plugin allow to save trees only for tasks defined in config +``/etc/koji-hub/plugins/save_failed_tree.conf``. Option +``allowed_methods`` contains list of comma-delimited names of tasks. Default +configuration contains line: ``allowed_methods = buildArch``. Anybody is allowed to create this type of task (and download tarball). .. warning:: diff --git a/plugins/hub/save_failed_tree.conf b/plugins/hub/save_failed_tree.conf new file mode 100644 index 0000000..af31ad7 --- /dev/null +++ b/plugins/hub/save_failed_tree.conf @@ -0,0 +1,7 @@ +# config file for the Koji save-failed-trees plugin + +[permissions] +# task methods for whose can be triggered buildroot export +# * can be used to allow everything. In such case it must be only component +# on line. Otherwise multiple values are delimited by comma. +allowed_methods = buildArch diff --git a/plugins/hub/save_failed_tree.py b/plugins/hub/save_failed_tree.py index 7f98602..02f6838 100644 --- a/plugins/hub/save_failed_tree.py +++ b/plugins/hub/save_failed_tree.py @@ -1,4 +1,5 @@ import sys +import ConfigParser import koji from koji.plugin import export @@ -7,23 +8,36 @@ import kojihub __all__ = ('saveFailedTree',) +CONFIG_FILE = '/etc/koji-hub/plugins/save_failed_tree.conf' +config = None +allowed_methods = None + @export def saveFailedTree(taskID, full=False, **opts): '''xmlrpc method for creating saveFailedTree task. If arguments are invalid, error message is returned. Otherwise task id of newly created task is returned.''' + global config, allowed_methods # let it raise errors taskID = int(taskID) full = bool(full) + # read configuration only once + if config is None: + config = ConfigParser.SafeConfigParser() + config.read(CONFIG_FILE) + allowed_methods = config.get('permissions', 'allowed_methods').split() + if len(allowed_methods) == 1 and allowed_methods[0] == '*': + allowed_methods = '*' + task_info = kojihub.Task(taskID).getInfo() if task_info['state'] != koji.TASK_STATES['FAILED']: - return 'Task %s has not failed.' % taskID - elif task_info['method'] != 'buildArch': - # TODO: allowed tasks could be defined in plugin hub config - return 'Only buildArch tasks can upload buildroot (Task %(id)s is %(method)s).' % task_info + return 'Task %s has not failed. Only failed tasks can upload their buildroots.' % taskID + elif allowed_methods != '*' and task_info['method'] not in allowed_methods: + return 'Only %s tasks can upload their buildroots (Task %s is %s).' % \ + (', '.join(allowed_methods), task_info['id'], task_info['method']) # owner? # permissions? From e1ac33793e3be92558c392cf0c7e889d94ce062c Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 5/17] permissions --- diff --git a/cli/koji b/cli/koji index ca9a8a6..859758f 100755 --- a/cli/koji +++ b/cli/koji @@ -7229,15 +7229,22 @@ def handle_save_failed_tree(options, session, args): try: task_id = session.saveFailedTree(taskID, opts.full) except koji.GenericError as e: - if 'Invalid method' in str(e): - print "* The save_failed_tree plugin appears to not be installed" \ - " on the koji hub. Please contact the administrator." - raise - - if type(task_id) != int: - print 'Error: %s' % task_id - return - + m = str(e) + if 'Only failed tasks can upload their buildroots.' in m: + print _("Only failed tasks can upload their buildroots.") + elif 'tasks can upload their buildroots (Task' in m: + print _("Task of this type has disabled support for uploading" \ + " buildroot. (configurable on hub)") + elif 'Invalid method' in m: + print _("* The save_failed_tree plugin appears to not be installed" \ + " on the koji hub. Please contact the administrator.") + if logger.isEnabledFor(logging.DEBUG): + tb_str = ''.join(traceback.format_exception(*sys.exc_info())) + logger.debug(tb_str) + return 1 + except koji.ActionNotAllowed: + print _("Only task owner or admin can run this task.") + return 1 if not opts.quiet: print "Created task:", task_id print "Task info: %s/taskinfo?taskID=%s" % (options.weburl, task_id) diff --git a/plugins/hub/save_failed_tree.py b/plugins/hub/save_failed_tree.py index 02f6838..c818632 100644 --- a/plugins/hub/save_failed_tree.py +++ b/plugins/hub/save_failed_tree.py @@ -1,6 +1,7 @@ import sys import ConfigParser import koji +from koji.context import context from koji.plugin import export sys.path.insert(0, '/usr/share/koji-hub/') @@ -34,12 +35,12 @@ def saveFailedTree(taskID, full=False, **opts): task_info = kojihub.Task(taskID).getInfo() if task_info['state'] != koji.TASK_STATES['FAILED']: - return 'Task %s has not failed. Only failed tasks can upload their buildroots.' % taskID + raise koji.PreBuildError, 'Task %s has not failed. Only failed tasks can upload their buildroots.' % taskID elif allowed_methods != '*' and task_info['method'] not in allowed_methods: - return 'Only %s tasks can upload their buildroots (Task %s is %s).' % \ + raise koji.PreBuildError, 'Only %s tasks can upload their buildroots (Task %s is %s).' % \ (', '.join(allowed_methods), task_info['id'], task_info['method']) - # owner? - # permissions? + elif task_info["owner"] != context.session.user_id and not context.session.assertPerm('admin'): + raise koji.ActionNotAllowed, "only owner of failed task or 'admin' can run this task" args = koji.encode_args(taskID, full, **opts) taskopts = { diff --git a/tests/test_cli/test_save_failed_tree.py b/tests/test_cli/test_save_failed_tree.py index 90488fe..b2ad7df 100644 --- a/tests/test_cli/test_save_failed_tree.py +++ b/tests/test_cli/test_save_failed_tree.py @@ -110,19 +110,28 @@ class TestSaveFailedTree(unittest.TestCase): self.parser.parse_args.return_value = [options, arguments] self.assertRaises(Exception, cli.handle_save_failed_tree, self.options, self.session, self.args) + cli.logger = mock.MagicMock() # plugin not installed arguments = [123] self.parser.parse_args.return_value = [options, arguments] self.session.saveFailedTree.side_effect = koji.GenericError("Invalid method") - self.assertRaises(koji.GenericError, cli.handle_save_failed_tree, - self.options, self.session, self.args) + cli.handle_save_failed_tree(self.options, self.session, self.args) + actual = stdout.getvalue() + self.assertTrue('The save_failed_tree plugin appears to not be installed' in actual) + + # Task which is not FAILED + stdout.seek(0) + stdout.truncate() + self.session.saveFailedTree.side_effect = koji.PreBuildError('Only failed tasks can upload their buildroots.') + cli.handle_save_failed_tree(self.options, self.session, self.args) + actual = stdout.getvalue() + self.assertTrue('Only failed tasks can upload their buildroots.' in actual) - # something wrong happened in task + # Disabled/unsupported task stdout.seek(0) stdout.truncate() - self.session.saveFailedTree.return_value = 'xyz' - self.session.saveFailedTree.side_effect = None + self.session.saveFailedTree.side_effect = koji.PreBuildError('tasks can upload their buildroots (Task') cli.handle_save_failed_tree(self.options, self.session, self.args) actual = stdout.getvalue() - self.assertEqual(actual, 'Error: xyz\n') + self.assertTrue('Task of this type has disabled support for uploading' in actual) From 6f76fc88f9270d4db3900267c556ac08d188b2b5 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 6/17] make stripped paths configurable --- diff --git a/docs/source/plugins.rst b/docs/source/plugins.rst index b3d98ad..b3c9572 100644 --- a/docs/source/plugins.rst +++ b/docs/source/plugins.rst @@ -17,6 +17,17 @@ build failed. Reconstructing this environment via mock needn't end with exactly same structure (due to builder settings, etc.). In such case this plugin can be used to retrieve tarball with complete mock tree. +Additional feature is that some paths from buildroot can be left out from +tarball. Feature can be configured via +`/etc/kojid/plugins/save_failed_tree.conf` file. Currently only field +filters.paths is used and it consists of globs (standard python's fnmatch is +used) separated by ':'. + +.. code-block:: ini + + [filters] + paths = /etc/*.keytab:/tmp/secret_data + .. warning:: For security reasons, currently all ``/tmp/krb5cc*`` and ``/etc/*.keytab`` files are removed from tarball. If we found some other dangerous pieces, diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py index 223e821..4b82b9a 100644 --- a/plugins/builder/save_failed_tree.py +++ b/plugins/builder/save_failed_tree.py @@ -1,19 +1,30 @@ import fnmatch import os import tarfile +import ConfigParser import koji.tasks as tasks from __main__ import BuildRoot __all__ = ('SaveFailedTreeTask',) +CONFIG_FILE = '/etc/kojid/plugins/save_failed_tree.conf' +config = None -def omit_ccache(tarinfo): - if fnmatch.fnmatch(tarinfo.name, '*/tmp/krb5cc') or \ - fnmatch.fnmatch(tarinfo.name, '*/etc/*.keytab'): +def omit_paths(tarinfo): + if any([fnmatch.fnmatch(tarinfo.name, f) for f in config['path_filters']]): return None else: return tarinfo +def read_config(): + global config + cp = ConfigParser.SafeConfigParser() + cp.read(CONFIG_FILE) + config = { + 'path_filters': [], + } + if cp.has_option('filters', 'paths'): + config['path_filters'] = cp.get('filters', 'paths').split(':') class SaveFailedTreeTask(tasks.BaseTaskHandler): Methods = ['saveFailedTree'] @@ -21,6 +32,7 @@ class SaveFailedTreeTask(tasks.BaseTaskHandler): def handler(self, taskID, full=False): self.logger.debug("Starting saving buildroots for task %d [full=%s]" % (taskID, full)) + read_config() tar_path = os.path.join(self.workdir, 'broots-task-%s.tar.gz' % taskID) f = tarfile.open(tar_path, "w:gz") for broot in self.session.listBuildroots(taskID=taskID): @@ -31,7 +43,7 @@ class SaveFailedTreeTask(tasks.BaseTaskHandler): else: path = os.path.join(path, 'builddir') self.logger.debug("Adding buildroot: %s" % path) - f.add(path, filter=omit_ccache) + f.add(path, filter=omit_paths) f.close() self.logger.debug("Uploading %s to hub." % tar_path) self.uploadFile(tar_path) From eee4ace19a0b6287470301cb9e0acb09b74bac20 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 7/17] removing already done TODOs --- diff --git a/docs/source/plugins.rst b/docs/source/plugins.rst index b3c9572..d3c29f0 100644 --- a/docs/source/plugins.rst +++ b/docs/source/plugins.rst @@ -64,7 +64,5 @@ is allowed to create this type of task (and download tarball). TODO ---- - * Make allowed task types configurable on hub - * Restricted access (original owner, special permission, ...) * Separate volume/directory on hub * garbage collector + policy for retaining generated tarballs From 087a0aa33966463fe411ba0eb3c48f55bdb8c337 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 8/17] config file --- diff --git a/plugins/builder/save_failed_tree.conf b/plugins/builder/save_failed_tree.conf new file mode 100644 index 0000000..5e16d4c --- /dev/null +++ b/plugins/builder/save_failed_tree.conf @@ -0,0 +1,2 @@ +[filters] +paths = */tmp/krb5cc:*/etc/*.keytab From 06c5257d4caee6819046916b4f3a775eb1165871 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:40:17 +0000 Subject: [PATCH 9/17] use print function everywhere --- diff --git a/cli/koji b/cli/koji index 859758f..88c5c22 100755 --- a/cli/koji +++ b/cli/koji @@ -7231,23 +7231,23 @@ def handle_save_failed_tree(options, session, args): except koji.GenericError as e: m = str(e) if 'Only failed tasks can upload their buildroots.' in m: - print _("Only failed tasks can upload their buildroots.") + print(_("Only failed tasks can upload their buildroots.")) elif 'tasks can upload their buildroots (Task' in m: - print _("Task of this type has disabled support for uploading" \ - " buildroot. (configurable on hub)") + print(_("Task of this type has disabled support for uploading" \ + " buildroot. (configurable on hub)")) elif 'Invalid method' in m: - print _("* The save_failed_tree plugin appears to not be installed" \ - " on the koji hub. Please contact the administrator.") + print(_("* The save_failed_tree plugin appears to not be installed" \ + " on the koji hub. Please contact the administrator.")) if logger.isEnabledFor(logging.DEBUG): tb_str = ''.join(traceback.format_exception(*sys.exc_info())) logger.debug(tb_str) return 1 except koji.ActionNotAllowed: - print _("Only task owner or admin can run this task.") + print(_("Only task owner or admin can run this task.")) return 1 if not opts.quiet: - print "Created task:", task_id - print "Task info: %s/taskinfo?taskID=%s" % (options.weburl, task_id) + print("Created task:", task_id) + print("Task info: %s/taskinfo?taskID=%s" % (options.weburl, task_id)) if opts.nowait: return From 003cc8ec8c0272ede7812518b7e889a0b85f665f Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:41:42 +0000 Subject: [PATCH 10/17] utilize multiple volume support --- diff --git a/plugins/builder/save_failed_tree.conf b/plugins/builder/save_failed_tree.conf index 5e16d4c..84df846 100644 --- a/plugins/builder/save_failed_tree.conf +++ b/plugins/builder/save_failed_tree.conf @@ -1,2 +1,5 @@ +[global] +volume = DEFAULT + [filters] paths = */tmp/krb5cc:*/etc/*.keytab diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py index 4b82b9a..d1b618a 100644 --- a/plugins/builder/save_failed_tree.py +++ b/plugins/builder/save_failed_tree.py @@ -22,9 +22,12 @@ def read_config(): cp.read(CONFIG_FILE) config = { 'path_filters': [], + 'volume': None, } if cp.has_option('filters', 'paths'): config['path_filters'] = cp.get('filters', 'paths').split(':') + if cp.has_option('general', 'volume'): + config['volume'] = cp.get('general', 'volume').strip() class SaveFailedTreeTask(tasks.BaseTaskHandler): Methods = ['saveFailedTree'] @@ -46,6 +49,6 @@ class SaveFailedTreeTask(tasks.BaseTaskHandler): f.add(path, filter=omit_paths) f.close() self.logger.debug("Uploading %s to hub." % tar_path) - self.uploadFile(tar_path) + self.uploadFile(tar_path, volume=config['volume']) os.unlink(tar_path) self.logger.debug("Finished saving buildroots for task %d" % taskID) From e97c2d0599fbbff2d9dc9dc53d555f7b3cc23812 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:48:13 +0000 Subject: [PATCH 11/17] remove unnecessary code --- diff --git a/cli/koji b/cli/koji index 88c5c22..545d5be 100755 --- a/cli/koji +++ b/cli/koji @@ -7207,7 +7207,6 @@ def handle_save_failed_tree(options, session, args): usage = _("usage: %prog save-failed-tree [options] taskID") usage += _("\n(Specify the --help global option for a list of other help options)") parser = OptionParser(usage=usage) - parser.disable_interspersed_args() parser.add_option("-f", "--full", action="store_true", default=False, help=_("Download whole tree, if not specified, only builddir will be downloaded")) parser.add_option("--quiet", action="store_true", From 18e9921361a8bcfd690d7eacf4c4b9f7c38e51dd Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 12:48:39 +0000 Subject: [PATCH 12/17] handle unknown exception in main --- diff --git a/cli/koji b/cli/koji index 545d5be..5d782ad 100755 --- a/cli/koji +++ b/cli/koji @@ -7237,9 +7237,8 @@ def handle_save_failed_tree(options, session, args): elif 'Invalid method' in m: print(_("* The save_failed_tree plugin appears to not be installed" \ " on the koji hub. Please contact the administrator.")) - if logger.isEnabledFor(logging.DEBUG): - tb_str = ''.join(traceback.format_exception(*sys.exc_info())) - logger.debug(tb_str) + else: + raise return 1 except koji.ActionNotAllowed: print(_("Only task owner or admin can run this task.")) From a4179a97e5c5200cfc433792a61fe8617323374f Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 14:04:40 +0000 Subject: [PATCH 13/17] Fail if host is disabled --- diff --git a/plugins/hub/save_failed_tree.py b/plugins/hub/save_failed_tree.py index c818632..46ee865 100644 --- a/plugins/hub/save_failed_tree.py +++ b/plugins/hub/save_failed_tree.py @@ -16,9 +16,10 @@ allowed_methods = None @export def saveFailedTree(taskID, full=False, **opts): - '''xmlrpc method for creating saveFailedTree task. If arguments are - invalid, error message is returned. Otherwise task id of newly created - task is returned.''' + """Create saveFailedTree task + + If arguments are invalid, error message is returned. Otherwise task id of + newly created task is returned.""" global config, allowed_methods # let it raise errors @@ -35,12 +36,14 @@ def saveFailedTree(taskID, full=False, **opts): task_info = kojihub.Task(taskID).getInfo() if task_info['state'] != koji.TASK_STATES['FAILED']: - raise koji.PreBuildError, 'Task %s has not failed. Only failed tasks can upload their buildroots.' % taskID + raise koji.PreBuildError("Task %s has not failed. Only failed tasks can upload their buildroots." % taskID) elif allowed_methods != '*' and task_info['method'] not in allowed_methods: - raise koji.PreBuildError, 'Only %s tasks can upload their buildroots (Task %s is %s).' % \ - (', '.join(allowed_methods), task_info['id'], task_info['method']) + raise koji.PreBuildError("Only %s tasks can upload their buildroots (Task %s is %s)." % \ + (', '.join(allowed_methods), task_info['id'], task_info['method'])) elif task_info["owner"] != context.session.user_id and not context.session.assertPerm('admin'): - raise koji.ActionNotAllowed, "only owner of failed task or 'admin' can run this task" + raise koji.ActionNotAllowed("Only owner of failed task or 'admin' can run this task.") + elif not kojihub.get_host(task_info['host_id'])['enabled']: + raise koji.PreBuildError("Host is disabled.") args = koji.encode_args(taskID, full, **opts) taskopts = { From 5078a93b45193b05810c9c8814d46cc39b4f3510 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 14:33:37 +0000 Subject: [PATCH 14/17] check that correct builder is used --- diff --git a/Makefile b/Makefile index 9a0bd34..bc8767f 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,7 @@ git-clean: test: coverage erase - PYTHONPATH=hub/.:plugins/hub/. nosetests --with-coverage --cover-package . + PYTHONPATH=hub/.:plugins/hub/.:plugins/builder/. nosetests --with-coverage --cover-package . coverage html @echo Coverage report in htmlcov/index.html diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py index d1b618a..f5fec8a 100644 --- a/plugins/builder/save_failed_tree.py +++ b/plugins/builder/save_failed_tree.py @@ -2,6 +2,8 @@ import fnmatch import os import tarfile import ConfigParser + +import koji import koji.tasks as tasks from __main__ import BuildRoot @@ -38,7 +40,10 @@ class SaveFailedTreeTask(tasks.BaseTaskHandler): read_config() tar_path = os.path.join(self.workdir, 'broots-task-%s.tar.gz' % taskID) f = tarfile.open(tar_path, "w:gz") + host_id = self.session.host.getHost()['id'] for broot in self.session.listBuildroots(taskID=taskID): + if broot['host_id'] != host_id: + raise koji.GenericError("Task is run on wrong builder.") broot = BuildRoot(self.session, self.options, broot['id']) path = broot.rootdir() if full: diff --git a/tests/test_plugins/test_save_failed_tree_builder.py b/tests/test_plugins/test_save_failed_tree_builder.py new file mode 100644 index 0000000..76dedf9 --- /dev/null +++ b/tests/test_plugins/test_save_failed_tree_builder.py @@ -0,0 +1,113 @@ +import mock +import os +import sys +import unittest + +# alter pythonpath to not load hub plugin +sys.path = [os.path.join(os.path.dirname(__file__), '../../plugins/builder')] + sys.path +#raise(Exception(sys.path)) + +import koji +# inject builder data +from tests.test_builder.loadkojid import kojid +import __main__ +__main__.BuildRoot = kojid.BuildRoot + +from save_failed_tree import SaveFailedTreeTask + +class TestSaveFailedTree(unittest.TestCase): + def setUp(self): + self.session = mock.MagicMock() + self.session.host.getHost.return_value = {'id': 1} + options = mock.MagicMock() + options.workdir = '/tmp/nonexistentdirectory' + options.mockdir = '/tmp/mockdir' + options.name = 'name' + self.t = SaveFailedTreeTask(123, 'saveFailedTree', {}, self.session, options) + + @mock.patch('os.unlink') + @mock.patch('tarfile.open') + def testNonExistentTask(self, tarfile, os_unlink): + # empty tarball + tfile = mock.MagicMock(name='tfile') + tarfile.return_value = tfile + + self.t.handler(1) + + tarfile.assert_called_once_with( + '/tmp/nonexistentdirectory/tasks/123/123/broots-task-1.tar.gz', + 'w:gz' + ) + tfile.add.assert_not_called() + tfile.close.assert_called_once_with() + os_unlink.assert_called_once_with('/tmp/nonexistentdirectory/tasks/123/123/broots-task-1.tar.gz') + + @mock.patch('os.unlink') + @mock.patch('tarfile.open') + def testCorrect(self, tarfile, os_unlink): + def getBuildroot(bid): + tmp = { + 'tag_name': 'tag_name', + 'repo_id': 'repo_id', + } + if bid == 1: + tmp['task_id'] = 1000 + tmp['arch'] = 'x86_64' + tmp['tag_id'] = 5000 + elif bid == 2: + tmp['task_id'] = 1001 + tmp['arch'] = 'i386' + tmp['tag_id'] = 5001 + return tmp + + self.session.getBuildroot.side_effect = getBuildroot + tfile = mock.MagicMock(name='tfile') + tfile.add = mock.MagicMock() + tarfile.return_value = tfile + # simplified return values, only id should be used + self.session.listBuildroots.return_value = [{'id': 1, 'host_id': 1}, {'id': 2, 'host_id': 1}] + + self.t.handler(1) + + tarfile.assert_called_once_with( + '/tmp/nonexistentdirectory/tasks/123/123/broots-task-1.tar.gz', + 'w:gz' + ) + self.assertEqual(tfile.add.call_args_list[0][0][0], '/tmp/mockdir/tag_name-1-repo_id/root/builddir') + self.assertEqual(tfile.add.call_args_list[1][0][0], '/tmp/mockdir/tag_name-2-repo_id/root/builddir') + self.assertEqual(len(tfile.add.call_args_list), 2) + tfile.close.assert_called_once_with() + os_unlink.assert_called_once_with('/tmp/nonexistentdirectory/tasks/123/123/broots-task-1.tar.gz') + + @mock.patch('os.unlink') + @mock.patch('tarfile.open') + def testWrongBuilder(self, tarfile, os_unlink): + def getBuildroot(bid): + tmp = { + 'tag_name': 'tag_name', + 'repo_id': 'repo_id', + } + if bid == 1: + tmp['task_id'] = 1000 + tmp['arch'] = 'x86_64' + tmp['tag_id'] = 5000 + elif bid == 2: + tmp['task_id'] = 1001 + tmp['arch'] = 'i386' + tmp['tag_id'] = 5001 + return tmp + + self.session.getBuildroot.side_effect = getBuildroot + tfile = mock.MagicMock(name='tfile') + tarfile.return_value = tfile + # simplified return values, only id should be used + self.session.listBuildroots.return_value = [{'id': 1, 'host_id': 2}, {'id': 2, 'host_id': 2}] + + with self.assertRaises(koji.GenericError): + self.t.handler(1) + + def testFull(self): + pass + + def testFailUpload(self): + pass From d72a5b4f53d76ccc6a3e931eb02233a0586d1a17 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 28 2017 14:33:39 +0000 Subject: [PATCH 15/17] change config separator to whitespace --- diff --git a/docs/source/plugins.rst b/docs/source/plugins.rst index d3c29f0..e6e775d 100644 --- a/docs/source/plugins.rst +++ b/docs/source/plugins.rst @@ -21,12 +21,12 @@ Additional feature is that some paths from buildroot can be left out from tarball. Feature can be configured via `/etc/kojid/plugins/save_failed_tree.conf` file. Currently only field filters.paths is used and it consists of globs (standard python's fnmatch is -used) separated by ':'. +used) separated by whitespaces. .. code-block:: ini [filters] - paths = /etc/*.keytab:/tmp/secret_data + paths = /etc/*.keytab /tmp/secret_data .. warning:: For security reasons, currently all ``/tmp/krb5cc*`` and ``/etc/*.keytab`` diff --git a/plugins/builder/save_failed_tree.conf b/plugins/builder/save_failed_tree.conf index 84df846..66274c5 100644 --- a/plugins/builder/save_failed_tree.conf +++ b/plugins/builder/save_failed_tree.conf @@ -2,4 +2,4 @@ volume = DEFAULT [filters] -paths = */tmp/krb5cc:*/etc/*.keytab +paths = */tmp/krb5cc */etc/*.keytab diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py index f5fec8a..1a22db6 100644 --- a/plugins/builder/save_failed_tree.py +++ b/plugins/builder/save_failed_tree.py @@ -27,7 +27,7 @@ def read_config(): 'volume': None, } if cp.has_option('filters', 'paths'): - config['path_filters'] = cp.get('filters', 'paths').split(':') + config['path_filters'] = cp.get('filters', 'paths').split() if cp.has_option('general', 'volume'): config['volume'] = cp.get('general', 'volume').strip() From dda05bafe0d550799ba0a331b6f00610080b35c6 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Mar 30 2017 08:22:35 +0000 Subject: [PATCH 16/17] refactor --- diff --git a/cli/koji b/cli/koji index 5d782ad..5a234fd 100755 --- a/cli/koji +++ b/cli/koji @@ -7202,50 +7202,63 @@ def handle_runroot(options, session, args): sys.exit(1) return + def handle_save_failed_tree(options, session, args): "Create tarball with whole buildtree" - usage = _("usage: %prog save-failed-tree [options] taskID") + usage = _("usage: %prog save-failed-tree [options] ID") usage += _("\n(Specify the --help global option for a list of other help options)") parser = OptionParser(usage=usage) parser.add_option("-f", "--full", action="store_true", default=False, help=_("Download whole tree, if not specified, only builddir will be downloaded")) - parser.add_option("--quiet", action="store_true", - help=_("Do not print the task information"), default=options.quiet) + parser.add_option("-t", "--task", action="store_const", dest="mode", + const="task", default="task", + help=_("Treat ID as a task ID (the default)")) + parser.add_option("-r", "--buildroot", action="store_const", dest="mode", + const="buildroot", + help=_("Treat ID as a buildroot ID")) + parser.add_option("--quiet", action="store_true", default=options.quiet, + help=_("Do not print the task information")) parser.add_option("--nowait", action="store_true", help=_("Don't wait on build")) (opts, args) = parser.parse_args(args) if len(args) != 1: - parser.error(_("List exactly one taskID")) + parser.error(_("List exactly one task or buildroot ID")) try: - taskID = int(args[0]) + id_val = int(args[0]) except ValueError: - parser.error(_("Task ID must be an integer.")) + parser.error(_("ID must be an integer")) activate_session(session) + + if opts.mode == "buildroot": + br_id = id_val + else: + brs = [b['id'] for b in session.listBuildroots(taskID=id_val)] + if not brs: + print(_("No buildroots for task %s") % id_val) + return 1 + br_id = max(brs) + if len(brs) > 1: + print(_("Multiple buildroots for task. Choosing last one (%s)") % br_id) + try: - task_id = session.saveFailedTree(taskID, opts.full) + task_id = session.saveFailedTree(br_id, opts.full) except koji.GenericError as e: m = str(e) - if 'Only failed tasks can upload their buildroots.' in m: - print(_("Only failed tasks can upload their buildroots.")) - elif 'tasks can upload their buildroots (Task' in m: - print(_("Task of this type has disabled support for uploading" \ - " buildroot. (configurable on hub)")) - elif 'Invalid method' in m: - print(_("* The save_failed_tree plugin appears to not be installed" \ - " on the koji hub. Please contact the administrator.")) - else: - raise - return 1 - except koji.ActionNotAllowed: - print(_("Only task owner or admin can run this task.")) - return 1 + if 'Invalid method' in m: + print(_("* The save_failed_tree plugin appears to not be " + "installed on the koji hub. Please contact the " + "administrator.")) + return 1 + raise + if not opts.quiet: - print("Created task:", task_id) - print("Task info: %s/taskinfo?taskID=%s" % (options.weburl, task_id)) + print(_("Created task %s for buildroot %s") % (task_id, br_id)) + print("Task info: %s/taskinfo?taskID=%s" + % (options.weburl, task_id)) if opts.nowait: return diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py index 1a22db6..d9b325c 100644 --- a/plugins/builder/save_failed_tree.py +++ b/plugins/builder/save_failed_tree.py @@ -12,12 +12,14 @@ __all__ = ('SaveFailedTreeTask',) CONFIG_FILE = '/etc/kojid/plugins/save_failed_tree.conf' config = None + def omit_paths(tarinfo): if any([fnmatch.fnmatch(tarinfo.name, f) for f in config['path_filters']]): return None else: return tarinfo + def read_config(): global config cp = ConfigParser.SafeConfigParser() @@ -31,29 +33,37 @@ def read_config(): if cp.has_option('general', 'volume'): config['volume'] = cp.get('general', 'volume').strip() + class SaveFailedTreeTask(tasks.BaseTaskHandler): Methods = ['saveFailedTree'] _taskWeight = 3.0 - def handler(self, taskID, full=False): - self.logger.debug("Starting saving buildroots for task %d [full=%s]" % (taskID, full)) + def handler(self, buildrootID, full=False): + self.logger.debug("Saving buildroot %d [full=%s]", buildrootID, full) read_config() - tar_path = os.path.join(self.workdir, 'broots-task-%s.tar.gz' % taskID) - f = tarfile.open(tar_path, "w:gz") + + brinfo = self.session.getBuildroot(buildrootID) host_id = self.session.host.getHost()['id'] - for broot in self.session.listBuildroots(taskID=taskID): - if broot['host_id'] != host_id: - raise koji.GenericError("Task is run on wrong builder.") - broot = BuildRoot(self.session, self.options, broot['id']) - path = broot.rootdir() - if full: - self.logger.debug("Adding buildroot (full): %s" % path) - else: - path = os.path.join(path, 'builddir') - self.logger.debug("Adding buildroot: %s" % path) - f.add(path, filter=omit_paths) + if brinfo['host_id'] != host_id: + raise koji.GenericError("Task is run on wrong builder") + broot = BuildRoot(self.session, self.options, brinfo['id']) + path = broot.rootdir() + + if full: + self.logger.debug("Adding buildroot (full): %s" % path) + else: + path = os.path.join(path, 'builddir') + self.logger.debug("Adding buildroot: %s" % path) + if not os.path.exists(path): + raise koji.GenericError("Buildroot directory is missing: %s" % path) + + tar_path = os.path.join(self.workdir, 'broot-%s.tar.gz' % buildrootID) + self.logger.debug("Creating buildroot archive %s", tar_path) + f = tarfile.open(tar_path, "w:gz") + f.add(path, filter=omit_paths) f.close() - self.logger.debug("Uploading %s to hub." % tar_path) + + self.logger.debug("Uploading %s to hub", tar_path) self.uploadFile(tar_path, volume=config['volume']) os.unlink(tar_path) - self.logger.debug("Finished saving buildroots for task %d" % taskID) + self.logger.debug("Finished saving buildroot %s", buildrootID) diff --git a/plugins/hub/save_failed_tree.py b/plugins/hub/save_failed_tree.py index 46ee865..3a98128 100644 --- a/plugins/hub/save_failed_tree.py +++ b/plugins/hub/save_failed_tree.py @@ -15,7 +15,7 @@ allowed_methods = None @export -def saveFailedTree(taskID, full=False, **opts): +def saveFailedTree(buildrootID, full=False, **opts): """Create saveFailedTree task If arguments are invalid, error message is returned. Otherwise task id of @@ -23,7 +23,7 @@ def saveFailedTree(taskID, full=False, **opts): global config, allowed_methods # let it raise errors - taskID = int(taskID) + buildrootID = int(buildrootID) full = bool(full) # read configuration only once @@ -34,6 +34,8 @@ def saveFailedTree(taskID, full=False, **opts): if len(allowed_methods) == 1 and allowed_methods[0] == '*': allowed_methods = '*' + brinfo = kojihub.get_buildroot(buildrootID, strict=True) + taskID = brinfo['task_id'] task_info = kojihub.Task(taskID).getInfo() if task_info['state'] != koji.TASK_STATES['FAILED']: raise koji.PreBuildError("Task %s has not failed. Only failed tasks can upload their buildroots." % taskID) @@ -45,8 +47,8 @@ def saveFailedTree(taskID, full=False, **opts): elif not kojihub.get_host(task_info['host_id'])['enabled']: raise koji.PreBuildError("Host is disabled.") - args = koji.encode_args(taskID, full, **opts) + args = koji.encode_args(buildrootID, full, **opts) taskopts = { - 'assign': task_info['host_id'], + 'assign': brinfo['host_id'], } return kojihub.make_task('saveFailedTree', args, **taskopts) From 74db04d5a82e77ef9e78e6643d8de6d5067faaa1 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Mar 30 2017 09:24:41 +0000 Subject: [PATCH 17/17] updated tests --- diff --git a/plugins/builder/save_failed_tree.py b/plugins/builder/save_failed_tree.py index d9b325c..138572f 100644 --- a/plugins/builder/save_failed_tree.py +++ b/plugins/builder/save_failed_tree.py @@ -43,6 +43,8 @@ class SaveFailedTreeTask(tasks.BaseTaskHandler): read_config() brinfo = self.session.getBuildroot(buildrootID) + if brinfo is None: + raise koji.GenericError("Nonexistent buildroot: %s" % buildrootID) host_id = self.session.host.getHost()['id'] if brinfo['host_id'] != host_id: raise koji.GenericError("Task is run on wrong builder") diff --git a/tests/test_cli/test_save_failed_tree.py b/tests/test_cli/test_save_failed_tree.py index b2ad7df..b8acf12 100644 --- a/tests/test_cli/test_save_failed_tree.py +++ b/tests/test_cli/test_save_failed_tree.py @@ -27,12 +27,14 @@ class TestSaveFailedTree(unittest.TestCase): def test_handle_save_failed_tree_simple(self, activate_session_mock): # koji save-failed-tree 123456 task_id = 123456 + broot_id = 321 arguments = [task_id] options = mock.MagicMock() options.full = False options.nowait = True self.parser.parse_args.return_value = [options, arguments] self.session.getAPIVersion.return_value = koji.API_VERSION + self.session.listBuildroots.return_value = [{'id': 321}] # Mock out the xmlrpc server self.session.saveFailedTree.return_value = 123 @@ -42,18 +44,46 @@ class TestSaveFailedTree(unittest.TestCase): # Finally, assert that things were called as we expected. activate_session_mock.assert_called_once_with(self.session) - self.session.saveFailedTree.assert_called_once_with(task_id, options.full) + self.session.listBuildroots.assert_called_once_with(taskID=task_id) + self.session.saveFailedTree.assert_called_once_with(broot_id, options.full) + + @mock.patch('koji_cli.activate_session') + def test_handle_save_failed_tree_buildroots(self, activate_session_mock): + # koji save-failed-tree --buildroot 123456 + broot_id = 321 + arguments = [broot_id] + options = mock.MagicMock() + options.full = False + options.nowait = True + options.mode = "buildroot" + self.parser.parse_args.return_value = [options, arguments] + self.session.getAPIVersion.return_value = koji.API_VERSION + self.session.listBuildroots.return_value = [{'id': 321}] + + # Mock out the xmlrpc server + self.session.saveFailedTree.return_value = 123 + + # Run it and check immediate output + cli.handle_save_failed_tree(self.options, self.session, self.args) + + # Finally, assert that things were called as we expected. + activate_session_mock.assert_called_once_with(self.session) + self.session.listBuildroots.assert_not_called() + self.session.saveFailedTree.assert_called_once_with(broot_id, options.full) + @mock.patch('koji_cli.activate_session') def test_handle_save_failed_tree_full(self, activate_session_mock): # koji save-failed-tree 123456 --full task_id = 123456 + broot_id = 321 arguments = [task_id] options = mock.MagicMock() options.full = True options.nowait = True self.parser.parse_args.return_value = [options, arguments] self.session.getAPIVersion.return_value = koji.API_VERSION + self.session.listBuildroots.return_value = [{'id': 321}] # Mock out the xmlrpc server self.session.saveFailedTree.return_value = 123 @@ -63,13 +93,15 @@ class TestSaveFailedTree(unittest.TestCase): # Finally, assert that things were called as we expected. activate_session_mock.assert_called_once_with(self.session) - self.session.saveFailedTree.assert_called_once_with(task_id, options.full) + self.session.listBuildroots.assert_called_once_with(taskID=task_id) + self.session.saveFailedTree.assert_called_once_with(broot_id, options.full) @mock.patch('koji_cli.activate_session') @mock.patch('koji_cli.watch_tasks') def test_handle_save_failed_tree_wait(self, watch_tasks_mock, activate_session_mock): # koji save-failed-tree 123456 --full task_id = 123456 + broot_id = 321 arguments = [task_id] options = mock.MagicMock() options.full = True @@ -77,6 +109,7 @@ class TestSaveFailedTree(unittest.TestCase): options.quiet = False self.parser.parse_args.return_value = [options, arguments] self.session.getAPIVersion.return_value = koji.API_VERSION + self.session.listBuildroots.return_value = [{'id': 321}] # Mock out the xmlrpc server spawned_id = 123 @@ -86,7 +119,8 @@ class TestSaveFailedTree(unittest.TestCase): cli.handle_save_failed_tree(self.options, self.session, self.args) # Finally, assert that things were called as we expected. - self.session.saveFailedTree.assert_called_once_with(task_id, options.full) + self.session.listBuildroots.assert_called_once_with(taskID=task_id) + self.session.saveFailedTree.assert_called_once_with(broot_id, options.full) activate_session_mock.assert_called_once_with(self.session) self.session.logout.assert_called_once_with() watch_tasks_mock.assert_called_once_with(self.session, [spawned_id], @@ -102,6 +136,7 @@ class TestSaveFailedTree(unittest.TestCase): self.parser.parse_args.return_value = [options, arguments] self.parser.error.side_effect = Exception() self.session.getAPIVersion.return_value = koji.API_VERSION + self.session.listBuildroots.return_value = [{'id': 321}] self.assertRaises(Exception, cli.handle_save_failed_tree, self.options, self.session, self.args) @@ -120,18 +155,9 @@ class TestSaveFailedTree(unittest.TestCase): actual = stdout.getvalue() self.assertTrue('The save_failed_tree plugin appears to not be installed' in actual) - # Task which is not FAILED - stdout.seek(0) - stdout.truncate() - self.session.saveFailedTree.side_effect = koji.PreBuildError('Only failed tasks can upload their buildroots.') - cli.handle_save_failed_tree(self.options, self.session, self.args) - actual = stdout.getvalue() - self.assertTrue('Only failed tasks can upload their buildroots.' in actual) - - # Disabled/unsupported task - stdout.seek(0) - stdout.truncate() - self.session.saveFailedTree.side_effect = koji.PreBuildError('tasks can upload their buildroots (Task') - cli.handle_save_failed_tree(self.options, self.session, self.args) - actual = stdout.getvalue() - self.assertTrue('Task of this type has disabled support for uploading' in actual) + # Task which is not FAILED, disabled in config, wrong owner + self.session.saveFailedTree.side_effect = koji.PreBuildError('placeholder') + with self.assertRaises(koji.PreBuildError) as cm: + cli.handle_save_failed_tree(self.options, self.session, self.args) + e = cm.exception + self.assertEqual(e, self.session.saveFailedTree.side_effect) diff --git a/tests/test_plugins/test_save_failed_tree_builder.py b/tests/test_plugins/test_save_failed_tree_builder.py index 76dedf9..56c92a5 100644 --- a/tests/test_plugins/test_save_failed_tree_builder.py +++ b/tests/test_plugins/test_save_failed_tree_builder.py @@ -27,34 +27,37 @@ class TestSaveFailedTree(unittest.TestCase): @mock.patch('os.unlink') @mock.patch('tarfile.open') - def testNonExistentTask(self, tarfile, os_unlink): - # empty tarball + def testNonExistentBuildroot(self, tarfile, os_unlink): tfile = mock.MagicMock(name='tfile') tarfile.return_value = tfile + self.session.getBuildroot.return_value = None - self.t.handler(1) + with self.assertRaises(koji.GenericError) as cm: + self.t.handler(1) + self.assertTrue('Nonexistent buildroot' in str(cm.exception)) - tarfile.assert_called_once_with( - '/tmp/nonexistentdirectory/tasks/123/123/broots-task-1.tar.gz', - 'w:gz' - ) + tarfile.assert_not_called() tfile.add.assert_not_called() - tfile.close.assert_called_once_with() - os_unlink.assert_called_once_with('/tmp/nonexistentdirectory/tasks/123/123/broots-task-1.tar.gz') + tfile.close.assert_not_called() + os_unlink.assert_not_called() + @mock.patch('os.path.exists') @mock.patch('os.unlink') @mock.patch('tarfile.open') - def testCorrect(self, tarfile, os_unlink): + def testCorrect(self, tarfile, os_unlink, os_exists): def getBuildroot(bid): tmp = { 'tag_name': 'tag_name', 'repo_id': 'repo_id', + 'host_id': 1, } if bid == 1: + tmp['id'] = 1 tmp['task_id'] = 1000 tmp['arch'] = 'x86_64' tmp['tag_id'] = 5000 elif bid == 2: + tmp['id'] = 2 tmp['task_id'] = 1001 tmp['arch'] = 'i386' tmp['tag_id'] = 5001 @@ -64,20 +67,19 @@ class TestSaveFailedTree(unittest.TestCase): tfile = mock.MagicMock(name='tfile') tfile.add = mock.MagicMock() tarfile.return_value = tfile - # simplified return values, only id should be used - self.session.listBuildroots.return_value = [{'id': 1, 'host_id': 1}, {'id': 2, 'host_id': 1}] + os_exists.return_value = True self.t.handler(1) tarfile.assert_called_once_with( - '/tmp/nonexistentdirectory/tasks/123/123/broots-task-1.tar.gz', + '/tmp/nonexistentdirectory/tasks/123/123/broot-1.tar.gz', 'w:gz' ) + + tfile.add.assert_called_once() self.assertEqual(tfile.add.call_args_list[0][0][0], '/tmp/mockdir/tag_name-1-repo_id/root/builddir') - self.assertEqual(tfile.add.call_args_list[1][0][0], '/tmp/mockdir/tag_name-2-repo_id/root/builddir') - self.assertEqual(len(tfile.add.call_args_list), 2) tfile.close.assert_called_once_with() - os_unlink.assert_called_once_with('/tmp/nonexistentdirectory/tasks/123/123/broots-task-1.tar.gz') + os_unlink.assert_called_once_with('/tmp/nonexistentdirectory/tasks/123/123/broot-1.tar.gz') @mock.patch('os.unlink') @mock.patch('tarfile.open') @@ -86,12 +88,15 @@ class TestSaveFailedTree(unittest.TestCase): tmp = { 'tag_name': 'tag_name', 'repo_id': 'repo_id', + 'host_id': 2000, } if bid == 1: + tmp['id'] = 1 tmp['task_id'] = 1000 tmp['arch'] = 'x86_64' tmp['tag_id'] = 5000 elif bid == 2: + tmp['id'] = 2 tmp['task_id'] = 1001 tmp['arch'] = 'i386' tmp['tag_id'] = 5001 @@ -100,8 +105,6 @@ class TestSaveFailedTree(unittest.TestCase): self.session.getBuildroot.side_effect = getBuildroot tfile = mock.MagicMock(name='tfile') tarfile.return_value = tfile - # simplified return values, only id should be used - self.session.listBuildroots.return_value = [{'id': 1, 'host_id': 2}, {'id': 2, 'host_id': 2}] with self.assertRaises(koji.GenericError): self.t.handler(1)