Display correct RHCOS version in Components for 4.Y and 5.Y releases#780
Open
sdodson wants to merge 3 commits into
Open
Display correct RHCOS version in Components for 4.Y and 5.Y releases#780sdodson wants to merge 3 commits into
sdodson wants to merge 3 commits into
Conversation
The Components list enrichment (URLs to RHCOS release browser) could miss RHCOS 10 entries when oc outputs names like "CoreOS 10.2" (period directly after "10") or when the generic "CoreOS" name is used with a 10.x version string. JSON: switch from exact component.Name match to strings.HasPrefix for the "Red Hat Enterprise Linux CoreOS 10" prefix so that "CoreOS 10.2", "CoreOS 10.20", etc. are all enriched with release browser URLs. Markdown regexes: broaden the RHCOS 10 patterns from `CoreOS 10(?: \d+\.\d+)?` to `CoreOS 10(?:[. ]\d[\d.]*)?` so they match "CoreOS 10.2" (period-separated) in addition to the existing "CoreOS 10" and "CoreOS 10 10.2" (space-separated) formats. Markdown fallback: when the generic RHCOS regex matches (no "10" in the component name) but the version string starts with 10.x, label the component as "Red Hat Enterprise Linux CoreOS 10" instead of the unversioned name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
OpenShift 5.Y ships RHCOS 10 (and potentially RHCOS 9 during transition). Extend the version check so that Major >= 5 is also recognized, matching the same dual-RHCOS rendering path already used for 4.21+. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
When both rhel-coreos (RHCOS 9) and rhel-coreos-10 (RHCOS 10) exist in a payload, oc adm release info chooses which one to show in the Components section based on its own logic (often alphabetically or by tag priority). This meant 4.21 nightlies showed RHCOS 10 even though RHCOS 9 is the primary node OS for 4.Y releases. This change post-processes the changelog markdown to swap the RHCOS component when needed: - 4.Y releases prefer rhel-coreos (RHCOS 9) in the Components section - 5.Y+ releases prefer rhel-coreos-10 (RHCOS 10) in the Components section The Node Image Info section continues to show both RHCOS versions with full package details for dual-RHCOS releases (4.21+, 5.Y+). Implementation: - Add PreferredMachineOSTag() to determine the preferred machine-OS tag based on OpenShift major version - Extend TransformMarkDownOutput() to accept ReleaseInfo and toImage parameters for querying available machine-OS streams - Add swapRHCOSComponentIfNeeded() to detect and swap the RHCOS component in the Components section when both versions exist - Update all callers (changelog-preview, http_changelog, tests) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED
Contributor
📝 WalkthroughWalkthroughAdds OpenShift 5+ support to dual-RHCOS detection and introduces ChangesRHCOS 10 dual-stream changelog enrichment
Sequence Diagram(s)sequenceDiagram
participant HTTP as http_changelog.go
participant Transform as TransformMarkDownOutput
participant Swap as swapRHCOSComponentIfNeeded
participant RI as releaseInfo
participant Browser as Release Browser URL
HTTP->>Transform: markdown, tags, arch, c.releaseInfo, toImage.GenerateDigestPullSpec()
Transform->>Swap: markdown, releaseInfo, toImage, arch
Swap->>RI: GetMachineOSStreams(toImage)
RI-->>Swap: available streams list
Swap->>RI: GetReleaseAnnotations(preferredTag)
RI-->>Swap: build annotations with machine-os version
Swap->>Browser: construct rhel-10.x stream URL
Browser-->>Swap: enriched URL
Swap-->>Transform: rewritten ### Components section
Transform-->>HTTP: fully transformed markdown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sdodson The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/rhcos/rhcos_test.go (1)
296-297:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTighten this assertion to catch duplicate infobox regressions.
Line 296 currently only fails when count is
< 2, so outputs with 3+ infoboxes would still pass. With exactly two CoreOS component lines in the fixture, this should assert equality.Suggested patch
- if strings.Count(out, "coreos-base-alert") < 2 { - t.Fatalf("expected two CoreOS base layer infoboxes, got:\n%s", out) + if got := strings.Count(out, "coreos-base-alert"); got != 2 { + t.Fatalf("expected exactly 2 CoreOS base layer infoboxes, got %d:\n%s", got, out) }🤖 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/rhcos/rhcos_test.go` around lines 296 - 297, The assertion in the test at line 296 uses a less-than comparison (< 2) which only catches cases where the count is below 2, but allows 3 or more occurrences to pass the test. Change the comparison operator from < to == in the strings.Count(out, "coreos-base-alert") check to enforce that exactly two CoreOS base layer infoboxes are present, matching the expected behavior from the test 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.
Inline comments:
In `@pkg/rhcos/rhcos_test.go`:
- Around line 346-348: The test starting at line 346 validates the label rewrite
for the RHCOS 10 fallback-format markdown case but does not verify the stream
URL assertion. Add an additional assertion after the existing label validation
to check that the output contains the expected stream URL such as "rhel-10.2" to
ensure link-enrichment is working correctly for the fallback parsing path when
CoreOS is upgraded from 10.x. This protects against regressions in the URL
enrichment for this specific code path.
In `@pkg/rhcos/rhcos.go`:
- Around line 99-104: The regex pattern in the reComponentsSection variable uses
a required `\n\n###` lookahead that prevents matching when the Components
section is the last section in the markdown file. Modify the regex pattern to
make the trailing `\n\n###` optional or use an alternation pattern that matches
either the trailing `\n\n###` or the end of the string, ensuring the Components
section is captured regardless of whether it appears at the end of the document.
- Around line 209-212: The reComponentLine.ReplaceAllString call on line 211 is
replacing all occurrences of Red Hat Enterprise Linux CoreOS lines in the
componentsSection, which causes multiple RHCOS version lines to be replaced with
the same enriched component. Instead of using ReplaceAllString, use a method
that replaces only the first occurrence or the specific targeted line, such as
replacing only the exact match of the componentsSection variable itself rather
than using a pattern-based replacement that could match multiple lines. This
ensures only the intended RHCOS component line is updated, not all matching
lines in the section.
---
Outside diff comments:
In `@pkg/rhcos/rhcos_test.go`:
- Around line 296-297: The assertion in the test at line 296 uses a less-than
comparison (< 2) which only catches cases where the count is below 2, but allows
3 or more occurrences to pass the test. Change the comparison operator from < to
== in the strings.Count(out, "coreos-base-alert") check to enforce that exactly
two CoreOS base layer infoboxes are present, matching the expected behavior from
the test fixture.
🪄 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: 2741f5d7-6667-4263-8d7f-69f3ee463817
📒 Files selected for processing (6)
cmd/release-controller-api/http_changelog.gohack/changelog-preview/main.gopkg/release-controller/semver.gopkg/release-controller/semver_dual_rhcos_test.gopkg/rhcos/rhcos.gopkg/rhcos/rhcos_test.go
Comment on lines
+346
to
+348
| if !strings.Contains(out, "Red Hat Enterprise Linux CoreOS 10") { | ||
| t.Fatalf("expected RHCOS 10 label when version starts with 10.x, got:\n%s", out) | ||
| } |
Contributor
There was a problem hiding this comment.
Add a stream URL assertion for the fallback-format markdown test.
Line 346 validates label rewrite, but this case also exercises fallback parsing for CoreOS upgraded from 10.x...; adding a rhel-10.2 URL assertion would protect against link-enrichment regressions in this exact path.
Suggested patch
if !strings.Contains(out, "Red Hat Enterprise Linux CoreOS 10") {
t.Fatalf("expected RHCOS 10 label when version starts with 10.x, got:\n%s", out)
}
+ if !strings.Contains(out, "rhel-10.2") {
+ t.Fatalf("expected rhel-10.2 stream URL in fallback format output, got:\n%s", out)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if !strings.Contains(out, "Red Hat Enterprise Linux CoreOS 10") { | |
| t.Fatalf("expected RHCOS 10 label when version starts with 10.x, got:\n%s", out) | |
| } | |
| if !strings.Contains(out, "Red Hat Enterprise Linux CoreOS 10") { | |
| t.Fatalf("expected RHCOS 10 label when version starts with 10.x, got:\n%s", out) | |
| } | |
| if !strings.Contains(out, "rhel-10.2") { | |
| t.Fatalf("expected rhel-10.2 stream URL in fallback format output, got:\n%s", out) | |
| } |
🤖 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/rhcos/rhcos_test.go` around lines 346 - 348, The test starting at line
346 validates the label rewrite for the RHCOS 10 fallback-format markdown case
but does not verify the stream URL assertion. Add an additional assertion after
the existing label validation to check that the output contains the expected
stream URL such as "rhel-10.2" to ensure link-enrichment is working correctly
for the fallback parsing path when CoreOS is upgraded from 10.x. This protects
against regressions in the URL enrichment for this specific code path.
Comment on lines
+99
to
+104
| reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)\n\n###`) | ||
| componentsMatch := reComponentsSection.FindStringSubmatch(markdown) | ||
| if componentsMatch == nil { | ||
| // Can't find Components section | ||
| return markdown, nil | ||
| } |
Contributor
There was a problem hiding this comment.
Handle terminal Components sections in the matcher.
At Line 99, the regex requires a following \n\n###, so swap is skipped when ### Components is the last section in the markdown.
Suggested fix
- reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)\n\n###`)
+ reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)(?:\n\n###|\z)`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)\n\n###`) | |
| componentsMatch := reComponentsSection.FindStringSubmatch(markdown) | |
| if componentsMatch == nil { | |
| // Can't find Components section | |
| return markdown, nil | |
| } | |
| reComponentsSection := regexp.MustCompile(`(?s)(### Components.*?)(?:\n\n###|\z)`) | |
| componentsMatch := reComponentsSection.FindStringSubmatch(markdown) | |
| if componentsMatch == nil { | |
| // Can't find Components section | |
| return markdown, nil | |
| } |
🤖 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/rhcos/rhcos.go` around lines 99 - 104, The regex pattern in the
reComponentsSection variable uses a required `\n\n###` lookahead that prevents
matching when the Components section is the last section in the markdown file.
Modify the regex pattern to make the trailing `\n\n###` optional or use an
alternation pattern that matches either the trailing `\n\n###` or the end of the
string, ensuring the Components section is captured regardless of whether it
appears at the end of the document.
Comment on lines
+209
to
+212
| reComponentLine := regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS[^\n]+`) | ||
|
|
||
| newComponentsSection := reComponentLine.ReplaceAllString(componentsSection, enrichedComponent) | ||
| markdown = strings.Replace(markdown, componentsSection, newComponentsSection, 1) |
Contributor
There was a problem hiding this comment.
Replace only the targeted RHCOS line, not every match.
At Line 211, ReplaceAllString rewrites all Red Hat Enterprise Linux CoreOS... lines. If both RHCOS 9 and 10 lines are present, both can be replaced with the same component.
Suggested fix
- reComponentLine := regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS[^\n]+`)
-
- newComponentsSection := reComponentLine.ReplaceAllString(componentsSection, enrichedComponent)
+ lines := strings.Split(componentsSection, "\n")
+ for i, line := range lines {
+ if !strings.HasPrefix(line, "* Red Hat Enterprise Linux CoreOS") {
+ continue
+ }
+ isRHCOS10 := strings.Contains(line, rhelCoreOs10)
+ if (currentlyShown == "rhel-coreos-10" && isRHCOS10) || (currentlyShown == "rhel-coreos" && !isRHCOS10) {
+ lines[i] = enrichedComponent
+ break
+ }
+ }
+ newComponentsSection := strings.Join(lines, "\n")
markdown = strings.Replace(markdown, componentsSection, newComponentsSection, 1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| reComponentLine := regexp.MustCompile(`\* Red Hat Enterprise Linux CoreOS[^\n]+`) | |
| newComponentsSection := reComponentLine.ReplaceAllString(componentsSection, enrichedComponent) | |
| markdown = strings.Replace(markdown, componentsSection, newComponentsSection, 1) | |
| lines := strings.Split(componentsSection, "\n") | |
| for i, line := range lines { | |
| if !strings.HasPrefix(line, "* Red Hat Enterprise Linux CoreOS") { | |
| continue | |
| } | |
| isRHCOS10 := strings.Contains(line, rhelCoreOs10) | |
| if (currentlyShown == "rhel-coreos-10" && isRHCOS10) || (currentlyShown == "rhel-coreos" && !isRHCOS10) { | |
| lines[i] = enrichedComponent | |
| break | |
| } | |
| } | |
| newComponentsSection := strings.Join(lines, "\n") | |
| markdown = strings.Replace(markdown, componentsSection, newComponentsSection, 1) |
🤖 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/rhcos/rhcos.go` around lines 209 - 212, The
reComponentLine.ReplaceAllString call on line 211 is replacing all occurrences
of Red Hat Enterprise Linux CoreOS lines in the componentsSection, which causes
multiple RHCOS version lines to be replaced with the same enriched component.
Instead of using ReplaceAllString, use a method that replaces only the first
occurrence or the specific targeted line, such as replacing only the exact match
of the componentsSection variable itself rather than using a pattern-based
replacement that could match multiple lines. This ensures only the intended
RHCOS component line is updated, not all matching lines in the section.
Contributor
|
@sdodson: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ReleaseTagIsDualRHCOSto include 5.Y releases (previously only 4.21+)CoreOS 10.2 ...andCoreOS 10 10.2 ...formatsrhel-coreosandrhel-coreos-10), swap the Components section to prefer RHCOS 9 for 4.Y releases and RHCOS 10 for 5.Y+ releasesTest plan
4.22.0-0.nightly-2026-06-17-223903from4.22.0-0.nightly-2026-06-17-152812: Components shows RHCOS 9.8, Node Image Info shows both RHCOS 9 and RHCOS 10 streams5.0.0-0.nightly-2026-06-17-170645from5.0.0-0.nightly-2026-06-17-022519: Components shows RHCOS 10 with version upgrade, Node Image Info shows RHCOS 10 stream🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests