PR#70: Allow filtering by flavor when scheduling a compose - fedora-qa/fedora_openqa

fedora-qa / fedora_openqa

#70 Allow filtering by flavor when scheduling a compose

Closed 5 years ago by adamwill. Opened 5 years ago by adamwill.

want-flavors into master

Allow filtering by flavor when scheduling a compose

Adam Williamson • 5 years ago

d5731aa

fedora_openqa/cli.py

file modified

+8 -2

		`@@ -47,13 +47,16 @@`
		`extraparams = None`
		`if args.updates:`
		`extraparams = {'GRUBADD': "inst.updates={0}".format(args.updates)}`
		`+ flavors = None`
		`+ if args.flavors:`
		`+ flavors = args.flavors.split(',')`
		`arches = None`
		`if args.arches:`
		`arches = args.arches.split(',')`
		`try:`
		`(_, jobs) = schedule.jobs_from_compose(`
		`args.location, force=args.force, extraparams=extraparams,`
		`- openqa_hostname=args.openqa_hostname, arches=arches)`
		`+ openqa_hostname=args.openqa_hostname, arches=arches, flavors=flavors)`
		`except schedule.TriggerException as err:`
		`logger.warning("No jobs run! %s", err)`
		`sys.exit(1)`
		`@@ -167,7 +170,10 @@`
		`parser_compose.set_defaults(func=command_compose)`
		`parser_compose.add_argument(`
		`"--arches", '-a', help="Comma-separated list of arches to schedule jobs for (if not specified, "`
		`- "all arches will be scheduled)", metavar='ARCH')`
		`+ "all arches will be scheduled)", metavar='ARCHES')`
		`+ parser_compose.add_argument(`
		`+ "--flavors", help="Comma-separated list of flavors to schedule jobs for (if not specified, "`
		`+ "all flavors will be scheduled)", metavar='FLAVORS')`

		`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')`

fedora_openqa/schedule.py

file modified

+20 -2

		`@@ -218,7 +218,8 @@`
		`return output["ids"]`


		`- def jobs_from_compose(location, wanted=None, force=False, extraparams=None, openqa_hostname=None, arches=None):`
		`+ def jobs_from_compose(location, wanted=None, force=False, extraparams=None, openqa_hostname=None, arches=None,`
		`+ flavors=None):`
		`"""Schedule jobs against a specific compose. Returns a 2-tuple`
		`of the compose ID and the list of job IDs.`

		`@@ -250,6 +251,15 @@`
		`arches is a list of arches to schedule jobs for; if specified,`
		`the image list will be filtered by the arches listed. If not`
		`specified, jobs are scheduled for all arches in the image list.`
		`+`
		`+ flavors is a list of flavors to schedule jobs for; if specified,`
		`+ the image list will be filtered to images whose flavor is in this`
		`+ list. For convenience, case is ignored.`
		`+`
		`+ Of course arches and flavors are redundant with and less capable`
		`+ than WANTED, but are used to back a convenience feature in the`
		`+ CLI, letting you quickly schedule jobs for specific flavor(s)`
		`+ and/or arch(es) without having to edit a WANTED file.`
		`"""`
		`if not wanted:`
		`wanted = WANTED`
		`@@ -270,9 +280,14 @@`
		`return ('', [])`
		`logger.debug("Finding images for compose %s in location %s", rel.cid, location)`
		`images = _get_images(rel, wanted=wanted)`
		`+ if flavors:`
		`+ flavors = [flavor.lower() for flavor in flavors]`
		`+ logger.debug("Only scheduling jobs for flavors %s", ' '.join(flavors))`
		`+ images = [img for img in images if img[0].lower() in flavors]`
		`if arches:`
		`logger.debug("Only scheduling jobs for arches %s", ' '.join(arches))`
		`images = [img for img in images if img[1] in arches]`
		`+`
		`if len(images) == 0:`
		`raise TriggerException("Compose found, but no available images")`
		`jobs = []`
		`@@ -292,7 +307,10 @@`
		`if score > univs.get(arch, [None, 0])[1]:`
		`univs[arch] = (param_urls, score, subvariant, imagetype)`

		`- # now schedule universal jobs`
		`+ # now schedule universal jobs...unless 'flavors' was passed and`
		`+ # 'universal' wasn't in it`
		`+ if flavors and 'universal' not in flavors:`
		`+ univs = {}`
		`if univs:`
		`for (arch, (param_urls, _, subvariant, imagetype)) in univs.items():`
		`# We are assuming that ISO_URL is present in param_urls. This could create problem when`

tests/test_cli.py

file modified

+21 -8

		`@@ -55,6 +55,8 @@`
		`assert fakejfc.call_args[1]['force'] is False`
		`# should pass arches as 'None'`
		`assert fakejfc.call_args[1]['arches'] is None`
		`+ # should pass flavors as 'None'`
		`+ assert fakejfc.call_args[1]['flavors'] is None`

		`def test_force(self, fakejfc):`
		`"""Test with -f (force)."""`
		`@@ -82,30 +84,41 @@`
		`# should add extra params`
		`assert fakejfc.call_args[1]['extraparams'] == {'GRUBADD': "inst.updates=https://www.foo.com/updates.img"}`

		`- def test_arch(self, fakejfc):`
		`- """Test with --arches."""`
		`+ @pytest.mark.parametrize(`
		`+ ("arg", "values"),`
		`+ [`
		`+ ('arches', ('x86_64', 'i386,armhfp')),`
		`+ ('flavors', ('server-boot-iso', 'workstation-live-iso,universal')),`
		`+ ]`
		`+ )`
		`+ def test_arches_flavors(self, fakejfc, arg, values):`
		`+ """Test with --arches and --flavors, using parametrization.`
		`+ 'arg' is the argument name. 'values' is a tuple of two value`
		`+ strings, the first a single appropriate value for the arg, the`
		`+ second a comma-separated list of two appropriate values.`
		`+ """`
		`args = cli.parse_args([`
		`'compose',`
		`'https://kojipkgs.fedoraproject.org/compose/rawhide/Fedora-24-20160113.n.1/compose',`
		`- '--arches=x86_64'`
		`+ '--{0}={1}'.format(arg, values[0])`
		`])`
		`with pytest.raises(SystemExit) as excinfo:`
		`cli.command_compose(args)`
		`# should exit 0`
		`assert not excinfo.value.code`
		`- # should specify arch as single-item list`
		`- assert fakejfc.call_args[1]['arches'] == ['x86_64']`
		`+ # should specify arch/flavor as single-item list`
		`+ assert fakejfc.call_args[1][arg] == [values[0]]`
		`args = cli.parse_args([`
		`'compose',`
		`'https://kojipkgs.fedoraproject.org/compose/rawhide/Fedora-24-20160113.n.1/compose',`
		`- '--arches=i386,armhfp'`
		`+ '--{0}={1}'.format(arg, values[1])`
		`])`
		`with pytest.raises(SystemExit) as excinfo:`
		`cli.command_compose(args)`
		`# should exit 0`
		`assert not excinfo.value.code`
		`- # should specify arch as multi-item list`
		`- assert fakejfc.call_args[1]['arches'] == ['i386', 'armhfp']`
		`+ # should specify arches/flavors as multi-item list`
		`+ assert fakejfc.call_args[1][arg] == values[1].split(',')`

		`def test_nojobs(self, fakejfc):`
		`"""Test exits 1 when no jobs are run."""`

tests/test_schedule.py

file modified

+14

		`@@ -369,6 +369,20 @@`
		`# 7 images (6 i386, 1 armhfp), 1 universal arch (i386)`
		`assert fakerun.call_count == 8`

		`+ # check flavors is handled properly`
		`+ fakerun.reset_mock()`
		`+ ret = schedule.jobs_from_compose(COMPURL, flavors=['server-boot-iso', 'workstation-live-iso', 'foobar'])`
		`+ # two of those flavors we have images for (2 images each), one we don't;`
		`+ # universal SHOULD NOT be scheduled`
		`+ assert fakerun.call_count == 4`
		`+`
		`+ # check flavors and arches is handled properly`
		`+ fakerun.reset_mock()`
		`+ ret = schedule.jobs_from_compose(COMPURL, flavors=['server-boot-iso', 'workstation-live-iso'], arches=['x86_64'])`
		`+ # we have one x86_64 image for each flavor, so 2`
		`+ assert fakerun.call_count == 2`
		`+`
		`+`
		`# check triggerexception is raised when appropriate`
		`with mock.patch('fedfind.release.get_release', side_effect=ValueError("Oops!")):`
		`with pytest.raises(schedule.TriggerException):`

adamwill commented 5 years ago

You can already do this by using a WANTED file, but that's a lot
of work if you just quickly want to schedule jobs only for one
ISO or whatever. It's much easier to just be able to pass in a
comma-separated list of flavors on the CLI, like this.

Signed-off-by: Adam Williamson awilliam@redhat.com

adamwill commented 5 years ago

@lruzicka I was just gonna merge this, but I'm sending it as a PR to give you a bit of practice in PR review and also maybe as a way for you to look into how the scheduler works a bit :) Please let me know what you think, thanks.

adamwill commented 5 years ago

I wound up merging this because I sorta needed it.

Pull-Request has been closed by adamwill

5 years ago