Prevent premature acceptance/rejection

[bradmwilliams]

Prevent premature acceptance or rejection of releases before the creation job completes.

The PayloadAcceptedController and PayloadRejectedController evaluated job results and overrides without first confirming the release creation job had succeeded. This caused two problems:

1. Releases with only informing jobs (no blocking jobs) were accepted immediately because the informing-only path checked job.AggregateState == JobStatePending, which missed jobs with unset/null aggregate state. These were treated as "not pending" and the release was accepted before any verification ran.
2. Releases with no tests at all were accepted instantly — zero blocking + zero informing jobs meant the loop body never executed, falling through to Accepted=True before the release image even existed.

Changes

PayloadAcceptedController:

• Added a guard requiring ReleaseCreationJobResult.Status == Success before evaluating job results or overrides. A release cannot be accepted (even manually) until the creation job succeeds, since there is no physical release to point to.
• Creation job failure still takes highest precedence (Accepted=False).
• Fixed the informing-only check from == JobStatePending to != JobStateSuccess && != JobStateFailure, correctly treating unset/Unknown/Pending states as "not done yet."

PayloadRejectedController:

• Same creation job success guard added, placed after the creation-job-failed check and before overrides/job evaluation.

Precedence order (both controllers):

1. Creation job failed → terminal (Accepted=False / Rejected=True)
2. Creation job not yet succeeded → Unknown (wait)
3. Manual override → accept/reject
4. Job results → normal evaluation

Tests:

• Updated EmptyPayload to expect Unknown instead of True
• Renamed CreationJobUnknownDoesNotShortCircuit to CreationJobUnknownBlocksAcceptance
• Added InformingJobsWithUnsetStateNoBlockingJobs test case
• All test fixtures with job results now include a successful creation job result

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

Summary by CodeRabbit

• Bug Fixes
  • Release payload acceptance is now gated on the release creation job reaching Success; pending/unknown/unset results keep the acceptance outcome unresolved.
  • Release payload rejection is no longer evaluated (including manual overrides) unless the release creation job result is Success.
  • Manual acceptance overrides no longer fall through into subsequent rejection evaluation, while override reasons/messages are preserved.
  • Failed creation jobs continue to surface clear failure details.
• Tests
  • Updated and expanded controller unit tests for acceptance/rejection gating across unset/unknown/failed scenarios.

[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: 53615c9d-939c-4448-b0dc-e1df9380ca1a

📥 Commits

Reviewing files that changed from the base of the PR and between c5f8600 and 79881aa.

📒 Files selected for processing (1)

• pkg/cmd/release-payload-controller/payload_rejected_controller_test.go

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

• pkg/cmd/release-payload-controller/payload_rejected_controller_test.go

---

📝 Walkthrough

Walkthrough

Release payload acceptance and rejection evaluation now waits for the release creation job to succeed. Failed creation jobs still produce failure conditions, while pending or unknown states leave evaluation unresolved. Tests update expected statuses and prerequisite job state.

Changes

Release payload conditions

Layer / File(s)	Summary
Acceptance condition gating pkg/cmd/release-payload-controller/payload_accepted_controller.go, pkg/cmd/release-payload-controller/payload_accepted_controller_test.go	Acceptance remains Unknown until the release creation job succeeds; failed jobs remain rejected, with updated precedence and synchronization expectations.
Rejection condition gating pkg/cmd/release-payload-controller/payload_rejected_controller.go, pkg/cmd/release-payload-controller/payload_rejected_controller_test.go	Rejection evaluation runs only after creation-job success, manual acceptance returns immediately, and tests cover override and blocking-job precedence.

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

Possibly related PRs

• openshift/release-controller#794: Also changes release payload acceptance gating based on prerequisite job states.

Suggested labels:lgtm

Suggested reviewers:hoxhaeris, AlexNPavel, thiagoalessio

🚥 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: delaying acceptance and rejection decisions until creation is complete.
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]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

136-153: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Gate manual overrides before evaluating them.

The creation-success check is after the override switch, so a pending/unknown/unset creation job can still return ConditionPayloadRejected=True for a manual rejection (or evaluate manual acceptance early). Move this gate before the switch, keeping the failed-job branch first.

Proposed fix

+	// The release creation job must succeed before rejection can be evaluated+	if payload.Status.ReleaseCreationJobResult.Status != v1alpha1.ReleaseCreationJobSuccess {+ return rejectedCondition+	}+
// Check for "Accepted" PayloadOverride
switch payload.Spec.PayloadOverride.Override {
case v1alpha1.ReleasePayloadOverrideRejected:
rejectedCondition.Status = metav1.ConditionTrue
rejectedCondition.Message = payload.Spec.PayloadOverride.Reason
rejectedCondition.Reason = ReleasePayloadManuallyRejectedReason
return rejectedCondition
case v1alpha1.ReleasePayloadOverrideAccepted:
rejectedCondition.Status = metav1.ConditionFalse
rejectedCondition.Message = payload.Spec.PayloadOverride.Reason
rejectedCondition.Reason = ReleasePayloadManuallyAcceptedReason
return rejectedCondition
}
--	if payload.Status.ReleaseCreationJobResult.Status != v1alpha1.ReleaseCreationJobSuccess {- return rejectedCondition-	}

🤖 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_rejected_controller.go` around
lines 136 - 153, Move the release-creation job success gate in the payload
rejection condition logic before the manual override switch. In the relevant
controller method, ensure non-successful ReleaseCreationJobResult returns the
default condition first, then evaluate ReleasePayloadOverrideRejected and
ReleasePayloadOverrideAccepted overrides only after that check.

🤖 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.
Outside diff comments:
In `@pkg/cmd/release-payload-controller/payload_rejected_controller.go`:
- Around line 136-153: Move the release-creation job success gate in the payload
rejection condition logic before the manual override switch. In the relevant
controller method, ensure non-successful ReleaseCreationJobResult returns the
default condition first, then evaluate ReleasePayloadOverrideRejected and
ReleasePayloadOverrideAccepted overrides only after that check.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 228d5092-7199-480d-9b28-f4d3f28d1d43

📥 Commits

Reviewing files that changed from the base of the PR and between 31afe97 and 7e6bff5.

📒 Files selected for processing (4)

• 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

[bradmwilliams]

/label tide/merge-method-squash

[coderabbitai]

🧹 Nitpick comments (1)

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

39-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression cases for incomplete creation jobs.

These fixtures cover only successful creation. Add rejection tests for unset, pending, and unknown creation-job states, including manual overrides, to verify evaluation remains Unknown and overrides are not applied before creation succeeds.

Also applies to: 135-137

🤖 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_rejected_controller_test.go`
around lines 39 - 41, Add regression cases in the release payload rejection
controller tests for unset, pending, and unknown ReleaseCreationJobResult.Status
values, including each with manual overrides configured. Assert evaluation
remains Unknown and overrides are ignored until creation succeeds, alongside the
existing successful creation fixture.

🤖 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.
Nitpick comments:
In `@pkg/cmd/release-payload-controller/payload_rejected_controller_test.go`:
- Around line 39-41: Add regression cases in the release payload rejection
controller tests for unset, pending, and unknown ReleaseCreationJobResult.Status
values, including each with manual overrides configured. Assert evaluation
remains Unknown and overrides are ignored until creation succeeds, alongside the
existing successful creation fixture.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c52ab819-849a-46d1-b29f-707021db9e21

📥 Commits

Reviewing files that changed from the base of the PR and between 7e6bff5 and 5b2c9d8.

📒 Files selected for processing (2)

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

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

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

[hoxhaeris]

/lgtm

[coderabbitai]

🧹 Nitpick comments (1)

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

67-178: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a creation-failure precedence regression case.

Add a case with ReleaseCreationJobFailed plus an acceptance override, asserting ConditionPayloadRejected=True with ReleasePayloadCreationJobFailedReason. This verifies failed creation remains terminal and cannot be masked by a manual override.

🤖 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_rejected_controller_test.go`
around lines 67 - 178, Add a regression table entry alongside the existing
creation-job override cases with ReleaseCreationJobResult.Status set to
ReleaseCreationJobFailed and PayloadOverride.Override set to
ReleasePayloadOverrideAccepted. Update the expected object to preserve the
failed creation status and assert ConditionPayloadRejected is True with Reason
set to ReleasePayloadCreationJobFailedReason, confirming failure precedence over
manual overrides.

🤖 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.
Nitpick comments:
In `@pkg/cmd/release-payload-controller/payload_rejected_controller_test.go`:
- Around line 67-178: Add a regression table entry alongside the existing
creation-job override cases with ReleaseCreationJobResult.Status set to
ReleaseCreationJobFailed and PayloadOverride.Override set to
ReleasePayloadOverrideAccepted. Update the expected object to preserve the
failed creation status and assert ConditionPayloadRejected is True with Reason
set to ReleasePayloadCreationJobFailedReason, confirming failure precedence over
manual overrides.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e008cfa1-6eff-45a0-a469-9dc5d013f285

📥 Commits

Reviewing files that changed from the base of the PR and between 5b2c9d8 and c5f8600.

📒 Files selected for processing (1)

• pkg/cmd/release-payload-controller/payload_rejected_controller_test.go

[bradmwilliams]

/hold
Another CodeRabbit nit

[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.