From 48a15fcb684f69a90f8fa6566ccff4df583abc56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Wed, 21 Aug 2024 16:36:33 +0200 Subject: [PATCH 1/3] Use new `KojiHelper` methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nikola Forró --- packit_service/worker/handlers/bodhi.py | 13 ++----------- packit_service/worker/handlers/mixin.py | 12 +++--------- tests/integration/test_bodhi_update.py | 18 ++++++++---------- tests/integration/test_issue_comment.py | 14 ++++---------- tests/integration/test_pr_comment.py | 3 +-- tests/unit/test_handler_mixin.py | 21 ++++++--------------- 6 files changed, 24 insertions(+), 57 deletions(-) diff --git a/packit_service/worker/handlers/bodhi.py b/packit_service/worker/handlers/bodhi.py index e5e9dcf8b..c9cd45e89 100644 --- a/packit_service/worker/handlers/bodhi.py +++ b/packit_service/worker/handlers/bodhi.py @@ -129,19 +129,10 @@ def get_builds_in_sidetag(self, dist_git_branch: str) -> Tuple[str, List[str]]: missing = dependencies - tagged_packages raise PackitException(f"Missing dependencies for Bodhi update: {missing}") - if not (candidate_tag := self.koji_helper.get_candidate_tag(dist_git_branch)): - raise PackitException(f"Failed to get candidate tag for {dist_git_branch}") - - stable_tags = self.koji_helper.get_stable_tags(candidate_tag) - nvrs = [] for package in dependencies: - latest_stable_nvr = max( - ( - self.koji_helper.get_latest_nvr_in_tag(package=package, tag=t) - for t in stable_tags + [candidate_tag] - ), - key=lambda nvr: NEVR.from_string(nvr), + latest_stable_nvr = self.koji_helper.get_latest_stable_nvr( + package, dist_git_branch, include_candidate=True ) latest_build_in_sidetag = max( (b for b in builds if b["package_name"] == package), diff --git a/packit_service/worker/handlers/mixin.py b/packit_service/worker/handlers/mixin.py index 27fcff433..49908f4c2 100644 --- a/packit_service/worker/handlers/mixin.py +++ b/packit_service/worker/handlers/mixin.py @@ -242,16 +242,10 @@ def koji_helper(self) -> KojiHelper: def _get_latest_build(self) -> dict: if not ( - candidate_tag := self.koji_helper.get_candidate_tag(self._dist_git_branch) - ): - raise PackitException( - f"Failed to get candidate tag for {self._dist_git_branch}" + build := self.koji_helper.get_latest_candidate_build( + self.project.repo, self._dist_git_branch ) - build = self.koji_helper.get_latest_build_in_tag( - package=self.project.repo, - tag=candidate_tag, - ) - if not build: + ): raise PackitException( f"No build found for package={self.project.repo} " f"and branch={self._dist_git_branch}" diff --git a/tests/integration/test_bodhi_update.py b/tests/integration/test_bodhi_update.py index 4b4e4c62e..74621c2ce 100644 --- a/tests/integration/test_bodhi_update.py +++ b/tests/integration/test_bodhi_update.py @@ -909,17 +909,15 @@ def test_bodhi_update_from_sidetag(koji_build_tagged, missing_dependency): sidetag_name ).and_return(builds_in_sidetag) - flexmock(KojiHelper).should_receive("get_candidate_tag").with_args( - "f40" - ).and_return("f40-updates-candidate") - flexmock(KojiHelper).should_receive("get_stable_tags").with_args( - "f40-updates-candidate" - ).and_return(["f40-updates", "f40"]) - flexmock(KojiHelper).should_receive("get_latest_nvr_in_tag").with_args( - package="python-specfile", tag=str + flexmock(KojiHelper).should_receive("get_latest_stable_nvr").with_args( + "python-specfile", + "f40", + include_candidate=True, ).and_return("python-specfile-0.30.0-1.fc40") - flexmock(KojiHelper).should_receive("get_latest_nvr_in_tag").with_args( - package="packit", tag=str + flexmock(KojiHelper).should_receive("get_latest_stable_nvr").with_args( + "packit", + "f40", + include_candidate=True, ).and_return("packit-0.98.0-1.fc40") flexmock(KojiHelper).should_receive("remove_sidetag").with_args(sidetag_name).times( 0 if missing_dependency else 1 diff --git a/tests/integration/test_issue_comment.py b/tests/integration/test_issue_comment.py index 33e11e04e..5129f717c 100644 --- a/tests/integration/test_issue_comment.py +++ b/tests/integration/test_issue_comment.py @@ -420,11 +420,8 @@ def test_issue_comment_retrigger_bodhi_update_handler( koji_builds=["python-teamcity-messages.fc38"], sidetag=None, ).and_return(("alias", "url")) - flexmock(KojiHelper).should_receive("get_candidate_tag").with_args( - "f38" - ).and_return("f38-updates-candidate") - flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( - package="python-teamcity-messages", tag="f38-updates-candidate" + flexmock(KojiHelper).should_receive("get_latest_candidate_build").with_args( + "python-teamcity-messages", "f38" ).and_return( { "nvr": "python-teamcity-messages.fc38", @@ -439,11 +436,8 @@ def test_issue_comment_retrigger_bodhi_update_handler( koji_builds=["python-teamcity-messages.fc37"], sidetag=None, ).and_return(("alias", "url")) - flexmock(KojiHelper).should_receive("get_candidate_tag").with_args( - "f37" - ).and_return("f37-updates-candidate") - flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( - package="python-teamcity-messages", tag="f37-updates-candidate" + flexmock(KojiHelper).should_receive("get_latest_candidate_build").with_args( + "python-teamcity-messages", "f37" ).and_return( { "nvr": "python-teamcity-messages.fc37", diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index b61c9dc52..e88146ac8 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -2545,8 +2545,7 @@ def test_bodhi_update_retrigger_via_dist_git_pr_comment(pagure_pr_comment_added) get_pr=lambda id: pr_mock, ) - flexmock(KojiHelper).should_receive("get_candidate_tag").and_return("tag") - flexmock(KojiHelper).should_receive("get_latest_build_in_tag").and_return( + flexmock(KojiHelper).should_receive("get_latest_candidate_build").and_return( {"nvr": "123", "build_id": 321, "state": 0, "task_id": 123} ) diff --git a/tests/unit/test_handler_mixin.py b/tests/unit/test_handler_mixin.py index 5ad0722da..d82ab576b 100644 --- a/tests/unit/test_handler_mixin.py +++ b/tests/unit/test_handler_mixin.py @@ -23,11 +23,8 @@ def __init__(self): ) self.data = flexmock(pr_id="123") - flexmock(KojiHelper).should_receive("get_candidate_tag").with_args( - "a_branch" - ).and_return("a_tag") - flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( - package="a_repo", tag="a_tag" + flexmock(KojiHelper).should_receive("get_latest_candidate_build").with_args( + "a_repo", "a_branch" ).and_return({"nvr": "1.0.0", "state": 1, "build_id": 123, "task_id": 321}) mixin = Test() data = [] @@ -88,17 +85,11 @@ def project_url(self) -> str: def branches(self): return ["f37", "f38"] - flexmock(KojiHelper).should_receive("get_candidate_tag").with_args( - "f37" - ).and_return("f37-updates-candidate") - flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( - package="a repo", tag="f37-updates-candidate" + flexmock(KojiHelper).should_receive("get_latest_candidate_build").with_args( + "a repo", "f37" ).and_return({"nvr": "1.0.1", "state": 1, "build_id": 123, "task_id": 321}) - flexmock(KojiHelper).should_receive("get_candidate_tag").with_args( - "f38" - ).and_return("f38-updates-candidate") - flexmock(KojiHelper).should_receive("get_latest_build_in_tag").with_args( - package="a repo", tag="f38-updates-candidate" + flexmock(KojiHelper).should_receive("get_latest_candidate_build").with_args( + "a repo", "f38" ).and_return({"nvr": "1.0.2", "state": 1, "build_id": 1234, "task_id": 4321}) mixin = Test() From 8a3d3ee145b3aaef8a308474652cb587abae692c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Wed, 21 Aug 2024 16:47:11 +0200 Subject: [PATCH 2/3] Handle `/packit koji-tag` dist-git PR comment command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nikola Forró --- packit_service/worker/checker/distgit.py | 23 +++- packit_service/worker/handlers/abstract.py | 1 + packit_service/worker/handlers/distgit.py | 81 ++++++++++++ packit_service/worker/tasks.py | 14 ++ tests/integration/test_pr_comment.py | 144 ++++++++++++++++++++- 5 files changed, 255 insertions(+), 8 deletions(-) diff --git a/packit_service/worker/checker/distgit.py b/packit_service/worker/checker/distgit.py index 0553280c9..1df36cf19 100644 --- a/packit_service/worker/checker/distgit.py +++ b/packit_service/worker/checker/distgit.py @@ -6,7 +6,10 @@ from packit.config.aliases import get_branches from packit_service.constants import MSG_GET_IN_TOUCH -from packit_service.utils import pr_labels_match_configuration +from packit_service.utils import ( + pr_labels_match_configuration, + get_packit_commands_from_comment, +) from packit_service.worker.checker.abstract import Checker, ActorChecker from packit_service.worker.checker.helper import DistgitAccountsChecker from packit_service.worker.events import ( @@ -93,24 +96,30 @@ def pre_check(self) -> bool: ) return False elif self.data.event_type in (PullRequestCommentPagureEvent.__name__,): + comment = self.data.event_dict.get("comment", "") + commands = get_packit_commands_from_comment( + comment, self.service_config.comment_command_prefix + ) + command = commands[0] if commands else "" commenter = self.data.actor logger.debug( - f"Triggering downstream koji build through comment by: {commenter}" + f"{'Tagging' if command == 'koji-tag' else 'Triggering'} " + f"downstream koji build through comment by: {commenter}" ) if not self.is_packager(commenter): msg = ( - f"koji-build retriggering through comment " - f"on PR identifier {self.data.pr_id} " + f"koji-build {'tagging' if command == 'koji-tag' else 'retriggering'} " + f"through comment on PR identifier {self.data.pr_id} " f"and project {self.data.project_url} " - f"done by {commenter} which is not a packager." + f"done by {commenter} who is not a packager." ) logger.info(msg) report_in_issue_repository( issue_repository=self.job_config.issue_repository, service_config=self.service_config, title=( - "Re-triggering downstream koji build " - "through comment in dist-git PR failed" + f"{'Tagging' if command == 'koji-tag' else 'Re-triggering'} " + "downstream koji build through comment in dist-git PR failed" ), message=msg + MSG_GET_IN_TOUCH, comment_to_existing=msg, diff --git a/packit_service/worker/handlers/abstract.py b/packit_service/worker/handlers/abstract.py index 8dacd7c84..aa894ea12 100644 --- a/packit_service/worker/handlers/abstract.py +++ b/packit_service/worker/handlers/abstract.py @@ -190,6 +190,7 @@ class TaskName(str, enum.Enum): pull_from_upstream = "task.pull_from_upstream" check_onboarded_projects = "task.check_onboarded_projects" koji_build_tag = "task.koji_build_tag" + tag_into_sidetag = "task.tag_into_sidetag" class Handler(PackitAPIProtocol, Config): diff --git a/packit_service/worker/handlers/distgit.py b/packit_service/worker/handlers/distgit.py index 1383d30c8..b35c4bfbc 100644 --- a/packit_service/worker/handlers/distgit.py +++ b/packit_service/worker/handlers/distgit.py @@ -16,6 +16,7 @@ from ogr.abstract import PullRequest, AuthMethod from ogr.services.github import GithubService from packit.config import JobConfig, JobType, Deployment +from packit.config import aliases from packit.config.package_config import PackageConfig from packit.exceptions import ( PackitException, @@ -1015,3 +1016,83 @@ def get_trigger_type_description(self) -> str: f"Fedora Koji build was re-triggered " f"by comment in issue {self.data.issue_id}." ) + + +@configured_as(job_type=JobType.koji_build) +@run_for_comment(command="koji-tag") +@reacts_to(event=PullRequestCommentPagureEvent) +class TagIntoSidetagHandler( + RetriableJobHandler, + ConfigFromEventMixin, + PackitAPIWithDownstreamMixin, + GetPagurePullRequestMixin, +): + task_name = TaskName.tag_into_sidetag + + _koji_helper: Optional[KojiHelper] = None + + @property + def koji_helper(self): + if not self._koji_helper: + self._koji_helper = KojiHelper() + return self._koji_helper + + @staticmethod + def get_checkers() -> Tuple[Type[Checker], ...]: + return (PermissionOnDistgit,) + + def run_for_branch(self, job_config: JobConfig, branch: str) -> None: + sidetag_group = SidetagGroupModel.get_or_create(job_config.sidetag_group) + sidetag = SidetagModel.get_or_create(sidetag_group, branch) + # we need Kerberos ticket to tag a build into sidetag + # and to create a new sidetag (if needed) + self.packit_api.init_kerberos_ticket() + if not sidetag.koji_name or not self.koji_helper.get_tag_info( + sidetag.koji_name + ): + tag_info = self.koji_helper.create_sidetag(branch) + if not tag_info: + logger.error(f"Failed to create sidetag for {branch}") + return + sidetag.set_koji_name(tag_info["name"]) + if not ( + nvr := self.koji_helper.get_latest_stable_nvr( + job_config.downstream_package_name, branch + ) + ): + logger.debug( + "Failed to get the latest stable build " + f"of {job_config.downstream_package_name} for {branch}" + ) + return + logger.debug(f"Tagging {nvr} into {sidetag.koji_name}") + self.koji_helper.tag_build(nvr, sidetag.koji_name) + + def run(self) -> TaskResults: + comment = self.data.event_dict.get("comment") + commands = get_packit_commands_from_comment( + comment, self.service_config.comment_command_prefix + ) + args = commands[1:] if len(commands) > 1 else "" + for job in self.package_config.get_job_views(): + if ( + job.type == JobType.koji_build + and job.sidetag_group + and job.dist_git_branches + ): + configured_branches = aliases.get_branches( + *job.dist_git_branches, default_dg_branch="rawhide" + ) + if "--all-branches" in args: + branches = configured_branches + elif self.pull_request.target_branch not in configured_branches: + continue + else: + branches = {self.pull_request.target_branch} + logger.debug( + "Running downstream Koji build tagging " + f"of {job.downstream_package_name} for {branches}" + ) + for branch in branches: + self.run_for_branch(job, branch) + return TaskResults(success=True, details={}) diff --git a/packit_service/worker/tasks.py b/packit_service/worker/tasks.py index a6bfe1490..26a82a50d 100644 --- a/packit_service/worker/tasks.py +++ b/packit_service/worker/tasks.py @@ -71,6 +71,7 @@ DownstreamKojiBuildHandler, RetriggerDownstreamKojiBuildHandler, PullFromUpstreamHandler, + TagIntoSidetagHandler, ) from packit_service.worker.handlers.forges import GithubFasVerificationHandler from packit_service.worker.handlers.koji import ( @@ -594,6 +595,19 @@ def run_koji_build_tag_handler(event: dict, package_config: dict, job_config: di return get_handlers_task_results(handler.run_job(), event) +@celery_app.task(bind=True, name=TaskName.tag_into_sidetag, base=TaskWithRetry) +def run_tag_into_sidetag_handler( + self, event: dict, package_config: dict, job_config: dict +): + handler = TagIntoSidetagHandler( + package_config=load_package_config(package_config), + job_config=load_job_config(job_config), + event=event, + celery_task=self, + ) + return get_handlers_task_results(handler.run_job(), event) + + def get_handlers_task_results(results: dict, event: dict) -> dict: # include original event to provide more info return {"job": results, "event": event} diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index e88146ac8..71813569b 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -56,6 +56,8 @@ BodhiUpdateGroupModel, BodhiUpdateTargetModel, SyncReleasePullRequestModel, + SidetagGroupModel, + SidetagModel, ) from packit_service.service.db_project_events import AddPullRequestEventToDb from packit_service.utils import ( @@ -68,7 +70,11 @@ from packit_service.worker.handlers.bodhi import ( RetriggerBodhiUpdateHandler, ) -from packit_service.worker.handlers.distgit import DownstreamKojiBuildHandler +from packit_service.worker.handlers.distgit import ( + DownstreamKojiBuildHandler, + TagIntoSidetagHandler, + aliases, +) from packit_service.worker.helpers.build.copr_build import CoprBuildJobHelper from packit_service.worker.helpers.build.koji_build import KojiBuildJobHelper from packit_service.worker.helpers.testing_farm import TestingFarmJobHelper @@ -85,6 +91,7 @@ run_testing_farm_handler, run_pull_from_upstream_handler, run_downstream_koji_build, + run_tag_into_sidetag_handler, ) from tests.spellbook import DATA_DIR, first_dict_value, get_parameters_from_results @@ -2944,3 +2951,138 @@ def _get_project(url, *_, **__): job_config=job_config, ) assert first_dict_value(results["job"])["success"] + + +@pytest.mark.parametrize( + "all_branches", + [False, True], +) +def test_koji_build_tag_via_dist_git_pr_comment(pagure_pr_comment_added, all_branches): + packit_yaml = ( + "{'specfile_path': 'python-teamcity-messages.spec', 'synced_files': []," + "'jobs': [{'trigger': 'commit', 'job': 'koji_build', 'sidetag_group': 'test'," + "'dist_git_branches': ['fedora-stable']}]," + "'downstream_package_name': 'python-ogr', 'issue_repository': " + "'https://github.com/namespace/repo'}" + ) + pagure_project = flexmock( + PagureProject, + full_repo_name="rpms/packit", + get_web_url=lambda: "https://src.fedoraproject.org/rpms/python-teamcity-messages", + default_branch="main", + ) + pagure_project.should_receive("get_files").with_args( + ref="main", filter_regex=r".+\.spec$" + ).and_return(["python-teamcity-messages.spec"]) + pagure_project.should_receive("get_file_content").with_args( + path=".packit.yaml", ref="main" + ).and_return(packit_yaml) + pagure_project.should_receive("get_files").with_args( + ref="main", recursive=False + ).and_return(["python-teamcity-messages.spec", ".packit.yaml"]) + + pagure_pr_comment_added["pullrequest"]["comments"][0]["comment"] = ( + "/packit koji-tag" + (" --all-branches" if all_branches else "") + ) + + project_event = flexmock( + job_config_trigger_type=JobConfigTriggerType.pull_request, id=123 + ) + flexmock(AddPullRequestEventToDb).should_receive("db_project_object").and_return( + project_event + ) + flexmock(PullRequestModel).should_receive("get_by_id").with_args(123).and_return( + project_event + ) + flexmock(PipelineModel).should_receive("create") + + db_project_object = flexmock( + id=12, + project_event_model_type=ProjectEventModelType.pull_request, + job_config_trigger_type=JobConfigTriggerType.pull_request, + ) + db_project_event = ( + flexmock() + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() + ) + flexmock(ProjectEventModel).should_receive("get_or_create").with_args( + type=ProjectEventModelType.pull_request, + event_id=12, + commit_sha="beaf90bcecc51968a46663f8d6f092bfdc92e682", + ).and_return(db_project_event) + flexmock(PullRequestModel).should_receive("get_or_create").with_args( + pr_id=pagure_pr_comment_added["pullrequest"]["id"], + namespace=pagure_pr_comment_added["pullrequest"]["project"]["namespace"], + repo_name=pagure_pr_comment_added["pullrequest"]["project"]["name"], + project_url=pagure_pr_comment_added["pullrequest"]["project"]["full_url"], + ).and_return(db_project_object) + + pr_mock = ( + flexmock(target_branch="f40") + .should_receive("comment") + .with_args("The task was accepted.") + .mock() + ) + flexmock( + PagureProject, + full_repo_name="rpms/jouduv-dort", + get_web_url=lambda: "https://src.fedoraproject.org/rpms/jouduv-dort", + default_branch="main", + get_pr=lambda id: pr_mock, + ) + + flexmock(TagIntoSidetagHandler).should_receive("pre_check").and_return(True) + flexmock(celery_group).should_receive("apply_async").once() + + flexmock(PackitAPI).should_receive("init_kerberos_ticket").and_return() + flexmock(aliases).should_receive("get_branches").with_args( + "fedora-stable", default_dg_branch="rawhide" + ).and_return({"f39", "f40"}) + + sidetag_group = flexmock() + flexmock(SidetagGroupModel).should_receive("get_or_create").with_args( + "test" + ).and_return(sidetag_group) + flexmock(SidetagModel).should_receive("get_or_create").with_args( + sidetag_group, "f39" + ).and_return(flexmock(koji_name="f39-build-side-12345")) + flexmock(SidetagModel).should_receive("get_or_create").with_args( + sidetag_group, "f40" + ).and_return(flexmock(koji_name="f40-build-side-12345")) + flexmock(KojiHelper).should_receive("get_tag_info").with_args( + "f39-build-side-12345" + ).and_return(flexmock()) + flexmock(KojiHelper).should_receive("get_tag_info").with_args( + "f40-build-side-12345" + ).and_return(flexmock()) + flexmock(KojiHelper).should_receive("get_latest_stable_nvr").with_args( + "python-ogr", "f39" + ).and_return("python-ogr-0.1-1.fc39") + flexmock(KojiHelper).should_receive("get_latest_stable_nvr").with_args( + "python-ogr", "f40" + ).and_return("python-ogr-0.1-1.fc40") + + if all_branches: + flexmock(KojiHelper).should_receive("tag_build").with_args( + "python-ogr-0.1-1.fc39", "f39-build-side-12345" + ).once() + flexmock(KojiHelper).should_receive("tag_build").with_args( + "python-ogr-0.1-1.fc40", "f40-build-side-12345" + ).once() + + flexmock(Pushgateway).should_receive("push").times(2).and_return() + + processing_results = SteveJobs().process_message(pagure_pr_comment_added) + event_dict, job, job_config, package_config = get_parameters_from_results( + processing_results + ) + assert json.dumps(event_dict) + results = run_tag_into_sidetag_handler( + event=event_dict, + package_config=package_config, + job_config=job_config, + ) + + assert first_dict_value(results["job"])["success"] From 3fa84a23a595e4ad3d6ddc0b9da049f150691313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Tue, 27 Aug 2024 15:20:52 +0200 Subject: [PATCH 3/3] Use types-setuptools in mypy pre-commit hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nikola Forró --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2fee2bbec..cc8fe501a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -61,7 +61,7 @@ repos: additional_dependencies: [ types-jwt, - types-pkg_resources, + types-setuptools, types-redis, types-requests, types-Flask,