Consider identifiers when retriggering (#2658)

Consider identifiers when retriggering

Related to #1886

Reviewed-by: Laura Barcziová
Reviewed-by: Matej Focko
Reviewed-by: Maja Massarini

alembic/versions/d625d6c1122f_add_identifier_to_copr_build.py

@@ -0,0 +1,31 @@
+"""Add identifier to copr build
+
+Revision ID: d625d6c1122f
+Revises: f69687c314c5
+Create Date: 2024-11-22 13:05:49.763917
+
+"""
+
+import sqlalchemy as sa
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "d625d6c1122f"
+down_revision = "f69687c314c5"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column(
+        "copr_build_targets", sa.Column("identifier", sa.String(), default="", nullable=True)
+    )
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column("copr_build_targets", "identifier")
+    # ### end Alembic commands ###

packit_service/models.py

@@ -162,10 +162,10 @@ def get_submitted_time_from_model(
 ) -> datetime:
     # TODO: unify `submitted_name` (or better -> create for both models `task_accepted_time`)
     # to delete this mess plz
-    if isinstance(model, CoprBuildTargetModel):
-        return model.build_submitted_time
-
-    return model.submitted_time
+    try:
+        return model.build_submitted_time  # type: ignore[union-attr]
+    except AttributeError:
+        return model.submitted_time  # type: ignore[union-attr]
 
 
 @overload
@@ -202,11 +202,11 @@ def get_most_recent_targets(
     for model in models:
         submitted_time_of_current_model = get_submitted_time_from_model(model)
         if (
-            most_recent_models.get(model.target) is None
-            or get_submitted_time_from_model(most_recent_models[model.target])
+            most_recent_models.get((model.target, model.identifier)) is None
+            or get_submitted_time_from_model(most_recent_models[(model.target, model.identifier)])
             < submitted_time_of_current_model
         ):
-            most_recent_models[model.target] = model
+            most_recent_models[(model.target, model.identifier)] = model
 
     return list(most_recent_models.values())
 
@@ -254,12 +254,14 @@ def filter_most_recent_target_names_by_status(
         Iterable["TFTTestRunTargetModel"],
     ],
     statuses_to_filter_with: list[str],
-) -> Optional[set[str]]:
+) -> Optional[set[tuple[str, str]]]:
     filtered_models = filter_most_recent_target_models_by_status(
         models,
         statuses_to_filter_with,
     )
-    return {model.target for model in filtered_models} if filtered_models else None
+    return (
+        {(model.target, model.identifier) for model in filtered_models} if filtered_models else None
+    )
 
 
 # https://github.com/python/mypy/issues/2477#issuecomment-313984522 ^_^
@@ -2102,6 +2104,8 @@ class CoprBuildTargetModel(GroupAndTargetModelConnector, Base):
 
     scan = relationship("OSHScanModel", back_populates="copr_build_target")
 
+    identifier = Column(String)
+
     def set_built_packages(self, built_packages):
         with sa_session_transaction(commit=True) as session:
             self.built_packages = built_packages
@@ -2297,6 +2301,7 @@ def create(
         status: BuildStatus,
         copr_build_group: "CoprBuildGroupModel",
         task_accepted_time: Optional[datetime] = None,
+        identifier: Optional[str] = None,
     ) -> "CoprBuildTargetModel":
         with sa_session_transaction(commit=True) as session:
             build = cls()
@@ -2307,6 +2312,7 @@ def create(
             build.web_url = web_url
             build.target = target
             build.task_accepted_time = task_accepted_time
+            build.identifier = identifier or ""
             session.add(build)
 
             copr_build_group.copr_build_targets.append(build)

packit_service/worker/events/comment.py

@@ -62,8 +62,8 @@ def __init__(
         comment_id: int,
         commit_sha: str = "",
         comment_object: Optional[Comment] = None,
-        build_targets_override: Optional[set[str]] = None,
-        tests_targets_override: Optional[set[str]] = None,
+        build_targets_override: Optional[set[tuple[str, str]]] = None,
+        tests_targets_override: Optional[set[tuple[str, str]]] = None,
     ) -> None:
         super().__init__(
             pr_id=pr_id,
@@ -93,7 +93,7 @@ def comment_object(self) -> Optional[Comment]:
         return self._comment_object
 
     @property
-    def build_targets_override(self) -> Optional[set[str]]:
+    def build_targets_override(self) -> Optional[set[tuple[str, str]]]:
         if not self._build_targets_override and "rebuild-failed" in self.comment:
             self._build_targets_override = (
                 super().get_all_build_targets_by_status(
@@ -104,7 +104,7 @@ def build_targets_override(self) -> Optional[set[str]]:
         return self._build_targets_override
 
     @property
-    def tests_targets_override(self) -> Optional[set[str]]:
+    def tests_targets_override(self) -> Optional[set[tuple[str, str]]]:
         if not self._tests_targets_override and "retest-failed" in self.comment:
             self._tests_targets_override = (
                 super().get_all_tf_targets_by_status(

packit_service/worker/events/event.py

@@ -72,8 +72,8 @@ def __init__(
         event_dict: Optional[dict],
         issue_id: Optional[int],
         task_accepted_time: Optional[datetime],
-        build_targets_override: Optional[list[str]],
-        tests_targets_override: Optional[list[str]],
+        build_targets_override: Optional[set[tuple[str, str]]],
+        tests_targets_override: Optional[set[tuple[str, str]]],
         branches_override: Optional[list[str]],
     ):
         self.event_type = event_type
@@ -119,8 +119,16 @@ def from_event_dict(cls, event: dict):
         time = event.get("task_accepted_time")
         task_accepted_time = datetime.fromtimestamp(time, timezone.utc) if time else None
 
-        build_targets_override = event.get("build_targets_override")
-        tests_targets_override = event.get("tests_targets_override")
+        build_targets_override = (
+            {(target, identifier_) for [target, identifier_] in event.get("build_targets_override")}
+            if event.get("build_targets_override")
+            else set()
+        )
+        tests_targets_override = (
+            {(target, identifier_) for [target, identifier_] in event.get("tests_targets_override")}
+            if event.get("tests_targets_override")
+            else set()
+        )
         branches_override = event.get("branches_override")
 
         return EventData(
@@ -508,17 +516,19 @@ def packages_config(self):
         raise NotImplementedError("Please implement me!")
 
     @property
-    def build_targets_override(self) -> Optional[set[str]]:
+    def build_targets_override(self) -> Optional[set[tuple[str, str]]]:
         """
-        Return the targets to use for building of the all targets from config
+        Return the targets and identifiers to use for building
+        of the all targets from config
         for the relevant events (e.g.rerunning of a single check).
         """
         return None
 
     @property
-    def tests_targets_override(self) -> Optional[set[str]]:
+    def tests_targets_override(self) -> Optional[set[tuple[str, str]]]:
         """
-        Return the targets to use for testing of the all targets from config
+        Return the targets and identifiers to use for testing
+        of the all targets from config
         for the relevant events (e.g.rerunning of a single check).
         """
         return None
@@ -638,34 +648,44 @@ def get_packages_config(self) -> Optional[PackageConfig]:
     def get_all_tf_targets_by_status(
         self,
         statuses_to_filter_with: list[str],
-    ) -> Optional[set[str]]:
+    ) -> Optional[set[tuple[str, str]]]:
         if self.commit_sha is None:
             return None
 
         logger.debug(
-            f"Getting failed Testing Farm targets for commit sha: {self.commit_sha}",
+            f"Getting Testing Farm targets for commit sha {self.commit_sha} "
+            f"and statuses {statuses_to_filter_with}",
         )
-        return filter_most_recent_target_names_by_status(
+        found_targets = filter_most_recent_target_names_by_status(
             models=TFTTestRunTargetModel.get_all_by_commit_target(
                 commit_sha=self.commit_sha,
             ),
             statuses_to_filter_with=statuses_to_filter_with,
         )
+        logger.debug(
+            f"Testing Farm found targets {found_targets}",
+        )
+        return found_targets
 
     def get_all_build_targets_by_status(
         self,
         statuses_to_filter_with: list[str],
-    ) -> Optional[set[str]]:
+    ) -> Optional[set[tuple[str, str]]]:
         if self.commit_sha is None or self.project.repo is None:
             return None
 
         logger.debug(
-            f"Getting failed COPR build targets for commit sha: {self.commit_sha}",
+            f"Getting COPR build targets for commit sha {self.commit_sha} "
+            f"and statuses {statuses_to_filter_with}",
         )
-        return filter_most_recent_target_names_by_status(
+        found_targets = filter_most_recent_target_names_by_status(
             models=CoprBuildTargetModel.get_all_by_commit(commit_sha=self.commit_sha),
             statuses_to_filter_with=statuses_to_filter_with,
         )
+        logger.debug(
+            f"Builds found targets {found_targets}",
+        )
+        return found_targets
 
     def get_non_serializable_attributes(self):
         return [

packit_service/worker/events/github.py

@@ -236,16 +236,24 @@ def __init__(
         )
         self.job_identifier = job_identifier
 
+    def _parse_target_and_identifier(self, target_string: str) -> tuple[str, str]:
+        """Parse target and identifier from check name target string."""
+        if ":" in target_string:
+            target, identifier = target_string.split(":")
+        else:
+            target, identifier = target_string, ""
+        return target, identifier
+
     @property
-    def build_targets_override(self) -> Optional[set[str]]:
+    def build_targets_override(self) -> Optional[set[tuple[str, str]]]:
         if self.check_name_job in {"rpm-build", "production-build", "koji-build"}:
-            return {self.check_name_target}
+            return {self._parse_target_and_identifier(self.check_name_target)}
         return None
 
     @property
-    def tests_targets_override(self) -> Optional[set[str]]:
+    def tests_targets_override(self) -> Optional[set[tuple[str, str]]]:
         if self.check_name_job == "testing-farm":
-            return {self.check_name_target}
+            return {self._parse_target_and_identifier(self.check_name_target)}
         return None
 
     @property

packit_service/worker/handlers/mixin.py

@@ -432,7 +432,7 @@ def copr_build_helper(self) -> CoprBuildJobHelper:
         # when reporting state of SRPM build built in Copr
         build_targets_override = (
             {
-                build.target
+                (build.target, build.identifier)
                 for build in CoprBuildTargetModel.get_all_by_build_id(
                     str(self.copr_event.build_id),
                 )

packit_service/worker/helpers/build/build_helper.py

@@ -53,8 +53,8 @@ def __init__(
         metadata: EventData,
         db_project_event: ProjectEventModel,
         job_config: JobConfig,
-        build_targets_override: Optional[set[str]] = None,
-        tests_targets_override: Optional[set[str]] = None,
+        build_targets_override: Optional[set[tuple[str, str]]] = None,
+        tests_targets_override: Optional[set[tuple[str, str]]] = None,
         pushgateway: Optional[Pushgateway] = None,
     ):
         super().__init__(
@@ -67,8 +67,8 @@ def __init__(
             pushgateway=pushgateway,
         )
         self.run_model: Optional[PipelineModel] = None
-        self.build_targets_override: Optional[set[str]] = build_targets_override
-        self.tests_targets_override: Optional[set[str]] = tests_targets_override
+        self.build_targets_override: Optional[set[tuple[str, str]]] = build_targets_override
+        self.tests_targets_override: Optional[set[tuple[str, str]]] = tests_targets_override
         self.pushgateway = pushgateway
 
         # lazy properties
@@ -199,7 +199,7 @@ def build_targets(self) -> set[str]:
         """
         if self.build_targets_override:
             logger.debug(f"Build targets override: {self.build_targets_override}")
-            return self.build_targets_all & self.build_targets_override
+            return self.build_targets_all & {target for target, _ in self.build_targets_override}
 
         return self.build_targets_all
 
@@ -270,13 +270,24 @@ def build_targets_for_test_job(self, test_job_config: JobConfig) -> set[str]:
 
         if self.build_targets_override:
             logger.debug(f"Build targets override: {self.build_targets_override}")
-            targets_override.update(self.build_targets_override)
+            targets_override.update(
+                [
+                    target
+                    for (target, identifier) in self.build_targets_override
+                    if identifier == (test_job_config.identifier or "")
+                ]
+            )
 
         if self.tests_targets_override:
             logger.debug(f"Test targets override: {self.tests_targets_override}")
             targets_override.update(
-                self.test_target2build_target_for_test_job(target, test_job_config)
-                for target in self.tests_targets_override
+                self.test_target2build_target_for_test_job(t, test_job_config)
+                for t in [
+                    target
+                    for (target, identifier) in self.tests_targets_override
+                    if identifier
+                    == (test_job_config.identifier if test_job_config.identifier else "")
+                ]
             )
 
         return configured_targets & targets_override if targets_override else configured_targets
@@ -314,14 +325,22 @@ def tests_targets_for_test_job(self, test_job_config: JobConfig) -> set[str]:
 
         if self.build_targets_override:
             logger.debug(f"Build targets override: {self.build_targets_override}")
-            for target in self.build_targets_override:
-                targets_override.update(
-                    self.build_target2test_targets_for_test_job(target, test_job_config),
-                )
+            for target, identifier in self.build_targets_override:
+                if identifier == (test_job_config.identifier if test_job_config.identifier else ""):
+                    targets_override.update(
+                        self.build_target2test_targets_for_test_job(target, test_job_config),
+                    )
 
         if self.tests_targets_override:
             logger.debug(f"Test targets override: {self.tests_targets_override}")
-            targets_override.update(self.tests_targets_override)
+            targets_override.update(
+                [
+                    target
+                    for target, identifier in self.tests_targets_override
+                    if identifier
+                    == (test_job_config.identifier if test_job_config.identifier else "")
+                ]
+            )
 
         return (
             configured_targets & targets_override

packit_service/worker/helpers/build/copr_build.py

@@ -80,8 +80,8 @@ def __init__(
         metadata: EventData,
         db_project_event: ProjectEventModel,
         job_config: JobConfig,
-        build_targets_override: Optional[set[str]] = None,
-        tests_targets_override: Optional[set[str]] = None,
+        build_targets_override: Optional[set[tuple[str, str]]] = None,
+        tests_targets_override: Optional[set[tuple[str, str]]] = None,
         pushgateway: Optional[Pushgateway] = None,
         celery_task: Optional[CeleryTask] = None,
         copr_build_group_id: Optional[int] = None,
@@ -605,6 +605,7 @@ def _get_or_create_build_group(self) -> CoprBuildGroupModel:
                 status=BuildStatus.waiting_for_srpm,
                 copr_build_group=group,
                 task_accepted_time=self.metadata.task_accepted_time,
+                identifier=self.job_config.identifier,
             )
 
         if unprocessed_chroots:

packit_service/worker/helpers/build/koji_build.py

@@ -47,8 +47,8 @@ def __init__(
         metadata: EventData,
         db_project_event: ProjectEventModel,
         job_config: JobConfig,
-        build_targets_override: Optional[set[str]] = None,
-        tests_targets_override: Optional[set[str]] = None,
+        build_targets_override: Optional[set[tuple[str, str]]] = None,
+        tests_targets_override: Optional[set[tuple[str, str]]] = None,
     ):
         super().__init__(
             service_config=service_config,