install: make install and upgrade values declarative

cmd/kubectl-ate/internal/cmd/install.go

@@ -82,7 +82,8 @@ var installCmd = &cobra.Command{
 
 Installs the substrate-crds, substrate-podcert, substrate-atelet and substrate
 Helm releases in order, and creates the CA pools and secrets they need.
-Existing secrets are kept. Rerunning install resumes a failed install.
+Existing secrets are kept. Rerunning install resumes a failed install. Once
+the substrate release is deployed, install refuses to run; use upgrade.
 
 Without --config, install uses a single-replica PostgreSQL for evaluation and
 a RustFS for snapshots. On a terminal it asks for these choices first and
@@ -128,10 +129,11 @@ kubectl ate: the certificate controller, atelet, the CRDs, then the substrate
 release. It doesn't roll WorkerPool worker images, which
 docs/reference/version-skew-policy.md upgrades after atelet.
 
-Each release keeps the values it was installed with. The config's
-releases values are layered on top. Upgrade also reads timeout and
-microvm.assetBucket from the config. It refuses downgrades, skipped minor
-releases, and CRD changes that would strand stored objects.
+Upgrade doesn't reuse the values a release was installed with. Each
+release's values come from the chart and the config, the same way install
+sets them, so pass the config you installed with, changed as needed.
+It refuses downgrades, skipped minor releases, and CRD changes that would
+strand stored objects.
 
 --dry-run prints each release's manifest diff and changes nothing.`,
 	Args: cobra.NoArgs,

pkg/kagent/install/bootstrap.go

@@ -346,21 +346,10 @@ func (in *Installer) configureObjectStore(ctx context.Context, _ func(string)) (
 		return "", err
 	}
 	if in.opts.ObjectStore.Type != ObjectStoreBundled {
-		// A resumed install keeps the values the releases already have.
 		atelet, substrate := in.externalStoreValues()
-		vals := map[release]map[string]any{}
-		for r, typed := range map[release]map[string]any{ateletRelease: atelet, substrateRelease: substrate} {
-			rel, err := in.helm.get(r)
-			if err != nil {
-				return "", err
-			}
-			var stored map[string]any
-			if rel != nil {
-				stored = rel.Config
-			}
-			vals[r] = mergeValues(stored, in.user[r.name], typed)
-		}
-		return in.checkExternalObjectStore(ctx, vals[substrateRelease], vals[ateletRelease])
+		return in.checkExternalObjectStore(ctx,
+			mergeValues(in.user[substrateRelease.name], substrate),
+			mergeValues(in.user[ateletRelease.name], atelet))
 	}
 	if ok, err := secretExists(ctx, in.kube, SystemNamespace, rustfsCredentialsSecret); ok || err != nil {
 		return "already present", err

pkg/kagent/install/helm.go

@@ -159,7 +159,8 @@ func (h *helmClient) newUpgrade(r release) (*action.Upgrade, error) {
 	}
 	up := action.NewUpgrade(cfg)
 	up.Namespace = r.namespace
-	up.ResetThenReuseValues = true
+	// Don't reuse the release's previous values.
+	up.ResetValues = true
 	return up, nil
 }
 

pkg/kagent/install/install_test.go

@@ -17,6 +17,7 @@ package install
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"reflect"
 	"slices"
@@ -280,24 +281,29 @@ func TestCheckExternalObjectStore(t *testing.T) {
 	}
 }
 
+// Install and upgrade set the same values from the config.
 func TestExternalObjectStoreValues(t *testing.T) {
-	in := &Installer{opts: Options{Config: Config{ObjectStore: ObjectStore{Type: ObjectStoreGCS}, Postgres: Postgres{Type: PostgresExternal}}}}
-	defaults, forced, err := in.ateletValues(context.Background())
-	if err != nil {
-		t.Fatal(err)
-	}
-	if defaults != nil {
-		t.Errorf("atelet defaults = %v, want none", defaults)
-	}
-	if got := forced["objectStore"].(map[string]any)["backend"]; got != "gcs" {
-		t.Errorf("atelet objectStore.backend = %v, want gcs", got)
-	}
-	_, forced, err = in.substrateValues(context.Background())
-	if err != nil {
-		t.Fatal(err)
-	}
-	if got := forced["rustfs"].(map[string]any)["enabled"]; got != false {
-		t.Errorf("substrate rustfs.enabled = %v, want false", got)
+	for _, upgrading := range []bool{false, true} {
+		t.Run(fmt.Sprintf("upgrading=%v", upgrading), func(t *testing.T) {
+			in := &Installer{upgrading: upgrading, opts: Options{Config: Config{ObjectStore: ObjectStore{Type: ObjectStoreGCS}, Postgres: Postgres{Type: PostgresExternal}}}}
+			defaults, forced, err := in.ateletValues(context.Background())
+			if err != nil {
+				t.Fatal(err)
+			}
+			if defaults != nil {
+				t.Errorf("atelet defaults = %v, want none", defaults)
+			}
+			if got, _ := lookupString(forced, "objectStore", "backend"); got != "gcs" {
+				t.Errorf("atelet objectStore.backend = %q, want gcs", got)
+			}
+			_, forced, err = in.substrateValues(context.Background())
+			if err != nil {
+				t.Fatal(err)
+			}
+			if rustfs, _ := forced["rustfs"].(map[string]any); rustfs["enabled"] != false {
+				t.Errorf("substrate rustfs = %v, want enabled false", forced["rustfs"])
+			}
+		})
 	}
 }
 

pkg/kagent/install/installer.go

@@ -163,9 +163,9 @@ func (in *Installer) Run(ctx context.Context, steps []Step) error {
 	return nil
 }
 
-// Install runs InstallSteps. Rerunning it is safe and resumes a failed
-// install: existing secrets are kept and existing releases are upgraded in
-// place when their chart version matches.
+// Install runs InstallSteps. It refuses a finished install. Rerunning it
+// resumes a failed install: existing secrets are kept and existing releases
+// are upgraded in place when their chart version matches.
 func (in *Installer) Install(ctx context.Context) error {
 	steps, err := in.InstallSteps()
 	if err != nil {

pkg/kagent/install/steps.go

@@ -54,6 +54,11 @@ func (in *Installer) preflight(ctx context.Context, progress func(string)) (stri
 	if err != nil {
 		return "", err
 	}
+	if rel != nil && rel.Info.Status == common.StatusDeployed {
+		// The substrate release is the last one, so the install finished.
+		// Changing it is upgrade's job, which can diff it first.
+		return "", fmt.Errorf("release %s %s is already installed; use kubectl ate upgrade to change it", rel.Name, rel.Chart.Metadata.Version)
+	}
 	if rel == nil {
 		// ate-setup installs with kustomize, which this command can't manage.
 		_, err := in.kube.AppsV1().Deployments(SystemNamespace).Get(ctx, "ate-controller", metav1.GetOptions{})
@@ -121,7 +126,7 @@ func (in *Installer) helmStep(r release, vf valuesFunc) func(context.Context, fu
 				return "", fmt.Errorf("release %s has chart version %s, want %s; use kubectl ate upgrade", r.name, got, want)
 			}
 		}
-		vals, err := in.releaseValues(ctx, r, vf, cur == nil)
+		vals, err := in.releaseValues(ctx, r, vf)
 		if err != nil {
 			return "", err
 		}
@@ -144,28 +149,20 @@ func (in *Installer) helmStep(r release, vf valuesFunc) func(context.Context, fu
 	}
 }
 
-// releaseValues returns the values to install or upgrade r with. Installer
-// defaults apply only to a first install. An upgrade reuses the release's
-// values, so defaults would override what the user set before.
-func (in *Installer) releaseValues(ctx context.Context, r release, vf valuesFunc, first bool) (map[string]any, error) {
+// releaseValues returns the values to install or upgrade r with. The
+// release's previous values aren't reused.
+func (in *Installer) releaseValues(ctx context.Context, r release, vf valuesFunc) (map[string]any, error) {
 	defaults, forced, err := vf(ctx)
 	if err != nil {
 		return nil, err
 	}
-	if !first {
-		defaults = nil
-	}
 	return mergeValues(defaults, in.user[r.name], forced), nil
 }
 
 // ateletValues point atelet at the substrate release's RustFS, or at the
 // backend of an external store.
 func (in *Installer) ateletValues(context.Context) (map[string]any, map[string]any, error) {
 	forced := map[string]any{"substrateReleaseName": substrateRelease.name}
-	if in.upgrading {
-		// The release keeps its storage values.
-		return nil, forced, nil
-	}
 	if in.opts.ObjectStore.Type != ObjectStoreBundled {
 		atelet, _ := in.externalStoreValues()
 		return nil, mergeValues(forced, atelet), nil
@@ -187,10 +184,6 @@ func (in *Installer) substrateValues(ctx context.Context) (map[string]any, map[s
 		// atelet is its own release so it can upgrade first.
 		"atelet": map[string]any{"enabled": false},
 	}
-	if in.upgrading {
-		// The release keeps its storage and database values.
-		return nil, forced, nil
-	}
 	if in.opts.ObjectStore.Type == ObjectStoreBundled {
 		access, secret, err := in.rustfsCredentials(ctx)
 		if err != nil {

pkg/kagent/install/upgrade.go

@@ -254,7 +254,7 @@ func (in *Installer) diffStep(r release, vf valuesFunc, out io.Writer) func(cont
 		if err != nil {
 			return "", err
 		}
-		vals, err := in.releaseValues(ctx, r, vf, false)
+		vals, err := in.releaseValues(ctx, r, vf)
 		if err != nil {
 			return "", err
 		}

pkg/kagent/install/upgrade_test.go

@@ -107,23 +107,3 @@ func TestWriteDiff(t *testing.T) {
 		t.Errorf("identical manifests: n = %d, output %q", n, buf.String())
 	}
 }
-
-// Upgrade runs without install options, so it must keep the stored storage
-// values instead of forcing new ones.
-func TestUpgradeKeepsStorageValues(t *testing.T) {
-	in := &Installer{upgrading: true}
-	for _, f := range []valuesFunc{in.ateletValues, in.substrateValues} {
-		defaults, forced, err := f(context.Background())
-		if err != nil {
-			t.Fatal(err)
-		}
-		if defaults != nil {
-			t.Errorf("defaults = %v, want none", defaults)
-		}
-		for _, k := range []string{"objectStore", "rustfs", "postgres"} {
-			if v, ok := forced[k]; ok {
-				t.Errorf("forced %s = %v on upgrade", k, v)
-			}
-		}
-	}
-}