Skip to content

docs(helm): align teardown release and namespace with install (NVBug 6622509) - #2596

Open
kheiss-uwzoo wants to merge 4 commits into
NVIDIA:mainfrom
kheiss-uwzoo:docs/6622509-helm-readme-teardown
Open

docs(helm): align teardown release and namespace with install (NVBug 6622509)#2596
kheiss-uwzoo wants to merge 4 commits into
NVIDIA:mainfrom
kheiss-uwzoo:docs/6622509-helm-readme-teardown

Conversation

@kheiss-uwzoo

Copy link
Copy Markdown
Collaborator

Summary

  • Fix the Helm README full-teardown path so it uses the same REL / NS values as the recommended install (retriever / default), instead of the chart name nemo-retriever and a retriever namespace that the quickstart never creates.
  • Stop hiding helm uninstall failure with 2>/dev/null || true. Confirm the target release first, then verify it is gone before deleting leftover NIM resources.
  • Scope NIMCache / NIMService / PVC cleanup to the named CRs this chart creates. Keep namespace-wide kubectl delete ... --all only as an explicit warning, not the advertised command.

NVBugs: https://nvbugspro.nvidia.com/bug/6622509

This is a docs-only change to nemo_retriever/helm/README.md. No chart templates, values, or runtime behavior.

Why

Literal execution of the old teardown block uninstalled a release that did not exist, suppressed the Helm error, and left the actual retriever release (and GPU / NIMCache / PVC workloads) running. nemo-retriever is the chart name, not the Helm release name.

Test plan

  • Recommended install and full teardown share REL=retriever and NS=default.
  • helm uninstall is not wrapped in 2>/dev/null || true.
  • NIM CR delete uses the four core chart names plus optional names in prose, not --all.
  • Install with the documented recommended command, run the new teardown steps, and confirm the Helm release, service Deployment, named NIM CRs, and intended PVCs are gone (cluster verification; not run in this PR).
  • If you installed into a non-default namespace, confirm the procedure tells you to replace NS and to run helm list --all-namespaces when the target is empty.

PR scope check (nrl-pr-scope / focused-git-pr)

Check Result
Base main
Commits 1 — docs(helm): align teardown release and namespace with install (NVBug 6622509)
Files 1 — nemo_retriever/helm/README.md
Red flags None for this task (Helm README is the intended file; no templates, values.yaml, or workflows)

Overlap: draft #2595 also edits this README (copy-paste ellipsis). This PR rewrites the keep-on-uninstall example and drops the leftover ... in that block.

pre-draft: leakage, mkdocs --strict, ::a, ::p, ::r on the diff vs main

Base: upstream/main
Files: nemo_retriever/helm/README.md

Check Result
Leakage (page roles + see [ CTAs) PASS — helm README is an allowed path; no see [ CTAs in the file. The leakage script vs stale origin/main also scanned leftover untracked custom-metadata.md (not in this diff).
Allowed paths PASS — 1 documentation file
mkdocs --strict N/A for this file (not in MkDocs nav). Strict build failed on untracked leftover pages custom-metadata.md / user-defined-stages.md that are not in this diff.
::a audit PASS — chart name nemo-retriever (Chart.yaml); example release retriever; keepOnUninstall default true; core CR names match templates (95%)
::p polish Applied — numbered teardown steps; shared REL / NS on install and teardown
::r style 95% — new prose has no contractions, uses refer to / do not, and introduces command blocks with complete sentences. Unchanged README contractions and em dashes were left in place.

Code drift (not in this docs PR): nemo_retriever/helm/templates/nims/*.yaml do not set app.kubernetes.io/instance, so label-scoped NIM CR deletion is not available without an eng change. Teardown deletes by the chart's fixed CR names instead.

PR: this draft

…6622509)

The full-teardown block used the chart name and a namespace the quickstart never creates, then hid the failed uninstall. Share REL/NS with the recommended install and verify cleanup against the intended release.
@kheiss-uwzoo
kheiss-uwzoo requested a review from jperez999 August 26, 2026 20:21
@kheiss-uwzoo kheiss-uwzoo self-assigned this Aug 26, 2026
@kheiss-uwzoo kheiss-uwzoo added doc Improvements or additions to documentation 26.08.1 labels Aug 26, 2026
@kheiss-uwzoo
kheiss-uwzoo marked this pull request as ready for review August 27, 2026 16:27
@kheiss-uwzoo
kheiss-uwzoo requested review from a team as code owners August 27, 2026 16:27
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The documentation now aligns the recommended Helm installation and teardown release identity and replaces namespace-wide cleanup with explicit, inspectable resource deletion.

  • Defines consistent REL=retriever and NS=default values for installation and teardown.
  • Verifies the Helm target before cleanup and no longer suppresses uninstall failures.
  • Covers all default core and optional NIM resources, supported name overrides, and corresponding PVCs.
  • Warns against namespace-wide deletion when unrelated resources may be present.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
nemo_retriever/helm/README.md The revised Helm commands resolve the previously reported teardown omissions and add safer target verification and resource-scoping guidance.

Reviews (3): Last reviewed commit: "docs(helm): include optional NIM names i..." | Re-trigger Greptile

Comment thread nemo_retriever/helm/README.md Outdated
The advertised delete commands only listed the four core CRs and PVCs, so enabled optional NIMs and nimServiceName overrides were left behind.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

26.08.1 doc Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant