From 6c602208b03f1071de18913ca1d6bb448ad34f04 Mon Sep 17 00:00:00 2001 From: Eitan Yarmush Date: Sun, 8 Nov 2026 13:25:09 +0000 Subject: [PATCH 1/2] kubectl-ate: take install charts as an fs.FS A program that embeds the installer can pass charts it ships itself instead of writing them to a directory. Signed-off-by: Eitan Yarmush --- cmd/kubectl-ate/internal/cmd/install.go | 5 ++++- cmd/kubectl-ate/internal/install/installer.go | 11 +++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cmd/kubectl-ate/internal/cmd/install.go b/cmd/kubectl-ate/internal/cmd/install.go index f249b5cfda..e2b1e21304 100644 --- a/cmd/kubectl-ate/internal/cmd/install.go +++ b/cmd/kubectl-ate/internal/cmd/install.go @@ -179,7 +179,10 @@ data.`, // from. Without --config the config is install.DefaultConfig. The terminal is // nil when there is none. Close it with closeTerminal. func options(cmd *cobra.Command) (install.Options, *os.File, error) { - opts := install.Options{ChartDir: chartDir} + var opts install.Options + if chartDir != "" { + opts.Charts = os.DirFS(chartDir) + } var err error switch configPath { case "": diff --git a/cmd/kubectl-ate/internal/install/installer.go b/cmd/kubectl-ate/internal/install/installer.go index eaa18915ef..235ffe8e4e 100644 --- a/cmd/kubectl-ate/internal/install/installer.go +++ b/cmd/kubectl-ate/internal/install/installer.go @@ -20,7 +20,6 @@ import ( "fmt" "io" "io/fs" - "os" "time" apiextclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" @@ -68,8 +67,8 @@ type Options struct { Config Kubeconfig string Context string - // ChartDir loads charts from a directory instead of the embedded copies. - ChartDir string + // Charts holds a directory per chart. Nil uses the embedded charts. + Charts fs.FS // HelmLog receives Helm's debug log. Nil discards it. HelmLog io.Writer // DryRun makes Upgrade write manifest diffs to Out instead of applying. @@ -119,9 +118,9 @@ func New(opts Options, r Reporter) (*Installer, error) { if err != nil { return nil, fmt.Errorf("while creating Kubernetes client: %w", err) } - charts := fs.FS(chartsfs.FS) - if opts.ChartDir != "" { - charts = os.DirFS(opts.ChartDir) + charts := opts.Charts + if charts == nil { + charts = chartsfs.FS } user, err := opts.userValues() if err != nil { From 9d1eaad8230ec0830f54bead752941a775acf985 Mon Sep 17 00:00:00 2001 From: Eitan Yarmush Date: Sun, 8 Nov 2026 13:31:19 +0000 Subject: [PATCH 2/2] kubectl-ate: split installer options per operation and export steps Upgrade and Uninstall take their own options, so Options holds only the cluster, charts and config. Step, Run and InstallSteps are exported so a caller can add steps to an install, such as setting up its own database before StepCheckDatabase. Signed-off-by: Eitan Yarmush --- cmd/kubectl-ate/internal/cmd/install.go | 24 +++--- .../internal/install/install_test.go | 27 +++++++ cmd/kubectl-ate/internal/install/installer.go | 76 ++++++++++--------- cmd/kubectl-ate/internal/install/uninstall.go | 48 +++++++----- cmd/kubectl-ate/internal/install/upgrade.go | 45 ++++++----- 5 files changed, 138 insertions(+), 82 deletions(-) diff --git a/cmd/kubectl-ate/internal/cmd/install.go b/cmd/kubectl-ate/internal/cmd/install.go index e2b1e21304..50a4739750 100644 --- a/cmd/kubectl-ate/internal/cmd/install.go +++ b/cmd/kubectl-ate/internal/cmd/install.go @@ -37,7 +37,7 @@ var ( chartDir string noTUI bool upgradeDryRun bool - uninstallOpts install.Options + uninstallOpts install.UninstallOptions uninstallYes bool ) @@ -141,9 +141,14 @@ releases, and CRD changes that would strand stored objects. return err } defer closeTerminal(tty) - opts.DryRun, opts.Out = upgradeDryRun, cmd.OutOrStdout() - return runInstaller(cmd, opts, tty, "upgrade", (*install.Installer).Upgrade, - "Agent Substrate is upgraded.") + up := install.UpgradeOptions{DryRun: upgradeDryRun, Out: cmd.OutOrStdout()} + // The diff goes to stdout, so it can't share the terminal with the TUI. + if up.DryRun { + tty = nil + } + return runInstaller(cmd, opts, tty, "upgrade", func(in *install.Installer, ctx context.Context) error { + return in.Upgrade(ctx, up) + }, "Agent Substrate is upgraded.") }, } @@ -166,12 +171,13 @@ data.`, return err } defer closeTerminal(tty) - opts.IncludeCRDs, opts.Purge = uninstallOpts.IncludeCRDs, uninstallOpts.Purge + un := uninstallOpts if !uninstallYes { - opts.Confirm = confirmFunc(cmd, tty) + un.Confirm = confirmFunc(cmd, tty) } - return runInstaller(cmd, opts, tty, "uninstall", (*install.Installer).Uninstall, - "Agent Substrate is uninstalled.") + return runInstaller(cmd, opts, tty, "uninstall", func(in *install.Installer, ctx context.Context) error { + return in.Uninstall(ctx, un) + }, "Agent Substrate is uninstalled.") }, } @@ -294,7 +300,7 @@ func newInstallerWith(cmd *cobra.Command, opts install.Options, r install.Report // lines otherwise. The TUI shows the error itself, so the returned error is // marked as reported. func runInstaller(cmd *cobra.Command, opts install.Options, tty *os.File, title string, fn func(*install.Installer, context.Context) error, summary string) error { - if !useTUI(cmd, tty) || opts.DryRun { + if !useTUI(cmd, tty) { in, err := newInstaller(cmd, opts) if err != nil { return err diff --git a/cmd/kubectl-ate/internal/install/install_test.go b/cmd/kubectl-ate/internal/install/install_test.go index 41f202ef77..245cca59f8 100644 --- a/cmd/kubectl-ate/internal/install/install_test.go +++ b/cmd/kubectl-ate/internal/install/install_test.go @@ -17,7 +17,9 @@ package install import ( "bytes" "context" + "io" "reflect" + "slices" "strings" "testing" @@ -334,3 +336,28 @@ func TestExternalStoreValuesS3(t *testing.T) { t.Errorf("substrate values = %v, want %v", substrate, wantSubstrate) } } + +func TestInstallStepsDatabase(t *testing.T) { + tests := []struct { + postgres PostgresType + wantCheck bool + }{ + {PostgresBundled, false}, + {PostgresExternal, true}, + } + for _, tc := range tests { + t.Run(string(tc.postgres), func(t *testing.T) { + cfg := DefaultConfig() + cfg.Postgres.Type = tc.postgres + in := &Installer{opts: Options{Config: cfg}, report: PlainReporter{W: io.Discard}} + steps, err := in.InstallSteps() + if err != nil { + t.Fatal(err) + } + got := slices.ContainsFunc(steps, func(s Step) bool { return s.Name == StepCheckDatabase }) + if got != tc.wantCheck { + t.Errorf("has %q = %t, want %t", StepCheckDatabase, got, tc.wantCheck) + } + }) + } +} diff --git a/cmd/kubectl-ate/internal/install/installer.go b/cmd/kubectl-ate/internal/install/installer.go index 235ffe8e4e..45ec7df55c 100644 --- a/cmd/kubectl-ate/internal/install/installer.go +++ b/cmd/kubectl-ate/internal/install/installer.go @@ -71,17 +71,6 @@ type Options struct { Charts fs.FS // HelmLog receives Helm's debug log. Nil discards it. HelmLog io.Writer - // DryRun makes Upgrade write manifest diffs to Out instead of applying. - DryRun bool - Out io.Writer - // IncludeCRDs makes Uninstall delete the CRDs and every custom resource. - IncludeCRDs bool - // Purge makes Uninstall delete the namespaces, with their CA keys, - // secrets and database volume, and the published trust bundles. - Purge bool - // Confirm is shown what Uninstall will delete and returns whether to go - // on. Nil means go on. - Confirm func(ctx context.Context, summary string) (bool, error) } // Installer runs install steps against one cluster. @@ -139,46 +128,61 @@ func New(opts Options, r Reporter) (*Installer, error) { }, nil } -// step is one unit of work. run returns a short summary for the report. -type step struct { - name string - run func(ctx context.Context, progress func(string)) (string, error) +// Step is one unit of work. Run returns a short summary for the report. +type Step struct { + Name string + Run func(ctx context.Context, progress func(string)) (string, error) } -func (in *Installer) run(ctx context.Context, steps []step) error { +// StepCheckDatabase is the name of the install step that checks the +// connection Secrets for an external database. A caller that sets up the +// database itself inserts its steps before this one. +const StepCheckDatabase = "Check database secrets" + +// Run runs steps in order and reports each one. It stops at the first error. +func (in *Installer) Run(ctx context.Context, steps []Step) error { names := make([]string, len(steps)) for i, s := range steps { - names[i] = s.name + names[i] = s.Name } in.report.Report(Event{Kind: Plan, Steps: names}) for _, s := range steps { - in.report.Report(Event{Kind: StepStarted, Step: s.name}) + in.report.Report(Event{Kind: StepStarted, Step: s.Name}) start := time.Now() progress := func(msg string) { - in.report.Report(Event{Kind: StepProgress, Step: s.name, Msg: msg}) + in.report.Report(Event{Kind: StepProgress, Step: s.Name, Msg: msg}) } - msg, err := s.run(ctx, progress) + msg, err := s.Run(ctx, progress) elapsed := time.Since(start) if err != nil { - in.report.Report(Event{Kind: StepFailed, Step: s.name, Err: err, Elapsed: elapsed}) - return fmt.Errorf("%s: %w", s.name, err) + in.report.Report(Event{Kind: StepFailed, Step: s.Name, Err: err, Elapsed: elapsed}) + return fmt.Errorf("%s: %w", s.Name, err) } - in.report.Report(Event{Kind: StepDone, Step: s.name, Msg: msg, Elapsed: elapsed}) + in.report.Report(Event{Kind: StepDone, Step: s.Name, Msg: msg, Elapsed: elapsed}) } return nil } -// Install installs every release in order. 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. 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. func (in *Installer) Install(ctx context.Context) error { - if err := in.opts.validate(); err != nil { + steps, err := in.InstallSteps() + if err != nil { return err } + return in.Run(ctx, steps) +} + +// InstallSteps returns the steps that install every release in order. +func (in *Installer) InstallSteps() ([]Step, error) { + if err := in.opts.validate(); err != nil { + return nil, err + } if in.opts.Postgres.Type == PostgresBundled { in.report.Report(Event{Kind: Note, Msg: "Installing a single-replica PostgreSQL for evaluation only. It has no backups or high availability. Set postgres.type to external with a managed database for anything else."}) } - steps := []step{ + steps := []Step{ {"Check cluster", in.preflight}, {"Install CRDs", in.helmStep(crdsRelease, noValues)}, {"Create pod certificate CAs", in.createPodCertCAs}, @@ -190,19 +194,19 @@ func (in *Installer) Install(ctx context.Context) error { } if in.opts.Postgres.Type == PostgresBundled { steps = append(steps, - step{"Install PostgreSQL", in.applyPostgres}, - step{"Set up database", in.setupDatabase}, + Step{"Install PostgreSQL", in.applyPostgres}, + Step{"Set up database", in.setupDatabase}, ) } else { - steps = append(steps, step{"Check database secrets", in.checkDatabaseSecrets}) + steps = append(steps, Step{StepCheckDatabase, in.checkDatabaseSecrets}) } - steps = append(steps, step{"Install Substrate", in.helmStep(substrateRelease, in.substrateValues)}) + steps = append(steps, Step{"Install Substrate", in.helmStep(substrateRelease, in.substrateValues)}) if in.opts.MicroVM.Enabled { steps = append(steps, - step{"Stage microVM assets", in.stageMicroVMAssets}, - step{"Apply microVM SandboxConfig", in.applyMicroVMSandboxConfig}, + Step{"Stage microVM assets", in.stageMicroVMAssets}, + Step{"Apply microVM SandboxConfig", in.applyMicroVMSandboxConfig}, ) } - steps = append(steps, step{"Check API", in.checkAPI}) - return in.run(ctx, steps) + steps = append(steps, Step{"Check API", in.checkAPI}) + return steps, nil } diff --git a/cmd/kubectl-ate/internal/install/uninstall.go b/cmd/kubectl-ate/internal/install/uninstall.go index cea244ec66..375ba5a228 100644 --- a/cmd/kubectl-ate/internal/install/uninstall.go +++ b/cmd/kubectl-ate/internal/install/uninstall.go @@ -36,21 +36,33 @@ import ( "go.ate.dev/substrate/internal/clustertrustbundle" ) -// ErrAborted is returned when Options.Confirm declines an uninstall. +// ErrAborted is returned when UninstallOptions.Confirm declines an uninstall. var ErrAborted = errors.New("aborted") var workerPoolsGVR = atev1.GroupVersion.WithResource("workerpools") +// UninstallOptions configures Uninstall. +type UninstallOptions struct { + // IncludeCRDs deletes the CRDs and every custom resource. + IncludeCRDs bool + // Purge deletes the namespaces, with their CA keys, secrets and database + // volume, and the published trust bundles. + Purge bool + // Confirm is shown what Uninstall will delete and returns whether to go + // on. Nil means go on. + 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 -// Options.IncludeCRDs or Options.Purge say otherwise. -func (in *Installer) Uninstall(ctx context.Context) error { - summary, err := in.inventory(ctx) +// opts.IncludeCRDs or opts.Purge say otherwise. +func (in *Installer) Uninstall(ctx context.Context, opts UninstallOptions) error { + summary, err := in.inventory(ctx, opts) if err != nil { return err } - if in.opts.Confirm != nil { - ok, err := in.opts.Confirm(ctx, summary) + if opts.Confirm != nil { + ok, err := opts.Confirm(ctx, summary) if err != nil { return err } @@ -58,34 +70,34 @@ func (in *Installer) Uninstall(ctx context.Context) error { return ErrAborted } } - steps := []step{ + steps := []Step{ {"Delete worker pools", in.deleteWorkerPools}, {"Uninstall Substrate", in.uninstallStep(substrateRelease)}, {"Remove PostgreSQL", in.removePostgres}, {"Uninstall atelet", in.uninstallStep(ateletRelease)}, {"Uninstall certificate controller", in.uninstallStep(podCertRelease)}, } - if in.opts.IncludeCRDs { - steps = append(steps, step{"Uninstall CRDs", in.uninstallStep(crdsRelease)}) + if opts.IncludeCRDs { + steps = append(steps, Step{"Uninstall CRDs", in.uninstallStep(crdsRelease)}) } - if in.opts.Purge { + if opts.Purge { steps = append(steps, - step{"Delete trust bundles", in.deleteTrustBundles}, - step{"Delete namespaces", in.deleteNamespaces}, + Step{"Delete trust bundles", in.deleteTrustBundles}, + Step{"Delete namespaces", in.deleteNamespaces}, ) } - return in.run(ctx, steps) + return in.Run(ctx, steps) } -// inventory describes what Uninstall will remove, for Options.Confirm. -func (in *Installer) inventory(ctx context.Context) (string, error) { +// inventory describes what Uninstall will remove, for opts.Confirm. +func (in *Installer) inventory(ctx context.Context, opts UninstallOptions) (string, error) { var installed []string for _, r := range allReleases { rel, err := in.helm.get(r) if err != nil { return "", err } - if rel != nil && (r != crdsRelease || in.opts.IncludeCRDs) { + if rel != nil && (r != crdsRelease || opts.IncludeCRDs) { installed = append(installed, r.name) } } @@ -98,10 +110,10 @@ func (in *Installer) inventory(ctx context.Context) (string, error) { if pools != nil { fmt.Fprintf(&b, "WorkerPools to delete: %d, and every actor running on them\n", len(pools.Items)) } - if in.opts.IncludeCRDs { + if opts.IncludeCRDs { b.WriteString("CRDs to delete, with every WorkerPool, SandboxConfig and CSIDriverConfig\n") } - if in.opts.Purge { + if opts.Purge { fmt.Fprintf(&b, "Namespaces to delete: %s and %s, with their CA keys, secrets and database volume\n", SystemNamespace, PodCertNamespace) } return b.String(), nil diff --git a/cmd/kubectl-ate/internal/install/upgrade.go b/cmd/kubectl-ate/internal/install/upgrade.go index 48cd7a4716..e57eaf01d8 100644 --- a/cmd/kubectl-ate/internal/install/upgrade.go +++ b/cmd/kubectl-ate/internal/install/upgrade.go @@ -45,28 +45,34 @@ var upgradeOrder = []struct { {substrateRelease, "Upgrade Substrate"}, } -// Upgrade moves every release to the charts in this binary. With -// Options.DryRun it writes each release's manifest diff to Options.Out. -func (in *Installer) Upgrade(ctx context.Context) error { +// UpgradeOptions configures Upgrade. +type UpgradeOptions struct { + // DryRun writes each release's manifest diff to Out instead of applying. + DryRun bool + Out io.Writer +} + +// Upgrade moves every release to the charts in this binary. +func (in *Installer) Upgrade(ctx context.Context, opts UpgradeOptions) error { in.upgrading = true - steps := []step{ + steps := []Step{ {"Check versions", in.checkVersions}, {"Check CRD storage versions", in.checkCRDStorage}, } - if in.opts.DryRun { + if opts.DryRun { for _, u := range upgradeOrder { - steps = append(steps, step{"Diff " + u.r.name, in.diffStep(u.r, in.valuesFor(u.r))}) + steps = append(steps, Step{"Diff " + u.r.name, in.diffStep(u.r, in.valuesFor(u.r), opts.Out)}) } - return in.run(ctx, steps) + return in.Run(ctx, steps) } steps = append(steps, - step{"Create pod certificate CAs", in.createPodCertCAs}, - step{upgradeOrder[0].name, in.helmStep(podCertRelease, noValues)}, - step{"Wait for trust bundles", in.waitTrustBundles}, - step{"Create identity secrets", in.createIdentitySecrets}, + Step{"Create pod certificate CAs", in.createPodCertCAs}, + Step{upgradeOrder[0].name, in.helmStep(podCertRelease, noValues)}, + Step{"Wait for trust bundles", in.waitTrustBundles}, + Step{"Create identity secrets", in.createIdentitySecrets}, ) for _, u := range upgradeOrder[1:] { - steps = append(steps, step{u.name, in.helmStep(u.r, in.valuesFor(u.r))}) + steps = append(steps, Step{u.name, in.helmStep(u.r, in.valuesFor(u.r))}) } // A new release may pin new microVM assets. microVM, err := in.hasMicroVMSandboxConfig(ctx) @@ -75,12 +81,12 @@ func (in *Installer) Upgrade(ctx context.Context) error { } if microVM { steps = append(steps, - step{"Stage microVM assets", in.stageMicroVMAssets}, - step{"Apply microVM SandboxConfig", in.applyMicroVMSandboxConfig}, + Step{"Stage microVM assets", in.stageMicroVMAssets}, + Step{"Apply microVM SandboxConfig", in.applyMicroVMSandboxConfig}, ) } - steps = append(steps, step{"Check API", in.checkAPI}) - return in.run(ctx, steps) + steps = append(steps, Step{"Check API", in.checkAPI}) + return in.Run(ctx, steps) } // hasMicroVMSandboxConfig reports whether the microvm SandboxConfig exists. @@ -224,8 +230,9 @@ func checkStoredVersions(ctx context.Context, c apiextclient.Interface, crds []a return nil } -// diffStep writes the change an upgrade of r would make to its manifest. -func (in *Installer) diffStep(r release, vf valuesFunc) func(context.Context, func(string)) (string, error) { +// diffStep writes the change an upgrade of r would make to its manifest to +// 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) if err != nil { @@ -243,7 +250,7 @@ func (in *Installer) diffStep(r release, vf valuesFunc) func(context.Context, fu if err != nil { return "", err } - n, err := writeDiff(in.opts.Out, r.name, cur.Manifest, next) + n, err := writeDiff(out, r.name, cur.Manifest, next) if err != nil { return "", err }