From 399f56bd30b018897c95e9ddffa1840e187fc6d2 Mon Sep 17 00:00:00 2001 From: Lukas Brabec Date: Jul 23 2018 12:01:42 +0000 Subject: [PATCH 1/6] code cleanup: sti support removed --- diff --git a/data/ansible/library/__init__.py b/data/ansible/library/__init__.py deleted file mode 100644 index e69de29..0000000 --- a/data/ansible/library/__init__.py +++ /dev/null diff --git a/data/ansible/library/acquire_subject.py b/data/ansible/library/acquire_subject.py deleted file mode 100644 index 373122b..0000000 --- a/data/ansible/library/acquire_subject.py +++ /dev/null @@ -1,60 +0,0 @@ -#!/usr/bin/python -# TODO: Make coding more python3-ish -from __future__ import (absolute_import, division) -__metaclass__ = type - -from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.koji import KojiDirective - -try: - import libtaskotron.exceptions as exc - from libtaskotron import check -except ImportError: - libtaskotron_found = False -else: - libtaskotron_found = True - - -def error_handler(mod): - raise exc.TaskotronError('Unsupported subject type') - -def rpm_handler(mod): - mod.params['action'] = 'download' - mod.params['koji_build'] = mod.params['taskotron_item'] - kojidirective = KojiDirective() - data = kojidirective.process(mod) - - return ' '.join(data['downloaded_rpms']) - -def infer_subject_handler(mod): - #FIXME move subject strings to apropriate collection - if mod.params['taskotron_item_type'] == check.ReportType.KOJI_BUILD: - return rpm_handler - - return error_handler - -def main(): - mod = AnsibleModule( - argument_spec=dict( - arch=dict(required=True), - taskotron_item=dict(required=True), - taskotron_item_type=dict(required=True), - target_dir=dict(required=False, default='.') - ) - ) - - # TODO: check args for completeness - if not libtaskotron_found: - mod.fail_json(msg="The libtaskotron python module is required") - - subject_handler = infer_subject_handler(mod) - try: - subjects = subject_handler(mod) - except exc.TaskotronError as e: - mod.fail_json(msg=e.msg) - - mod.exit_json(msg="Successfuly acquired subjects", changed=True, subjects=subjects) - - -if __name__ == '__main__': - main() diff --git a/data/ansible/library/taskotron_koji.py b/data/ansible/library/taskotron_koji.py deleted file mode 100644 index 79bc699..0000000 --- a/data/ansible/library/taskotron_koji.py +++ /dev/null @@ -1,48 +0,0 @@ -#!/usr/bin/python -# TODO: Make coding more python3-ish -from __future__ import (absolute_import, division) -__metaclass__ = type - -from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.koji import KojiDirective - -try: - import libtaskotron.exceptions as exc -except ImportError: - libtaskotron_found = False -else: - libtaskotron_found = True - - -def main(): - mod = AnsibleModule( - argument_spec=dict( - action=dict(required=True), - arch=dict(required=False, default=['noarch']), - workdir=dict(required=False, default="/tmp/firstmod"), - arch_exclude=dict(required=False), - build_log=dict(required=False, default=False, type="bool"), - debuginfo=dict(required=False, default=False, type="bool"), - koji_build=dict(required=False), - koji_tag=dict(required=False), - src=dict(required=False, default=False, type="bool"), - target_dir=dict(required=False, default='.') - ) - ) - - # TODO: check args for completeness - if not libtaskotron_found: - mod.fail_json(msg="The libtaskotron python module is required") - - try: - kojidirective = KojiDirective() - data = kojidirective.process(mod) - except exc.TaskotronError as e: - mod.fail_json(msg=e) - - subjects = ' '.join(data['downloaded_rpms']) - - mod.exit_json(msg="worky!", changed=True, subjects=subjects) - -if __name__ == '__main__': - main() diff --git a/data/ansible/module_utils/__init__.py b/data/ansible/module_utils/__init__.py deleted file mode 100644 index e69de29..0000000 --- a/data/ansible/module_utils/__init__.py +++ /dev/null diff --git a/data/ansible/module_utils/koji.py b/data/ansible/module_utils/koji.py deleted file mode 100644 index da0b207..0000000 --- a/data/ansible/module_utils/koji.py +++ /dev/null @@ -1,114 +0,0 @@ -from __future__ import print_function -import ast -from libtaskotron.ext.fedora.koji_utils import KojiClient -from libtaskotron.ext.fedora import rpm_utils -import libtaskotron.exceptions as exc - -class KojiDirective(object): - # FIXME: don't duplicate code with koji_directive.py - - def __init__(self, koji_session=None): - super(KojiDirective, self).__init__() - if koji_session is None: - self.koji = KojiClient() - else: - self.koji = koji_session - - def process(self, mod): - # process params - valid_actions = ['download', 'download_tag', 'download_latest_stable'] - action = mod.params['action'] - if action not in valid_actions: - raise exc.TaskotronDirectiveError('%s is not a valid action for koji ' - 'directive' % action) - - if 'arch' not in mod.params: - detected_args = ', '.join(mod.params.keys()) - raise exc.TaskotronDirectiveError( - "The koji directive requires 'arch' as an argument. Detected " - "arguments: %s" % detected_args) - - # this is supposedly safe enough to use on raw input but should be double checked - # http://stackoverflow.com/questions/1894269/convert-string-representation-of-list-to-list-in-python - arches = ast.literal_eval(mod.params['arch']) - if not isinstance(arches, list): - raise exc.TaskotronError("arches must be a list") - - if arches and ('all' not in arches) and ('noarch' not in arches): - arches.append('noarch') - - arch_exclude_string = mod.params.get('arch_exclude', None) - if arch_exclude_string is None: - arch_exclude = [] - else: - arch_exclude = ast.literal_eval(mod.params['arch_exclude']) - - debuginfo = mod.params.get('debuginfo', False) - src = mod.params.get('src', False) - build_log = mod.params.get('build_log', False) - - if not isinstance(arch_exclude, list): - print("arch_exclude: {}".format(type(arch_exclude))) - raise Exception("arch_exclude must be a list") - # download files - output_data = {} - - if action == 'download': - if 'koji_build' not in mod.params: - detected_args = ', '.join(mod.params.keys()) - raise exc.TaskotronDirectiveError( - "The koji directive requires 'koji_build' for the 'download' " - "action. Detected arguments: %s" % detected_args) - - nvr = rpm_utils.rpmformat(mod.params['koji_build'], 'nvr') - output_data['downloaded_rpms'] = self.koji.get_nvr_rpms( - nvr, mod.params['target_dir'], arches=arches, arch_exclude=arch_exclude, - debuginfo=debuginfo, src=src) - - elif action == 'download_tag': - if 'koji_tag' not in mod.params: - detected_args = ', '.join(mod.params.keys()) - raise exc.TaskotronDirectiveError( - "The koji directive requires 'koji_tag' for the 'download_tag' " - "action. Detected arguments: %s" % detected_args) - - koji_tag = mod.params['koji_tag'] - - output_data['downloaded_rpms'] = self.koji.get_tagged_rpms( - koji_tag, mod.params['target_dir'], arches=arches, arch_exclude=arch_exclude, - debuginfo=debuginfo, src=src) - - elif action == 'download_latest_stable': - if 'koji_build' not in mod.params: - detected_args = ', '.join(mod.params.keys()) - raise exc.TaskotronDirectiveError( - "The koji directive requires 'koji_build' for the 'download_latest_stable' " - "action. Detected arguments: %s" % detected_args) - - name = rpm_utils.rpmformat(mod.params['koji_build'], 'n') - disttag = rpm_utils.get_dist_tag(mod.params['koji_build']) - # we need to do 'fc22' -> 'f22' conversion - tag = disttag.replace('c', '') - - # first we need to check updates tag and if that fails, the latest - # stable nvr is in the base repo - tags = ['%s-updates' % tag, tag] - nvr = self.koji.latest_by_tag(tags, name) - - output_data['downloaded_rpms'] = self.koji.get_nvr_rpms( - nvr, mod.params['target_dir'], arch_exclude=arch_exclude, - arches=arches, debuginfo=debuginfo, src=src) - - # download build.log if requested - if build_log: - if action in ('download', 'download_latest_stable'): - ret_log = self.koji.get_build_log( - nvr, mod.params['target_dir'], arches=arches, arch_exclude=arch_exclude) - output_data['downloaded_logs'] = ret_log['ok'] - output_data['log_errors'] = ret_log['error'] - else: - #log.warn("Downloading build logs is not supported for action '%s', ignoring.", - # action) - print("Downloading build logs is not supported for action '%s', ignoring." % action) - - return output_data \ No newline at end of file diff --git a/data/ansible/runner.yml b/data/ansible/runner.yml index 16a3713..88bf049 100644 --- a/data/ansible/runner.yml +++ b/data/ansible/runner.yml @@ -103,10 +103,22 @@ when: taskotron_keepalive_minutes|int > 0 delegate_to: localhost - - name: Include either generic or STI execution tasks - import_tasks: "{{ exec_tasks }}" - static: yes - # variable 'task' is registered here + - name: Run {{ test_playbook }} + become: '{{ become_root }}' + become_user: root + shell: > + ansible-playbook "{{ client_taskdir }}/{{ test_playbook }}" + --inventory=localhost, + --connection=local + -e '@{{ artifacts }}/taskotron/{{ varsfile }}' + &> "{{ artifacts }}/ansible.log" + environment: + TEST_ARTIFACTS: "{{ artifacts }}" + # Make task output "pretty printed" (structured) + # https://serverfault.com/a/846232 + ANSIBLE_STDOUT_CALLBACK: 'debug' + ignore_errors: yes + register: task - name: Delete secrets file: diff --git a/data/ansible/tasks_generic.yml b/data/ansible/tasks_generic.yml deleted file mode 100644 index 9099dbc..0000000 --- a/data/ansible/tasks_generic.yml +++ /dev/null @@ -1,19 +0,0 @@ -# Tasks to be run with a generic Taskotron task. This is executed on the -# minion. The available variables are described in runner.yml. - -- name: Run {{ test_playbook }} - become: '{{ become_root }}' - become_user: root - shell: > - ansible-playbook "{{ client_taskdir }}/{{ test_playbook }}" - --inventory=localhost, - --connection=local - -e '@{{ artifacts }}/taskotron/{{ varsfile }}' - &> "{{ artifacts }}/ansible.log" - environment: - TEST_ARTIFACTS: "{{ artifacts }}" - # Make task output "pretty printed" (structured) - # https://serverfault.com/a/846232 - ANSIBLE_STDOUT_CALLBACK: 'debug' - ignore_errors: yes - register: task diff --git a/data/ansible/tasks_sti.yml b/data/ansible/tasks_sti.yml deleted file mode 100644 index a044d98..0000000 --- a/data/ansible/tasks_sti.yml +++ /dev/null @@ -1,64 +0,0 @@ -# Tasks to be run with a STI task. This is executed on the minion. The -# available variables are described in runner.yml. - -- name: Print maintenance warning - debug: - msg: > - WARNING! STANDARD TEST INTERFACE TASKS EXECUTION IS NOT MAINTAINED IN THE - MOMENT! - - - Perhaps you wanted to run this as a generic task? - -- name: Acquire subject - acquire_subject: - arch: - - '{{ taskotron_arch }}' - target_dir: "{{ client_taskdir }}" - taskotron_item: "{{ taskotron_item }}" - taskotron_item_type: "{{ taskotron_item_type }}" - register: acquired_subject - -- debug: var=acquired_subject - -- name: Run {{ test_playbook }} - become: '{{ become_root }}' - become_user: root - # FIXME add context tags - shell: > - ansible-playbook "{{ client_taskdir }}/{{ test_playbook }}" - --inventory="{{ sti_inventory }}" - --connection=local - -e artifacts="{{ artifacts }}" - -e subjects="{{ acquired_subject['subjects'] }}" - &> "{{ artifacts }}/ansible.log" - environment: - TEST_SUBJECTS: "{{ acquired_subject['subjects'] }}" - TEST_ARTIFACTS: "{{ artifacts }}" - # Make task output "pretty printed" (structured) - # https://serverfault.com/a/846232 - ANSIBLE_STDOUT_CALLBACK: 'debug' - ignore_errors: yes - register: task - -- name: Save exit code - copy: - content: "{{ task.rc }}" - dest: "{{ artifacts }}/taskotron/test.rc" - -- name: Set outcome based on exit code - set_fact: outcome={{ (task.rc == 0) | ternary('PASSED', 'FAILED') }} - -- name: Generate ResultsDB result file - # FIXME: type of result - # FIXME: checkname - shell: > - taskotron_result - -f "{{ artifacts }}/taskotron/results.yml" - -i "{{ taskotron_item }}" - -o "{{ outcome }}" - -t koji_build - -a "{{ artifacts }}/test.log" - -c "{{ test_playbook }}" #FIXME something like "pkg.firefox.tests.yml" - args: - creates: "{{ artifacts }}/taskotron/results.yml" diff --git a/libtaskotron/executor.py b/libtaskotron/executor.py index 4be3d8a..d4b721f 100644 --- a/libtaskotron/executor.py +++ b/libtaskotron/executor.py @@ -262,15 +262,10 @@ class Executor(object): a list of repos (strings) to install on the minion minion_repos_ignore_errors whether to ignore errors when adding minion repos - sti_inventory - path to inventory file to be used with STI tests (local inside - client_taskdir or global) taskdir path to directory with test suite (on overlord) taskotron_arch architecture of taskotron_item to be tested - taskotron_generic_task - whether to run a generic or STI task taskotron_item_type item under test taskotron_item @@ -304,7 +299,6 @@ class Executor(object): # default values vars_['become_root'] = True - vars_['taskotron_generic_task'] = False vars_['heartbeat_interval'] = 120 vars_['taskotron_keepalive_minutes'] = 0 vars_['taskotron_match_host_arch'] = False @@ -336,17 +330,6 @@ class Executor(object): cfg.supported_arches for binarch in arch_utils.Arches.binary[arch]] vars_['test_playbook'] = test_playbook - if vars_['taskotron_generic_task']: - vars_['exec_tasks'] = 'tasks_generic.yml' - else: - vars_['exec_tasks'] = 'tasks_sti.yml' - - if os.path.isfile(os.path.join(vars_['taskdir'], 'inventory')): - vars_['sti_inventory'] = os.path.join(vars_['client_taskdir'], - 'inventory') - else: - vars_['sti_inventory'] = '/usr/share/ansible/inventory' - return vars_ def _dump_playbook_vars(self, playbook_vars): @@ -532,8 +515,7 @@ class Executor(object): self._run_playbook(test_playbook, ipaddr, playbook_vars) # report results - if playbook_vars['taskotron_generic_task']: - self._report_results(test_playbook) + self._report_results(test_playbook) except exc.TaskotronInterruptError as e: log.error('Caught system interrupt during execution of ' 'playbook %s: %s. Not executing any other playbooks.', diff --git a/testing/test_executor.py b/testing/test_executor.py index 152c373..9b7ff02 100644 --- a/testing/test_executor.py +++ b/testing/test_executor.py @@ -476,30 +476,6 @@ class TestExecutor(): ] assert 'tests.yml' in playbooks or 'tests_copy.yml' in playbooks - def test_execute_sti_no_report(self, monkeypatch): - '''Shouldn't try to report results for STI tasks''' - self.playbook.write(PLAYBOOK_STI) - mock_check_syntax = mock.Mock() - monkeypatch.setattr(executor.Executor, '_check_playbook_syntax', - mock_check_syntax) - mock_spawn_vm = mock.Mock() - monkeypatch.setattr(executor.Executor, '_spawn_vm', mock_spawn_vm) - mock_run_playbook = mock.Mock(return_value=(None, False)) - monkeypatch.setattr(executor.Executor, '_run_playbook', - mock_run_playbook) - mock_report_results = mock.Mock() - monkeypatch.setattr(executor.Executor, '_report_results', - mock_report_results) - - success = self.executor.execute() - - assert success == True - mock_check_syntax.assert_called_once() - mock_spawn_vm.assert_not_called() - mock_run_playbook.assert_called_once() - assert mock_run_playbook.call_args[0][1] == '127.0.0.1' - mock_report_results.assert_not_called() - def test_execute_interrupted(self, monkeypatch): '''Should halt execution and return when interrupted''' mock_check_syntax = mock.Mock() From 9cd6c62724c1b6b29967b8fdf9ee8ea29a12f655 Mon Sep 17 00:00:00 2001 From: Lukas Brabec Date: Jul 23 2018 12:01:42 +0000 Subject: [PATCH 2/6] code cleanup: multiple tests*.yml support removed --- diff --git a/libtaskotron/executor.py b/libtaskotron/executor.py index d4b721f..5345e41 100644 --- a/libtaskotron/executor.py +++ b/libtaskotron/executor.py @@ -486,62 +486,59 @@ class Executor(object): results or the execution was interrupted (e.g. a system signal). :rtype: bool ''' - test_playbooks = fnmatch.filter(os.listdir(self.arg_data['taskdir']), - 'tests*.yml') - if not test_playbooks: - raise exc.TaskotronError('No tests*.yml found in dir %s' % + test_playbook = 'tests.yml' + + if not os.path.exists(os.path.join(self.arg_data['taskdir'], test_playbook)): + raise exc.TaskotronError('No tests.yml found in dir %s' % self.arg_data['taskdir']) failed = [] - for test_playbook in test_playbooks: - playbook_vars = None + playbook_vars = None + try: + # syntax check + self._check_playbook_syntax(os.path.join( + self.arg_data['taskdir'], test_playbook)) + + # compute variables + playbook_vars = self._create_playbook_vars(test_playbook) + + # spawn VM if needed + ipaddr = self.ipaddr + if ipaddr is None: + ipaddr = self._spawn_vm(self.arg_data['uuid'], + playbook_vars) + + # execute + log.info('Running playbook %s on machine: %s', test_playbook, + ipaddr) + self._run_playbook(test_playbook, ipaddr, playbook_vars) + + # report results + self._report_results(test_playbook) + except exc.TaskotronInterruptError as e: + log.error('Caught system interrupt during execution of ' + 'playbook %s: %s.', + test_playbook, e) + failed.append(test_playbook) + except exc.TaskotronError as e: + log.error('Error during execution of playbook %s: %s', + test_playbook, e) + failed.append(test_playbook) + finally: try: - # syntax check - self._check_playbook_syntax(os.path.join( - self.arg_data['taskdir'], test_playbook)) - - # compute variables - playbook_vars = self._create_playbook_vars(test_playbook) - - # spawn VM if needed - ipaddr = self.ipaddr - if ipaddr is None: - ipaddr = self._spawn_vm(self.arg_data['uuid'], - playbook_vars) - - # execute - log.info('Running playbook %s on machine: %s', test_playbook, - ipaddr) - self._run_playbook(test_playbook, ipaddr, playbook_vars) - - # report results - self._report_results(test_playbook) - except exc.TaskotronInterruptError as e: - log.error('Caught system interrupt during execution of ' - 'playbook %s: %s. Not executing any other playbooks.', - test_playbook, e) - failed.append(test_playbook) - break - except exc.TaskotronError as e: - log.error('Error during execution of playbook %s: %s', - test_playbook, e) - failed.append(test_playbook) - finally: - try: - if playbook_vars and config.get_config().profile != config.ProfileName.TESTING: - os.remove(playbook_vars['taskotron_secrets_file']) - except OSError as e: - log.warning("Could not delete the secrets file at %r. %s", - playbook_vars['taskotron_secrets_file'], e) - if self.task_vm is not None: - if self.arg_data['no_destroy']: - log.info('Not destroying disposable client as ' - 'requested, access it at: %s . Skipping any ' - 'other playbooks.', ipaddr) - break - else: - self.task_vm.teardown() - log.info('Playbook execution finished: %s', test_playbook) + if playbook_vars and config.get_config().profile != config.ProfileName.TESTING: + os.remove(playbook_vars['taskotron_secrets_file']) + except OSError as e: + log.warning("Could not delete the secrets file at %r. %s", + playbook_vars['taskotron_secrets_file'], e) + if self.task_vm is not None: + if self.arg_data['no_destroy']: + log.info('Not destroying disposable client as ' + 'requested, access it at: %s .', ipaddr) + else: + self.task_vm.teardown() + + log.info('Playbook execution finished: %s', test_playbook) if failed: log.error('Some playbooks failed during execution: %s', diff --git a/testing/test_executor.py b/testing/test_executor.py index 9b7ff02..50bfc05 100644 --- a/testing/test_executor.py +++ b/testing/test_executor.py @@ -393,38 +393,6 @@ class TestExecutor(): mock_run_playbook.assert_not_called() mock_report_results.assert_not_called() - def test_execute_more_playbooks(self, monkeypatch): - '''Should execute all found playbooks''' - mock_check_syntax = mock.Mock() - monkeypatch.setattr(executor.Executor, '_check_playbook_syntax', - mock_check_syntax) - mock_run_playbook = mock.Mock(return_value=(None, True)) - monkeypatch.setattr(executor.Executor, '_run_playbook', - mock_run_playbook) - mock_report_results = mock.Mock() - monkeypatch.setattr(executor.Executor, '_report_results', - mock_report_results) - self.playbook.copy(self.taskdir.join('tests_copy.yml')) - - success = self.executor.execute() - - assert success == True - assert mock_check_syntax.call_count == 2 - assert mock_run_playbook.call_count == 2 - playbooks = [ - mock_run_playbook.call_args_list[0][0][0], - mock_run_playbook.call_args_list[1][0][0] - ] - assert 'tests.yml' in playbooks - assert 'tests_copy.yml' in playbooks - assert mock_report_results.call_count == 2 - playbooks = [ - mock_report_results.call_args_list[0][0][0], - mock_report_results.call_args_list[1][0][0] - ] - assert 'tests.yml' in playbooks - assert 'tests_copy.yml' in playbooks - def test_execute_error(self, monkeypatch): '''Should raise on playbook errors''' mock_check_syntax = mock.Mock() @@ -444,38 +412,6 @@ class TestExecutor(): mock_run_playbook.assert_called_once() mock_report_results.assert_not_called() - def test_execute_more_playbooks_error(self, monkeypatch): - '''Should execute all found playbooks even if some has error''' - mock_check_syntax = mock.Mock() - monkeypatch.setattr(executor.Executor, '_check_playbook_syntax', - mock_check_syntax) - mock_run_playbook = mock.Mock(side_effect=[ - exc.TaskotronError, - (None, True)]) - monkeypatch.setattr(executor.Executor, '_run_playbook', - mock_run_playbook) - mock_report_results = mock.Mock() - monkeypatch.setattr(executor.Executor, '_report_results', - mock_report_results) - self.playbook.copy(self.taskdir.join('tests_copy.yml')) - - success = self.executor.execute() - - assert success == False - assert mock_check_syntax.call_count == 2 - assert mock_run_playbook.call_count == 2 - playbooks = [ - mock_run_playbook.call_args_list[0][0][0], - mock_run_playbook.call_args_list[1][0][0] - ] - assert 'tests.yml' in playbooks - assert 'tests_copy.yml' in playbooks - assert mock_report_results.call_count == 1 - playbooks = [ - mock_report_results.call_args_list[0][0][0] - ] - assert 'tests.yml' in playbooks or 'tests_copy.yml' in playbooks - def test_execute_interrupted(self, monkeypatch): '''Should halt execution and return when interrupted''' mock_check_syntax = mock.Mock() From d9b2e3821be2eff9f20aca80e25892874b73bc6d Mon Sep 17 00:00:00 2001 From: Kamil Páral Date: Jul 23 2018 13:21:39 +0000 Subject: [PATCH 3/6] Revert "code cleanup: multiple tests*.yml support removed" This reverts commit 9cd6c62724c1b6b29967b8fdf9ee8ea29a12f655. Let's not remove multiple tests.yml support just yet, just remove distgit support. --- diff --git a/libtaskotron/executor.py b/libtaskotron/executor.py index 5345e41..d4b721f 100644 --- a/libtaskotron/executor.py +++ b/libtaskotron/executor.py @@ -486,59 +486,62 @@ class Executor(object): results or the execution was interrupted (e.g. a system signal). :rtype: bool ''' - test_playbook = 'tests.yml' - - if not os.path.exists(os.path.join(self.arg_data['taskdir'], test_playbook)): - raise exc.TaskotronError('No tests.yml found in dir %s' % + test_playbooks = fnmatch.filter(os.listdir(self.arg_data['taskdir']), + 'tests*.yml') + if not test_playbooks: + raise exc.TaskotronError('No tests*.yml found in dir %s' % self.arg_data['taskdir']) failed = [] - playbook_vars = None - try: - # syntax check - self._check_playbook_syntax(os.path.join( - self.arg_data['taskdir'], test_playbook)) - - # compute variables - playbook_vars = self._create_playbook_vars(test_playbook) - - # spawn VM if needed - ipaddr = self.ipaddr - if ipaddr is None: - ipaddr = self._spawn_vm(self.arg_data['uuid'], - playbook_vars) - - # execute - log.info('Running playbook %s on machine: %s', test_playbook, - ipaddr) - self._run_playbook(test_playbook, ipaddr, playbook_vars) - - # report results - self._report_results(test_playbook) - except exc.TaskotronInterruptError as e: - log.error('Caught system interrupt during execution of ' - 'playbook %s: %s.', - test_playbook, e) - failed.append(test_playbook) - except exc.TaskotronError as e: - log.error('Error during execution of playbook %s: %s', - test_playbook, e) - failed.append(test_playbook) - finally: + for test_playbook in test_playbooks: + playbook_vars = None try: - if playbook_vars and config.get_config().profile != config.ProfileName.TESTING: - os.remove(playbook_vars['taskotron_secrets_file']) - except OSError as e: - log.warning("Could not delete the secrets file at %r. %s", - playbook_vars['taskotron_secrets_file'], e) - if self.task_vm is not None: - if self.arg_data['no_destroy']: - log.info('Not destroying disposable client as ' - 'requested, access it at: %s .', ipaddr) - else: - self.task_vm.teardown() - - log.info('Playbook execution finished: %s', test_playbook) + # syntax check + self._check_playbook_syntax(os.path.join( + self.arg_data['taskdir'], test_playbook)) + + # compute variables + playbook_vars = self._create_playbook_vars(test_playbook) + + # spawn VM if needed + ipaddr = self.ipaddr + if ipaddr is None: + ipaddr = self._spawn_vm(self.arg_data['uuid'], + playbook_vars) + + # execute + log.info('Running playbook %s on machine: %s', test_playbook, + ipaddr) + self._run_playbook(test_playbook, ipaddr, playbook_vars) + + # report results + self._report_results(test_playbook) + except exc.TaskotronInterruptError as e: + log.error('Caught system interrupt during execution of ' + 'playbook %s: %s. Not executing any other playbooks.', + test_playbook, e) + failed.append(test_playbook) + break + except exc.TaskotronError as e: + log.error('Error during execution of playbook %s: %s', + test_playbook, e) + failed.append(test_playbook) + finally: + try: + if playbook_vars and config.get_config().profile != config.ProfileName.TESTING: + os.remove(playbook_vars['taskotron_secrets_file']) + except OSError as e: + log.warning("Could not delete the secrets file at %r. %s", + playbook_vars['taskotron_secrets_file'], e) + if self.task_vm is not None: + if self.arg_data['no_destroy']: + log.info('Not destroying disposable client as ' + 'requested, access it at: %s . Skipping any ' + 'other playbooks.', ipaddr) + break + else: + self.task_vm.teardown() + log.info('Playbook execution finished: %s', test_playbook) if failed: log.error('Some playbooks failed during execution: %s', diff --git a/testing/test_executor.py b/testing/test_executor.py index 50bfc05..9b7ff02 100644 --- a/testing/test_executor.py +++ b/testing/test_executor.py @@ -393,6 +393,38 @@ class TestExecutor(): mock_run_playbook.assert_not_called() mock_report_results.assert_not_called() + def test_execute_more_playbooks(self, monkeypatch): + '''Should execute all found playbooks''' + mock_check_syntax = mock.Mock() + monkeypatch.setattr(executor.Executor, '_check_playbook_syntax', + mock_check_syntax) + mock_run_playbook = mock.Mock(return_value=(None, True)) + monkeypatch.setattr(executor.Executor, '_run_playbook', + mock_run_playbook) + mock_report_results = mock.Mock() + monkeypatch.setattr(executor.Executor, '_report_results', + mock_report_results) + self.playbook.copy(self.taskdir.join('tests_copy.yml')) + + success = self.executor.execute() + + assert success == True + assert mock_check_syntax.call_count == 2 + assert mock_run_playbook.call_count == 2 + playbooks = [ + mock_run_playbook.call_args_list[0][0][0], + mock_run_playbook.call_args_list[1][0][0] + ] + assert 'tests.yml' in playbooks + assert 'tests_copy.yml' in playbooks + assert mock_report_results.call_count == 2 + playbooks = [ + mock_report_results.call_args_list[0][0][0], + mock_report_results.call_args_list[1][0][0] + ] + assert 'tests.yml' in playbooks + assert 'tests_copy.yml' in playbooks + def test_execute_error(self, monkeypatch): '''Should raise on playbook errors''' mock_check_syntax = mock.Mock() @@ -412,6 +444,38 @@ class TestExecutor(): mock_run_playbook.assert_called_once() mock_report_results.assert_not_called() + def test_execute_more_playbooks_error(self, monkeypatch): + '''Should execute all found playbooks even if some has error''' + mock_check_syntax = mock.Mock() + monkeypatch.setattr(executor.Executor, '_check_playbook_syntax', + mock_check_syntax) + mock_run_playbook = mock.Mock(side_effect=[ + exc.TaskotronError, + (None, True)]) + monkeypatch.setattr(executor.Executor, '_run_playbook', + mock_run_playbook) + mock_report_results = mock.Mock() + monkeypatch.setattr(executor.Executor, '_report_results', + mock_report_results) + self.playbook.copy(self.taskdir.join('tests_copy.yml')) + + success = self.executor.execute() + + assert success == False + assert mock_check_syntax.call_count == 2 + assert mock_run_playbook.call_count == 2 + playbooks = [ + mock_run_playbook.call_args_list[0][0][0], + mock_run_playbook.call_args_list[1][0][0] + ] + assert 'tests.yml' in playbooks + assert 'tests_copy.yml' in playbooks + assert mock_report_results.call_count == 1 + playbooks = [ + mock_report_results.call_args_list[0][0][0] + ] + assert 'tests.yml' in playbooks or 'tests_copy.yml' in playbooks + def test_execute_interrupted(self, monkeypatch): '''Should halt execution and return when interrupted''' mock_check_syntax = mock.Mock() From 62b22aae93d4edbaf2a674570e0ef23722aac3d7 Mon Sep 17 00:00:00 2001 From: Kamil Páral Date: Jul 23 2018 14:18:34 +0000 Subject: [PATCH 4/6] add back taskotron_generic_task and require it to be set So that unadjusted ansible playbooks don't throw arcane errors mid execution. This way it ensures the playbook is intended to be executed inside Taskotron. --- diff --git a/libtaskotron/executor.py b/libtaskotron/executor.py index d4b721f..9403344 100644 --- a/libtaskotron/executor.py +++ b/libtaskotron/executor.py @@ -266,6 +266,8 @@ class Executor(object): path to directory with test suite (on overlord) taskotron_arch architecture of taskotron_item to be tested + taskotron_generic_task + whether this is a Taskotron task, or a random ansible playbook taskotron_item_type item under test taskotron_item @@ -299,6 +301,7 @@ class Executor(object): # default values vars_['become_root'] = True + vars_['taskotron_generic_task'] = False vars_['heartbeat_interval'] = 120 vars_['taskotron_keepalive_minutes'] = 0 vars_['taskotron_match_host_arch'] = False @@ -503,6 +506,11 @@ class Executor(object): # compute variables playbook_vars = self._create_playbook_vars(test_playbook) + if not playbook_vars['taskotron_generic_task']: + raise exc.TaskotronPlaybookError('This playbook is not ' + 'marked as a Taskotron generic task. See ' + 'documentation how to write a task.') + # spawn VM if needed ipaddr = self.ipaddr if ipaddr is None: diff --git a/testing/test_executor.py b/testing/test_executor.py index 9b7ff02..0775415 100644 --- a/testing/test_executor.py +++ b/testing/test_executor.py @@ -444,6 +444,42 @@ class TestExecutor(): mock_run_playbook.assert_called_once() mock_report_results.assert_not_called() + def test_execute_not_taskotron_task(self, monkeypatch): + '''Should raise error when not marked as a taskotron generic task''' + playbook = yaml.safe_load(PLAYBOOK)[0] + playbook['vars'].pop('taskotron_generic_task') + self.playbook.remove() + self.playbook.write(yaml.safe_dump([playbook])) + monkeypatch.setattr(executor.Executor, '_check_playbook_syntax', + mock.Mock()) + mock_run_playbook = mock.Mock() + monkeypatch.setattr(executor.Executor, '_run_playbook', + mock_run_playbook) + + success = self.executor.execute() + + assert success == False + mock_run_playbook.assert_not_called() + + @pytest.mark.parametrize('taskotron_generic_task', [False, None]) + def test_execute_taskotron_task_False(self, monkeypatch, + taskotron_generic_task): + '''Should raise error when taskotron_generic_task is False''' + playbook = yaml.safe_load(PLAYBOOK)[0] + playbook['vars']['taskotron_generic_task'] = taskotron_generic_task + self.playbook.remove() + self.playbook.write(yaml.safe_dump([playbook])) + mock_run_playbook = mock.Mock() + monkeypatch.setattr(executor.Executor, '_check_playbook_syntax', + mock.Mock()) + monkeypatch.setattr(executor.Executor, '_run_playbook', + mock_run_playbook) + + success = self.executor.execute() + + assert success == False + mock_run_playbook.assert_not_called() + def test_execute_more_playbooks_error(self, monkeypatch): '''Should execute all found playbooks even if some has error''' mock_check_syntax = mock.Mock() From 21da4c8ac8c4d8edb790f1deb26d6d74831c43fe Mon Sep 17 00:00:00 2001 From: Kamil Páral Date: Jul 23 2018 14:28:26 +0000 Subject: [PATCH 5/6] remove extra references to distgit STI --- diff --git a/data/ansible/runner.yml b/data/ansible/runner.yml index 88bf049..f84f607 100644 --- a/data/ansible/runner.yml +++ b/data/ansible/runner.yml @@ -1,7 +1,6 @@ # The main playbook for running a task through Taskotron. This is executed # either locally on the overlord or (more usually) remotely on a minion -# machine. This playbook either includes generic or STI tasks, depending on -# the job to be run. +# machine. - hosts: all remote_user: root diff --git a/libtaskotron/executor.py b/libtaskotron/executor.py index 9403344..f4028d7 100644 --- a/libtaskotron/executor.py +++ b/libtaskotron/executor.py @@ -250,8 +250,6 @@ class Executor(object): whether to run playbooks as root client_taskdir path to directory with test suite (on minion) - exec_tasks - path to taskotron tasks playbook (generic or STI) heartbeat_file heartbeat will appear in this file heartbeat_interval diff --git a/libtaskotron/main.py b/libtaskotron/main.py index c5dfe63..999d873 100644 --- a/libtaskotron/main.py +++ b/libtaskotron/main.py @@ -69,7 +69,7 @@ def get_argparser(): pass parser = argparse.ArgumentParser(epilog=ITEM_TYPE_DOCS, formatter_class=CustomFormatter) - parser.add_argument("taskdir", help="taskdir with STI playbook to run") + parser.add_argument("taskdir", help="taskdir with tests.yml* playbook(s) to run") parser.add_argument("-a", "--arch", choices=["i386", "x86_64", "armhfp", "noarch"], default='noarch', help="architecture specifying the item to be checked. 'noarch' value " diff --git a/testing/functest_data.py b/testing/functest_data.py index e9672a0..57740e5 100644 --- a/testing/functest_data.py +++ b/testing/functest_data.py @@ -12,8 +12,6 @@ import subprocess from libtaskotron import config -TASKS_FILES = ['tasks_generic.yml', 'tasks_sti.yml'] - @pytest.mark.usefixtures('setup') class TestAnsible(): '''Test contents of data/ansible directory''' @@ -25,10 +23,8 @@ class TestAnsible(): self.ansible_dir = os.path.join(self.data_dir, 'ansible') self.runner_file = 'runner.yml' - @pytest.mark.parametrize('tasks_file', TASKS_FILES) - def test_runner_syntax(self, tasks_file): + def test_runner_syntax(self): '''Syntax check for runner.yml and tasks_*.yml''' - cmd = ['ansible-playbook', '--syntax-check', self.runner_file, - '--extra-vars=exec_tasks=%s' % tasks_file] + cmd = ['ansible-playbook', '--syntax-check', self.runner_file] # use cwd so that our ansible.cfg gets used subprocess.check_call(cmd, cwd=self.ansible_dir) diff --git a/testing/test_executor.py b/testing/test_executor.py index 0775415..30d3f05 100644 --- a/testing/test_executor.py +++ b/testing/test_executor.py @@ -31,14 +31,6 @@ PLAYBOOK=''' - debug: msg: This is a sample debug printout from a Taskotron generic task ''' -PLAYBOOK_STI=''' -- hosts: localhost - # this saves a lot of time when running in mock (without network) - gather_facts: no - tasks: - - debug: - msg: This is a sample debug printout from an STI task -''' @pytest.mark.usefixtures('setup') From 0f9c6d16568ff6819687d3857a7d476f1aeef4b0 Mon Sep 17 00:00:00 2001 From: Kamil Páral Date: Jul 23 2018 14:33:40 +0000 Subject: [PATCH 6/6] adjust documentation to state we don't support vanilla STI --- diff --git a/docs/source/standard-test-interface.rst b/docs/source/standard-test-interface.rst index 6f03405..b5e754c 100644 --- a/docs/source/standard-test-interface.rst +++ b/docs/source/standard-test-interface.rst @@ -31,10 +31,8 @@ STI is needed. We try to keep as close to the STI specification as possible and maintain full compatibility, just extend the parts where the specification is lacking. -.. note:: Taskotron also includes support for plain STI tests (i.e. specific, - not generic tasks). However, this support is experimental and not maintained - at the moment. There are other systems which handle this area, at least in - Fedora. See CI_. +.. note:: If you need to execute vanilla STI tests (i.e. specific, not generic + tasks), have a look at other systems described at CI_. .. _taskotron-sti: @@ -55,6 +53,8 @@ from STI_ (specification is either extended or modified): * The item/subject is not downloaded and installed automatically as required by STI. +* Custom ansible inventory files are not supported. + * The playbook's exit code is not used for generating PASS/FAIL result automatically (and in our case, for submitting the result to ResultsDB_). Instead, the task must create ``{{artifacts}}/taskotron/results.yml`` file in