kubectl-ate: per-operation installer options and exported steps

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,20 +171,24 @@ 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.")
 	},
 }
 
 // options reads the config and finds the terminal to read keys and answers
 // 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 "":
@@ -291,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

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)
+			}
+		})
+	}
+}

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,21 +67,10 @@ 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.
-	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.
@@ -119,9 +107,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 {
@@ -140,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},
@@ -191,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
 }

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

@@ -36,56 +36,68 @@ 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
 		}
 		if !ok {
 			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