feat(bundler)!: uniform NNN-folder bundle layout via localformat (#662)

Introduce pkg/bundler/deployer/localformat — a deployer-agnostic writer
that produces the NNN-<component>/ numbered folder layout. Each folder
is one of two kinds: upstream-helm (no Chart.yaml; upstream.env carries
{CHART,REPO,VERSION}) or local-helm (Chart.yaml + templates/). Mixed
components (Helm + raw manifests) emit two adjacent folders: a primary
upstream-helm + an injected -post local-helm wrapper. Each folder
carries a rendered install.sh; deploy.sh is now a generic loop with
no per-component branching, only name-matched special-case blocks.

Rewire --deployer helm to consume localformat. Generator.Generate
delegates per-component content (values.yaml, cluster-values.yaml,
Chart.yaml, templates/, install.sh) to localformat.Write and owns only
top-level orchestration files (README.md, deploy.sh, undeploy.sh,
checksums). Deletes ~195 lines of dead code: generateComponentDirectories,
buildComponentDataList (full version), writeClusterValuesFile,
hasYAMLObjects (relocated to localformat). Deletes ComponentData fields
HasChart, HasManifests, IsKustomize.

Rewrite deploy.sh.tmpl: generic install loop replaces ~90 lines of
{{ if .IsKustomize/.HasChart/.HasManifests }} branching. Delete the
~50-line CRD-race retry block — its trigger (pre-apply raw manifests)
is structurally eliminated; mixed components now apply manifests
post-install via the injected -post wrapped chart, where Helm owns
CRD ordering natively. Rewrite undeploy.sh.tmpl to a generic reverse
loop. Preserve name-matched blocks (skyhook taint cleanup, kai-scheduler
async timeout/diagnostics, DRA kubelet-plugin restart, orphaned-CRD
scan) around the loops; their simplification is deferred.

Refactor helm_test.go: introduce assertBundleGolden helper (mirroring
localformat's writer_test) + 5 golden-file scenarios under testdata/
exercising upstream-helm-only, manifest-only, mixed gpu-operator,
kai-scheduler-present, skyhook-present bundles. Goldens double as
reference examples of rendered output. Delete 11 redundant tests
covering removed code or duplicating golden coverage; add focused
error-path tests. Net: 48 → 37 tests, 3408 → 2130 lines (−37%),
coverage 87.6% → 87.9%.

Update bundler_test.go, handler_test.go, and 6 chainsaw e2e fixtures
to assert on NNN-prefixed paths.

Breaking changes for --deployer helm consumers:
- Bundle directory layout: <component>/ → NNN-<component>/
- Mixed components emit two folders (primary + -post)
- Raw manifests for mixed components apply post-install only
- CRD-race retry block removed; pre-apply mechanism gone
- No more per-component README.md (top-level README still present)

Docs: docs/user/cli-reference.md updated with new layout, folder kinds,
mixed-component -post injection, and breaking-change callout.
docs/contributor/component.md describes localformat as the shared
base format and the load-bearing invariants (localformat never writes
deployer-specific files; install.sh never name-customized; Write is
deterministic).

Author: lockwobr

docs/contributor/component.md

@@ -32,7 +32,7 @@ The bundler system converts RecipeInput objects into deployment artifacts. Artif
 **Load-bearing invariants** (don't violate without changing the design):
 
 1. **`localformat` never writes deployer-specific files.** `deploy.sh`, `helmfile.yaml`, argocd `Application` CRs, Flux `HelmRelease`s — all produced by the respective deployer after `Write()` returns. This separation is what makes a single layout consumable by every deployer.
-2. **`install.sh` is never name-customized.** It is rendered from one of exactly two templates (`install-upstream-helm.sh.tmpl`, `install-local-helm.sh.tmpl`), parameterized only by data (name, namespace, upstream ref). Name-keyed quirks (kai-scheduler async timeout, skyhook taint cleanup, DRA restart, orphan-CRD scan) stay in `deploy.sh` as name-matched blocks — not in `install.sh`. This is the structural barrier that prevents per-folder scripts from accumulating drift.
+2. **`install.sh` is never name-customized.** It is rendered from one of exactly two templates (`install-upstream-helm.sh.tmpl`, `install-local-helm.sh.tmpl`), parameterized only by data (name, namespace, upstream ref). Name-keyed quirks (kai-scheduler async timeout, nodewright-operator taint cleanup, DRA restart, orphan-CRD scan) stay in `deploy.sh` as name-matched blocks — not in `install.sh`. This is the structural barrier that prevents per-folder scripts from accumulating drift.
 3. **`Write` is deterministic and idempotent.** Same inputs → same on-disk bytes → same `Folder` slice. Map iteration is sorted; no timestamps or random suffixes are embedded.
 
 For the full classification table, base-format invariants, and the helm deployer's call site, see `pkg/bundler/deployer/localformat/doc.go` (godoc) and `pkg/bundler/deployer/helm/helm.go::Generate`. Further design history: ticket [#662](https://github.com/NVIDIA/aicr/issues/662).

docs/user/cli-reference.md

@@ -1156,7 +1156,7 @@ bundles/
 
 Previous releases used a flat `<component>/` layout with `manifests/` siblings and a `--deployer helm` script that branched on component kind. The new format is uniform:
 
-- All folders carry a rendered `install.sh`. The top-level `deploy.sh` is a generic loop with no per-component branching — name-matched special-case blocks (skyhook taint cleanup, kai-scheduler async timeout, orphan-CRD scan, DRA kubelet-plugin restart) live around the loop, not inside it.
+- All folders carry a rendered `install.sh`. The top-level `deploy.sh` is a generic loop with no per-component branching — name-matched special-case blocks (nodewright-operator taint cleanup, kai-scheduler async timeout, orphan-CRD scan, DRA kubelet-plugin restart) live around the loop, not inside it.
 - Raw manifests for mixed components now apply **post-install only**, via the injected `-post` wrapped chart. The earlier pre-apply mechanism with a CRD-race retry wrapper is gone — Helm now owns CRD ordering for mixed components natively.
 - Tooling that parsed bundle paths by bare component name must account for the `NNN-` prefix.
 

pkg/bundler/bundler_test.go

@@ -423,11 +423,10 @@ func TestMake_SetEnabledOverridesPrecedence(t *testing.T) {
 			}
 
 			// When included, the component appears as the second numbered folder
-			// (gpu-operator is 001, aws-ebs-csi-driver is 002). When excluded,
-			// neither numbering nor flat name should exist.
-			_, statErr1 := os.Stat(filepath.Join(tmpDir, "002-aws-ebs-csi-driver"))
-			_, statErr2 := os.Stat(filepath.Join(tmpDir, "aws-ebs-csi-driver"))
-			included := !os.IsNotExist(statErr1) || !os.IsNotExist(statErr2)
+			// (gpu-operator is 001, aws-ebs-csi-driver is 002). The flat layout
+			// is gone in this PR — only assert against the numbered path.
+			_, statErr := os.Stat(filepath.Join(tmpDir, "002-aws-ebs-csi-driver"))
+			included := !os.IsNotExist(statErr)
 
 			if included != tt.expectIncluded {
 				t.Errorf("aws-ebs-csi-driver included=%v, want %v", included, tt.expectIncluded)

pkg/bundler/deployer/helm/helm_test.go

@@ -1430,7 +1430,7 @@ func TestUndeployScript_PreflightSkipListCoversManifestDeletedReleases(t *testin
 			ComponentRefs: []recipe.ComponentRef{
 				{Name: "cert-manager", Namespace: "cert-manager", Chart: "cert-manager", Version: "v1.17.2", Source: "https://charts.jetstack.io"},
 				{Name: "kgateway", Namespace: "kgateway-system", Chart: "kgateway", Version: "v0.1.0", Source: "https://example.invalid/charts"},
-				{Name: "skyhook-operator", Namespace: "skyhook", Chart: "skyhook-operator", Version: "v0.1.0", Source: "https://example.invalid/charts"},
+				{Name: "nodewright-operator", Namespace: "skyhook", Chart: "nodewright-operator", Version: "v0.1.0", Source: "https://example.invalid/charts"},
 			},
 			DeploymentOrder: []string{"cert-manager", "kgateway", "nodewright-operator"},
 		},
@@ -1471,7 +1471,7 @@ func TestUndeployScript_PreflightSkipListCoversManifestDeletedReleases(t *testin
 	}
 
 	out := stdout.String()
-	for _, want := range []string{"skip:skyhook-operator", "skip:kgateway", "check:cert-manager"} {
+	for _, want := range []string{"skip:nodewright-operator", "skip:kgateway", "check:cert-manager"} {
 		if !strings.Contains(out, want) {
 			t.Errorf("expected %q in output; stdout=%q stderr=%q", want, out, stderr.String())
 		}
@@ -1795,7 +1795,7 @@ func TestUndeployScript_DynamoPlatformOwnsExplicitGroveCRDs(t *testing.T) {
 // The goldens double as reference examples of what a rendered bundle looks
 // like for each common shape: upstream-helm-only, manifest-only, mixed
 // (upstream + raw manifests → primary + -post folder), kai-scheduler (async
-// block), and skyhook-operator (pre-install taint cleanup block).
+// block), and nodewright-operator (pre-install taint cleanup block).
 
 func TestBundleGolden_UpstreamHelmOnly(t *testing.T) {
 	outDir := t.TempDir()
@@ -1911,19 +1911,24 @@ func TestBundleGolden_KaiSchedulerPresent(t *testing.T) {
 	assertBundleGolden(t, outDir, "testdata/kai_scheduler_present")
 }
 
-func TestBundleGolden_SkyhookPresent(t *testing.T) {
+func TestBundleGolden_NodewrightPresent(t *testing.T) {
 	outDir := t.TempDir()
+	// Mirror the production registry: component name "nodewright-operator"
+	// but the upstream chart is still named "skyhook-operator". This shape
+	// is what real recipes have post-rename — the registry component name
+	// drives the name-matched taint cleanup blocks; the chart name drives
+	// helm install.
 	g := &Generator{
 		RecipeResult: singleComponentRecipe(
-			"skyhook-operator", "skyhook", "skyhook-operator", "v0.1.0",
+			"nodewright-operator", "skyhook", "skyhook-operator", "v0.1.0",
 			"https://example.invalid/charts"),
-		ComponentValues: map[string]map[string]any{"skyhook-operator": {}},
+		ComponentValues: map[string]map[string]any{"nodewright-operator": {}},
 		Version:         "v1.0.0",
 	}
 	if _, err := g.Generate(context.Background(), outDir); err != nil {
 		t.Fatalf("Generate: %v", err)
 	}
-	assertBundleGolden(t, outDir, "testdata/skyhook_present")
+	assertBundleGolden(t, outDir, "testdata/nodewright_present")
 }
 
 // ---------------------------------------------------------------------------

pkg/bundler/deployer/helm/templates/README.md.tmpl

@@ -24,13 +24,7 @@ via its own `install.sh`:
 | Component | Version | Namespace | Source |
 |-----------|---------|-----------|--------|
 {{ range .Components -}}
-{{ if .IsKustomize -}}
-| {{ .Name }} | Kustomize | {{ .Tag }} | {{ .Namespace }} | {{ .Repository }} ({{ .Path }}) |
-{{ else if .HasChart -}}
-| {{ .Name }} | Helm | {{ .Version }} | {{ .Namespace }} | {{ .ChartName }} ({{ .Repository }}) |
-{{ else -}}
-| {{ .Name }} | Manifests | | {{ .Namespace }} | |
-{{ end -}}
+| {{ .Name }} | {{ if .Version }}{{ .Version }}{{ else }}N/A{{ end }} | {{ .Namespace }} | {{ if .Repository }}{{ .ChartName }} ({{ .Repository }}){{ else }}local{{ end }} |
 {{ end }}
 
 {{ if .Constraints }}
@@ -60,7 +54,7 @@ Use `--no-wait` to skip Helm chart-level waiting where AICR uses `--wait` (keeps
 ./deploy.sh --no-wait
 ```
 
-> **Note:** The deploy script's final status reflects install/apply results. If `--best-effort` was used, one or more components may still have failed; check warning lines and logs. This does **not** mean the cluster is ready for GPU workloads. On fresh GPU nodes, cluster convergence (Nodewright node tuning, GPU operator operand rollout, DRA kubelet plugin registration) continues asynchronously after the script exits. See the [AICR CLI Reference](https://github.com/NVIDIA/aicr/blob/main/docs/user/cli-reference.md#deploy-script-behavior-deploysh) for details.
+> **Note:** The deploy script's final status reflects install/apply results. If `--best-effort` was used, one or more components may still have failed; check warning lines and logs. This does **not** guarantee the cluster is ready to schedule workloads — operator-driven cluster convergence (CRD reconciliation, node tuning, plugin registration, etc.) continues asynchronously after the script exits, in operator-specific ways. See the [AICR CLI Reference](https://github.com/NVIDIA/aicr/blob/main/docs/user/cli-reference.md#deploy-script-behavior-deploysh) for details.
 
 ## Manual Installation
 
@@ -71,56 +65,24 @@ with the right arguments baked in. To install a single component manually:
 cd NNN-<component-name>
 bash install.sh
 ```
-{{ else if .HasChart -}}
-```bash
-{{ if .IsOCI -}}
-helm upgrade --install {{ .Name }} {{ .Repository }}/{{ .ChartName }} \
-  --version {{ .Version }} \
-  -n {{ .Namespace }} --create-namespace \
-  -f {{ .Name }}/values.yaml \
-  -f {{ .Name }}/cluster-values.yaml \
-  --wait --timeout 10m
-{{ else -}}
-helm upgrade --install {{ .Name }} {{ .ChartName }} \
-  --repo {{ .Repository }} \
-  --version {{ .Version }} \
-  -n {{ .Namespace }} --create-namespace \
-  -f {{ .Name }}/values.yaml \
-  -f {{ .Name }}/cluster-values.yaml \
-  --wait --timeout 10m
-{{ end -}}
-```
-{{ end -}}
-{{ if .HasManifests }}
-```bash
-kubectl apply -f {{ .Name }}/manifests/
-```
-{{ end }}
-{{ end }}
 
 ## Customization
 
-Each Helm component has two values files in its directory:
-
-- `values.yaml` — resolved configuration from the recipe. Edit to override defaults:
+Each component folder has its own `values.yaml` (static) and `cluster-values.yaml`
+(dynamic, per-cluster). Edit either before deploying:
 
-  ```bash
-  vim gpu-operator/values.yaml
-  ```
-
-- `cluster-values.yaml` — install-time parameters. Any paths declared with
-  `aicr bundle --dynamic <component>:<path>` are pulled out of `values.yaml`
-  and placed here for you to fill in. The file is always created (empty if
-  no dynamic paths were declared) and passed to `helm upgrade --install`
-  alongside `values.yaml` by both `deploy.sh` and the per-component commands
-  in the "Manual Installation" section above.
+```bash
+vim NNN-<component-name>/values.yaml
+vim NNN-<component-name>/cluster-values.yaml
+```
 
 ## Upgrade
 
 Re-run the per-component install.sh to upgrade an already-installed release:
 
 ```bash
-helm upgrade <component> <chart> --version <version> -n <namespace> -f <component>/values.yaml -f <component>/cluster-values.yaml --wait --timeout 10m
+cd NNN-<component-name>
+bash install.sh
 ```
 
 ## Uninstall
@@ -144,20 +106,27 @@ helm uninstall {{ .Name }} -n {{ .Namespace }}
 ### Check deployment status
 
 ```bash
-kubectl get pods -A | grep -E 'gpu-operator|network-operator|cert-manager'
+kubectl get pods -A | grep -E '{{ range $i, $c := .Components }}{{ if $i }}|{{ end }}{{ $c.Name }}{{ end }}'
 ```
 
 ### View component logs
 
+Inspect a single component's pods (replace `<component>` and `<namespace>`
+with one of the entries from the table above):
+
 ```bash
-kubectl logs -n gpu-operator -l app=gpu-operator
+kubectl logs -n <namespace> -l app.kubernetes.io/instance=<component>
 ```
 
+{{- $hasGPU := false -}}
+{{- range .Components -}}{{- if eq .Name "gpu-operator" -}}{{- $hasGPU = true -}}{{- end -}}{{- end -}}
+{{- if $hasGPU }}
 ### Verify GPU access
 
 ```bash
 kubectl get nodes -o jsonpath='{.items[*].status.allocatable}' | jq '.["nvidia.com/gpu"]'
 ```
+{{- end }}
 
 ## References
 

pkg/bundler/deployer/helm/templates/deploy.sh.tmpl

@@ -31,7 +31,7 @@ set -euo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
 # Run helm commands from a temp directory to prevent local chart directories
-# (e.g., bundle/001-skyhook-operator/) from shadowing remote chart references.
+# (e.g., bundle/001-nodewright-operator/) from shadowing remote chart references.
 HELM_WORKDIR="$(mktemp -d)"
 trap 'rm -rf "${HELM_WORKDIR}"; exit 130' INT TERM
 trap 'rm -rf "${HELM_WORKDIR}"' EXIT
@@ -263,17 +263,17 @@ if [[ "${nodewright_available}" == "0" || -z "${nodewright_available}" ]]; then
   # Extract the taint key from bundle values. The YAML value is a taint string
   # like "custom.io/gate=true:NoSchedule" — extract the key before the first "=".
   # If runtimeRequiredTaint is not set, use the default skyhook.nvidia.com.
-  SKYHOOK_TAINT_KEY="skyhook.nvidia.com"
-  # Locate skyhook-operator's NNN-prefixed directory at runtime.
-  skyhook_dir="$(ls -d "${SCRIPT_DIR}"/[0-9][0-9][0-9]-skyhook-operator 2>/dev/null | head -1)"
-  if [[ -n "${skyhook_dir}" && -f "${skyhook_dir}/values.yaml" ]]; then
-    custom_taint_line=$(grep 'runtimeRequiredTaint:' "${skyhook_dir}/values.yaml" 2>/dev/null || true)
+  NODEWRIGHT_TAINT_KEY="skyhook.nvidia.com"
+  # Locate nodewright-operator's NNN-prefixed directory at runtime.
+  nodewright_dir="$(ls -d "${SCRIPT_DIR}"/[0-9][0-9][0-9]-nodewright-operator 2>/dev/null | head -1)"
+  if [[ -n "${nodewright_dir}" && -f "${nodewright_dir}/values.yaml" ]]; then
+    custom_taint_line=$(grep 'runtimeRequiredTaint:' "${nodewright_dir}/values.yaml" 2>/dev/null || true)
     if [[ -n "${custom_taint_line}" ]]; then
       taint_value=$(echo "${custom_taint_line}" | sed 's/.*runtimeRequiredTaint:[[:space:]]*//' | tr -d '"' | tr -d "'")
       if [[ -n "${taint_value}" ]]; then
         # Handle both key=value:effect and key:effect formats
-        SKYHOOK_TAINT_KEY="${taint_value%%=*}"
-        SKYHOOK_TAINT_KEY="${SKYHOOK_TAINT_KEY%%:*}"
+        NODEWRIGHT_TAINT_KEY="${taint_value%%=*}"
+        NODEWRIGHT_TAINT_KEY="${NODEWRIGHT_TAINT_KEY%%:*}"
       fi
     fi
   fi
@@ -312,7 +312,13 @@ for dir in "${SCRIPT_DIR}"/[0-9][0-9][0-9]-*/; do
   dir="${dir%/}"
   base="${dir##*/}"
   name="${base#[0-9][0-9][0-9]-}"
-  echo "Installing ${name} (${base})..."
+  # Source the namespace from the folder's install.sh — not the folder
+  # basename — because the helm release name and its target namespace can
+  # differ (e.g. nodewright-operator → namespace skyhook; gpu-operator-post →
+  # namespace gpu-operator). cleanup_helm_hooks and the kai diagnostics
+  # both operate on the namespace.
+  namespace=$(awk '{ for (i=1;i<NF;i++) if ($i=="--namespace") { print $(i+1); exit } }' "${dir}/install.sh")
+  echo "Installing ${name} (${base}) into namespace ${namespace}..."
 
   # Derive wait args: global --wait/--no-wait behavior + per-component timeout.
   COMPONENT_HELM_TIMEOUT="${HELM_TIMEOUT}"
@@ -343,13 +349,13 @@ for dir in "${SCRIPT_DIR}"/[0-9][0-9][0-9]-*/; do
       break
     fi
     attempt=$((attempt + 1))
-    dump_kai_scheduler_helm_diagnostics "${name}"
+    dump_kai_scheduler_helm_diagnostics "${namespace}"
     if [[ ${attempt} -gt ${COMPONENT_MAX_RETRIES} ]]; then
       echo "ERROR: ${name} install failed after ${attempt} attempts"
       helm_failed "${name}"
       break
     fi
-    cleanup_helm_hooks "${name}"
+    cleanup_helm_hooks "${namespace}"
     wait_secs=$(backoff_seconds "${attempt}")
     echo "RETRY: ${name} install failed (attempt ${attempt}/${COMPONENT_MAX_RETRIES}), retrying in ${wait_secs}s..."
     sleep "${wait_secs}"

pkg/bundler/deployer/helm/templates/undeploy.sh.tmpl

@@ -469,28 +469,41 @@ for dir in $(ls -1d "${SCRIPT_DIR}"/[0-9][0-9][0-9]-*/ 2>/dev/null | sort -r); d
     continue
   fi
   echo "Uninstalling ${name} (${ns})..."
+  # For local-helm folders (Chart.yaml + templates/), kubectl-delete the
+  # rendered templates BEFORE helm uninstall. The templates may carry
+  # helm.sh/hook annotations (post-install/post-upgrade) which helm does
+  # not track or clean up on uninstall — so without this pre-delete, the
+  # operator gets removed but its hook-created CRs (and their finalizers)
+  # linger. Doing the delete while the controller is still running lets
+  # finalizers clear naturally.
+  if [[ -d "${dir}/templates" ]]; then
+    for tpl in "${dir}/templates/"*.yaml; do
+      [[ -f "${tpl}" ]] || continue
+      kubectl delete -n "${ns}" -f "${tpl}" --ignore-not-found --timeout="${HELM_TIMEOUT}s" || true
+    done
+  fi
   helm_force_uninstall "${name}" "${ns}"
   delete_release_cluster_resources "${name}" "${ns}"
   delete_orphaned_webhooks_for_ns "${ns}"
 done
 
-# Remove skyhook node taints that persist after operator removal.
-# Skyhook taints nodes during kernel tuning. The taint key is configurable
+# Remove nodewright node taints that persist after operator removal.
+# Nodewright taints nodes during kernel tuning. The taint key is configurable
 # via runtimeRequiredTaint (defaults to skyhook.nvidia.com).
 {{- range .ComponentsReversed }}
-{{- if eq .Name "skyhook-operator" }}
-SKYHOOK_TAINT_KEY="skyhook.nvidia.com"
-# Locate skyhook-operator's NNN-prefixed directory at runtime.
-skyhook_dir="$(ls -d "${SCRIPT_DIR}"/[0-9][0-9][0-9]-skyhook-operator 2>/dev/null | head -1)"
-if [[ -n "${skyhook_dir}" && -f "${skyhook_dir}/values.yaml" ]]; then
-  custom_taint_line=$(grep 'runtimeRequiredTaint:' "${skyhook_dir}/values.yaml" 2>/dev/null || true)
+{{- if eq .Name "nodewright-operator" }}
+NODEWRIGHT_TAINT_KEY="skyhook.nvidia.com"
+# Locate nodewright-operator's NNN-prefixed directory at runtime.
+nodewright_dir="$(ls -d "${SCRIPT_DIR}"/[0-9][0-9][0-9]-nodewright-operator 2>/dev/null | head -1)"
+if [[ -n "${nodewright_dir}" && -f "${nodewright_dir}/values.yaml" ]]; then
+  custom_taint_line=$(grep 'runtimeRequiredTaint:' "${nodewright_dir}/values.yaml" 2>/dev/null || true)
   if [[ -n "${custom_taint_line}" ]]; then
     # Extract value after "runtimeRequiredTaint:", then extract taint key before "="
     taint_value=$(echo "${custom_taint_line}" | sed 's/.*runtimeRequiredTaint:[[:space:]]*//' | tr -d '"' | tr -d "'")
     if [[ -n "${taint_value}" ]]; then
       # Handle both key=value:effect and key:effect formats
-      SKYHOOK_TAINT_KEY="${taint_value%%=*}"
-      SKYHOOK_TAINT_KEY="${SKYHOOK_TAINT_KEY%%:*}"
+      NODEWRIGHT_TAINT_KEY="${taint_value%%=*}"
+      NODEWRIGHT_TAINT_KEY="${NODEWRIGHT_TAINT_KEY%%:*}"
     fi
   fi
 fi