fix(recipes): use Helm manifest-only pattern for gke-nccl-tcpxo

[yuanchen8911]

Summary

The registry entry for gke-nccl-tcpxo uses a top-level manifest: block that is not parsed by the ComponentConfig struct, so manifest.defaultNamespace: kube-system is silently ignored. After #706 wrapped manifest-only components as local Helm charts, the generated install.sh emits helm upgrade --install ... --namespace --create-namespace (empty namespace value) → bundler fails on every post-#706 GKE-COS H100 training run.

Fix: declare gke-nccl-tcpxo with the established manifest-only Helm pattern (empty defaultRepository), matching nodewright-customizations.

Motivation / Context

PR #715's CI failed both Tier 2: h100-gke-cos-training and Tier 2: h100-gke-cos-training-kubeflow with:

Error: create: failed to create: namespaces "--create-namespace" not found

Root-cause:

1. Latent metadata bug. recipes/registry.yaml declares the namespace under manifest:, but the registry's Go schema only parses helm: and kustomize:. manifest: is dead config.
2. Exposed by deploy-path migration. Pre-feat(bundler)!: uniform NNN-folder bundle layout via localformat (#662) #706, manifest-only components were installed via raw kubectl apply -f .../manifests/. The manifest YAML carries inline metadata.namespace: kube-system, so kubectl routed correctly without needing ComponentRef.Namespace.
3. feat(bundler)!: uniform NNN-folder bundle layout via localformat (#662) #706 (feat(bundler)\!: uniform NNN-folder bundle layout via localformat) wraps every component — manifest-only included — as a local Helm chart. The generated install.sh now always uses helm upgrade --install ... --namespace <ns> --create-namespace, which requires ComponentRef.Namespace. With the unparsed manifest: block, that field is empty.

The result on a post-#706 GKE-COS bundle is:

helm upgrade --install gke-nccl-tcpxo ./ \
  --namespace  --create-namespace \      # ← empty namespace

Shell argument collapsing makes Helm parse --create-namespace as the namespace name → fails. gke-nccl-tcpxo is the only component still using the unparsed manifest: form. nodewright-customizations already uses the correct pattern.

Change

   - name: gke-nccl-tcpxo
     displayName: gke-nccl-tcpxo
     valueOverrideKeys:
       - gkenccltcpxo
-    manifest:
+    helm:
+      # Manifest-only component - no external Helm chart, uses manifestFiles
+      defaultRepository: ""
       defaultNamespace: kube-system

Net change: 1 file, +3/-1 lines. No other component is affected.

Fixes: post-#706 GKE-COS H100 training bundle generation
Related: #706 (the deploy-path migration that exposed the bug), #715 (the first PR whose CI completed far enough to surface it)

Type of Change

• Bug fix (non-breaking change that fixes an issue)

Component(s) Affected

• Recipe engine / data (pkg/recipe) — registry data only
• Bundlers (pkg/bundler, pkg/component/*) — fixes generated install.sh

Implementation Notes

• The fix uses the same pattern already established for nodewright-customizations (also a manifest-only component): declare under helm: with an empty defaultRepository. This routes correctly through the post-feat(bundler)!: uniform NNN-folder bundle layout via localformat (#662) #706 local-Helm wrapper without changing the manifest content itself.
• No values schema or recipe changes; no overlay touched.

Testing

End-to-end repro and fix verification:

$ aicr recipe --service gke --accelerator h100 --intent training --os cos -o /tmp/recipe.yaml
$ aicr bundle -r /tmp/recipe.yaml -o /tmp/bundle
$ grep -A1 'helm upgrade' /tmp/bundle/*-gke-nccl-tcpxo/install.sh
helm upgrade --install gke-nccl-tcpxo ./ \
  --namespace kube-system --create-namespace \

Pre-fix the same command produced --namespace --create-namespace (empty).

make lint                        # 0 issues
go test ./pkg/recipe/... ./pkg/bundler/...   # all pass

Sanity-checked AKS-training bundle still generates correctly (no regression on non-GKE recipes).

Risk Assessment

• Low — Single-file, 3-line registry data change. Switches gke-nccl-tcpxo from a never-parsed manifest: block to the well-established manifest-only helm: pattern (same as nodewright-customizations). No code, no values, no overlays touched.

Rollout notes: Bundles regenerated post-merge for any GKE-COS recipe will produce a working install.sh for gke-nccl-tcpxo. Existing installations are unaffected until re-bundled.

Checklist

• Tests pass locally (make test with -race)
• Linter passes (make lint)
• I did not skip/disable tests to make CI green
• I added/updated tests for new functionality (N/A — registry data fix; consider follow-up regression test asserting non-empty --namespace for manifest-only install.sh)
• I updated docs if user-facing behavior changed (N/A — internal data fix)
• Changes follow existing patterns in the codebase (matches nodewright-customizations)
• Commits are cryptographically signed (git commit -S)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: adbfddba-887f-44a1-9b0f-09abe2b86c4c

📥 Commits

Reviewing files that changed from the base of the PR and between 4746872 and afc1026.

📒 Files selected for processing (1)

• recipes/registry.yaml

---

📝 Walkthrough

Walkthrough

The recipes/registry.yaml entry for component gke-nccl-tcpxo is modified: the top-level manifest block is replaced with a helm block marked as a manifest-only component. The helm block adds defaultRepository: "" and preserves defaultNamespace: kube-system. Total lines changed: +3/-1.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: converting gke-nccl-tcpxo from manifest to Helm manifest-only pattern.
Description check	✅ Passed	The description thoroughly explains the bug, root cause, fix, testing, and impact, directly relating to the changeset in recipes/registry.yaml.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Review rate limit: 9/10 reviews remaining, refill in 6 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[yuanchen8911]

Absorbing this fix into #715 — same 3-line registry change now lands as part of the phase-1 PR (commit 102fb8fa in #715, with Closes: #718 in the trailer). Same diff, same verification; folding it in to unblock #715's CI without a sequencing dependency. Bug history preserved in the #715 commit body.