Skip to content

Commit bda8b64

Browse files
committed
Fix multi-release retries
Signed-off-by: Nikola Forró <nforro@redhat.com> Assisted-by: Claude Opus 4.6 via Claude Code
1 parent 379f24d commit bda8b64

3 files changed

Lines changed: 252 additions & 3 deletions

File tree

packit_service/worker/handlers/distgit.py

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,8 @@ def __init__(
238238
event: dict,
239239
celery_task: Task,
240240
sync_release_run_id: Optional[int] = None,
241+
retry_tag: Optional[str] = None,
242+
retry_version: Optional[str] = None,
241243
):
242244
super().__init__(
243245
package_config=package_config,
@@ -247,6 +249,8 @@ def __init__(
247249
)
248250
self._project_url = self.data.project_url
249251
self._sync_release_run_id = sync_release_run_id
252+
self._retry_tag = retry_tag
253+
self._retry_version = retry_version
250254
self.helper: Optional[SyncReleaseHelper] = None
251255

252256
@property
@@ -316,6 +320,8 @@ def sync_branch(
316320
# is not retried also automatically
317321
kargs = self.celery_task.task.request.kwargs.copy()
318322
kargs["sync_release_run_id"] = model.id
323+
kargs["retry_tag"] = tag
324+
kargs["retry_version"] = version
319325
# https://docs.celeryq.dev/en/stable/userguide/tasks.html#retrying
320326
# https://docs.celeryq.dev/en/stable/reference/celery.app.task.html#celery.app.task.Task.retry
321327
self.celery_task.task.retry(
@@ -344,8 +350,30 @@ def sync_branch(
344350

345351
return downstream_pr, additional_prs
346352

347-
def _get_or_create_sync_release_run(self, project_event_model=None) -> SyncReleaseModel:
348-
if self._sync_release_run_id is not None:
353+
def _get_or_create_sync_release_run(
354+
self,
355+
project_event_model: Optional[ProjectEventModel] = None,
356+
tag: Optional[str] = None,
357+
version: Optional[str] = None,
358+
) -> SyncReleaseModel:
359+
"""Get the existing sync release run model if retrying, or create a new one.
360+
361+
On retry, the existing model is reused only when the tag and version
362+
match the retry parameters, so that other versions in a multi-version
363+
run get their own models.
364+
365+
Args:
366+
project_event_model: The project event model associated with the run.
367+
tag: The tag of the release being synced.
368+
version: The version of the release being synced.
369+
370+
Returns:
371+
The SyncReleaseModel representing the run.
372+
"""
373+
if self._sync_release_run_id is not None and (
374+
(self._retry_tag is None and self._retry_version is None)
375+
or (tag == self._retry_tag and version == self._retry_version)
376+
):
349377
return SyncReleaseModel.get_by_id(self._sync_release_run_id)
350378

351379
sync_release_model, _ = SyncReleaseModel.create_with_new_run(
@@ -605,7 +633,9 @@ def _run_for_release(
605633
if tag and self.data.event_type == anitya.NewHotness.event_type()
606634
else None
607635
)
608-
sync_release_run_model = self._get_or_create_sync_release_run(release_event)
636+
sync_release_run_model = self._get_or_create_sync_release_run(
637+
release_event, tag=tag, version=version
638+
)
609639
branches_to_run = [target.branch for target in sync_release_run_model.sync_release_targets]
610640
logger.debug(
611641
f"Branches to run {self.job_config.type} "
@@ -720,13 +750,17 @@ def __init__(
720750
event: dict,
721751
celery_task: Task,
722752
sync_release_run_id: Optional[int] = None,
753+
retry_tag: Optional[str] = None,
754+
retry_version: Optional[str] = None,
723755
):
724756
super().__init__(
725757
package_config=package_config,
726758
job_config=job_config,
727759
event=event,
728760
celery_task=celery_task,
729761
sync_release_run_id=sync_release_run_id,
762+
retry_tag=retry_tag,
763+
retry_version=retry_version,
730764
)
731765

732766
@staticmethod
@@ -782,13 +816,17 @@ def __init__(
782816
event: dict,
783817
celery_task: Task,
784818
sync_release_run_id: Optional[int] = None,
819+
retry_tag: Optional[str] = None,
820+
retry_version: Optional[str] = None,
785821
):
786822
super().__init__(
787823
package_config=package_config,
788824
job_config=job_config,
789825
event=event,
790826
celery_task=celery_task,
791827
sync_release_run_id=sync_release_run_id,
828+
retry_tag=retry_tag,
829+
retry_version=retry_version,
792830
)
793831
if self.data.event_type in (pagure.pr.Comment.event_type(),):
794832
# use upstream project URL when retriggering from dist-git PR

packit_service/worker/tasks.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,12 +469,16 @@ def run_propose_downstream_handler(
469469
package_config: dict,
470470
job_config: dict,
471471
sync_release_run_id: Optional[int] = None,
472+
retry_tag: Optional[str] = None,
473+
retry_version: Optional[str] = None,
472474
):
473475
handler = ProposeDownstreamHandler(
474476
package_config=load_package_config(package_config),
475477
job_config=load_job_config(job_config),
476478
event=event,
477479
sync_release_run_id=sync_release_run_id,
480+
retry_tag=retry_tag,
481+
retry_version=retry_version,
478482
celery_task=self,
479483
)
480484
return get_handlers_task_results(handler.run_job(), event)
@@ -494,12 +498,16 @@ def run_pull_from_upstream_handler(
494498
package_config: dict,
495499
job_config: dict,
496500
sync_release_run_id: Optional[int] = None,
501+
retry_tag: Optional[str] = None,
502+
retry_version: Optional[str] = None,
497503
):
498504
handler = PullFromUpstreamHandler(
499505
package_config=load_package_config(package_config),
500506
job_config=load_job_config(job_config),
501507
event=event,
502508
sync_release_run_id=sync_release_run_id,
509+
retry_tag=retry_tag,
510+
retry_version=retry_version,
503511
celery_task=self,
504512
)
505513
return get_handlers_task_results(handler.run_job(), event)

tests/integration/test_new_hotness_update.py

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,209 @@ class AnityaTestProjectModel(AnityaProjectModel):
579579
assert first_dict_value(results["job"])["success"]
580580

581581

582+
def test_retry_pull_from_upstream_multi_version(new_hotness_update):
583+
"""On retry with multiple versions, only the failed version should reuse its
584+
SyncReleaseModel. Other versions must create their own models."""
585+
new_hotness_update["trigger"]["msg"]["message"]["upstream_versions"] = ["7.0.3", "7.0.4"]
586+
587+
class AnityaTestProjectModel(AnityaProjectModel):
588+
pass
589+
590+
db_project_object = flexmock(
591+
id=12,
592+
project_event_model_type=ProjectEventModelType.anitya_multiple_versions,
593+
job_config_trigger_type=JobConfigTriggerType.release,
594+
project=AnityaTestProjectModel(),
595+
)
596+
project_event = (
597+
flexmock(type=ProjectEventModelType.anitya_multiple_versions, event_id=12)
598+
.should_receive("get_project_event_object")
599+
.and_return(db_project_object)
600+
.mock()
601+
)
602+
run_model = flexmock(PipelineModel)
603+
flexmock(ProjectEventModel).should_receive("get_or_create").with_args(
604+
type=ProjectEventModelType.anitya_multiple_versions,
605+
event_id=12,
606+
commit_sha=None,
607+
).and_return(project_event)
608+
flexmock(AnityaMultipleVersionsModel).should_receive("get_or_create").with_args(
609+
versions=["7.0.3", "7.0.4"],
610+
project_name="redis",
611+
project_id=4181,
612+
package="redis",
613+
).and_return(db_project_object)
614+
615+
# The retried model (for version 7.0.4 which failed download previously)
616+
retried_target = flexmock(status="retry", id=1234, branch="main")
617+
retried_model = flexmock(id=123, sync_release_targets=[retried_target])
618+
flexmock(SyncReleaseModel).should_receive("get_by_id").with_args(123).and_return(
619+
retried_model,
620+
).once()
621+
622+
# A new model should be created for version 7.0.3 (never attempted before)
623+
new_target = flexmock(status="queued", id=1235, branch="main")
624+
new_model = flexmock(id=124, sync_release_targets=[])
625+
flexmock(SyncReleaseModel).should_receive("create_with_new_run").with_args(
626+
status=SyncReleaseStatus.running,
627+
project_event_model=project_event,
628+
job_type=SyncReleaseJobType.pull_from_upstream,
629+
package_name="redis",
630+
).and_return(new_model, run_model).once()
631+
flexmock(SyncReleaseTargetModel).should_receive("create").with_args(
632+
status=SyncReleaseTargetStatus.queued,
633+
branch="main",
634+
).and_return(new_target).once()
635+
636+
flexmock(SyncReleasePullRequestModel).should_receive("get_or_create").with_args(
637+
pr_id=21,
638+
namespace="downstream-namespace",
639+
repo_name="downstream-repo",
640+
project_url="https://src.fedoraproject.org/rpms/downstream-repo",
641+
target_branch=str,
642+
url=str,
643+
).and_return(flexmock(sync_release_targets=[flexmock()]))
644+
645+
packit_yaml = (
646+
"{'specfile_path': 'hello-world.spec', "
647+
"jobs: [{trigger: release, job: pull_from_upstream, metadata: {targets:[]}}]}"
648+
)
649+
flexmock(Github, get_repo=lambda full_name_or_id: None)
650+
distgit_project = flexmock(
651+
get_files=lambda ref, recursive: [".packit.yaml"],
652+
get_file_content=lambda path, ref, headers: packit_yaml,
653+
full_repo_name="rpms/redis",
654+
repo="redis",
655+
namespace="rpms",
656+
is_private=lambda: False,
657+
default_branch="main",
658+
)
659+
660+
lp = flexmock(LocalProject, refresh_the_arguments=lambda: None)
661+
flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: lp)
662+
lp.working_dir = ""
663+
lp.git_project = distgit_project
664+
flexmock(DistGit).should_receive("local_project").and_return(lp)
665+
666+
flexmock(Allowlist, check_and_report=True)
667+
668+
flexmock(
669+
packit_service.worker.handlers.distgit,
670+
).should_receive("get_monitoring_metadata").and_return(
671+
flexmock(all_versions=True),
672+
)
673+
674+
service_config = ServiceConfig().get_service_config()
675+
flexmock(service_config).should_receive("get_project").with_args(
676+
"https://src.fedoraproject.org/rpms/redis",
677+
required=False,
678+
).and_return(distgit_project)
679+
flexmock(service_config).should_receive("get_project").with_args(
680+
"https://src.fedoraproject.org/rpms/redis",
681+
).and_return(distgit_project)
682+
683+
target_project = (
684+
flexmock(namespace="downstream-namespace", repo="downstream-repo")
685+
.should_receive("get_web_url")
686+
.and_return("https://src.fedoraproject.org/rpms/downstream-repo")
687+
.mock()
688+
)
689+
pr = (
690+
flexmock(
691+
id=21,
692+
url="some_url",
693+
target_project=target_project,
694+
description="some-title",
695+
)
696+
.should_receive("comment")
697+
.mock()
698+
)
699+
# 7.0.4 (retried version) succeeds this time
700+
flexmock(PackitAPI).should_receive("sync_release").with_args(
701+
dist_git_branch="main",
702+
versions=["7.0.4"],
703+
create_pr=True,
704+
local_pr_branch_suffix="update-pull_from_upstream",
705+
use_downstream_specfile=True,
706+
add_pr_instructions=True,
707+
resolved_bugs=["rhbz#2106196"],
708+
release_monitoring_project_id=4181,
709+
sync_acls=True,
710+
pr_description_footer=DistgitAnnouncement.get_announcement(),
711+
add_new_sources=True,
712+
fast_forward_merge_branches=set(),
713+
).and_return((pr, {})).once().ordered()
714+
# 7.0.3 (new version) also succeeds
715+
flexmock(PackitAPI).should_receive("sync_release").with_args(
716+
dist_git_branch="main",
717+
versions=["7.0.3"],
718+
create_pr=True,
719+
local_pr_branch_suffix="update-pull_from_upstream",
720+
use_downstream_specfile=True,
721+
add_pr_instructions=True,
722+
resolved_bugs=["rhbz#2106196"],
723+
release_monitoring_project_id=4181,
724+
sync_acls=True,
725+
pr_description_footer=DistgitAnnouncement.get_announcement(),
726+
add_new_sources=True,
727+
fast_forward_merge_branches=set(),
728+
).and_return((pr, {})).once().ordered()
729+
flexmock(PackitAPI).should_receive("clean")
730+
731+
for target, model in [
732+
(retried_target, retried_model),
733+
(new_target, new_model),
734+
]:
735+
flexmock(target).should_receive("set_status").with_args(
736+
status=SyncReleaseTargetStatus.running,
737+
).once()
738+
flexmock(target).should_receive("set_downstream_pr_url").with_args(
739+
downstream_pr_url="some_url",
740+
)
741+
flexmock(target).should_receive("set_downstream_prs").with_args(
742+
downstream_prs=list,
743+
).once()
744+
flexmock(target).should_receive("set_status").with_args(
745+
status=SyncReleaseTargetStatus.submitted,
746+
).once()
747+
flexmock(target).should_receive("set_start_time").once()
748+
flexmock(target).should_receive("set_finished_time").once()
749+
flexmock(target).should_receive("set_logs").once()
750+
flexmock(model).should_receive("set_status").with_args(
751+
status=SyncReleaseStatus.finished,
752+
).once()
753+
model.should_receive("get_package_name").and_return(None)
754+
755+
flexmock(IsRunConditionSatisfied).should_receive("pre_check").and_return(True)
756+
757+
flexmock(AddReleaseEventToDb).should_receive("db_project_object").and_return(
758+
flexmock(
759+
job_config_trigger_type=JobConfigTriggerType.release,
760+
id=123,
761+
project_event_model_type=ProjectEventModelType.release,
762+
),
763+
)
764+
flexmock(group).should_receive("apply_async").once()
765+
flexmock(Pushgateway).should_receive("push").times(2).and_return()
766+
flexmock(shutil).should_receive("rmtree").with_args("")
767+
768+
processing_results = SteveJobs().process_message(new_hotness_update)
769+
event_dict, _, job_config, package_config = get_parameters_from_results(
770+
processing_results,
771+
)
772+
assert json.dumps(event_dict)
773+
774+
# Simulate retry: pass sync_release_run_id and retry_version for 7.0.4
775+
results = run_pull_from_upstream_handler(
776+
package_config=package_config,
777+
event=event_dict,
778+
job_config=job_config,
779+
sync_release_run_id=123,
780+
retry_version="7.0.4",
781+
)
782+
assert first_dict_value(results["job"])["success"]
783+
784+
582785
def test_new_hotness_update_non_git(new_hotness_update, sync_release_model_non_git):
583786
model = flexmock(status="queued", id=1234, branch="main")
584787
flexmock(SyncReleaseTargetModel).should_receive("create").with_args(

0 commit comments

Comments
 (0)