From be38d1b2df56f7a33a8a295bd2c838df0fac4559 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Jan 29 2019 10:16:02 +0000 Subject: Handle scheduling update tests on Koji tasks We quite often want to run the update tests on a Koji task (not a Bodhi update) for some reason - usually to test a potential fix for an issue, or at a maintainer's request to test a change before it is merged upstream and officially sent out as an update. Up till now I've always hacked up utils.pm on the staging server by hand to do this, which is horrible. Together with a commit to the distri, this should allow us to do it in a nice, sane way via the CLI. It's mostly just tweaking the variable names and CLI bits as you'd expect, but there's a bit of subtlety to it because of the installer tests that use %ADVISORY% as a variable substitution in the disk image name; you can't do something like `%ADVISORY or KOJITASK%`, sadly, so I had to have almost-redundant variables ADVISORY, KOJITASK and ADVISORY_OR_TASK (we could kinda just live with ADVISORY_OR_TASK except I didn't want to drop ADVISORY as it's an unnecessary change from previous behavior). Signed-off-by: Adam Williamson --- diff --git a/fedora_openqa/cli.py b/fedora_openqa/cli.py index bfbdb06..cefa337 100644 --- a/fedora_openqa/cli.py +++ b/fedora_openqa/cli.py @@ -73,12 +73,19 @@ def command_compose(args): sys.exit() -def command_update(args): - """Schedule openQA jobs for a specified update.""" +def command_update_task(args): + """Schedule openQA jobs for a specified update or task.""" flavors = [] if args.flavor: flavors = [args.flavor] - jobs = schedule.jobs_from_update(args.update, args.release, flavors=flavors, force=args.force, + if hasattr(args, 'task'): + if not args.task.isdigit(): + logger.error("Koji task ID must be all digits!") + sys.exit(1) + buildarg = args.task + else: + buildarg = args.update + jobs = schedule.jobs_from_update(buildarg, args.release, flavors=flavors, force=args.force, openqa_hostname=args.openqa_hostname, arch=args.arch) print("Scheduled jobs: {0}".format(', '.join((str(job) for job in jobs)))) sys.exit() @@ -169,20 +176,30 @@ def parse_args(args=None): "--arches", '-a', help="Comma-separated list of arches to schedule jobs for (if not specified, " "all arches will be scheduled)", metavar='ARCH') + # parser_update and parser_task are nearly the same, so... parser_update = subparsers.add_parser('update', description="Schedule jobs for a specific update.") parser_update.add_argument('update', help="The update ID (e.g. 'FEDORA-2017-b07d628952')", metavar='UPDATE') - parser_update.add_argument('release', help="The release the update is for (e.g. '25')", type=int, metavar="NN") - parser_update.add_argument('--flavor', help="A single flavor to schedule jobs for (e.g. 'server'), " - "otherwise jobs will be scheduled for all update flavors") - parser_update.add_argument( - "--openqa-hostname", help="openQA host to schedule jobs on (default: client library " - "default)", metavar='HOSTNAME') - parser_update.add_argument( - '--arch', '-a', help="Arch to schedule jobs for (default: x86_64)", metavar='ARCH') - parser_update.add_argument( - '--force', '-f', help="For each flavor, schedule jobs even if there " - "are existing, non-cancelled jobs for the update for that flavor", action='store_true') - parser_update.set_defaults(func=command_update) + parser_task = subparsers.add_parser('task', description="Schedule jobs for a specific Koji task.") + parser_task.add_argument('task', help="The task ID (e.g. '32099714')", metavar='TASK') + for updtaskparser in [parser_update, parser_task]: + if updtaskparser is parser_update: + targetstr = 'update' + else: + targetstr = 'task' + updtaskparser.add_argument('release', help="The release the {0} is for (e.g. '25')".format(targetstr), + type=int, metavar="NN") + updtaskparser.add_argument('--flavor', help="A single flavor to schedule jobs for (e.g. 'server'), " + "otherwise jobs will be scheduled for all update flavors") + updtaskparser.add_argument( + "--openqa-hostname", help="openQA host to schedule jobs on (default: client library " + "default)", metavar='HOSTNAME') + updtaskparser.add_argument( + '--arch', '-a', help="Arch to schedule jobs for (default: x86_64)", metavar='ARCH') + updtaskparser.add_argument( + '--force', '-f', help="For each flavor, schedule jobs even if there are existing, non-cancelled jobs " + "for the {0} for that flavor".format(targetstr), action='store_true') + parser_update.set_defaults(func=command_update_task) + parser_task.set_defaults(func=command_update_task) parser_report = subparsers.add_parser( 'report', description="Map openQA job results to Wikitcms test results and either log them to output or " diff --git a/fedora_openqa/schedule.py b/fedora_openqa/schedule.py index b7e081f..9703bce 100644 --- a/fedora_openqa/schedule.py +++ b/fedora_openqa/schedule.py @@ -321,29 +321,29 @@ def jobs_from_compose(location, wanted=None, force=False, extraparams=None, open return (rel.cid, jobs) def jobs_from_update(update, version, flavors=None, force=False, extraparams=None, openqa_hostname=None, arch=None): - """Schedule jobs for a specific Fedora update. update is the - advisory ID, version is the release number, flavors defines which - update tests should be run (valid values are the 'flavdict' keys). - force, extraparams and openqa_hostname are as for - jobs_from_compose. To explain the HDD_1 and START_AFTER_TEST - settings: most tests in the 'update' scenario are shared with the - 'compose' scenario. Many of them specify START_AFTER_TEST as - 'install_default_upload' and HDD_1 as the disk image that - install_default_upload creates, so that in the 'compose' scenario, - these tests run after install_default_upload and use the image it - creates. For update testing, there is no install_default_upload - test; we instead want to run these tests using the pre-existing - createhdds-created base image. So here, we specify the appropriate - HDD_1 value, and an empty value for START_AFTER_TEST, so the - scheduler will not try to create a dependency on the non-existent - install_default_upload test, and the correct disk image will be - used. There are a *few* tests where we do *not* want to override - these values, however: the tests where there really is a dependency - in both scenarios (e.g. the cockpit_basic test has to run after the - cockpit_default test and use the disk image it uploads). For these - tests, we specify the values in the templates as +START_AFTER_TEST - and +HDD_1; the + makes those values win over the ones we pass in - here. + """Schedule jobs for a specific Fedora update (or scratch build). + update is the advisory ID or task ID, version is the release + number, flavors defines which update tests should be run (valid + values arethe 'flavdict' keys). force, extraparams and + openqa_hostname are as for jobs_from_compose. To explain the HDD_1 + and START_AFTER_TEST settings: most tests in the 'update' scenario + are shared with the 'compose' scenario. Many of them specify + START_AFTER_TEST as 'install_default_upload' and HDD_1 as the disk + image that install_default_upload creates, so that in the compose + scenario, these tests run after install_default_upload and use the + image it creates. For update testing, there is no install_default_ + upload test; we instead want to run these tests using the pre- + existing createhdds-created base image. So here, we specify the + appropriate HDD_1 value, and an empty value for START_AFTER_TEST, + so the scheduler will not try to create a dependency on the non- + existent install_default_upload test, and the correct disk image + will be used. There are a *few* tests where we do *not* want to + override these values, however: the tests where there really is a + dependency in both scenarios (e.g. the cockpit_basic test has to + run after the cockpit_default test and use the disk image it + uploads). For these tests, we specify the values in the templates + as +START_AFTER_TEST and +HDD_1; the + makes those values win over + the ones we pass in here. """ version = str(version) try: @@ -357,7 +357,12 @@ def jobs_from_update(update, version, flavors=None, force=False, extraparams=Non if not arch: # set a default in a way that works neatly with the CLI bits arch = 'x86_64' - build = 'Update-{0}'.format(update) + if update.isdigit(): + # Koji task ID: treat as a non-reported scratch build test + build = "Kojitask-{0}-NOREPORT".format(update) + else: + # normal update case + build = 'Update-{0}'.format(update) if extraparams: build = '{0}-EXTRA'.format(build) flavdict = { @@ -384,12 +389,23 @@ def jobs_from_update(update, version, flavors=None, force=False, extraparams=Non 'CURRREL': currrel, }, } + # we'll set ADVISORY and ADVISORY_OR_TASK for updates, and KOJITASK and + # ADVISORY_OR_TASK for Koji tasks. I'd probably have designed this more + # cleanly if doing it from scratch, but we started with updates and added + # tasks later and it's a bit messy. We need to have a consistently-named + # variable for the distri templates substitution, there's no mechanism to + # do 'substitute for this var or this other var depending which exists' + if update.isdigit(): + advkey = 'KOJITASK' + else: + advkey = 'ADVISORY' baseparams = { 'DISTRI': 'fedora', 'VERSION': version, 'ARCH': arch, 'BUILD': build, - 'ADVISORY': update, + advkey: update, + 'ADVISORY_OR_TASK': update, # only obsolete pending jobs for same BUILD (i.e. update) '_ONLY_OBSOLETE_SAME_BUILD': '1', 'START_AFTER_TEST': '', @@ -405,7 +421,7 @@ def jobs_from_update(update, version, flavors=None, force=False, extraparams=Non baseparams['DEVELOPMENT'] = 1 except ValueError: # but don't fail to schedule if fedfind fails... - logger.warning("jobs_from_update: could not determine current or oldest release! Assuming update is " + logger.warning("jobs_from_update: could not determine current or oldest release! Assuming update/task is " "for stable release that is not the oldest stable.") oldest = 0 client = OpenQA_Client(openqa_hostname) @@ -427,7 +443,7 @@ def jobs_from_update(update, version, flavors=None, force=False, extraparams=Non currjobs = client.openqa_request('GET', 'jobs', params={'build': build, 'arch': arch})['jobs'] currjobs = [cjob for cjob in currjobs if cjob['settings']['FLAVOR'] == fullflav] if currjobs: - logger.info("jobs_from_update: Existing jobs found for update %s flavor %s arch %s, " + logger.info("jobs_from_update: Existing jobs found for update/task %s flavor %s arch %s, " "and force not set! No jobs scheduled.", update, flavor, arch) continue flavparams = flavdict[flavor] diff --git a/tests/test_cli.py b/tests/test_cli.py index 46ae5f8..8a3090a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -119,15 +119,28 @@ class TestCommandCompose: # should exit 1 assert excinfo.value.code == 1 +@pytest.mark.parametrize( + "target", + # update ID or task ID (to test both paths) + ['FEDORA-2017-b07d628952', '32099714'] +) @mock.patch('fedora_openqa.schedule.jobs_from_update', return_value=[1, 2], autospec=True) -def test_command_update(fakejfu, capsys): - """Test the command_update function.""" +def test_command_update_task(fakejfu, target, capsys): + """Test the command_update_task function.""" + if target.isdigit(): + targetarg = 'task' + else: + targetarg = 'update' args = cli.parse_args( - ['update', 'FEDORA-2017-b07d628952', '25'] + [targetarg, target, '25'] ) with pytest.raises(SystemExit) as excinfo: - cli.command_update(args) + cli.command_update_task(args) (out, _) = capsys.readouterr() + # first arg should be target + assert fakejfu.call_args[0][0] == target + # second should be release + assert fakejfu.call_args[0][1] == 25 # flavors kwarg should be false-y (not, e.g., [None]) assert not fakejfu.call_args[1]['flavors'] # should print out list of scheduled jobs @@ -139,34 +152,46 @@ def test_command_update(fakejfu, capsys): # check 'flavor' args = cli.parse_args( - ['update', 'FEDORA-2017-b07d628952', '25', '--flavor', 'server'] + ['update', target, '25', '--flavor', 'server'] ) with pytest.raises(SystemExit) as excinfo: - cli.command_update(args) + cli.command_update_task(args) # should exit 0 assert not excinfo.value.code assert fakejfu.call_args[1]['flavors'] == ['server'] # check 'force' args = cli.parse_args( - ['update', 'FEDORA-2017-b07d628952', '25', '--force'] + ['update', target, '25', '--force'] ) with pytest.raises(SystemExit) as excinfo: - cli.command_update(args) + cli.command_update_task(args) # should exit 0 assert not excinfo.value.code assert fakejfu.call_args[1]['force'] is True # check 'openqa_hostname' args = cli.parse_args( - ['update', 'FEDORA-2017-b07d628952', '25', '--openqa-hostname', 'openqa.example'] + ['update', target, '25', '--openqa-hostname', 'openqa.example'] ) with pytest.raises(SystemExit) as excinfo: - cli.command_update(args) + cli.command_update_task(args) # should exit 0 assert not excinfo.value.code assert fakejfu.call_args[1]['openqa_hostname'] == 'openqa.example' +# this should not in fact get hit, but just in case we *do* break the code, +# let's mock it to be safe... +@mock.patch('fedora_openqa.schedule.jobs_from_update', return_value=[1, 2], autospec=True) +def test_command_update_nonint(fakejfu): + args = cli.parse_args( + ['task', '123abc', '25'] + ) + with pytest.raises(SystemExit) as excinfo: + cli.command_update_task(args) + # should exit 1 + assert excinfo.value.code == 1 + @pytest.mark.parametrize( "jobargs,argname,expecteds", # args, which kwarg we expect report function to get, list of diff --git a/tests/test_schedule.py b/tests/test_schedule.py index b57d583..d0a6123 100644 --- a/tests/test_schedule.py +++ b/tests/test_schedule.py @@ -437,6 +437,7 @@ def test_jobs_from_update(fakeclient, fakecurrr, fakecurrs): 'ARCH': 'x86_64', 'BUILD': 'Update-FEDORA-2017-b07d628952', 'ADVISORY': 'FEDORA-2017-b07d628952', + 'ADVISORY_OR_TASK': 'FEDORA-2017-b07d628952', '_ONLY_OBSOLETE_SAME_BUILD': '1', 'START_AFTER_TEST': '', 'FLAVOR': 'updates-server-upgrade', @@ -448,6 +449,7 @@ def test_jobs_from_update(fakeclient, fakecurrr, fakecurrs): 'ARCH': 'x86_64', 'BUILD': 'Update-FEDORA-2017-b07d628952', 'ADVISORY': 'FEDORA-2017-b07d628952', + 'ADVISORY_OR_TASK': 'FEDORA-2017-b07d628952', '_ONLY_OBSOLETE_SAME_BUILD': '1', 'START_AFTER_TEST': '', 'FLAVOR': 'updates-workstation-upgrade', @@ -459,6 +461,7 @@ def test_jobs_from_update(fakeclient, fakecurrr, fakecurrs): 'ARCH': 'x86_64', 'BUILD': 'Update-FEDORA-2017-b07d628952', 'ADVISORY': 'FEDORA-2017-b07d628952', + 'ADVISORY_OR_TASK': 'FEDORA-2017-b07d628952', '_ONLY_OBSOLETE_SAME_BUILD': '1', 'START_AFTER_TEST': '', 'HDD_1': 'disk_f25_server_3_x86_64.img', @@ -470,6 +473,7 @@ def test_jobs_from_update(fakeclient, fakecurrr, fakecurrs): 'ARCH': 'x86_64', 'BUILD': 'Update-FEDORA-2017-b07d628952', 'ADVISORY': 'FEDORA-2017-b07d628952', + 'ADVISORY_OR_TASK': 'FEDORA-2017-b07d628952', '_ONLY_OBSOLETE_SAME_BUILD': '1', 'START_AFTER_TEST': '', 'HDD_1': 'disk_f25_desktop_4_x86_64.img', @@ -482,6 +486,7 @@ def test_jobs_from_update(fakeclient, fakecurrr, fakecurrs): 'ARCH': 'x86_64', 'BUILD': 'Update-FEDORA-2017-b07d628952', 'ADVISORY': 'FEDORA-2017-b07d628952', + 'ADVISORY_OR_TASK': 'FEDORA-2017-b07d628952', '_ONLY_OBSOLETE_SAME_BUILD': '1', 'START_AFTER_TEST': '', 'FLAVOR': 'updates-installer', @@ -601,4 +606,38 @@ def test_jobs_from_update(fakeclient, fakecurrr, fakecurrs): if 'HDD_1' in post[0][2]: assert post[0][2]['HDD_1'] in ['disk_f25_server_3_ppc64le.img', 'disk_f25_desktop_4_ppc64le.img'] +@mock.patch('fedfind.helpers.get_current_stables', return_value=[28, 29]) +@mock.patch('fedfind.helpers.get_current_release', return_value=29) +@mock.patch('fedora_openqa.schedule.OpenQA_Client', autospec=True) +def test_jobs_from_update_kojitask(fakeclient, fakecurrr, fakecurrs): + """Test jobs_from_update works as expected when passed a Koji task + ID. We don't need to recheck everything, just the differing vars. + """ + # the OpenQA_Client instance mock + fakeinst = fakeclient.return_value + # for now, return no 'jobs' (for the dupe query), one 'id' (for + # the post request) + fakeinst.openqa_request.return_value = {'jobs': [], 'ids': [1]} + # simple case + ret = schedule.jobs_from_update('32099714', '28', flavors=['installer']) + # should get one job for one flavor + assert ret == [1] + # find the POST calls + posts = [call for call in fakeinst.openqa_request.call_args_list if call[0][0] == 'POST'] + # one flavor, one call + assert len(posts) == 1 + parmdict = posts[0][0][2] + assert parmdict == { + 'DISTRI': 'fedora', + 'VERSION': '28', + 'ARCH': 'x86_64', + 'BUILD': 'Kojitask-32099714-NOREPORT', + 'KOJITASK': '32099714', + 'ADVISORY_OR_TASK': '32099714', + '_ONLY_OBSOLETE_SAME_BUILD': '1', + 'START_AFTER_TEST': '', + 'FLAVOR': 'updates-installer', + 'CURRREL': '29', + } + # vim: set textwidth=120 ts=8 et sw=4: