Skip running Koji builds if they are triggered already (#2559)

Skip running Koji builds if they are triggered already

Fixes #2503
Requires packit/packit#2435

TODO:

 test it

RELEASE NOTES BEGIN
Before triggering the non-scratch Koji builds, we now check whether there is not already a build in progress or completed for the same NVR.
RELEASE NOTES END

Reviewed-by: Nikola Forró
Reviewed-by: Laura Barcziová

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")
 

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

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,

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

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()