From 6fe5965ff4421ba9967a9dce996a092816f08d11 Mon Sep 17 00:00:00 2001 From: Laura Barcziova Date: Fri, 4 Oct 2024 07:26:21 +0200 Subject: [PATCH] Skip running Koji builds if they are triggered already Fixes #2503 Requires packit/packit#2435 --- packit_service/worker/handlers/distgit.py | 39 ++++++++++++++++++ tests/integration/test_dg_commit.py | 15 +++++++ tests/integration/test_issue_comment.py | 6 +++ tests/integration/test_koji_build.py | 50 ++++++++++++++++++----- tests/integration/test_pr_comment.py | 3 ++ 5 files changed, 102 insertions(+), 11 deletions(-) diff --git a/packit_service/worker/handlers/distgit.py b/packit_service/worker/handlers/distgit.py index 63fa572e2..5379ee3c9 100644 --- a/packit_service/worker/handlers/distgit.py +++ b/packit_service/worker/handlers/distgit.py @@ -25,6 +25,7 @@ PackitDownloadFailedException, ReleaseSkippedPackitException, ) +from packit.utils.koji_helper import KojiHelper from packit_service import sentry_integration from packit_service.config import PackageConfigGetter, ServiceConfig from packit_service.constants import ( @@ -35,6 +36,7 @@ DEFAULT_RETRY_BACKOFF, MSG_RETRIGGER_DISTGIT, RETRY_LIMIT_RELEASE_ARCHIVE_DOWNLOAD_ERROR, + KojiBuildState, ) from packit_service.models import ( SyncReleasePullRequestModel, @@ -737,6 +739,8 @@ class AbstractDownstreamKojiBuildHandler( topic = "org.fedoraproject.prod.pagure.git.receive" task_name = TaskName.downstream_koji_build + _koji_helper: Optional[KojiHelper] = None + def __init__( self, package_config: PackageConfig, @@ -756,6 +760,12 @@ def __init__( self._packit_api = None self._koji_group_model_id = koji_group_model_id + @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 ( @@ -791,6 +801,27 @@ def _get_or_create_koji_group_model(self) -> KojiBuildGroupModel: def get_branches(self) -> List[str]: """Get a list of branch (names) to be built in koji""" + def is_already_triggered(self, branch: str) -> bool: + """ + Check if the build was already triggered + (building or completed state). + """ + nvr = self.packit_api.dg.get_nvr(branch) + existing_build = self.koji_helper.get_build_info(nvr) + + if existing_build: + raw_state = existing_build["state"] + if (state := KojiBuildState.from_number(raw_state)) in ( + KojiBuildState.building, + KojiBuildState.complete, + ): + logger.debug( + f"Koji build with matching NVR ({nvr}) found with state {state}" + ) + return True + + return False + def run(self) -> TaskResults: errors = {} @@ -806,6 +837,14 @@ def run(self) -> TaskResults: ) continue + if not self.job_config.scratch and self.is_already_triggered(branch): + logger.info( + f"Skipping downstream Koji build for branch {branch} " + f"that was already triggered." + ) + koji_build_model.set_status("skipped") + continue + logger.debug(f"Running downstream Koji build for {branch}") koji_build_model.set_status("pending") diff --git a/tests/integration/test_dg_commit.py b/tests/integration/test_dg_commit.py index aaab3c7b1..d3ad053be 100644 --- a/tests/integration/test_dg_commit.py +++ b/tests/integration/test_dg_commit.py @@ -286,6 +286,9 @@ def test_downstream_koji_build(sidetag_group): from_upstream=False, koji_target=koji_target if sidetag_group else None, ).and_return("") + flexmock(DownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) processing_results = SteveJobs().process_message(distgit_commit_event()) event_dict, job, job_config, package_config = get_parameters_from_results( processing_results @@ -373,6 +376,9 @@ def test_downstream_koji_build_failure_no_issue(): from_upstream=False, koji_target=None, ).and_raise(PackitException, "Some error") + flexmock(DownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) pagure_project_mock.should_receive("get_issue_list").times(0) pagure_project_mock.should_receive("create_issue").times(0) @@ -463,6 +469,9 @@ def test_downstream_koji_build_failure_issue_created(): from_upstream=False, koji_target=None, ).and_raise(PackitException, "Some error") + flexmock(DownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) issue_project_mock = flexmock(GithubProject) issue_project_mock.should_receive("get_issue_list").and_return([]).once() @@ -561,6 +570,9 @@ def test_downstream_koji_build_failure_issue_comment(): from_upstream=False, koji_target=None, ).and_raise(PackitException, "Some error") + flexmock(DownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) issue_project_mock = flexmock(GithubProject) issue_project_mock.should_receive("get_issue_list").and_return( @@ -747,6 +759,9 @@ def test_downstream_koji_build_where_multiple_branches_defined(jobs_config): from_upstream=False, koji_target=None, ).once().and_return("") + flexmock(DownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) processing_results = SteveJobs().process_message(distgit_commit_event()) assert len(processing_results) == 1 diff --git a/tests/integration/test_issue_comment.py b/tests/integration/test_issue_comment.py index d59e814a3..24af676a4 100644 --- a/tests/integration/test_issue_comment.py +++ b/tests/integration/test_issue_comment.py @@ -542,6 +542,9 @@ def test_issue_comment_retrigger_koji_build_handler( flexmock(RetriggerDownstreamKojiBuildHandler).should_receive( "local_project" ).and_return(flexmock()) + flexmock(RetriggerDownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) results = run_retrigger_downstream_koji_build( package_config=package_config, @@ -605,6 +608,9 @@ def test_issue_comment_retrigger_koji_build_error_msg( message=msg, comment_to_existing=msg, ).once() + flexmock(RetriggerDownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) run_retrigger_downstream_koji_build( package_config=package_config, diff --git a/tests/integration/test_koji_build.py b/tests/integration/test_koji_build.py index be92d3d8b..4834f79e6 100644 --- a/tests/integration/test_koji_build.py +++ b/tests/integration/test_koji_build.py @@ -9,8 +9,10 @@ from ogr.services.github import GithubProject from ogr.services.pagure import PagureProject -from packit.exceptions import PackitException +from packit.api import PackitAPI from packit.config import JobConfigTriggerType +from packit.exceptions import PackitException +from packit.utils.koji_helper import KojiHelper from packit_service.config import PackageConfigGetter from packit_service.models import ( GitBranchModel, @@ -19,22 +21,22 @@ ProjectEventModel, KojiBuildGroupModel, ) -from packit_service.worker.jobs import SteveJobs -from packit_service.worker.monitoring import Pushgateway -from packit_service.worker.celery_task import CeleryTask -from packit_service.worker.tasks import ( - run_downstream_koji_build, - run_downstream_koji_build_report, -) -from packit_service.worker.handlers.distgit import ( - DownstreamKojiBuildHandler, -) from packit_service.models import ( ProjectEventModelType, ) +from packit_service.worker.celery_task import CeleryTask from packit_service.worker.events.pagure import ( PushPagureEvent, ) +from packit_service.worker.handlers.distgit import ( + DownstreamKojiBuildHandler, +) +from packit_service.worker.jobs import SteveJobs +from packit_service.worker.monitoring import Pushgateway +from packit_service.worker.tasks import ( + run_downstream_koji_build, + run_downstream_koji_build_report, +) from tests.conftest import koji_build_completed_rawhide, koji_build_start_rawhide from tests.spellbook import first_dict_value, get_parameters_from_results @@ -216,9 +218,35 @@ def test_koji_build_error_msg(distgit_push_packit): message=msg, comment_to_existing=msg, ).once() + flexmock(DownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) run_downstream_koji_build( package_config=package_config, event=event_dict, job_config=job_config, ) + + +@pytest.mark.parametrize( + "build_info, result", + [ + (None, False), + ({"state": 0}, True), + ({"state": 1}, True), + ({"state": 2}, False), + ], +) +def test_is_already_triggered(build_info, result): + flexmock(PackitAPI).should_receive("dg").and_return( + flexmock(get_nvr=lambda branch: "some-nvr") + ) + flexmock(KojiHelper).should_receive("get_build_info").and_return(build_info) + + DownstreamKojiBuildHandler( + package_config=flexmock(), + job_config=flexmock(), + event={}, + celery_task=flexmock(), + ).is_already_triggered("rawhide") is result diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index 93fbe21ad..5ca205156 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -2434,6 +2434,9 @@ def test_koji_build_retrigger_via_dist_git_pr_comment(pagure_pr_comment_added): ) flexmock(DownstreamKojiBuildHandler).should_receive("pre_check").and_return(True) + flexmock(DownstreamKojiBuildHandler).should_receive( + "is_already_triggered" + ).and_return(False) flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: flexmock()) flexmock(LocalProject, refresh_the_arguments=lambda: None) flexmock(celery_group).should_receive("apply_async").once()