[release-1.0] Backport installer step lists, Options.Version and declarative values

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

@@ -34,8 +34,6 @@ import (
 	releasev1 "helm.sh/helm/v4/pkg/release/v1"
 	"helm.sh/helm/v4/pkg/storage/driver"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
-
-	"go.ate.dev/substrate/internal/version"
 )
 
 // release names a Helm release and the chart it installs.
@@ -159,7 +157,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
 }
 
@@ -219,7 +218,7 @@ func asV1(rel any, err error) (*releasev1.Release, error) {
 }
 
 // loadChart reads the chart in directory name of charts.
-func loadChart(charts fs.FS, name string) (*chart.Chart, error) {
+func loadChart(charts fs.FS, name, tag string) (*chart.Chart, error) {
 	var files []*archive.BufferedFile
 	err := fs.WalkDir(charts, name, func(p string, d fs.DirEntry, err error) error {
 		if err != nil || d.IsDir() {
@@ -239,7 +238,7 @@ func loadChart(charts fs.FS, name string) (*chart.Chart, error) {
 	if err != nil {
 		return nil, fmt.Errorf("while loading chart %s: %w", path.Base(name), err)
 	}
-	stampVersion(ch, version.Version)
+	stampVersion(ch, tag)
 	return ch, nil
 }
 

pkg/kagent/install/install_test.go

@@ -17,6 +17,7 @@ package install
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"reflect"
 	"slices"
@@ -90,6 +91,7 @@ func TestStampVersion(t *testing.T) {
 		{"v1.2.3-rc.1", "1.2.3-rc.1", "v1.2.3-rc.1"},
 		{"dev", "0.1.0", "0.1.0"},
 		{"v1.2.3-4-gabcdef-dirty+meta", "0.1.0", "0.1.0"},
+		{"", "0.1.0", "0.1.0"},
 	}
 	for _, tc := range tests {
 		t.Run(tc.tag, func(t *testing.T) {
@@ -105,10 +107,13 @@ func TestStampVersion(t *testing.T) {
 func TestLoadEmbeddedCharts(t *testing.T) {
 	for _, r := range allReleases {
 		t.Run(r.chart, func(t *testing.T) {
-			ch, err := loadChart(chartsfs.FS, r.chart)
+			ch, err := loadChart(chartsfs.FS, r.chart, "v1.2.3")
 			if err != nil {
 				t.Fatal(err)
 			}
+			if ch.Metadata.Version != "1.2.3" || ch.Metadata.AppVersion != "v1.2.3" {
+				t.Errorf("got version %q app %q, want 1.2.3 v1.2.3", ch.Metadata.Version, ch.Metadata.AppVersion)
+			}
 			if ch.Name() != r.chart {
 				t.Errorf("chart name = %q, want %q", ch.Name(), r.chart)
 			}
@@ -280,24 +285,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

@@ -30,6 +30,7 @@ import (
 	chartsfs "go.ate.dev/substrate/charts"
 	"go.ate.dev/substrate/internal/ateclient"
 	"go.ate.dev/substrate/internal/installdefaults"
+	"go.ate.dev/substrate/internal/version"
 )
 
 const (
@@ -71,6 +72,10 @@ type Options struct {
 	Charts fs.FS
 	// HelmLog receives Helm's debug log. Nil discards it.
 	HelmLog io.Writer
+	// Version is the release tag, like v1.0.1, stamped on the charts. Their
+	// images default to it. Empty uses version.Version, which kubectl-ate sets
+	// at build time.
+	Version string
 }
 
 // Installer runs install steps against one cluster.
@@ -107,6 +112,9 @@ func New(opts Options, r Reporter) (*Installer, error) {
 	if err != nil {
 		return nil, fmt.Errorf("while creating Kubernetes client: %w", err)
 	}
+	if opts.Version == "" {
+		opts.Version = version.Version
+	}
 	charts := opts.Charts
 	if charts == nil {
 		charts = chartsfs.FS
@@ -163,9 +171,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{})
@@ -96,7 +101,7 @@ func noValues(context.Context) (map[string]any, map[string]any, error) { return
 // real upgrade.
 func (in *Installer) helmStep(r release, vf valuesFunc) func(context.Context, func(string)) (string, error) {
 	return func(ctx context.Context, progress func(string)) (string, error) {
-		ch, err := loadChart(in.charts, r.chart)
+		ch, err := loadChart(in.charts, r.chart, in.opts.Version)
 		if err != nil {
 			return "", err
 		}
@@ -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/uninstall.go

@@ -53,11 +53,10 @@ type UninstallOptions struct {
 	Confirm func(ctx context.Context, summary string) (bool, error)
 }
 
-// Uninstall removes the releases in the reverse of install order. It keeps
-// CRDs, secrets, the database volume and the namespaces unless
-// opts.IncludeCRDs or opts.Purge say otherwise.
+// Uninstall shows UninstallSummary to opts.Confirm, then runs
+// UninstallSteps.
 func (in *Installer) Uninstall(ctx context.Context, opts UninstallOptions) error {
-	summary, err := in.inventory(ctx, opts)
+	summary, err := in.UninstallSummary(ctx, opts)
 	if err != nil {
 		return err
 	}
@@ -70,6 +69,14 @@ func (in *Installer) Uninstall(ctx context.Context, opts UninstallOptions) error
 			return ErrAborted
 		}
 	}
+	return in.Run(ctx, in.UninstallSteps(opts))
+}
+
+// UninstallSteps returns the steps that remove the releases in the reverse of
+// install order. They keep CRDs, secrets, the database volume and the
+// namespaces unless opts.IncludeCRDs or opts.Purge say otherwise. They don't
+// call opts.Confirm.
+func (in *Installer) UninstallSteps(opts UninstallOptions) []Step {
 	steps := []Step{
 		{"Delete worker pools", in.deleteWorkerPools},
 		{"Uninstall Substrate", in.uninstallStep(substrateRelease)},
@@ -86,11 +93,11 @@ func (in *Installer) Uninstall(ctx context.Context, opts UninstallOptions) error
 			Step{"Delete namespaces", in.deleteNamespaces},
 		)
 	}
-	return in.Run(ctx, steps)
+	return steps
 }
 
-// inventory describes what Uninstall will remove, for opts.Confirm.
-func (in *Installer) inventory(ctx context.Context, opts UninstallOptions) (string, error) {
+// UninstallSummary describes what UninstallSteps will remove.
+func (in *Installer) UninstallSummary(ctx context.Context, opts UninstallOptions) (string, error) {
 	var installed []string
 	for _, r := range allReleases {
 		rel, err := in.helm.get(r)