PR#456: Add command for requesting side-tags in Koji - rpkg

rpkg

#456 Add command for requesting side-tags in Koji

Merged 4 years ago by lsedlar. Opened 4 years ago by lsedlar.

lsedlar/rpkg side-tags into master

Add commands for interacting with Koji side-tag plugin

Lubomír Sedlář • 4 years ago

7d1c609

pyrpkg/__init__.py

file modified

+15

		`@@ -2569,6 +2569,9 @@`
		`# Run the command`
		`self._run_command(cmd, shell=True)`

		`+ def list_side_tags(self, base_tag=None, user=None):`
		`+ return self.kojisession.listSideTags(basetag=base_tag, user=user)`
		`+`
		`def local(self, localargs, arch=None, hashtype=None, builddir=None,`
		`buildrootdir=None, define=None):`
		`"""rpmbuild locally for given arch.`
		`@@ -3191,6 +3194,18 @@`
		`cmd.extend([project, srpm_name])`
		`self._run_command(cmd)`

		`+ def remove_side_tag(self, tag):`
		`+ self.kojisession.removeSideTag(tag)`
		`+`
		`+ def request_side_tag(self, base_tag=None):`
		`+ if not base_tag:`
		`+ build_target = self.kojisession.getBuildTarget(self.target)`
		`+ if not build_target:`
		`+ raise rpkgError("Unknown build target: %s" % self.target)`
		`+ base_tag = build_target["build_tag_name"]`
		`+`
		`+ return self.kojisession.createSideTag(base_tag)`
		`+`
		`def retire(self, message):`
		`"""Delete all tracked files and commit a new dead.package file for rpms`
		`or dead.module file for modules.`

pyrpkg/cli.py

file modified

+52

		`@@ -465,6 +465,7 @@`
		`self.register_import_srpm()`
		`self.register_install()`
		`self.register_lint()`
		`+ self.register_list_side_tags()`
		`self.register_local()`
		`self.register_mockbuild()`
		`self.register_mock_config()`
		`@@ -481,6 +482,8 @@`
		`self.register_prep()`
		`self.register_pull()`
		`self.register_push()`
		`+ self.register_remove_side_tag()`
		`+ self.register_request_side_tag()`
		`self.register_retire()`
		`self.register_scratch_build()`
		`self.register_sources()`
		`@@ -1015,6 +1018,20 @@`
		`help='Use a specific configuration file for rpmlint')`
		`lint_parser.set_defaults(command=self.lint)`

		`+ def register_list_side_tags(self):`
		`+ """Register the list-side-tags target"""`
		`+`
		`+ parser = self.subparsers.add_parser(`
		`+ "list-side-tags", help="List existing side-tags",`
		`+ )`
		`+ group = parser.add_mutually_exclusive_group()`
		`+ group.add_argument("--mine", action="store_true", help="List only my side tags")`
		`+ group.add_argument(`
		`+ "--user", dest="tag_owner", help="List side tags created by this user",`
		`+ )`
		`+ group.add_argument("--base-tag", help="List only tags based on this base")`
		`+ parser.set_defaults(command=self.list_side_tags)`
		`+`
		`def register_local(self):`
		`"""Register the local target"""`

		`@@ -1320,6 +1337,22 @@`
		`push_parser.add_argument('--force', '-f', help='Force push', action='store_true')`
		`push_parser.set_defaults(command=self.push)`

		`+ def register_remove_side_tag(self):`
		`+ """Register remove-side-tag command."""`
		`+ parser = self.subparsers.add_parser(`
		`+ "remove-side-tag", help="Remove a side tag (without merging packages)"`
		`+ )`
		`+ parser.add_argument("TAG", help="name of tag to be deleted")`
		`+ parser.set_defaults(command=self.remove_side_tag)`
		`+`
		`+ def register_request_side_tag(self):`
		`+ """Register command line parser for subcommand request-side-tag """`
		`+ parser = self.subparsers.add_parser(`
		`+ "request-side-tag", help="Create a new side tag"`
		`+ )`
		`+ parser.add_argument("--base-tag", help="name of base tag")`
		`+ parser.set_defaults(command=self.request_side_tag)`
		`+`
		`def register_retire(self):`
		`"""Register the retire target"""`

		`@@ -2064,6 +2097,12 @@`
		`def lint(self):`
		`self.cmd.lint(self.args.info, self.args.rpmlintconf)`

		`+ def list_side_tags(self):`
		`+ user = self.args.tag_owner or (self.user if self.args.mine else None)`
		`+ tags = self.cmd.list_side_tags(base_tag=self.args.base_tag, user=user)`
		`+ for tag in sorted(tags, key=lambda t: t["name"]):`
		`+ print("%(name)s\t(id %(id)d)" % tag)`
		`+`
		`def local(self):`
		`self.sources()`

		`@@ -2456,6 +2495,19 @@`
		`'credential.useHttpPath': 'true'}`
		`self.cmd.push(getattr(self.args, 'force', False), extra_config)`

		`+ def remove_side_tag(self):`
		`+ self.cmd.remove_side_tag(self.args.TAG)`
		`+ print("Tag deleted.")`
		`+`
		`+ def request_side_tag(self):`
		`+ tag_info = self.cmd.request_side_tag(base_tag=self.args.base_tag)`
		`+ print("Side tag '%(name)s' (id %(id)d) created." % tag_info)`
		`+ print("Use '%s build --target=%s' to use it." % (self.name, tag_info["name"]))`
		`+ print(`
		`+ "Use '%s wait-repo %s' to wait for the build repo to be generated."`
		`+ % (self.cmd.build_client, tag_info["name"])`
		`+ )`
		`+`
		`def retire(self):`
		`# Skip if package/module is already retired...`
		`if os.path.isfile(os.path.join(self.cmd.path, 'dead.package')):`

tests/test_side_tag.py

file added

+164

		`@@ -0,0 +1,164 @@`
		`+ # -- coding: utf-8 --`
		`+`
		`+ import logging`
		`+ import os`
		`+`
		`+ import koji`
		`+ import mock`
		`+ import six`
		`+ from six.moves import StringIO, configparser`
		`+`
		`+ import pyrpkg.cli`
		`+`
		`+ from utils import CommandTestCase`
		`+`
		`+ if six.PY2:`
		`+ ConfigParser = configparser.SafeConfigParser`
		`+ else:`
		`+ # The SafeConfigParser class has been renamed to ConfigParser in Python 3.2.`
		`+ ConfigParser = configparser.ConfigParser`
		`+`
		`+`
		`+ class BaseCase(CommandTestCase):`
		`+ def new_cli(self, args):`
		`+ config = ConfigParser()`
		`+ fixtures_dir = os.path.join(os.path.dirname(__file__), "fixtures")`
		`+ config.read(os.path.join(fixtures_dir, "rpkg.conf"))`
		`+`
		`+ client = pyrpkg.cli.cliClient(config, name="rpkg")`
		`+ client.setupLogging(pyrpkg.log)`
		`+ pyrpkg.log.setLevel(logging.CRITICAL)`
		`+ client.do_imports()`
		`+ cmd = ["rpkg", "--path", self.cloned_repo_path] + args`
		`+ with mock.patch("sys.argv", new=cmd):`
		`+ client.parse_cmdline()`
		`+`
		`+ client.cmd._kojisession = mock.Mock()`
		`+`
		`+ return client`
		`+`
		`+`
		`+ class RequestSideTagTestCase(BaseCase):`
		`+ def test_guess_base_tag(self):`
		`+ cli = self.new_cli(["request-side-tag"])`
		`+ cli.cmd._kojisession.getBuildTarget.return_value = {"build_tag_name": "base"}`
		`+ cli.cmd._kojisession.createSideTag.return_value = {"name": "side", "id": 123}`
		`+ with mock.patch("sys.stdout", new_callable=StringIO) as mock_out:`
		`+ cli.request_side_tag()`
		`+`
		`+ output = mock_out.getvalue()`
		`+ self.assertIn("Side tag 'side' (id 123) created.", output)`
		`+ self.assertIn("Use 'rpkg build --target=side' to use it.", output)`
		`+`
		`+ self.assertEqual(`
		`+ cli.cmd._kojisession.createSideTag.call_args_list, [mock.call("base")]`
		`+ )`
		`+ self.assertEqual(`
		`+ cli.cmd._kojisession.getBuildTarget.call_args_list,`
		`+ [mock.call(cli.cmd.target)],`
		`+ )`
		`+`
		`+ def test_explicit_base_tag(self):`
		`+ cli = self.new_cli(["request-side-tag", "--base-tag=f30-build"])`
		`+ cli.cmd._kojisession.createSideTag.return_value = {"name": "side", "id": 123}`
		`+ with mock.patch("sys.stdout", new_callable=StringIO) as mock_out:`
		`+ cli.request_side_tag()`
		`+`
		`+ output = mock_out.getvalue()`
		`+ self.assertIn("Side tag 'side' (id 123) created.", output)`
		`+ self.assertIn("Use 'rpkg build --target=side' to use it.", output)`
		`+`
		`+ self.assertEqual(`
		`+ cli.cmd._kojisession.createSideTag.call_args_list, [mock.call("f30-build")]`
		`+ )`
		`+`
		`+ def test_failure(self):`
		`+ def raise_error(args, *kwargs):`
		`+ raise koji.GenericError("a problem")`
		`+`
		`+ cli = self.new_cli(["request-side-tag", "--base-tag=foobar"])`
		`+ cli.cmd._kojisession.createSideTag.side_effect = raise_error`
		`+`
		`+ with self.assertRaises(Exception) as ctx:`
		`+ cli.request_side_tag()`
		`+`
		`+ self.assertIn("a problem", str(ctx.exception))`
		`+`
		`+`
		`+ class ListSideTagTestCase(BaseCase):`
		`+ def test_list_all(self):`
		`+ cli = self.new_cli(["list-side-tags"])`
		`+ cli.cmd._kojisession.listSideTags.return_value = [`
		`+ dict(name="f31-build-side-456", id=1),`
		`+ dict(name="f30-build-side-1", id=2),`
		`+ ]`
		`+ with mock.patch("sys.stdout", new_callable=StringIO) as mock_out:`
		`+ cli.list_side_tags()`
		`+`
		`+ self.assertEqual(`
		`+ cli.cmd._kojisession.listSideTags.call_args_list,`
		`+ [mock.call(basetag=None, user=None)],`
		`+ )`
		`+`
		`+ self.assertEqual(`
		`+ mock_out.getvalue(),`
		`+ "f30-build-side-1\t(id 2)\nf31-build-side-456\t(id 1)\n",`
		`+ )`
		`+`
		`+ def list_for_base_tag(self):`
		`+ cli = self.new_cli(["list-side-tags", "--base-tag=f30-build"])`
		`+ cli.cmd._kojisession.listSideTags.return_value = [`
		`+ dict(name="f30-build-side-456", id=1)`
		`+ ]`
		`+ cli.list_side_tags()`
		`+`
		`+ self.assertEqual(`
		`+ cli.cmd._kojisession.listSideTags.call_args_list,`
		`+ [mock.call(basetag="f30-build", user=None)],`
		`+ )`
		`+`
		`+ def list_mine(self):`
		`+ cli = self.new_cli(["list-side-tags", "--mine"])`
		`+ cli.user = "devel"`
		`+ cli.cmd._kojisession.listSideTags.return_value = [`
		`+ dict(name="f30-build-side-456", id=1)`
		`+ ]`
		`+ cli.list_side_tags()`
		`+`
		`+ self.assertEqual(`
		`+ cli.cmd._kojisession.listSideTags.call_args_list,`
		`+ [mock.call(basetag=None, user="devel")],`
		`+ )`
		`+`
		`+ def list_for_user(self):`
		`+ cli = self.new_cli(["list-side-tags", "--user=jdoe"])`
		`+ cli.cmd._kojisession.listSideTags.return_value = [`
		`+ dict(name="f30-build-side-456", id=1)`
		`+ ]`
		`+ cli.list_side_tags()`
		`+`
		`+ self.assertEqual(`
		`+ cli.cmd._kojisession.listSideTags.call_args_list,`
		`+ [mock.call(basetag=None, user="jdoe")],`
		`+ )`
		`+`
		`+`
		`+ class RemoveSideTagTestCase(BaseCase):`
		`+ def test_success_remove(self):`
		`+ cli = self.new_cli(["remove-side-tag", "f30-build-side-123"])`
		`+ cli.remove_side_tag()`
		`+ self.assertEqual(`
		`+ cli.cmd._kojisession.removeSideTag.call_args_list,`
		`+ [mock.call("f30-build-side-123")],`
		`+ )`
		`+`
		`+ def test_fail_to_remove(self):`
		`+ def raise_error(args, *kwargs):`
		`+ raise koji.GenericError("a problem")`
		`+`
		`+ cli = self.new_cli(["remove-side-tag", "f30-build-side-123"])`
		`+ cli.cmd._kojisession.removeSideTag.side_effect = raise_error`
		`+ with self.assertRaises(Exception) as ctx:`
		`+ cli.remove_side_tag()`
		`+`
		`+ self.assertIn("a problem", str(ctx.exception))`

lsedlar commented 4 years ago

There is a Koji plugin that can create side-tags [0]. This patch adds support for a command to request new side-tags.

[0] https://pagure.io/sidetag-koji-plugin

It is used like this:

$ rpkg request-side-tag [--base-tag=FOO]

The base tag is used as a parent of the new side tag. If not given, rpkg will find build tag of current target and use it.

The plugin creates both tag and target (with the same name), so the output of rpkg contains a suggestion on how to submit builds to the new
target.

Fixes: https://pagure.io/fedpkg/issue/329

This has not yet been tested as staging Koji does not let me create a side tag since I'm not an admin.

rebased onto 1c18004b50496fe3fd222d16255ffaa6ca4d1d2f

4 years ago

lsedlar commented 4 years ago

CC @pingou

pingou commented on line 50 of tests/test_request_side_tag.py 4 years ago

Maybe we should specify what 123 is in the output? (I see if from the mock line above, it's the side-tag's id, but as a user this may not be clear if you just have this line)

pingou commented 4 years ago

Are there any risk associated with the base tag? Could someone, say, by-pass CI by using a inappropriate base-tag?

lsedlar commented on line 50 of tests/test_request_side_tag.py 4 years ago

Good point, I'll add indication that it's ID of the tag – (id 123).

lsedlar commented 4 years ago

Are there any risk associated with the base tag? Could someone, say, by-pass CI by using a inappropriate base-tag?

I don't think so, the base tag is what is tested by the hub policy. That should make it possible to prevent any abuse by only allowing side tags from correct bases.

rebased onto 0493787d277feb73a74ec56d466401bec97eb0f4

4 years ago

onosek commented 4 years ago

A nice piece of code (:thumbsup:)

Edited 4 years ago by onosek

lsedlar commented 4 years ago

PR is updated to include list-side-tags and remove-side-tag commands.

rebased onto c997b0630353f6e6803e77e9350efb675fe3a44a

4 years ago

rebased onto 7d1c609

4 years ago

onosek commented 4 years ago

The updated code also looks cool.

lsedlar commented 4 years ago

The plugin in stage is now fixed:

# Running in a cloned package repo on f30 branch
$ fedpkg-stage request-side-tag
Side tag 'f30-build-side-7677' (id 7677) created.
Use 'fedpkg-stage build --target=f30-build-side-7677' to use it.
Use 'stg-koji wait-repo f30-build-side-7677' to wait for the build repo to be generated.
$ fedpkg-stage list-side-tags
f30-build-side-7669 (id 7669)
f30-build-side-7671 (id 7671)
f30-build-side-7677 (id 7677)
$ fedpkg-stage list-side-tags --mine
f30-build-side-7677 (id 7677)
$ fedpkg-stage remove-side-tag f30-build-side-7669
Could not execute remove_side_tag: This is not your sidetag
ERROR:pyrpkg:Could not execute remove_side_tag: This is not your sidetag
$ fedpkg-stage remove-side-tag f30-build-side-7677
Tag deleted.
$

I have not tested submitting any builds to the side tag, as the repo was not ready yet.

Pull-Request has been merged by lsedlar

4 years ago