fix(recipes): correct nvsentinel registry default to OCI source#725
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 11 minutes and 53 seconds. Comment |
74902e9 to
5e8ca09
Compare
The `nvsentinel` registry entry declared:
defaultRepository: https://helm.ngc.nvidia.com/nvidia
defaultChart: nvidia/nvsentinel
But the chart isn't published to the HTTPS NGC index — only to the
OCI registry at `oci://ghcr.io/nvidia/nvsentinel`. The defaults are
silently ignored today: every nvsentinel-using overlay sets its own
`source: oci://ghcr.io/nvidia` + chart `nvsentinel`, so the broken
HTTPS default never resolves. But anyone relying on the registry
defaults (e.g. via `aicr bundle` without explicit overlay overrides
on this entry) would hit the dead path.
Update the defaults to match what every overlay already uses:
defaultRepository: oci://ghcr.io/nvidia
defaultChart: nvsentinel
Same shape as the kai-scheduler entry post-NVIDIA#720 (OCI registry path
in `defaultRepository`, bare chart name in `defaultChart`). Verified
locally:
$ helm pull oci://ghcr.io/nvidia/nvsentinel --version v1.3.0
Pulled.
$ aicr bundle -r recipe.yaml -o /tmp/bundle
... generates upstream.env with
CHART='oci://ghcr.io/nvidia/nvsentinel'
REPO=''
VERSION='v1.3.0'
Note: other NGC HTTPS entries in the registry (gpu-operator,
network-operator, nodewright-operator, nvidia-dra-driver-gpu) are
unchanged — those charts are genuinely served by the HTTPS NGC
index. nvsentinel is special because it ships only via OCI.
Refs: NVIDIA#698 (Phase 1 follow-up NVIDIA#2)
5e8ca09 to
eb80519
Compare
Summary
The
nvsentinelentry inrecipes/registry.yamldeclares adefaultRepository(https://helm.ngc.nvidia.com/nvidia) anddefaultChart(nvidia/nvsentinel) that don't actually serve the chart — the HTTPS NGC index doesn't carry nvsentinel; onlyoci://ghcr.io/nvidia/nvsentineldoes. This PR aligns the registry defaults with the canonical OCI source that every overlay already uses.Motivation / Context
Surfaced during the #715 cross-review and tracked as Phase-1 follow-up item #2 on #698. The mismatch is silent / dead-code today because every
nvsentinel-using overlay (base.yaml, etc.) explicitly setssource: oci://ghcr.io/nvidia+ chartnvsentinel— the registry default never resolves. But anyone relying on the registry defaults (e.g. anaicr bundleinvocation that doesn't override the source) would hit the dead path.The fix mirrors the same pattern the
kai-schedulerentry uses post-#720: OCI registry path indefaultRepository, bare chart name indefaultChart.- name: nvsentinel ... helm: - defaultRepository: https://helm.ngc.nvidia.com/nvidia - defaultChart: nvidia/nvsentinel + defaultRepository: oci://ghcr.io/nvidia + defaultChart: nvsentinel defaultVersion: v1.3.0 defaultNamespace: nvsentinelNote on other NGC HTTPS entries
gpu-operator,network-operator,nodewright-operator, andnvidia-dra-driver-gpualso usedefaultRepository: https://helm.ngc.nvidia.com/...— those are intentionally unchanged. The HTTPS NGC index genuinely serves those charts.nvsentinelis the only NVIDIA-published component that ships exclusively via OCI, which is why its registry default was broken.Fixes: nvsentinel registry-default dead path
Related: #698 (Phase-1 follow-up item #2), #715 (Phase 1), #720 (Phase 2)
Type of Change
Component(s) Affected
pkg/recipe) — registry data onlyImplementation Notes
helm pull oci://ghcr.io/nvidia/nvsentinel --version v1.3.0succeeds.CHART='oci://ghcr.io/nvidia/nvsentinel'with emptyREPOin the per-componentupstream.env(correct OCI form).Testing
Risk Assessment
Rollout notes: No migration steps. The broken default is dead code today (overlays override it), so the fix has no behavior impact on existing bundles. Fresh bundles where the user doesn't override the source via overlay will now resolve correctly instead of failing.
Checklist
make lint)git commit -S)