From 2496181ef4e8790eafdf237d0bc34507c702b408 Mon Sep 17 00:00:00 2001 From: Eitan Yarmush Date: Sun, 8 Nov 2026 19:38:55 -0800 Subject: [PATCH 1/3] Merge pull request #163 from kagent-dev/installer-step-lists install: export the upgrade and uninstall steps (cherry picked from commit d8021f105f969e4accc6bec59a0fe9f80f90edb6) --- pkg/kagent/install/uninstall.go | 21 ++++++++++++++------- pkg/kagent/install/upgrade.go | 20 ++++++++++++++++---- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/pkg/kagent/install/uninstall.go b/pkg/kagent/install/uninstall.go index 375ba5a228..604916271c 100644 --- a/pkg/kagent/install/uninstall.go +++ b/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) diff --git a/pkg/kagent/install/upgrade.go b/pkg/kagent/install/upgrade.go index e57eaf01d8..f83b08430c 100644 --- a/pkg/kagent/install/upgrade.go +++ b/pkg/kagent/install/upgrade.go @@ -52,8 +52,20 @@ type UpgradeOptions struct { Out io.Writer } -// Upgrade moves every release to the charts in this binary. +// Upgrade runs UpgradeSteps. func (in *Installer) Upgrade(ctx context.Context, opts UpgradeOptions) error { + steps, err := in.UpgradeSteps(ctx, opts) + if err != nil { + return err + } + return in.Run(ctx, steps) +} + +// UpgradeSteps returns the steps that move every release to the charts in +// this binary. It reads the cluster to check for the microVM SandboxConfig. +// After it is called, the Installer's helm steps upgrade releases across +// chart versions. +func (in *Installer) UpgradeSteps(ctx context.Context, opts UpgradeOptions) ([]Step, error) { in.upgrading = true steps := []Step{ {"Check versions", in.checkVersions}, @@ -63,7 +75,7 @@ func (in *Installer) Upgrade(ctx context.Context, opts UpgradeOptions) error { for _, u := range upgradeOrder { steps = append(steps, Step{"Diff " + u.r.name, in.diffStep(u.r, in.valuesFor(u.r), opts.Out)}) } - return in.Run(ctx, steps) + return steps, nil } steps = append(steps, Step{"Create pod certificate CAs", in.createPodCertCAs}, @@ -77,7 +89,7 @@ func (in *Installer) Upgrade(ctx context.Context, opts UpgradeOptions) error { // A new release may pin new microVM assets. microVM, err := in.hasMicroVMSandboxConfig(ctx) if err != nil { - return err + return nil, err } if microVM { steps = append(steps, @@ -86,7 +98,7 @@ func (in *Installer) Upgrade(ctx context.Context, opts UpgradeOptions) error { ) } steps = append(steps, Step{"Check API", in.checkAPI}) - return in.Run(ctx, steps) + return steps, nil } // hasMicroVMSandboxConfig reports whether the microvm SandboxConfig exists. From a866ec19272da41dac9d02dff70098c5b208efd6 Mon Sep 17 00:00:00 2001 From: Eitan Yarmush Date: Mon, 9 Nov 2026 05:15:21 -0800 Subject: [PATCH 2/3] install: take the chart version from Options.Version (#162) * install: stamp charts from the substrate module version kubectl-ate sets version.Version with ldflags, but a program that imports the installer doesn't. Its embedded charts then keep 0.1.0 and pull images that don't exist. Take the tag from the go.ate.dev/substrate dependency in the build info instead, and skip pseudo-versions. Signed-off-by: Eitan Yarmush * install: take the chart version from Options.Version Replace the build-info search with an option. The caller knows which substrate release it depends on, and the search had to guess around replaces and pseudo-versions. Signed-off-by: Eitan Yarmush --------- Signed-off-by: Eitan Yarmush (cherry picked from commit 2c6fd6e6889598159a23f294e4d76877d4cc604e) --- pkg/kagent/install/helm.go | 6 ++---- pkg/kagent/install/install_test.go | 6 +++++- pkg/kagent/install/installer.go | 8 ++++++++ pkg/kagent/install/steps.go | 2 +- pkg/kagent/install/upgrade.go | 6 +++--- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/kagent/install/helm.go b/pkg/kagent/install/helm.go index ee1e501cd1..d5a4a1e5d5 100644 --- a/pkg/kagent/install/helm.go +++ b/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. @@ -219,7 +217,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 +237,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 } diff --git a/pkg/kagent/install/install_test.go b/pkg/kagent/install/install_test.go index 245cca59f8..9cf3842494 100644 --- a/pkg/kagent/install/install_test.go +++ b/pkg/kagent/install/install_test.go @@ -90,6 +90,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 +106,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) } diff --git a/pkg/kagent/install/installer.go b/pkg/kagent/install/installer.go index 45ec7df55c..2694346e69 100644 --- a/pkg/kagent/install/installer.go +++ b/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 diff --git a/pkg/kagent/install/steps.go b/pkg/kagent/install/steps.go index 481255ef75..6041430d39 100644 --- a/pkg/kagent/install/steps.go +++ b/pkg/kagent/install/steps.go @@ -96,7 +96,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 } diff --git a/pkg/kagent/install/upgrade.go b/pkg/kagent/install/upgrade.go index f83b08430c..4e04543dbe 100644 --- a/pkg/kagent/install/upgrade.go +++ b/pkg/kagent/install/upgrade.go @@ -138,7 +138,7 @@ func (in *Installer) checkVersions(_ context.Context, progress func(string)) (st if st := cur.Info.Status; st.IsPending() { return "", fmt.Errorf("release %s is %s; run helm rollback or helm uninstall on it, then retry", r.name, st) } - ch, err := loadChart(in.charts, r.chart) + ch, err := loadChart(in.charts, r.chart, in.opts.Version) if err != nil { return "", err } @@ -183,7 +183,7 @@ func checkSkew(from, to string) error { // checkCRDStorage refuses to drop a CRD version that objects are still // stored in, which would make them unreadable. func (in *Installer) checkCRDStorage(ctx context.Context, _ func(string)) (string, error) { - ch, err := loadChart(in.charts, crdsRelease.chart) + ch, err := loadChart(in.charts, crdsRelease.chart, in.opts.Version) if err != nil { return "", err } @@ -246,7 +246,7 @@ func checkStoredVersions(ctx context.Context, c apiextclient.Interface, crds []a // out. func (in *Installer) diffStep(r release, vf valuesFunc, out io.Writer) func(context.Context, func(string)) (string, error) { return func(ctx context.Context, _ 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 } From c3024b406de1c03f6e56a39584aa1c85d1f9a1c2 Mon Sep 17 00:00:00 2001 From: Eitan Yarmush Date: Mon, 9 Nov 2026 14:44:03 -0800 Subject: [PATCH 3/3] install: make install and upgrade values declarative (#165) Upgrades reused each release's previous values, so removing a value or switching storage modes from the config had no effect. Now every install and upgrade sets values from the chart and the config only. Install refuses once the substrate release is deployed, so changes to a finished install go through upgrade, which can diff them first. Signed-off-by: Eitan Yarmush (cherry picked from commit 691b2393529e7497f76d6b08d657ea0ff198fd84) --- cmd/kubectl-ate/internal/cmd/install.go | 12 ++++---- pkg/kagent/install/bootstrap.go | 17 ++--------- pkg/kagent/install/helm.go | 3 +- pkg/kagent/install/install_test.go | 40 ++++++++++++++----------- pkg/kagent/install/installer.go | 6 ++-- pkg/kagent/install/steps.go | 25 ++++++---------- pkg/kagent/install/upgrade.go | 2 +- pkg/kagent/install/upgrade_test.go | 20 ------------- 8 files changed, 48 insertions(+), 77 deletions(-) diff --git a/cmd/kubectl-ate/internal/cmd/install.go b/cmd/kubectl-ate/internal/cmd/install.go index 2b21932eec..70d984cccd 100644 --- a/cmd/kubectl-ate/internal/cmd/install.go +++ b/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, diff --git a/pkg/kagent/install/bootstrap.go b/pkg/kagent/install/bootstrap.go index f4cb5e810a..9bf43f8eed 100644 --- a/pkg/kagent/install/bootstrap.go +++ b/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 diff --git a/pkg/kagent/install/helm.go b/pkg/kagent/install/helm.go index d5a4a1e5d5..ef36ce1f0b 100644 --- a/pkg/kagent/install/helm.go +++ b/pkg/kagent/install/helm.go @@ -157,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 } diff --git a/pkg/kagent/install/install_test.go b/pkg/kagent/install/install_test.go index 9cf3842494..b80d9cc2f9 100644 --- a/pkg/kagent/install/install_test.go +++ b/pkg/kagent/install/install_test.go @@ -17,6 +17,7 @@ package install import ( "bytes" "context" + "fmt" "io" "reflect" "slices" @@ -284,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"]) + } + }) } } diff --git a/pkg/kagent/install/installer.go b/pkg/kagent/install/installer.go index 2694346e69..393495a0b0 100644 --- a/pkg/kagent/install/installer.go +++ b/pkg/kagent/install/installer.go @@ -171,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 { diff --git a/pkg/kagent/install/steps.go b/pkg/kagent/install/steps.go index 6041430d39..521deca08c 100644 --- a/pkg/kagent/install/steps.go +++ b/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,17 +149,13 @@ 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 } @@ -162,10 +163,6 @@ func (in *Installer) releaseValues(ctx context.Context, r release, vf valuesFunc // 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 { diff --git a/pkg/kagent/install/upgrade.go b/pkg/kagent/install/upgrade.go index 4e04543dbe..1762a61938 100644 --- a/pkg/kagent/install/upgrade.go +++ b/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 } diff --git a/pkg/kagent/install/upgrade_test.go b/pkg/kagent/install/upgrade_test.go index 852f840f19..3a3bbfcc2c 100644 --- a/pkg/kagent/install/upgrade_test.go +++ b/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) - } - } - } -}