Skip to content

revert(release): restore the chart publish existence check from #2221 - #2347

Closed
apartha-nv wants to merge 1 commit into
mainfrom
aparthasarat/restore-chart-existence-check
Closed

apartha-nv wants to merge 1 commit into
mainfrom
aparthasarat/restore-chart-existence-check

Conversation

@apartha-nv

@apartha-nv apartha-nv commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Why

Every Helm chart publish has failed since #2272 reverted #2221 on 2026-10-05. #2221 had correctly fixed the chart-existence probe (missing_helm_chart_output) to recognize the oras-go v2 "FetchReference ... not found" wording that Helm 3.21.4 actually emits for a missing OCI artifact, anchored to the specific requested chart:version reference. #2272 reverted that fix back to the original, never-correct two-phrase match ("manifest unknown", "status code: 404"), which Helm 3.21.4 has never emitted for this case.

CodeRabbit's own review on #2272 flagged this exact risk before merge: "A new chart version may fail to publish when Helm reports that it is missing. Restore the requested-reference check before merging." It was merged anyway.

Confirmed impact since the revert: every real first-time chart publish has failed with the identical error (nvca-operator v1.29.3, v1.29.4, v1.29.5 x2, vanity-gateway v0.6.0, event-ledger v0.4.0, openbao v0.33.0). The only run that succeeded in this period (event-ledger v0.5.0) did so by coincidentally matching an already-published, unchanged version, bypassing the broken check entirely.

See #2218 for the original, more detailed root-cause writeup (including that roughly 49 chart versions across the repo need republishing once this is fixed).

What changed

git revert of #2272, restoring #2221's implementation of missing_helm_chart_output verbatim: it now takes the requested chart:version reference, matches on a line ending in <reference>: not found (the oras-go v2 wording, deliberately not anchored to a specific operation name since oras uses "FetchReference" or "Resolve" depending on source), plus the legacy "manifest unknown" / "status code: 404" / "response status code 404" phrases, and still refuses anything else (including auth failures) so an existing chart is never silently treated as absent.

Customer Release Notes

Not customer visible.

Plan Summary

Not applicable.

Usage

Not applicable.

Testing

  • python3 -m unittest tools/ci/test-github-release.py (130 tests, all pass)
  • Clean revert with no conflicts
  • Cross-checked against the real failing production log text from the openbao v0.33.0, nvca-operator v1.29.3/1.29.4/1.29.5, vanity-gateway v0.6.0, and event-ledger v0.4.0 CI run failures

Notes

Once this merges, the roughly 49 chart versions tagged and released since the Sep 11 regression (per #2218) that are missing from the registry still need republishing. That is a separate operational follow-up, not part of this code fix.

References

Closes #2218

Related Pull Requests

Reverts #2272. Restores #2221.

Dependencies

None.

Summary by CodeRabbit

  • Bug Fixes
    • Helm chart publishing now recognizes a missing chart only when the response identifies the requested chart or reports a known missing-chart condition. Access-denied and authentication errors are handled as failures rather than as missing charts, and publishing stops with Helm’s error output. This helps prevent unrelated repository or credential problems from being mistaken for an absent chart.

@apartha-nv
apartha-nv requested a review from a team as a code owner October 7, 2026 08:55
@apartha-nv
apartha-nv requested a review from Max-NV October 7, 2026 08:55
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/nvcf/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: b77f01e4-2fc0-41cb-acb0-e7b068d81f38
📥 Commits

Reviewing files that changed from the base of the PR and between 726d305 and 030633d.

📒 Files selected for processing (2)
  • tools/ci/github-release
  • tools/ci/test-github-release.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Walkthrough

Walkthrough

The release script now recognizes additional Helm output for an absent chart. It checks the output against the requested chart reference. Tests cover absent-chart responses, unrelated not-found messages, and registry access failures.

Changes

Chart absence detection

Layer / File(s) Summary
Match Helm absence responses
tools/ci/github-release, tools/ci/test-github-release.py
The absence check recognizes known 404 responses and lines ending with the requested reference followed by : not found. The publish flow passes the chart reference to the check. Tests cover Helm response variants, reject mismatched references and registry access failures, and retain legacy missing-artifact signals.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to 03063

This change restores detection of absent Helm chart versions so chart publishes can proceed, while access and authentication failures still stop the run. No merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits syntax with the single type revert and scope release. It accurately describes reverting the release-chart change to restore the existence check.
Linked Issues check ✅ Passed Issue #2218 requires the release workflow to recognize a missing requested chart version and continue to publication, while stopping when the registry refuses access. `missing_helm_chart_output(output…
Out of Scope Changes check ✅ Passed All changes support issue #2218. The implementation update restores reference-specific absence detection. The added and updated tests cover the reported failure and ensure registry refusals do not aut…
Docstring Coverage ✅ Passed Docstring coverage is 92.86% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 1 files. (1 skipped: 1 …
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@apartha-nv

Copy link
Copy Markdown
Contributor Author

Closing without merging - confirmed nvcf-internal's dispatcher-triggered pipeline already publishes these charts successfully and independently (per the GitHub First POR, this is its intended job, not GitHub Actions'). The real fix here is removing the redundant GitHub-side chart publish path entirely, which Rohith is already actively working on. Leaving #2218 open for them to close via that work.

@apartha-nv apartha-nv closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chart publication fails for every new version: the absence check does not match the registry's reply

1 participant