RFC 0020: Two-Phase Release Workflow#178
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds ChangesTwo-Phase Release Workflow RFC
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)rfcs/0020-two-phase-release-workflow.mdmarkdownlint-cli2 wrapper config was not available before execution Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@rfcs/0000-two-phase-release-workflow.md`:
- Around line 134-135: Update the cross-RFC reference in
rfcs/0000-two-phase-release-workflow.md so it consistently points to the RFC
specified by the PR objective (change the citation that currently references RFC
0015 to RFC 0018), and make the same correction for the other occurrence noted
(lines around 469-470); ensure both the numeric RFC ID, link URL and title text
match RFC 0018 so implementers have a single normative source.
- Around line 5-6: The RFC currently uses placeholder tracking links
"Kuadrant/architecture#0000" for both the "RFC PR" and "Issue tracking" entries;
replace those placeholders with the actual pull request and issue references for
this proposal so both lines under the header point to the real PR and issue
(update the two occurrences of Kuadrant/architecture#0000).
- Around line 141-144: The “No phantom releases.” guarantee conflicts with the
phase order that creates release tags before building artifacts (see the “Tests
before artifacts.” / phase ordering in the two-phase release workflow), so
either relax the guarantee text or change the normative flow: update the “No
phantom releases.” paragraph to qualify the guarantee (e.g., “No phantom GitHub
Releases will be left if all rollback steps run; tags/images may require cleanup
on partial failures”), OR move tag/publish steps to occur after artifact build
and add explicit rollback steps that delete pushed tags and container images on
failure; reference and update the headings/sections titled “No phantom
releases.” and “Tests before artifacts.” and any flow descriptions that mention
creating tags/images before artifacts to keep wording and behavior consistent.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa633d1b-837c-49e0-98cd-dc41c74f122f
📒 Files selected for processing (1)
rfcs/0000-two-phase-release-workflow.md
| - RFC PR: [Kuadrant/architecture#0000](https://github.com/Kuadrant/architecture/pull/0000) | ||
| - Issue tracking: [Kuadrant/architecture#0000](https://github.com/Kuadrant/architecture/issues/0000) |
There was a problem hiding this comment.
Replace placeholder tracking links before merge.
Kuadrant/architecture#0000 for both PR and issue appears to be a template placeholder, which makes this RFC non-traceable in practice. Please update these to the real references for this proposal.
🤖 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 `@rfcs/0000-two-phase-release-workflow.md` around lines 5 - 6, The RFC
currently uses placeholder tracking links "Kuadrant/architecture#0000" for both
the "RFC PR" and "Issue tracking" entries; replace those placeholders with the
actual pull request and issue references for this proposal so both lines under
the header point to the real PR and issue (update the two occurrences of
Kuadrant/architecture#0000).
| - **No phantom releases.** | ||
| If any step fails, no GitHub Release exists. | ||
| The release either fully succeeds or does not happen. | ||
| - **Tests before artifacts.** |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
“No phantom releases” guarantee conflicts with the specified phase order.
The document guarantees “release either fully succeeds or does not happen”, but the workflow tags before artifact builds, and later acknowledges failures can leave pushed tags/images requiring manual cleanup. Please tighten the guarantee wording or change the normative flow/rollback requirements so these sections are consistent.
Also applies to: 279-299, 447-450
🤖 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 `@rfcs/0000-two-phase-release-workflow.md` around lines 141 - 144, The “No
phantom releases.” guarantee conflicts with the phase order that creates release
tags before building artifacts (see the “Tests before artifacts.” / phase
ordering in the two-phase release workflow), so either relax the guarantee text
or change the normative flow: update the “No phantom releases.” paragraph to
qualify the guarantee (e.g., “No phantom GitHub Releases will be left if all
rollback steps run; tags/images may require cleanup on partial failures”), OR
move tag/publish steps to occur after artifact build and add explicit rollback
steps that delete pushed tags and container images on failure; reference and
update the headings/sections titled “No phantom releases.” and “Tests before
artifacts.” and any flow descriptions that mention creating tags/images before
artifacts to keep wording and behavior consistent.
This RFC introduces the concept of a two-phase release workflow base off the security policy of no writes to a release branch, but also address a number of issue faced while releasing kuadrant 1.5.0 Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
fd02643 to
2babc0b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
rfcs/0020-two-phase-release-workflow.md (2)
113-113: 💤 Low valueMinor grammar and style improvements for clarity.
Several sentences can be tightened for clarity:
- Line 113: Add a comma for readability: "The version
0.0.0is a sentinel value, meaning 'targets latest / under active development.'"- Line 213: The phrase "On release branches" repeats twice in consecutive sentences. Consider combining: "On release branches, the version and all dependency versions must not be
0.0.0— they must be concrete released versions."- Line 364: Add comma before "but": "...could reduce this burden**,** but is out of scope..."
- Line 430: Remove hyphen from adverb: "loosely coupled workflows" (not "loosely-coupled").
- Line 461: Add comma before "but": "...is actively being designed**,** but is independent..."
Also applies to: 213-213, 364-364, 430-430, 461-461
🤖 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 `@rfcs/0020-two-phase-release-workflow.md` at line 113, Fix the following grammar and style issues in the RFC document: add a comma after "sentinel value" on line 113 to read "is a sentinel value, meaning"; on line 213, combine the two consecutive sentences that both start with "On release branches" into a single sentence to avoid repetition; on line 364, add a comma before "but" in the phrase "could reduce this burden, but is out of scope"; on line 430, remove the hyphen from "loosely-coupled" to make it "loosely coupled"; and on line 461, add a comma before "but" in the phrase "is actively being designed, but is independent". These changes improve readability and consistency of style throughout the document.Source: Linters/SAST tools
262-266: 💤 Low valueAdd language identifier to the code block.
The flowchart at line 264 is a fenced code block without a language identifier. For clarity and to satisfy Markdown linting, please specify a language (e.g.,
mermaid,text, orplaintext) or use an alternative notation (e.g., a list).Suggested fix
-``` +```text read-version → smoke-tests → tag → build artifacts → create-release -``` +```🤖 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 `@rfcs/0020-two-phase-release-workflow.md` around lines 262 - 266, The fenced code block containing the job flow description (read-version → smoke-tests → tag → build artifacts → create-release) is missing a language identifier on the opening fence. Add a language identifier such as `text` or `plaintext` to the opening triple backticks of this code block to satisfy Markdown linting requirements and improve clarity. For example, change the opening fence from triple backticks alone to triple backticks followed by the language identifier.Source: Linters/SAST tools
🤖 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 `@rfcs/0020-two-phase-release-workflow.md`:
- Around line 141-157: The "no phantom releases" guarantee in the principles
section overstates the scope by claiming "The release either fully succeeds or
does not happen," but the phase ordering documented later (which pushes git tags
before building artifacts) allows for partial failures where tags are created
even if subsequent artifact builds fail. To resolve this inconsistency, either
revise the guarantee wording in the "no phantom releases" bullet to explicitly
acknowledge that while no GitHub Release is created on failure, git tags and
partially published images may require manual cleanup, or restructure the phase
ordering to ensure all artifacts are built and validated before any tags are
pushed to the repository, with explicit rollback procedures for partial
failures.
- Around line 130-137: The release branch naming section contains a reference to
RFC 0015 for the release branch naming convention, but the PR objectives
indicate this should align with RFC 0018 instead. Verify which RFC number is the
correct authoritative source for release branch naming, then update the inline
link reference to the correct RFC in the "Release branch naming" section header
explanation, and also update any corresponding references to this RFC elsewhere
in the document (around the area that discusses release branch naming
principles) to ensure all citations point to the same authoritative RFC.
---
Nitpick comments:
In `@rfcs/0020-two-phase-release-workflow.md`:
- Line 113: Fix the following grammar and style issues in the RFC document: add
a comma after "sentinel value" on line 113 to read "is a sentinel value,
meaning"; on line 213, combine the two consecutive sentences that both start
with "On release branches" into a single sentence to avoid repetition; on line
364, add a comma before "but" in the phrase "could reduce this burden, but is
out of scope"; on line 430, remove the hyphen from "loosely-coupled" to make it
"loosely coupled"; and on line 461, add a comma before "but" in the phrase "is
actively being designed, but is independent". These changes improve readability
and consistency of style throughout the document.
- Around line 262-266: The fenced code block containing the job flow description
(read-version → smoke-tests → tag → build artifacts → create-release) is missing
a language identifier on the opening fence. Add a language identifier such as
`text` or `plaintext` to the opening triple backticks of this code block to
satisfy Markdown linting requirements and improve clarity. For example, change
the opening fence from triple backticks alone to triple backticks followed by
the language identifier.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1a7e049-c219-4fd5-87ef-b70381c70530
📒 Files selected for processing (1)
rfcs/0020-two-phase-release-workflow.md
| - **No phantom releases.** | ||
| If any step fails, no GitHub Release exists. | ||
| The release either fully succeeds or does not happen. | ||
| - **Tests before artifacts.** | ||
| Smoke tests must pass before any image is built or published. | ||
| No more published images that fail validation. | ||
| - **Human review gate.** | ||
| All code changes are reviewed through a standard pull request before release proceeds. | ||
| This satisfies Red Hat's Segregation of Duties policy. | ||
| - **Dependency safety.** | ||
| The version gate prevents operators from releasing before their operands, eliminating references to non-existent images. | ||
| - **Contained releases.** | ||
| The release workflow does not directly trigger workflows in other repositories. | ||
| It may create PRs to external repositories, but each component's release is self-contained and observable. | ||
| - **Branch protection compliance.** | ||
| All code changes go through PRs. | ||
| The release workflow only creates a tag - no commits to protected branches. |
There was a problem hiding this comment.
The "no phantom releases" guarantee requires qualification given the phase ordering.
The guarantee states: "If any step fails, no GitHub Release exists. The release either fully succeeds or does not happen." However, the phase ordering (line 262–289) creates and pushes git tags before building artifacts. If the "tag" phase succeeds but a subsequent "build artifacts" phase fails, the result is:
- No GitHub Release is created ✓ (promise kept)
- A git tag is pushed to the repository ✗ (requires manual cleanup)
- Possibly container images are partially published ✗ (requires manual cleanup)
This is not "fully succeeds or does not happen"—it is a partially succeeded state that violates the stated guarantee. The unresolved questions section (lines 447–450) acknowledges this gap ("tag is pushed, but an image build fails... may need manual cleanup"), but the guarantee wording should be updated to reflect the actual behaviour.
Please either:
- Tighten the guarantee to say "No GitHub Release is created if any job fails; other artifacts (tags, images) may require manual cleanup on partial failures", or
- Change the workflow phase ordering to ensure artifacts are published before tags are pushed, and add explicit rollback steps.
🤖 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 `@rfcs/0020-two-phase-release-workflow.md` around lines 141 - 157, The "no
phantom releases" guarantee in the principles section overstates the scope by
claiming "The release either fully succeeds or does not happen," but the phase
ordering documented later (which pushes git tags before building artifacts)
allows for partial failures where tags are created even if subsequent artifact
builds fail. To resolve this inconsistency, either revise the guarantee wording
in the "no phantom releases" bullet to explicitly acknowledge that while no
GitHub Release is created on failure, git tags and partially published images
may require manual cleanup, or restructure the phase ordering to ensure all
artifacts are built and validated before any tags are pushed to the repository,
with explicit rollback procedures for partial failures.
|
|
||
| - **Release branch naming standardization.** | ||
| Full standardization of the `release-X.Y` branch naming convention across all repositories, including migration of existing branches that use different conventions, is tracked as a companion effort. | ||
| See [RFC 0015](https://github.com/Kuadrant/architecture/blob/main/rfcs/0015-standardize-release-branch-naming.md) for details. |
There was a problem hiding this comment.
This should be the RFC 0018 instead. Another wrong reference was also pointed out by coderabbit above
Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
Summary
This RFC introduces a two-phase release workflow model for all Kuadrant component repositories, evolving the process described in RFC 0008.
The v1.5.0 release exposed systemic failures in the single-workflow release model: false success reporting, hidden workflow chains, silent downstream failures, policy violations from direct commits to protected branches, and missing dependency gating. These all stem from conflating code changes with artifact publishing in a single workflow.
What this RFC proposes
Every release is split into two distinct GitHub Actions workflows:
A human review gate (PR approval) sits between the two phases, satisfying Red Hat's Segregation of Duties policy and giving CI an opportunity to validate the release branch before any artifacts are published.
A
release.yamlfile is added to every component repository as the machine-readable source of truth for version and dependency information. A version gate CI check uses this file to prevent operators from releasing before their operand dependencies have published GitHub Releases.Key guarantees
Relationship to existing RFCs
release-X.Ybranch naming convention defined in RFC 0018 (Standardize Release Branch Naming)Summary by CodeRabbit