Skip to content

Revert "Extend Node Image Info to handle streams" #752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sdodson merged 1 commit into from
Apr 8, 2026
Merged

Revert "Extend Node Image Info to handle streams" #752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,3 @@ __pycache__/
coverage.out
golangci-lint.out
report.json

# Local output from hack/changelog-preview (go run … > sample-changelog.md)
hack/changelog-preview/sample-changelog.md
45 changes: 15 additions & 30 deletions cmd/release-controller-api/http_changelog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ func (c *Controller) getChangeLog(ch chan renderResult, chNodeInfo chan renderRe
return
}
ch <- renderResult{out: out}
return
}

out, err = rhcos.TransformMarkDownOutput(out, fromTag, toTag, architecture, archExtension)
Expand All @@ -87,31 +86,25 @@ func (c *Controller) getChangeLog(ch chan renderResult, chNodeInfo chan renderRe
return
}

toImagePullspec := toImage.GenerateDigestPullSpec()
fromImagePullspec := fromImage.GenerateDigestPullSpec()

// Request node image info when the changelog links to #node-image-info (CoreOS infobox) or when
// the target payload has discoverable machine-os streams (newer oc may omit RHCOS summary lines).
fetchNode := strings.Contains(out, "#node-image-info")
if !fetchNode {
streams, err := c.releaseInfo.ListMachineOSStreams(toImagePullspec)
if err != nil {
chNodeInfo <- renderResult{err: err}
return
}
fetchNode = len(streams) > 0
}
if !fetchNode {
// Only request node image info if it'll be rendered. Use the exact
// check that renderChangeLog does to know if to consume from us.
if !strings.Contains(out, "#node-image-info") {
chNodeInfo <- renderResult{}
return
}

nodeMD, err := rhcos.NodeImageSectionMarkdown(c.releaseInfo, fromImagePullspec, toImagePullspec, out)
toImagePullspec := toImage.GenerateDigestPullSpec()
rpmlist, err := c.releaseInfo.RpmList(toImagePullspec)
if err != nil {
chNodeInfo <- renderResult{err: err}
return
}
chNodeInfo <- renderResult{out: nodeMD}

rpmdiff, err := c.releaseInfo.RpmDiff(fromImage.GenerateDigestPullSpec(), toImagePullspec)
if err != nil {
chNodeInfo <- renderResult{err: err}
}

chNodeInfo <- renderResult{out: rhcos.RenderNodeImageInfo(out, rpmlist, rpmdiff)}
Comment on lines +89 to +107
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Send to chNodeInfo exactly once, or not at all.

renderChangeLog skips reading chNodeInfo when the markdown has no #node-image-info, and otherwise it consumes a single value. This block sends on the no-anchor path at Line 92 and can also send an error plus a final rendered result on the failure paths, so the producer goroutine blocks permanently in both cases.

Suggested fix 
 	// Only request node image info if it'll be rendered. Use the exact
 	// check that renderChangeLog does to know if to consume from us.
 	if !strings.Contains(out, "#node-image-info") {
-		chNodeInfo <- renderResult{}
 		return
 	}

 	toImagePullspec := toImage.GenerateDigestPullSpec()
 	rpmlist, err := c.releaseInfo.RpmList(toImagePullspec)
 	if err != nil {
 		chNodeInfo <- renderResult{err: err}
+		return
 	}

 	rpmdiff, err := c.releaseInfo.RpmDiff(fromImage.GenerateDigestPullSpec(), toImagePullspec)
 	if err != nil {
 		chNodeInfo <- renderResult{err: err}
+		return
 	}

 	chNodeInfo <- renderResult{out: rhcos.RenderNodeImageInfo(out, rpmlist, rpmdiff)}
📝 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
// Only request node image info if it'll be rendered. Use the exact
// check that renderChangeLog does to know if to consume from us.
if !strings.Contains(out, "#node-image-info") {
chNodeInfo <- renderResult{}
return
}
nodeMD, err := rhcos.NodeImageSectionMarkdown(c.releaseInfo, fromImagePullspec, toImagePullspec, out)
toImagePullspec := toImage.GenerateDigestPullSpec()
rpmlist, err := c.releaseInfo.RpmList(toImagePullspec)
if err != nil {
chNodeInfo <- renderResult{err: err}
return
}
chNodeInfo <- renderResult{out: nodeMD}
rpmdiff, err := c.releaseInfo.RpmDiff(fromImage.GenerateDigestPullSpec(), toImagePullspec)
if err != nil {
chNodeInfo <- renderResult{err: err}
}
chNodeInfo <- renderResult{out: rhcos.RenderNodeImageInfo(out, rpmlist, rpmdiff)}
// Only request node image info if it'll be rendered. Use the exact
// check that renderChangeLog does to know if to consume from us.
if !strings.Contains(out, "#node-image-info") {
return
}
toImagePullspec := toImage.GenerateDigestPullSpec()
rpmlist, err := c.releaseInfo.RpmList(toImagePullspec)
if err != nil {
chNodeInfo <- renderResult{err: err}
return
}
rpmdiff, err := c.releaseInfo.RpmDiff(fromImage.GenerateDigestPullSpec(), toImagePullspec)
if err != nil {
chNodeInfo <- renderResult{err: err}
return
}
chNodeInfo <- renderResult{out: rhcos.RenderNodeImageInfo(out, rpmlist, rpmdiff)}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/release-controller-api/http_changelog.go` around lines 89 - 107, The
producer goroutine currently may send to chNodeInfo multiple times (once on the
no-anchor path and again on error/normal paths), causing a possible permanent
block; update the logic in the block that checks strings.Contains(out,
"#node-image-info") and the subsequent calls to c.releaseInfo.RpmList and
c.releaseInfo.RpmDiff so that chNodeInfo is written exactly once: on the
no-anchor path send a single empty renderResult and return immediately, and on
the error paths send a single renderResult with the error and return immediately
(do not continue to call rhcos.RenderNodeImageInfo); finally, if no errors occur
send the rendered result from rhcos.RenderNodeImageInfo once. Ensure you
reference chNodeInfo, renderResult, c.releaseInfo.RpmList,
c.releaseInfo.RpmDiff, fromImage.GenerateDigestPullSpec,
toImage.GenerateDigestPullSpec, and rhcos.RenderNodeImageInfo when locating the
code to change.

}

func (c *Controller) renderChangeLog(w http.ResponseWriter, fromPull string, fromTag string, toPull string, toTag string, format string) {
Expand Down Expand Up @@ -180,17 +173,9 @@ func (c *Controller) renderChangeLog(w http.ResponseWriter, fromPull string, fro
fmt.Fprintf(w, `<p class="alert alert-danger">%s</p>`, fmt.Sprintf("Unable to show full changelog: %s", render.err))
}

needsNode := strings.Contains(render.out, "#node-image-info")
if !needsNode && render.err == nil && format != "json" {
toImage, err := releasecontroller.GetImageInfo(c.releaseInfo, c.architecture, toPull)
if err == nil {
streams, err2 := c.releaseInfo.ListMachineOSStreams(toImage.GenerateDigestPullSpec())
if err2 == nil && len(streams) > 0 {
needsNode = true
}
}
}
if !needsNode {
// only render a CoreOS diff if we need to; we can know this by
// checking if it links to the diff section we create here
if !strings.Contains(render.out, "#node-image-info") {
return
}

Expand Down
81 changes: 0 additions & 81 deletions hack/changelog-preview/main.go
View file
Open in desktop

This file was deleted.

147 changes: 0 additions & 147 deletions pkg/release-controller/machine_os_tags.go
View file
Open in desktop

This file was deleted.

78 changes: 0 additions & 78 deletions pkg/release-controller/machine_os_tags_test.go
View file
Open in desktop

This file was deleted.

Loading
Loading