Gate acceptance/rejection on release mirror job completion

[bradmwilliams]

Summary

• Only populate ReleaseMirrorCoordinates on ReleasePayloads when AlternateImageRepository and AlternateImageRepositorySecretName are both configured
• Gate the PayloadAccepted and PayloadRejected condition evaluation on the mirror job reaching a terminal state (Success or Failed), when a mirror job is configured — including manual overrides, since images need to be backed up before any decision is made
• A failed mirror job does not prevent acceptance or cause rejection — the failure is recorded in ReleaseMirrorJobResult.Status and evaluation proceeds normally
• Already-accepted or already-rejected payloads skip the mirror gate in both controllers to avoid retroactively reverting releases that predate this change

Precedence order

1. Creation job failed → reject (unchanged)
2. Creation job not success → wait (unchanged)
3. Mirror job not terminal → wait (new, skipped for already-decided payloads)
4. Manual overrides (unchanged)
5. Blocking/informing/upgrade job evaluation (unchanged)

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED

Summary by CodeRabbit

• Bug Fixes
  • Release mirror job coordinates are now set only when both alternate image repository settings are provided.
  • Payload acceptance and rejection evaluation now waits for configured mirror jobs to reach a terminal status before finalizing decisions.
  • Mirror job success or failure allows evaluation to continue; pending or unknown mirror jobs temporarily defer acceptance/rejection.
• Tests
  • Updated and added unit tests to cover mirror-job gating behavior for acceptance and rejection.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 14abe205-b30b-4bf5-bc38-1b583435258b

📥 Commits

Reviewing files that changed from the base of the PR and between 2ebacfe and 2403af7.

📒 Files selected for processing (3)

• pkg/cmd/release-payload-controller/payload_accepted_controller.go
• pkg/cmd/release-payload-controller/payload_accepted_controller_test.go
• pkg/cmd/release-payload-controller/payload_rejected_controller.go

🚧 Files skipped from review as they are similar to previous changes (2)

• pkg/cmd/release-payload-controller/payload_rejected_controller.go
• pkg/cmd/release-payload-controller/payload_accepted_controller.go

---

📝 Walkthrough

Walkthrough

Release payload creation now conditionally configures mirror coordinates. Acceptance and rejection controllers defer condition evaluation until configured mirror jobs reach terminal states, with expanded tests covering precedence and synchronization.

Changes

Mirror job flow

Layer / File(s)	Summary
Conditional mirror coordinates cmd/release-controller/sync_release_payload.go, cmd/release-controller/sync_release_payload_test.go	Mirror coordinates are populated only when both alternate repository settings are configured; payload expectations no longer require the field unconditionally.
Acceptance mirror-job gate pkg/cmd/release-payload-controller/payload_accepted_controller.go, pkg/cmd/release-payload-controller/payload_accepted_controller_test.go	Acceptance evaluation waits for configured mirror jobs to reach success or failure before processing overrides and job results, while preserving existing accepted or rejected states.
Rejection mirror-job gate pkg/cmd/release-payload-controller/payload_rejected_controller.go, pkg/cmd/release-payload-controller/payload_rejected_controller_test.go	Rejection evaluation returns Unknown while configured mirror jobs are pending, with coverage for mirror statuses and prior payload conditions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ReleasePayload
  participant AcceptanceController
  participant RejectionController
  participant MirrorJob
  ReleasePayload->>AcceptanceController: evaluate accepted condition
  AcceptanceController->>MirrorJob: read mirror job status
  MirrorJob-->>AcceptanceController: pending or terminal status
  AcceptanceController-->>ReleasePayload: defer or evaluate acceptance
  ReleasePayload->>RejectionController: evaluate rejected condition
  RejectionController->>MirrorJob: read mirror job status
  MirrorJob-->>RejectionController: pending or terminal status
  RejectionController-->>ReleasePayload: Unknown or evaluate rejection

Loading

Suggested reviewers: alexnpavel, hoxhaeris

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: gating acceptance and rejection on release mirror job completion.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

pkg/cmd/release-payload-controller/payload_accepted_controller.go (1)

141-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated mirror-job gate logic across both controllers.

The mirror-job "must reach terminal state, skip if already accepted/rejected" predicate is duplicated verbatim (modulo the condition variable name) in both controllers. A future change to the gate's semantics risks being applied to only one file.

• pkg/cmd/release-payload-controller/payload_accepted_controller.go#L141-L156: extract this predicate into a shared helper (e.g. mirrorJobPending(payload) bool) callable from both files.
• pkg/cmd/release-payload-controller/payload_rejected_controller.go#L144-L159: replace this block with a call to the same shared helper.

♻️ Example shared helper

// mirrorJobPending reports whether a configured mirror job has not yet reached
// a terminal state, and the payload has not already been accepted or rejected.
func mirrorJobPending(payload *v1alpha1.ReleasePayload) bool {
	if payload.Spec.PayloadCreationConfig.ReleaseMirrorCoordinates.ReleaseMirrorJobName == "" {
		return false
	}
	if v1helpers.IsConditionTrue(payload.Status.Conditions, v1alpha1.ConditionPayloadAccepted) ||
		v1helpers.IsConditionTrue(payload.Status.Conditions, v1alpha1.ConditionPayloadRejected) {
		return false
	}
	return payload.Status.ReleaseMirrorJobResult.Status != v1alpha1.ReleaseMirrorJobSuccess &&
		payload.Status.ReleaseMirrorJobResult.Status != v1alpha1.ReleaseMirrorJobFailed
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/cmd/release-payload-controller/payload_accepted_controller.go` around
lines 141 - 156, Extract the duplicated mirror-job pending predicate into a
shared helper, such as mirrorJobPending(payload), preserving the configured-job,
accepted/rejected, and terminal-status checks. In
pkg/cmd/release-payload-controller/payload_accepted_controller.go lines 141-156,
replace the inline gate with the helper while retaining the existing accepted
condition return; make the same replacement in
pkg/cmd/release-payload-controller/payload_rejected_controller.go lines 144-159,
retaining that controller’s existing return behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/cmd/release-payload-controller/payload_accepted_controller_test.go`:
- Line 299: Correct the section header above the mirror-job tests to state that
the mirror gate is evaluated before and blocks payload overrides, matching
MirrorJobBlocksOverrideAccepted, MirrorJobBlocksOverrideRejected, and the
implementation comment. Remove the incorrect “third precedence — after
overrides” wording without changing test behavior.

---

Nitpick comments:
In `@pkg/cmd/release-payload-controller/payload_accepted_controller.go`:
- Around line 141-156: Extract the duplicated mirror-job pending predicate into
a shared helper, such as mirrorJobPending(payload), preserving the
configured-job, accepted/rejected, and terminal-status checks. In
pkg/cmd/release-payload-controller/payload_accepted_controller.go lines 141-156,
replace the inline gate with the helper while retaining the existing accepted
condition return; make the same replacement in
pkg/cmd/release-payload-controller/payload_rejected_controller.go lines 144-159,
retaining that controller’s existing return behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 71efcc40-ab24-4dce-aa5c-f720ffda688e

📥 Commits

Reviewing files that changed from the base of the PR and between b3be4f5 and 2ebacfe.

📒 Files selected for processing (6)

• cmd/release-controller/sync_release_payload.go
• cmd/release-controller/sync_release_payload_test.go
• pkg/cmd/release-payload-controller/payload_accepted_controller.go
• pkg/cmd/release-payload-controller/payload_accepted_controller_test.go
• pkg/cmd/release-payload-controller/payload_rejected_controller.go
• pkg/cmd/release-payload-controller/payload_rejected_controller_test.go

💤 Files with no reviewable changes (1)

• cmd/release-controller/sync_release_payload_test.go

[bradmwilliams]

/hold
For codereview updates

[bradmwilliams]

/label tide/merge-method-squash

[hoxhaeris]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bradmwilliams, hoxhaeris

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [bradmwilliams,hoxhaeris]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[bradmwilliams]

/unhold

[openshift-ci]

@bradmwilliams: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.