Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions pkg/bundler/deployer/helmfile/helmfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,111 @@ func TestBuildHelmfile_SameNamespaceNeedsBare(t *testing.T) {
}
}

// TestBuildHelmfile_CreateNamespaceFromFolder asserts that buildHelmfile
// emits a per-release `createNamespace: false` override when a folder's
// CreateNamespace flag is false (the Talos privileged-namespace pattern),
// and emits no override (relying on helmDefaults.createNamespace: true)
// otherwise. The localformat writer already encodes the
// chart-owns-Namespace decision; the helmfile deployer must honor it or
// helm refuses to import the namespace it created out-of-band.
func TestBuildHelmfile_CreateNamespaceFromFolder(t *testing.T) {
folders := []localformat.Folder{
// Pre-injection folder that ships its own Namespace (Talos pattern):
// localformat would set CreateNamespace=false; expect the override.
{Index: 1, Dir: "001-gpu-operator-pre", Kind: localformat.KindLocalHelm,
Name: "gpu-operator-pre", Parent: "gpu-operator", CreateNamespace: false},
// Primary upstream-helm folder: no Namespace conflict possible from
// AICR's perspective; expect no per-release override.
{Index: 2, Dir: "002-gpu-operator", Kind: localformat.KindUpstreamHelm,
Name: "gpu-operator", Parent: "gpu-operator", CreateNamespace: true,
Upstream: &localformat.Upstream{Chart: "gpu-operator", Repo: "https://helm.ngc.nvidia.com/nvidia", Version: "v25.3.3"}},
}
ns := map[string]string{"gpu-operator": "privileged-gpu-operator"}
doc, err := buildHelmfile(folders, ns, nil)
if err != nil {
t.Fatalf("buildHelmfile() error = %v", err)
}
if len(doc.Releases) != 2 {
t.Fatalf("expected 2 releases, got %d", len(doc.Releases))
}
if doc.Releases[0].CreateNamespace == nil || *doc.Releases[0].CreateNamespace {
t.Errorf("release[0] (gpu-operator-pre) CreateNamespace = %v, want pointer to false — "+
"the chart owns the Namespace and helm must not create it out-of-band",
doc.Releases[0].CreateNamespace)
}
if doc.Releases[1].CreateNamespace != nil {
t.Errorf("release[1] (gpu-operator) CreateNamespace = %v, want nil — "+
"helmDefaults.createNamespace: true covers the common case; no per-release override should be emitted",
doc.Releases[1].CreateNamespace)
}
}

// TestGenerate_NamespaceOwningPreManifest is the integration counterpart:
// a pre-manifest containing a Namespace resource flows end-to-end through
// localformat.Write → buildHelmfile and surfaces as `createNamespace: false`
// on the pre-release in the rendered helmfile.yaml. This locks the
// os-talos failure mode the helmfile deployer originally hit (release
// creates namespace via --create-namespace, then the chart's Namespace
// template can't claim it).
func TestGenerate_NamespaceOwningPreManifest(t *testing.T) {
namespaceManifest := []byte("apiVersion: v1\n" +
"kind: Namespace\n" +
"metadata:\n" +
" name: privileged-gpu-operator\n" +
" labels:\n" +
" pod-security.kubernetes.io/enforce: privileged\n")
g := &Generator{
RecipeResult: recipeWith(
recipe.ComponentRef{
Name: "gpu-operator",
Namespace: "privileged-gpu-operator",
Chart: "gpu-operator",
Version: "v25.3.3",
Source: "https://helm.ngc.nvidia.com/nvidia",
Type: recipe.ComponentTypeHelm,
},
),
ComponentValues: map[string]map[string]any{"gpu-operator": {}},
ComponentPreManifests: map[string]map[string][]byte{
"gpu-operator": {"talos-namespace.yaml": namespaceManifest},
},
Version: testBundlerVersion,
}
outputDir := t.TempDir()
if _, err := g.Generate(context.Background(), outputDir); err != nil {
t.Fatalf("Generate() error = %v", err)
}
data, err := os.ReadFile(filepath.Join(outputDir, fileHelmfile))
if err != nil {
t.Fatalf("read helmfile.yaml: %v", err)
}
var doc Helmfile
if err := yaml.Unmarshal(data, &doc); err != nil {
t.Fatalf("parse helmfile.yaml: %v\n--- content ---\n%s", err, data)
}
if len(doc.Releases) != 2 {
t.Fatalf("expected 2 releases (gpu-operator-pre + gpu-operator), got %d", len(doc.Releases))
}
// release[0] is the injected -pre with the Namespace template.
if got := doc.Releases[0].Name; got != "gpu-operator-pre" {
t.Fatalf("release[0].name = %q, want %q", got, "gpu-operator-pre")
}
if doc.Releases[0].CreateNamespace == nil || *doc.Releases[0].CreateNamespace {
t.Errorf("release[0] (gpu-operator-pre) CreateNamespace = %v, want pointer to false — "+
"pre-manifest ships a kind: Namespace, so helm must not create it out-of-band",
doc.Releases[0].CreateNamespace)
}
// release[1] is the primary upstream-helm; no override expected.
if doc.Releases[1].CreateNamespace != nil {
t.Errorf("release[1] (gpu-operator) CreateNamespace = %v, want nil",
doc.Releases[1].CreateNamespace)
}
// Global default is unchanged — every other release still benefits.
if !doc.HelmDefaults.CreateNamespace {
t.Error("helmDefaults.createNamespace should remain true; only the affected release overrides it")
}
}

// TestComponentOverrides_ParityWithHelmDeployScript guards against silent
// drift between componentOverrides in this package and the hardcoded
// ASYNC_COMPONENTS / COMPONENT_HELM_TIMEOUT case block in
Expand Down
20 changes: 20 additions & 0 deletions pkg/bundler/deployer/helmfile/releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ type Release struct {
// Timeout is rendered only when the release overrides
// helmDefaults.timeout (e.g., kai-scheduler 20m).
Timeout int `yaml:"timeout,omitempty"`
// CreateNamespace is rendered only when explicitly set to false —
// the chart ships its own Namespace template, so helm must not
// create the namespace out-of-band. helmDefaults.createNamespace
// (true) applies otherwise. Pointer so a deliberate false survives
// YAML marshaling and a nil value omits the field entirely.
CreateNamespace *bool `yaml:"createNamespace,omitempty"`
}

// overrides carries per-component helm flag overrides.
Expand Down Expand Up @@ -155,6 +161,20 @@ func buildHelmfile(folders []localformat.Folder, namespaceByComponent map[string
}
}

// Override the global helmDefaults.createNamespace when this
// folder's chart owns the target Namespace (Talos privileged-
// namespace pre-injection). Without this, helmfile passes
// --create-namespace and helm creates the namespace out-of-band;
// the chart's later Namespace template then collides because
// the existing namespace lacks the release's ownership labels.
// Only emit the override when false — the global default
// already covers the true case and we want helmfile.yaml
// minimal.
if !f.CreateNamespace {
falseVal := false
rel.CreateNamespace = &falseVal
}

releases = append(releases, rel)
prevName = rel.Name
prevNamespace = ns
Expand Down
10 changes: 10 additions & 0 deletions pkg/bundler/deployer/localformat/folder.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,14 @@ type Folder struct {
Parent string // component this folder belongs to (== Name for primary)
Upstream *Upstream // set iff Kind == KindUpstreamHelm
Files []string // relative paths (to OutputDir) of files written in this folder
// CreateNamespace is true when the orchestration layer should pass
// --create-namespace to helm for this folder's release, false when
// the folder's chart ships its own Namespace resource (the Talos
// privileged-namespace pre-injection pattern). install.sh already
// honors this internally; the field exposes the same decision to
// out-of-band deployers (e.g., helmfile) that bypass install.sh.
// Helm 3 refuses to import a namespace it created out-of-band via
// --create-namespace because that namespace lacks the release's
// ownership annotations.
CreateNamespace bool
}
13 changes: 7 additions & 6 deletions pkg/bundler/deployer/localformat/local_helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,12 @@ func writeLocalHelmFolder(
files = append(files, filepath.Join(dir, "install.sh"))

return Folder{
Index: idx,
Dir: dir,
Kind: KindLocalHelm,
Name: name,
Parent: parent,
Files: files,
Index: idx,
Dir: dir,
Kind: KindLocalHelm,
Name: name,
Parent: parent,
Files: files,
CreateNamespace: effectiveCreateNamespace,
}, nil
}
4 changes: 4 additions & 0 deletions pkg/bundler/deployer/localformat/upstream_helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,5 +109,9 @@ func writeUpstreamHelmFolder(outputDir, dir string, idx int, c Component) (Folde
filepath.Join(dir, "upstream.env"),
filepath.Join(dir, "install.sh"),
},
// Upstream Helm folders don't ship AICR-rendered templates, so
// the chart-owns-Namespace detection in local_helm doesn't apply
// here. Default to true to match install.sh's --create-namespace.
CreateNamespace: true,
}, nil
}
4 changes: 4 additions & 0 deletions pkg/bundler/deployer/localformat/vendor_folder.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ func writeVendoredHelmFolder(
Name: c.Name,
Parent: c.Name,
Files: files,
// Vendored folders wrap an upstream chart that AICR does not
// render; the chart-owns-Namespace detection does not apply
// here. Default to true to match the upstream-helm path.
CreateNamespace: true,
}, rec, nil
}

Expand Down
Loading