diff --git a/README.md b/README.md index 95e7f2a..e745d2c 100644 --- a/README.md +++ b/README.md @@ -77,8 +77,8 @@ For how Deployah compares to DevSpace, Werf, Score, Epinio, and Kubero, see workers) - **Metrics** - ServiceMonitor / PodMonitor when Prometheus Operator is present - **Extras** - custom manifests and CRDs under `.deployah/` -- **Plan before apply** - `deployah plan` / `deployah deploy` with validation - and deploy guards +- **Plan, then deploy** - `deployah plan` inspects changes and is read-only; + `deployah deploy` validates, runs guards, and runs Helm ## Current limitations @@ -422,8 +422,8 @@ These work with every command: | `deployah validate ` | Also load the platform file and check the resolved configuration for that environment. | | `deployah resolve ` | Preview the fully resolved hostname, TLS mode, context, and runtime environment, offline. Prints FileValues and ExplicitValues; do not treat the output as secret-safe CI output. Use `--output json` for machine-readable output. | | `deployah resolve --environments` | List every environment from both files: where it is registered, its context (or the kubeconfig fallback), domains, and overrides. | -| `deployah plan ` | Preview what a deploy would change, without applying anything. Extra manifests from `.deployah/manifests/` appear in the diff; CRD files from `.deployah/crds/` are listed with Helm's install-only lifecycle, not applied. Use `--offline` to render with no cluster access, `--raw` for raw Kubernetes field paths instead of the compact Deployah vocabulary, `--yaml` to show changed fields as YAML blocks, `--drift` to also compare against live cluster state, `--detailed-exitcode` to exit 2 when changes are pending, or `--output json` for CI. | -| `deployah deploy ` | Deploy your project. Shows the plan and asks for confirmation before applying; use `-y`/`--yes` to skip the prompt, `--reapply` to upgrade even with no changes, `--skip-crds` to skip installing [chart CRDs](docs/custom-manifests-and-crds.md#helm-crd-lifecycle) on a fresh Helm install (a CRD added after that first install is not installed by a later deploy), `--explain` to print the resolution report first, `--force-hostname-change` to bypass the hostname guard, or `--resize-volumes` to grow [persistence](docs/workloads.md#growing-volumes) sizes. | +| `deployah plan ` | Inspect changes for an environment, without applying anything. Extra manifests from `.deployah/manifests/` appear in the diff; CRD files from `.deployah/crds/` are listed with Helm's install-only lifecycle, not applied. Use `--offline` to render with no cluster access, `--raw` for raw Kubernetes field paths instead of the compact Deployah vocabulary, `--yaml` to show changed fields as YAML blocks, `--drift` to also compare against live cluster state, `--detailed-exitcode` to exit 2 when changes are pending, or `--output json` for CI. | +| `deployah deploy ` | Deploy your project. Deployah validates the spec, runs deploy guards, then always runs a Helm install for a new release or a Helm upgrade for an existing one. Use `--skip-crds` to skip installing [chart CRDs](docs/custom-manifests-and-crds.md#helm-crd-lifecycle) on a fresh Helm install (a CRD added after that first install is not installed by a later deploy), `--explain` to print the resolution report first, `--force-hostname-change` to bypass the hostname guard, or `--resize-volumes` to grow [persistence](docs/workloads.md#growing-volumes) sizes. | | `deployah run ` | Run a spec task as a one-off Job. Wait is the default; `--detach` returns after create. `--count` / `--parallelism` override fanout for that run. | | `deployah status ` | Show the status of a deployed project. Use `--detailed` for pod details, `-e` for an environment. | | `deployah logs ` | Stream logs. Filter with `--component`, `-e`, `--container`, `--since`, `--tail`. Use `--no-follow` for a one-off read. | diff --git a/docs/adr/0005-semantic-plan-uses-previous-live-and-desired.md b/docs/adr/0005-semantic-plan-uses-previous-live-and-desired.md index 4f0d503..53f2a7c 100644 --- a/docs/adr/0005-semantic-plan-uses-previous-live-and-desired.md +++ b/docs/adr/0005-semantic-plan-uses-previous-live-and-desired.md @@ -28,11 +28,12 @@ Those states answer different questions and are independent: - Previous -> Desired: declarative / release intent change - Previous -> Live: drift -- Live -> Desired: visible resource consequence when this invocation - actually performs the relevant write (ADR-0011) +- Live -> Desired: visible resource consequence when the semantic + plan includes the corresponding write (ADR-0011) -Do not infer a deployment operation merely by comparing Live and -Desired. Ordinary modified drift alone does not run Helm (ADR-0006). +Do not infer a release change merely by comparing Live and Desired. +Ordinary drift alone is not a release change (ADR-0006). Whether a +requested deployment runs is ADR-0016. Drift is intrinsic. If Previous and Live are available, Previous -> Live is always evaluated. There is no opt-in drift mode. When there diff --git a/docs/adr/0006-semantic-plan-follows-helm-lifecycle.md b/docs/adr/0006-semantic-plan-follows-helm-lifecycle.md index 7a3a210..de9346d 100644 --- a/docs/adr/0006-semantic-plan-follows-helm-lifecycle.md +++ b/docs/adr/0006-semantic-plan-follows-helm-lifecycle.md @@ -19,23 +19,17 @@ install. ## Decision Deployah follows the lifecycle Helm actually performs. The semantic -plan describes the operations Deployah's real Helm execution path -would attempt. - -Helm runs when this invocation is a Helm install, when Helm-managed -release intent changed (Previous vs Desired, including hook -definitions), or when the operator supplies explicit Helm re-execution -intent. Ordinary Live drift, including a missing Live object, does not -by itself cause Helm to run. - -Explicit re-execution exists because Deployah may otherwise skip Helm -when release intent is unchanged. It is not a new HelmAction: -HelmAction remains Install, Upgrade, or None. On an existing release -it is an ordinary Upgrade; with no release it remains Install, with no -second upgrade after that install. Helm running does not fabricate -resource consequences (ADR-0011). Drift stays independent (ADR-0005). -Unchanged preDeploy and postDeploy hooks still run on that upgrade -(ADR-0014). +plan uses Helm lifecycle semantics to describe release changes and +their resource and task consequences. The plan does not decide +whether a requested deployment runs (ADR-0016). + +The plan reports a release change when the desired release differs +from the release Helm last recorded, including hook definitions. +Ordinary live drift, including a missing live object, is not by +itself a release change. A plan with no release change does not +invent resource consequences (ADR-0011). Drift stays independent +(ADR-0005). When Helm upgrades, unchanged preDeploy and postDeploy +hooks still run (ADR-0014). CRD lifecycle is ADR-0008. @@ -66,9 +60,9 @@ unknown final name does not make the semantic plan partial. ### Positive -- Plan operations match the Helm install, upgrade, and uninstall path - Deployah actually runs, including server-side apply on install - Create. +- Planned release changes and their consequences follow Helm's + install, upgrade, and uninstall lifecycle, including server-side + apply on an install Create. ### Negative diff --git a/docs/adr/0011-resource-consequences-are-distinct-from-release-change-and-drift.md b/docs/adr/0011-resource-consequences-are-distinct-from-release-change-and-drift.md index 6fa50e8..3d72324 100644 --- a/docs/adr/0011-resource-consequences-are-distinct-from-release-change-and-drift.md +++ b/docs/adr/0011-resource-consequences-are-distinct-from-release-change-and-drift.md @@ -6,24 +6,25 @@ Accepted ## Context -Release change, Drift, and the writes Helm will perform can disagree. -Collapsing them into one action invents Updates because Helm runs, or -Deletes of objects that no longer exist. +Release change, Drift, and planned write consequences can disagree. +Collapsing them into one action invents Updates because a planned +Helm lifecycle transition exists, or Deletes of objects that no +longer exist. The three states are defined in ADR-0005. This ADR owns the visible -resource consequence when this invocation actually performs the -relevant write. +resource consequence when the semantic plan includes the +corresponding write. ## Decision Visible resource consequences use this vocabulary: -- Create: the relevant object does not exist Live and this invocation - will create it. +- Create: the relevant object does not exist Live, and the planned + Helm lifecycle transition includes that create. - Update: the relevant Live object exists, differs from Desired, and - this invocation will write Desired. + the planned Helm lifecycle transition includes that write. - Delete: the object exists Live, leaves Desired/release state, and - Helm will delete it. + the planned Helm lifecycle transition includes that delete. - Retain: the object leaves Desired/release state, but Helm lifecycle policy intentionally leaves it Live. @@ -32,7 +33,7 @@ required output row. Do not add Replace. Immutable-field rejection and replacement feasibility are outside semantic planning (ADR-0004). Human-readable Create output shows the complete intended manifest. An -Update starts from Live when this invocation will write. +Update starts from Live when the semantic plan includes that write. Retain's primary case is Live `helm.sh/resource-policy: keep`. When that policy suppresses deletion, report Retain, not Delete. If @@ -45,15 +46,17 @@ Representative combinations: 1. Previous differs from Desired, Live already equals Desired: release change exists, Drift may exist, no resource consequence. Do not - invent an Update merely because Helm runs. + invent an Update merely because the plan includes a Helm lifecycle + transition. 2. Previous present, Desired absent, Live absent: release change and missing-resource drift, no Delete. 3. Previous present, Live absent, Desired unchanged: missing-resource - drift only. Missing Live state alone does not trigger Helm - (ADR-0006). -4. If Helm is already running for another release-intent change and a - Desired resource that existed in Previous is missing Live, show - Create plus missing-resource drift. + drift only. Missing Live state alone is not a release change + (ADR-0006). It does not decide whether a requested deployment + runs (ADR-0016). +4. When the planned Helm lifecycle transition includes a write for a + release change, and a Desired resource that existed in Previous is + missing Live, show Create plus missing-resource drift. Adoption is ADR-0012. @@ -67,5 +70,6 @@ Adoption is ADR-0012. ### Negative -- A missing Live object is not recreated until Helm runs for a - declared release-intent reason. +- The plan does not show a recreate for a missing Live object + unless there is a release change. A requested deploy of an + existing release still follows Helm upgrade (ADR-0016). diff --git a/docs/adr/0014-task-planning-follows-the-helm-managed-footprint.md b/docs/adr/0014-task-planning-follows-the-helm-managed-footprint.md index afebdee..ba17ea2 100644 --- a/docs/adr/0014-task-planning-follows-the-helm-managed-footprint.md +++ b/docs/adr/0014-task-planning-follows-the-helm-managed-footprint.md @@ -9,30 +9,32 @@ Accepted Deployah tasks map onto Helm hooks, ordinary Helm resources, or a one-time operation outside the release. Treating leftover hook Jobs as declarative Drift, or listing a current manual task as a deploy -resource, invents a lifecycle Helm does not run. When Helm runs is -ADR-0006. Resource consequences are ADR-0011. +resource, invents a lifecycle Helm does not run. The lifecycle the +plan describes is ADR-0006. Whether a requested deployment runs is +ADR-0016. Resource consequences are ADR-0011. ## Decision `preDeploy` and `postDeploy` definitions are Helm hooks. A change to -those definitions is release intent change. If Helm upgrades for -another reason, unchanged hook tasks still run because Helm executes -them as part of the upgrade. +those definitions is a release change. On a Helm upgrade, unchanged +hook tasks still run, because Helm executes them as part of the +upgrade. Scheduled tasks are ordinary Helm-managed resources, for example -CronJobs. A schedule task does not independently decide whether Helm -runs. +CronJobs. A schedule task is a release change only when that +resource changes. The task type does not by itself make one. A current manual task is outside the deploy semantic plan. It is not -shown as a deploy Task by itself, has no release Resource entry merely -because it exists in the spec, and does not trigger Helm. +shown as a deploy Task by itself, and has no release Resource entry +merely because it exists in the spec. Jobs or other objects created by executing `preDeploy` and `postDeploy` hooks are execution artifacts. Their existence, success, -failure, or leftover presence is not declarative Drift. They do not -independently trigger Helm. Semantic planning models the hook -definition, the task-level action, and whether that hook will run in -this invocation. +failure, or leftover presence is not declarative Drift. They are not +a release change. Semantic planning models the hook definition, the +task-level action, and whether that hook is part of the planned +release change. A requested deploy follows Helm's hook rules for the +install or upgrade it runs (ADR-0016). Changing phase between Helm-managed phases is a Task Update. diff --git a/docs/adr/0016-separate-semantic-planning-from-deployment-execution.md b/docs/adr/0016-separate-semantic-planning-from-deployment-execution.md new file mode 100644 index 0000000..711ba18 --- /dev/null +++ b/docs/adr/0016-separate-semantic-planning-from-deployment-execution.md @@ -0,0 +1,49 @@ +# ADR-0016: Separate semantic planning from deployment execution + +## Status + +Accepted + +## Context + +Deployah both inspects a deployment and carries one out. Inspection +describes release changes and their consequences. Execution carries +out a deployment the operator asked for. Those are different jobs. +Inspection must not decide whether that deployment runs. + +## Decision + +Planning and inspection are read-only. They describe release changes +and the resource and task consequences of those changes. Cluster +state informs drift and those consequences. It is not itself a +release change. + +A requested deployment runs after validation and guards succeed, +including any volume preparation that must happen before Helm. It +then runs Helm. Planning results do not determine whether that +deployment is executed. + +Helm chooses install or upgrade from whether the release already +exists. A release that does not exist yet is an install. An existing +release follows Helm's normal upgrade path. + +An unchanged desired deployment may produce no planned release +change, while explicitly running deploy still executes that upgrade +path. Hooks follow the same path. The upgrade can record a new +revision even when the plan shows no release change. + +## Consequences + +### Positive + +- Inspection can show no release change without blocking a later + deploy. +- Existing releases follow Helm's upgrade path, including hooks and + a new revision. + +### Negative + +- Deploy can record a new Helm revision when the plan showed no + release change. +- Hook tasks can run on that upgrade even when their definitions + did not change. diff --git a/docs/cli/deployah.md b/docs/cli/deployah.md index 07eed7c..bcb6dcd 100644 --- a/docs/cli/deployah.md +++ b/docs/cli/deployah.md @@ -32,7 +32,7 @@ deployah [flags] * [deployah init](deployah_init.md) - Creates deployah.yaml and a platform file so you can deploy. * [deployah list](deployah_list.md) - List deployed projects * [deployah logs](deployah_logs.md) - View logs for a deployed project -* [deployah plan](deployah_plan.md) - Preview the changes a deploy would make +* [deployah plan](deployah_plan.md) - Inspect changes for an environment * [deployah resolve](deployah_resolve.md) - Show the fully resolved configuration for an environment * [deployah run](deployah_run.md) - Run a spec task as a one-off Job * [deployah shell](deployah_shell.md) - Connect to a shell in a container diff --git a/docs/cli/deployah_deploy.md b/docs/cli/deployah_deploy.md index 35c2e4b..d9beefd 100644 --- a/docs/cli/deployah_deploy.md +++ b/docs/cli/deployah_deploy.md @@ -4,7 +4,7 @@ Deploy a project to a Kubernetes cluster on a given environment ### Synopsis -Deploy a project to a Kubernetes cluster on a given environment. Shows what would change and asks for confirmation before applying, unless --yes is set. +Deploy a project to a Kubernetes cluster for an environment. Deployah validates the spec and runs its deploy guards, then runs Helm: an install for a new release, an upgrade for an existing one. An existing release is always upgraded, even when nothing changed, so Helm creates a new revision and runs upgrade hooks. To inspect changes first, run `deployah plan `. ```text deployah deploy [flags] @@ -15,10 +15,8 @@ deployah deploy [flags] ```text --explain Print the resolution report before cluster checks (visible even when cluster is unreachable) --force-hostname-change Allow changing the resolved hostname even though it may break existing traffic (skips the hostname guard) - --reapply Upgrade the release even when the plan shows no changes --resize-volumes Allow persistence.size increases by expanding PVCs; StatefulSet controllers are orphan-deleted when needed so volumeClaimTemplates can be rewritten --skip-crds Skip installing CustomResourceDefinitions from the chart on a fresh Helm install - -y, --yes Apply without an interactive confirmation prompt ``` ### Options inherited from parent commands diff --git a/docs/cli/deployah_plan.md b/docs/cli/deployah_plan.md index 600f68a..e1bce55 100644 --- a/docs/cli/deployah_plan.md +++ b/docs/cli/deployah_plan.md @@ -1,10 +1,10 @@ ## deployah plan -Preview the changes a deploy would make +Inspect changes for an environment ### Synopsis -Render the chart for an environment and show what would change compared to the last successful release, without applying anything. +Render the chart for an environment and compare it with the last successful Helm release. With --drift, also compare the rendered manifests with live cluster state. Plan is read-only and never applies anything. ```text deployah plan [flags] diff --git a/docs/workloads.md b/docs/workloads.md index eaba908..aef19db 100644 --- a/docs/workloads.md +++ b/docs/workloads.md @@ -133,7 +133,7 @@ place (no orphan-delete). 1. Deploy with the flag: ```sh - deployah deploy --resize-volumes --yes + deployah deploy --resize-volumes ``` 1. Deployah then: diff --git a/examples/nginx/README.md b/examples/nginx/README.md index 22b1b6f..1cac5d7 100644 --- a/examples/nginx/README.md +++ b/examples/nginx/README.md @@ -10,7 +10,7 @@ You need Docker or Podman. From the repo root: ```sh cd examples/nginx deployah cluster up -deployah deploy local --yes +deployah deploy local deployah cluster status ``` diff --git a/internal/cmd/deploy/deploy.go b/internal/cmd/deploy/deploy.go index e0e4a98..37e8a65 100644 --- a/internal/cmd/deploy/deploy.go +++ b/internal/cmd/deploy/deploy.go @@ -2,7 +2,6 @@ package deploy import ( "context" - "errors" "fmt" "os" "slices" @@ -16,9 +15,7 @@ import ( "deployah.dev/deployah/internal/cmd/cmdopts" "deployah.dev/deployah/internal/extras" "deployah.dev/deployah/internal/k8s" - "deployah.dev/deployah/internal/plan/semantic" "deployah.dev/deployah/internal/readiness" - "deployah.dev/deployah/internal/render" "deployah.dev/deployah/internal/session" "deployah.dev/deployah/internal/spec" @@ -31,8 +28,6 @@ type Options struct { Explain bool `nabat:"explain"` ForceHostnameChange bool `nabat:"force-hostname-change"` ResizeVolumes bool `nabat:"resize-volumes"` - Yes bool `nabat:"yes"` - Reapply bool `nabat:"reapply"` SkipCRDs bool `nabat:"skip-crds"` } @@ -40,13 +35,11 @@ type Options struct { func Register(app *nabat.App) { app.MustCommand("deploy", nabat.WithDescription("Deploy a project to a Kubernetes cluster on a given environment"), - nabat.WithLongDescription("Deploy a project to a Kubernetes cluster on a given environment. Shows what would change and asks for confirmation before applying, unless --yes is set."), + nabat.WithLongDescription("Deploy a project to a Kubernetes cluster for an environment. Deployah validates the spec and runs its deploy guards, then runs Helm: an install for a new release, an upgrade for an existing one. An existing release is always upgraded, even when nothing changed, so Helm creates a new revision and runs upgrade hooks. To inspect changes first, run `deployah plan `."), nabat.WithArg("environment", "", nabat.WithRequired(), nabat.WithUsage("Environment to deploy to"), nabat.WithPrompt("Environment", "", nabat.WithHint("e.g. prod, staging"))), nabat.WithFlag("explain", false, nabat.WithUsage("Print the resolution report before cluster checks (visible even when cluster is unreachable)")), nabat.WithFlag("force-hostname-change", false, nabat.WithUsage("Allow changing the resolved hostname even though it may break existing traffic (skips the hostname guard)")), nabat.WithFlag("resize-volumes", false, nabat.WithUsage("Allow persistence.size increases by expanding PVCs; StatefulSet controllers are orphan-deleted when needed so volumeClaimTemplates can be rewritten")), - nabat.WithFlag("yes", false, nabat.WithShort('y'), nabat.WithUsage("Apply without an interactive confirmation prompt")), - nabat.WithFlag("reapply", false, nabat.WithUsage("Upgrade the release even when the plan shows no changes")), nabat.WithFlag("skip-crds", false, nabat.WithUsage("Skip installing CustomResourceDefinitions from the chart on a fresh Helm install")), nabat.WithExample(` # Deploy to production using the default spec path (./deployah.yaml) @@ -55,33 +48,19 @@ deployah deploy prod # Deploy to staging with an explicit spec path deployah deploy staging -s ./path/to/deployah.yaml -# Deploy without an interactive confirmation prompt (e.g. in CI) -deployah deploy prod --yes - # Skip chart CRDs on a first install deployah deploy prod --skip-crds # Show resolution report before deploying deployah deploy prod --explain -# Preview what a deploy would change, without touching the cluster -deployah plan prod --offline`), +# Inspect changes, then deploy +deployah plan prod +deployah deploy prod`), nabat.WithRun(runDeploy), ) } -// deployPlan bundles the shown diff with the render that produced it, so -// callers reuse one render instead of recomputing. helmAction is the -// semantic install, upgrade, or none decision for that same render. -// cleanup releases the chart temp dir behind result.ChartPath; runDeploy -// defers it. -type deployPlan struct { - diff *planengine.Plan - result *render.RenderResult - helmAction semantic.HelmAction - cleanup func() -} - func runDeploy(c *nabat.Context) error { opts := &Options{} if err := c.Bind(opts); err != nil { @@ -174,8 +153,8 @@ func runDeploy(c *nabat.Context) error { c.Logger().Debug("kubernetes client unavailable", "err", k8sErr) } - // Materialize self-signed TLS certs once, before any render, so the plan - // render and the real apply see identical bytes (see applyDeploy). + // Materialize self-signed TLS certs once, before Helm, so the resize + // preflight and the real apply see identical bytes. if resolvedSpec != nil { if tlsErr := cmdopts.MaterializeSelfSignedTLS(c, k8sClient, k8sErr, cluster.Namespace(), resolvedSpec); tlsErr != nil { return fmt.Errorf("materialize self-signed TLS: %w", tlsErr) @@ -192,20 +171,7 @@ func runDeploy(c *nabat.Context) error { } postRenderer := bundle.PostRendererFor() - plan, err := computePlan(c, helmClient, cluster, resolvedSpec, postRenderer, bundle.CRDs) - if err != nil { - return err - } - defer plan.cleanup() - planengine.StampChartCRDs(plan.diff, bundle.CRDDocs, plan.result.IsUpgrade, opts.SkipCRDs) - - textOpts := planengine.TextOptions{Mode: planengine.ModeCompact, Theme: c.Theme()} - if renderErr := planengine.RenderText(c.IO().Out, plan.diff, textOpts); renderErr != nil { - return fmt.Errorf("render plan: %w", renderErr) - } - // Hostname guard: block FQDN changes unless --force-hostname-change. - // Runs after the plan diff is shown, so a block is never a surprise. if resolvedSpec != nil { if guardErr := checkHostnameGuard(c, helmClient, manifest.Project, opts.Environment, resolvedSpec, opts.ForceHostnameChange); guardErr != nil { return guardErr @@ -226,11 +192,6 @@ func runDeploy(c *nabat.Context) error { return resizeFlagErr } - helmIdle := skipHelmApply(plan.result.IsUpgrade, plan.diff.HasChanges(), opts.Reapply) - if helmIdle { - return skipDeploy(c, k8sClient, k8sErr, plan) - } - if k8sErr == nil { if hasStatefulWithPersistence(manifest, opts.Environment) { if verErr := k8s.CheckMinimumVersion( @@ -244,105 +205,38 @@ func runDeploy(c *nabat.Context) error { } } - proceed, confirmErr := confirmApply(c, opts, "Apply these changes?") - if confirmErr != nil { - return confirmErr - } - if !proceed { - c.Println("Aborted.") - return nil - } - - return applyDeploy(c, sess, cluster, helmClient, platform, manifest, opts, resolvedSpec, plan, k8sClient, k8sErr, bundle, postRenderer, resizes) + return applyDeploy(c, sess, cluster, helmClient, platform, manifest, opts, resolvedSpec, envIdentity.ReleaseName(manifest.Project), k8sClient, k8sErr, bundle, postRenderer, resizes) } -// skipHelmApply reports whether deploy should exit without invoking Helm: -// an existing release whose rendered manifests are unchanged, unless -// --reapply is set. Fresh installs are never skipped, even when the -// ordinary Manifest is empty. Chart CRD files and --skip-crds do not -// affect this gate. -func skipHelmApply(isUpgrade, hasChanges, reapply bool) bool { - return isUpgrade && !hasChanges && !reapply -} - -// confirmApply gates the real apply behind --yes or an interactive prompt. -// proceed is false with a nil error on a clean "no"; err is non-nil when -// non-interactive without --yes ([nabat.ErrConfirmationRequired]), or the -// prompt fails. prompt must be non-empty. -func confirmApply(c *nabat.Context, opts *Options, prompt string) (proceed bool, err error) { - confirmed, confirmErr := c.Confirm(prompt, - nabat.WithYes(opts.Yes), - nabat.WithBypassHint("--yes"), - ) - if confirmErr != nil { - return false, confirmErr +// helmPreflight renders the chart on the client before a volume resize +// and does not keep the result. If rendering fails, the resize does not +// start, so PVCs stay as they are and StatefulSets are not deleted. +// Call it only when a resize is pending. +func helmPreflight(c *nabat.Context, helmClient session.HelmClient, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) error { + _, cleanup, err := helmClient.RenderManifests(c, resolved, postRenderer, crds) + if cleanup != nil { + defer cleanup() } - return confirmed, nil -} - -// computePlan renders the chart client-side and diffs it against the last -// successful release. It never mutates the cluster or Helm's release history. -// The caller must invoke deployPlan.cleanup when finished with the result. -func computePlan(c *nabat.Context, helmClient session.HelmClient, cluster *session.Cluster, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*deployPlan, error) { - p, result, cleanup, err := planengine.BuildPlan(c, helmClient, cluster.Context(), resolved, postRenderer, crds) if err != nil { - cleanup() - return nil, fmt.Errorf("%w%s", err, cmdopts.ClusterHint(err)) - } - helmAction, err := planengine.HelmActionFromRender(result) - if err != nil { - cleanup() - return nil, fmt.Errorf("determine helm release intent: %w", err) - } - return &deployPlan{diff: p, result: result, helmAction: helmAction, cleanup: cleanup}, nil -} - -// skipDeploy handles an unchanged upgrade: Helm is never invoked, but pod -// readiness for the release is still shown for parity with a real deploy. -func skipDeploy(c *nabat.Context, k8sClient kubernetes.Interface, k8sErr error, plan *deployPlan) error { - c.Success(fmt.Sprintf("No changes. Release %s unchanged (revision %d).", plan.diff.Header.Release, plan.diff.Header.Revision)) - return printReadiness(c, k8sClient, k8sErr, plan) -} - -func printReadiness(c *nabat.Context, k8sClient kubernetes.Interface, k8sErr error, plan *deployPlan) error { - if k8sErr != nil { - c.Logger().Debug("skipping readiness summary: k8s client unavailable", "err", k8sErr) - return nil - } - statuses, pollErr := readiness.Poll(c, k8sClient, plan.result.Namespace, plan.result.ReleaseName) - if pollErr != nil { - c.Logger().Debug("skipping readiness summary: poll failed", "err", pollErr) - return nil - } - if summary := readiness.Summary(statuses); summary != "" { - c.Println("Readiness: " + summary) + return fmt.Errorf("helm preflight before volume resize: %w%s", err, cmdopts.ClusterHint(err)) } return nil } -// applyDeploy re-renders and verifies determinism before the real Helm -// install/upgrade. When resizes is non-empty, PVC expansion (and -// StatefulSet orphan-delete when needed) run before Helm. -func applyDeploy(c *nabat.Context, sess *session.Session, cluster *session.Cluster, helmClient session.HelmClient, platform *spec.PlatformConfig, manifest *spec.Spec, opts *Options, resolved *spec.ResolvedSpec, plan *deployPlan, k8sClient kubernetes.Interface, k8sErr error, bundle *extras.Bundle, postRenderer postrenderer.PostRenderer, resizes []persistenceResize) error { - verify, verifyCleanup, err := helmClient.RenderManifests(c, resolved, postRenderer, bundle.CRDs) - if verifyCleanup != nil { - defer verifyCleanup() - } - if err != nil { - return fmt.Errorf("render manifests: %w%s", err, cmdopts.ClusterHint(err)) - } - // A mismatch means the chart is non-deterministic (e.g. embeds a - // timestamp), so what was shown isn't what would actually be installed. - if verify.Manifest != plan.result.Manifest { - return errors.New("rendered manifests changed between plan and apply; re-run 'deployah deploy' to see the current plan") - } - +// applyDeploy runs Helm install or upgrade. When resizes is non-empty, a +// client-side render preflight runs first, then PVC expansion (and +// StatefulSet orphan-delete when needed) run before Helm. releaseName is +// the Helm release for this project and environment. +func applyDeploy(c *nabat.Context, sess *session.Session, cluster *session.Cluster, helmClient session.HelmClient, platform *spec.PlatformConfig, manifest *spec.Spec, opts *Options, resolved *spec.ResolvedSpec, releaseName string, k8sClient kubernetes.Interface, k8sErr error, bundle *extras.Bundle, postRenderer postrenderer.PostRenderer, resizes []persistenceResize) error { if len(resizes) > 0 { if k8sErr != nil { return fmt.Errorf("resize volumes: kubernetes client unavailable: %w", k8sErr) } + if preflightErr := helmPreflight(c, helmClient, resolved, postRenderer, bundle.CRDs); preflightErr != nil { + return preflightErr + } c.Printf("Resizing volumes for %d component(s)...\n", len(resizes)) - if resizeErr := resizeVolumes(c, k8sClient, cluster.Namespace(), plan.result.ReleaseName, resizes); resizeErr != nil { + if resizeErr := resizeVolumes(c, k8sClient, cluster.Namespace(), releaseName, resizes); resizeErr != nil { return fmt.Errorf("%s: %w", resizeFailureHint(resizes), resizeErr) } } @@ -369,10 +263,10 @@ func applyDeploy(c *nabat.Context, sess *session.Session, cluster *session.Clust // failing the whole deploy. c.Logger().Debug("skipping deploy watcher: k8s client unavailable", "err", k8sErr) } else { - watcher = NewDeployWatcher(k8sClient, cluster.Namespace(), plan.result.ReleaseName) + watcher = NewDeployWatcher(k8sClient, cluster.Namespace(), releaseName) } - err = c.Status(func(st *nabat.Status) error { + err := c.Status(func(st *nabat.Status) error { var wg sync.WaitGroup var cancel context.CancelFunc if watcher != nil { diff --git a/internal/cmd/deploy/deploy_flow_test.go b/internal/cmd/deploy/deploy_flow_test.go index 0568653..2530562 100644 --- a/internal/cmd/deploy/deploy_flow_test.go +++ b/internal/cmd/deploy/deploy_flow_test.go @@ -16,28 +16,34 @@ package deploy import ( "errors" - "strings" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "helm.sh/helm/v4/pkg/release/common" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" "nabat.dev/nabat" "nabat.dev/nabat/nabattest" "deployah.dev/deployah/internal/extras" - "deployah.dev/deployah/internal/plan/semantic" + "deployah.dev/deployah/internal/helm" "deployah.dev/deployah/internal/render" "deployah.dev/deployah/internal/session" "deployah.dev/deployah/internal/spec" "deployah.dev/deployah/internal/target" "deployah.dev/deployah/internal/testing/nabatctx" - planengine "deployah.dev/deployah/internal/plan" + chart "helm.sh/helm/v4/pkg/chart/v2" v1 "helm.sh/helm/v4/pkg/release/v1" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + clienttesting "k8s.io/client-go/testing" ) // newClusterWithStub builds a Session and Cluster whose Kubernetes client is @@ -46,6 +52,7 @@ func newClusterWithStub(t *testing.T, stub *stubHelmClient, k8sClient kubernetes t.Helper() sess := session.New( + session.WithNamespace("default"), session.WithHelmFactory(func(*target.Target, session.HelmConfig) (session.HelmClient, error) { return stub, nil }), @@ -87,223 +94,151 @@ spec: replicas: 2 ` -const deployFlowManifestV2 = ` -apiVersion: apps/v1 -kind: Deployment -metadata: - name: web - namespace: default -spec: - replicas: 3 +const deploySpec = `apiVersion: v1-alpha.5 +project: web +components: + web: + image: nginx:latest + port: 80 +environments: + production: {} ` -// TestConfirmApply covers --yes and the non-interactive refusal path. -func TestConfirmApply(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - opts *Options - wantProceed bool - wantErrIs error - wantHint string - }{ - { - name: "yes skips prompt", - opts: &Options{Yes: true}, - wantProceed: true, - }, - { - name: "non-interactive without yes refuses", - opts: &Options{Yes: false}, - wantErrIs: nabat.ErrConfirmationRequired, - wantHint: "--yes", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - h := nabatctx.New(t, "test") - proceed, err := confirmApply(h.Context, tt.opts, "Apply these changes?") - if tt.wantErrIs != nil { - require.ErrorIs(t, err, tt.wantErrIs) - assert.False(t, proceed) - var ce *nabat.ConfirmationError - require.ErrorAs(t, err, &ce) - assert.Equal(t, tt.wantHint, ce.BypassHint) - return - } - require.NoError(t, err) - assert.Equal(t, tt.wantProceed, proceed) - }) - } -} - -// TestApplyDeploy_RenderMismatch_AbortsBeforeApply verifies that when the -// chart renders differently the second time (a non-deterministic template), -// applyDeploy aborts with a clear error and never calls InstallApp. -func TestApplyDeploy_RenderMismatch_AbortsBeforeApply(t *testing.T) { - t.Parallel() - - stub := &stubHelmClient{ - renderResults: []*render.RenderResult{ - testRenderResult(deployFlowManifestV1), // the apply-time re-render - }, - installErr: nil, // would only matter if InstallApp were (wrongly) called - } - sess, cluster := newClusterWithStub(t, stub, nil) - - planned := &deployPlan{ - diff: &planengine.Plan{}, - result: testRenderResult(deployFlowManifestV2), // differs from the re-render above - cleanup: func() {}, - } - - h := nabatctx.New(t, "test") - opts := &Options{Environment: "production"} - manifest := &spec.Spec{Project: "web"} +const deploySpecResize = `apiVersion: v1-alpha.5 +project: web +components: + web: + image: nginx:latest + port: 80 + persistence: + size: 2Gi + mountPath: /data +environments: + production: {} +` - err := applyDeploy(h.Context, sess, cluster, stub, nil, manifest, opts, nil, planned, nil, nil, &extras.Bundle{}, nil, nil) - require.Error(t, err) - assert.Contains(t, err.Error(), "changed between plan and apply") - assert.Equal(t, 1, stub.renderCallCount, "must re-render exactly once before comparing") +func writeDeploySpec(t *testing.T, body string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "deployah.yaml") + require.NoError(t, os.WriteFile(path, []byte(body), 0o600)) + return path } -// TestSkipDeploy_NoChanges_ShowsReadinessSummary verifies that skipping a -// no-op deploy still reports current pod readiness for the release, without -// calling Helm at all. -func TestSkipDeploy_NoChanges_ShowsReadinessSummary(t *testing.T) { - t.Parallel() - k8sClient := fake.NewSimpleClientset( - &corev1.Pod{ - Name: "web-1", Namespace: "default", - Labels: map[string]string{ - "app.kubernetes.io/instance": "web-production", - "app.kubernetes.io/component": "web", - }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, - Conditions: []corev1.PodCondition{}, - ContainerStatuses: []corev1.ContainerStatus{ - {Ready: true}, +func releaseWithComponent(component map[string]any) *v1.Release { + return &v1.Release{ + Name: "web-production", + Version: 1, + Manifest: deployFlowManifestV1, + Info: &v1.Info{Status: common.StatusDeployed}, + Chart: &chart.Chart{ + Values: map[string]any{ + "deployah": map[string]any{ + "resolved": map[string]any{ + "components": map[string]any{"web": component}, + }, }, }, }, - ) - h := nabatctx.New(t, "test") - plan := &deployPlan{ - diff: &planengine.Plan{ - Header: planengine.Header{Release: "web-production", Revision: 7}, - }, - result: testRenderResult(deployFlowManifestV1), - cleanup: func() {}, } +} - err := skipDeploy(h.Context, k8sClient, nil, plan) - require.NoError(t, err) - assert.Contains(t, h.Stderr.String(), "No changes. Release web-production unchanged (revision 7).") - assert.Contains(t, h.Stdout.String(), "Readiness:") - assert.Contains(t, h.Stdout.String(), "web: 1/1") +func runDeployCommand(t *testing.T, specBody string, stub *stubHelmClient) (stdout, stderr string, err error) { + t.Helper() + specPath := writeDeploySpec(t, specBody) + sess := session.New( + session.WithSpecPath(specPath), + session.WithHelmFactory(func(*target.Target, session.HelmConfig) (session.HelmClient, error) { + return stub, nil + }), + session.WithKubernetesFactory(func(*target.Target) (kubernetes.Interface, error) { + return nil, assertNever{} + }), + ) + h := nabatctx.New(t, "deployah") + Register(h.App) + require.NoError(t, h.App.OnPreRun(func(c *nabat.Context) error { + c.SetContext(session.WithContext(c.Context(), sess)) + return nil + })) + err = nabattest.Run(t, h.App, []string{"deploy", "production"}) + return h.Stdout.String(), h.Stderr.String(), err } -// TestSkipHelmApply locks the idle gate: upgrades with no rendered -// changes skip Helm unless --reapply is set. Fresh installs never skip, -// even when the ordinary Manifest is empty. --skip-crds is not a trigger. -func TestSkipHelmApply(t *testing.T) { +// TestRunDeploy_ExecutesHelm proves a fresh release and an unchanged +// existing release both invoke Helm once, with no render, plan, or prompt. +func TestRunDeploy_ExecutesHelm(t *testing.T) { t.Parallel() tests := []struct { - name string - isUpgrade bool - hasChanges bool - reapply bool - want bool + name string + stub *stubHelmClient }{ - {name: "fresh install empty manifest", isUpgrade: false, hasChanges: false, want: false}, - {name: "fresh install with changes", isUpgrade: false, hasChanges: true, want: false}, - {name: "idle upgrade", isUpgrade: true, hasChanges: false, want: true}, - {name: "idle upgrade reapply", isUpgrade: true, hasChanges: false, reapply: true, want: false}, - {name: "upgrade with changes", isUpgrade: true, hasChanges: true, want: false}, + { + name: "fresh release", + stub: &stubHelmClient{releaseErr: helm.ErrReleaseNotFound}, + }, + { + name: "unchanged existing release", + stub: &stubHelmClient{ + release: releaseWithComponent(map[string]any{"workloadKind": "Deployment"}), + renderResult: testRenderResult(deployFlowManifestV1), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tt.want, skipHelmApply(tt.isUpgrade, tt.hasChanges, tt.reapply)) + stdout, stderr, err := runDeployCommand(t, deploySpec, tt.stub) + require.NoError(t, err) + assert.NotErrorIs(t, err, nabat.ErrConfirmationRequired) + assert.Equal(t, 1, tt.stub.installCallCount) + assert.Equal(t, 0, tt.stub.renderCallCount) + assert.NotContains(t, stdout, "Plan:") + assert.NotContains(t, stdout, "No changes") + assert.NotContains(t, stdout, "Apply these changes?") + assert.Contains(t, stderr, "Deployed") }) } } -// TestDeployPlan_ChartCRDsMatchSkipCRDs keeps the displayed plan aligned -// with InstallApp skipCRDs: process on a fresh install, skip when requested, -// and never treat chart CRDs as actionable on upgrade. -func TestDeployPlan_ChartCRDsMatchSkipCRDs(t *testing.T) { +// TestRunDeploy_GuardsBlockBeforeHelm proves workload and resize guards +// stop the command before Helm or a render. +func TestRunDeploy_GuardsBlockBeforeHelm(t *testing.T) { t.Parallel() - yamlBody := "kind: CustomResourceDefinition\nmetadata:\n name: widgets.example.com\n" - docs := []extras.CRDDoc{{ - Path: "widget.yaml", - Kind: "CustomResourceDefinition", - Name: "widgets.example.com", - YAML: []byte(yamlBody), - }} tests := []struct { - name string - upgrade bool - skipCRDs bool - wantProcess bool - wantNote string - notContains []string - wantIdle bool + name string + specBody string + release *v1.Release + wantErr string }{ { - name: "fresh default processes", - wantProcess: true, - wantNote: "Helm install will process this chart CRD", - notContains: []string{"install-time CRD processing disabled"}, - }, - { - name: "fresh skip", - skipCRDs: true, - wantNote: "install-time CRD processing disabled", - notContains: []string{"+ CustomResourceDefinition/", "Helm install will process this chart CRD"}, + name: "kind change", + specBody: deploySpec, + release: releaseWithComponent(map[string]any{"workloadKind": "StatefulSet"}), + wantErr: "kind change", }, { - name: "upgrade ignores skip flag for lifecycle", - upgrade: true, - skipCRDs: true, - wantNote: "Helm upgrade will not process this chart CRD", - notContains: []string{"+ CustomResourceDefinition/", "install-time CRD processing disabled"}, - wantIdle: true, - }, - { - name: "upgrade without skip is not actionable", - upgrade: true, - wantNote: "Helm upgrade will not process this chart CRD", - notContains: []string{"+ CustomResourceDefinition/"}, - wantIdle: true, + name: "resize without flag", + specBody: deploySpecResize, + release: releaseWithComponent(map[string]any{"workloadKind": "Deployment", "persistenceSize": "1Gi"}), + wantErr: "requires --resize-volumes", }, } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { t.Parallel() - p := &planengine.Plan{} - planengine.StampChartCRDs(p, docs, tc.upgrade, tc.skipCRDs) - require.Len(t, p.ChartCRDs, 1) - assert.Equal(t, tc.wantProcess, p.ChartCRDs[0].WillProcess) - assert.Equal(t, tc.wantIdle, skipHelmApply(tc.upgrade, p.HasChanges(), false)) - var buf strings.Builder - require.NoError(t, planengine.RenderText(&buf, p, planengine.TextOptions{})) - got := buf.String() - assert.Contains(t, got, "CustomResourceDefinition/widgets.example.com") - assert.Contains(t, got, tc.wantNote) - for _, s := range tc.notContains { - assert.NotContains(t, got, s) - } + stub := &stubHelmClient{release: tt.release} + _, _, err := runDeployCommand(t, tt.specBody, stub) + require.Error(t, err) + assert.ErrorContains(t, err, tt.wantErr) + assert.Equal(t, 0, stub.installCallCount) + assert.Equal(t, 0, stub.renderCallCount) }) } } // TestApplyDeploy_PassesCRDsToInstall copies chart CRD files into Helm and -// maps Options.SkipCRDs onto Install.SkipCRDs. +// maps Options.SkipCRDs onto Install.SkipCRDs. A deploy with no resize +// does not render first. func TestApplyDeploy_PassesCRDsToInstall(t *testing.T) { t.Parallel() tests := []struct { @@ -317,23 +252,16 @@ func TestApplyDeploy_PassesCRDsToInstall(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - manifest := deployFlowManifestV1 - stub := &stubHelmClient{ - renderResults: []*render.RenderResult{testRenderResult(manifest)}, - } + stub := &stubHelmClient{} sess, cluster := newClusterWithStub(t, stub, nil) - planned := &deployPlan{ - diff: &planengine.Plan{Header: planengine.Header{Release: "web-production", Revision: 1}}, - result: testRenderResult(manifest), - cleanup: func() {}, - } h := nabatctx.New(t, "test") opts := &Options{Environment: "production", SkipCRDs: tc.skipCRDs} bundle := &extras.Bundle{CRDs: []extras.RawFile{{Path: "widget.yaml"}}} - err := applyDeploy(h.Context, sess, cluster, stub, nil, &spec.Spec{Project: "web"}, opts, nil, planned, nil, assertNever{}, bundle, nil, nil) + err := applyDeploy(h.Context, sess, cluster, stub, nil, &spec.Spec{Project: "web"}, opts, nil, "web-production", nil, assertNever{}, bundle, nil, nil) require.NoError(t, err) assert.Equal(t, 1, stub.installCallCount) + assert.Equal(t, 0, stub.renderCallCount) assert.Equal(t, tc.skipCRDs, stub.lastSkipCRDs) require.Len(t, stub.lastCRDs, 1) for _, want := range tc.wantStderr { @@ -347,29 +275,21 @@ func TestApplyDeploy_PassesCRDsToInstall(t *testing.T) { func TestApplyDeploy_PropagatesInstallError(t *testing.T) { t.Parallel() - manifest := deployFlowManifestV1 - stub := &stubHelmClient{ - renderResults: []*render.RenderResult{testRenderResult(manifest)}, - installErr: errors.New("helm boom"), - } + stub := &stubHelmClient{installErr: errors.New("helm boom")} sess, cluster := newClusterWithStub(t, stub, nil) - planned := &deployPlan{ - diff: &planengine.Plan{Header: planengine.Header{Release: "web-production", Revision: 1}}, - result: testRenderResult(manifest), - cleanup: func() {}, - } h := nabatctx.New(t, "test") opts := &Options{Environment: "production"} - err := applyDeploy(h.Context, sess, cluster, stub, nil, &spec.Spec{Project: "web"}, opts, nil, planned, nil, assertNever{}, &extras.Bundle{}, nil, nil) + err := applyDeploy(h.Context, sess, cluster, stub, nil, &spec.Spec{Project: "web"}, opts, nil, "web-production", nil, assertNever{}, &extras.Bundle{}, nil, nil) require.Error(t, err) assert.Contains(t, err.Error(), "deploy failed") assert.Contains(t, err.Error(), "helm boom") assert.Equal(t, 1, stub.installCallCount) + assert.Equal(t, 0, stub.renderCallCount) } -// TestDeployFlags_SkipCRDsAcceptedCRDsRemoved checks the deploy CLI surface. -func TestDeployFlags_SkipCRDsAcceptedCRDsRemoved(t *testing.T) { +// TestDeployFlags checks the deploy CLI surface. +func TestDeployFlags(t *testing.T) { t.Parallel() h := nabatctx.New(t, "deployah") @@ -380,123 +300,129 @@ func TestDeployFlags_SkipCRDsAcceptedCRDsRemoved(t *testing.T) { assert.Contains(t, help, "--skip-crds") assert.NotContains(t, help, "--crds") assert.NotContains(t, help, "create-replace") + assert.NotContains(t, help, "--yes") + assert.NotContains(t, help, "-y,") + assert.NotContains(t, help, "--reapply") + assert.NotContains(t, help, "confirmation") + assert.NotContains(t, help, "Shows what would change") err = nabattest.Run(t, h.App, []string{"deploy", "prod", "--crds", "create"}) require.Error(t, err) assert.NotContains(t, err.Error(), "cluster") + for _, args := range [][]string{ + {"deploy", "prod", "--yes"}, + {"deploy", "prod", "--reapply"}, + } { + rejected := nabatctx.New(t, "deployah") + Register(rejected.App) + runErr := nabattest.Run(t, rejected.App, args) + require.Error(t, runErr) + assert.NotContains(t, runErr.Error(), "cluster") + } + var opts Options assert.False(t, opts.SkipCRDs) } -func TestComputePlan_HelmAction(t *testing.T) { +// TestApplyDeploy_ResizePreflight proves a render failure stops before any +// volume mutation, and a successful preflight resizes before Helm. +func TestApplyDeploy_ResizePreflight(t *testing.T) { t.Parallel() - same := deployFlowManifestV1 - tests := []struct { - name string - release *v1.Release - result *render.RenderResult - want semantic.HelmAction - }{ - { - name: "fresh install", - result: testRenderResult(same), - want: semantic.HelmInstall, - }, - { - name: "unchanged upgrade", - release: deployedRelease(same), - result: upgradeRenderResult(same, same), - want: semantic.HelmNone, - }, - { - name: "changed upgrade", - release: deployedRelease(deployFlowManifestV1), - result: upgradeRenderResult(deployFlowManifestV1, deployFlowManifestV2), - want: semantic.HelmUpgrade, - }, - { - name: "previous wins over last successful release", - release: deployedRelease(deployFlowManifestV1), - result: upgradeRenderResult(deployFlowManifestV2, deployFlowManifestV2), - want: semantic.HelmNone, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - stub := &stubHelmClient{release: tt.release, renderResults: []*render.RenderResult{tt.result}} - _, cluster := newClusterWithStub(t, stub, nil) - h := nabatctx.New(t, "test") - plan, err := computePlan(h.Context, stub, cluster, deployResolved(), nil, nil) - require.NoError(t, err) - t.Cleanup(plan.cleanup) - assert.Equal(t, tt.want, plan.helmAction) - assert.Same(t, tt.result, plan.result) - }) - } -} -// TestComputePlan_LegacyDiffIgnoresArgsReorder documents the transitional -// divergence: dyff ignores a pure argument reorder, while the semantic -// action is an upgrade. The execution gate still uses the legacy diff. -func TestComputePlan_LegacyDiffIgnoresArgsReorder(t *testing.T) { - t.Parallel() - previous := deploymentArgs("--foo", "--bar") - desired := deploymentArgs("--bar", "--foo") - stub := &stubHelmClient{ - release: deployedRelease(previous), - renderResults: []*render.RenderResult{upgradeRenderResult(previous, desired)}, - } - _, cluster := newClusterWithStub(t, stub, nil) - h := nabatctx.New(t, "test") - plan, err := computePlan(h.Context, stub, cluster, deployResolved(), nil, nil) - require.NoError(t, err) - t.Cleanup(plan.cleanup) - assert.False(t, plan.diff.HasChanges()) - assert.Equal(t, semantic.HelmUpgrade, plan.helmAction) -} + resizes := []persistenceResize{{ + Component: "db", PreviousSize: "10Gi", NewSize: "20Gi", + StorageClass: "fast-ssd", Stateful: true, + }} -func TestComputePlan_UpgradeWithoutPrevious(t *testing.T) { - t.Parallel() - result := testRenderResult(deployFlowManifestV1) - result.IsUpgrade = true - stub := &stubHelmClient{ - release: deployedRelease(deployFlowManifestV1), - renderResults: []*render.RenderResult{result}, - } - _, cluster := newClusterWithStub(t, stub, nil) - h := nabatctx.New(t, "test") - plan, err := computePlan(h.Context, stub, cluster, deployResolved(), nil, nil) - require.Error(t, err) - assert.Nil(t, plan) - assert.ErrorContains(t, err, "determine helm release intent") - assert.Equal(t, 1, stub.cleanupCalls) + t.Run("render failure", func(t *testing.T) { + t.Parallel() + log := &deployCallLog{} + stub := &stubHelmClient{renderErr: errors.New("bad chart"), log: log} + client := resizeClient(log) + sess, cluster := newClusterWithStub(t, stub, client) + h := nabatctx.New(t, "test") + + err := applyDeploy(h.Context, sess, cluster, stub, nil, &spec.Spec{Project: "shop"}, &Options{Environment: "production"}, nil, "shop-production", client, nil, &extras.Bundle{}, nil, resizes) + require.Error(t, err) + assert.ErrorContains(t, err, "helm preflight before volume resize") + assert.Equal(t, []string{"render", "cleanup"}, log.snapshot()) + assert.Equal(t, 1, stub.cleanupCalls) + assert.Equal(t, 0, stub.installCallCount) + }) + + t.Run("success", func(t *testing.T) { + t.Parallel() + log := &deployCallLog{} + stub := &stubHelmClient{renderResult: testRenderResult(deployFlowManifestV1), log: log} + client := resizeClient(log) + sess, cluster := newClusterWithStub(t, stub, client) + h := nabatctx.New(t, "test") + + err := applyDeploy(h.Context, sess, cluster, stub, nil, &spec.Spec{Project: "shop"}, &Options{Environment: "production"}, nil, "shop-production", client, nil, &extras.Bundle{}, nil, resizes) + require.NoError(t, err) + assert.Equal(t, []string{"render", "cleanup", "pvc-update", "sts-delete", "install"}, log.snapshot()) + assert.Equal(t, 1, stub.renderCallCount) + assert.Equal(t, 1, stub.cleanupCalls) + assert.Equal(t, 1, stub.installCallCount) + }) } -func deployResolved() *spec.ResolvedSpec { - return &spec.ResolvedSpec{ - Spec: &spec.Spec{Project: "web"}, - Env: spec.EnvIdentity{Original: "production"}, +func resizeClient(log *deployCallLog) *fake.Clientset { + sc := &storagev1.StorageClass{ + Name: "fast-ssd", + AllowVolumeExpansion: new(true), } -} - -func deployedRelease(manifest string) *v1.Release { - return &v1.Release{ - Version: 2, - Manifest: manifest, - Info: &v1.Info{Status: common.StatusDeployed}, + sts := &appsv1.StatefulSet{ + Name: "shop-production-db", + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/instance": "shop-production", + spec.LabelComponent: "db", + }, } -} - -func upgradeRenderResult(previous, desired string) *render.RenderResult { - result := testRenderResult(desired) - result.IsUpgrade = true - result.Revision = 3 - result.Previous = &render.ReleaseIntent{Manifest: previous} - return result -} - -func deploymentArgs(first, second string) string { - return "apiVersion: apps/v1\nkind: Deployment\nmetadata:\n name: web\n namespace: default\nspec:\n template:\n spec:\n containers:\n - name: api\n args:\n - " + first + "\n - " + second + "\n" + qty10 := resource.MustParse("10Gi") + pvc := &corev1.PersistentVolumeClaim{ + Name: "data-shop-production-db-0", + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/instance": "shop-production", + spec.LabelComponent: "db", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceStorage: qty10}, + }, + StorageClassName: new("fast-ssd"), + }, + Status: corev1.PersistentVolumeClaimStatus{ + Capacity: corev1.ResourceList{corev1.ResourceStorage: qty10}, + Conditions: []corev1.PersistentVolumeClaimCondition{{ + Type: corev1.PersistentVolumeClaimFileSystemResizePending, + Status: corev1.ConditionTrue, + }}, + }, + } + pod := &corev1.Pod{ + Name: "db-0", + Namespace: "default", + Labels: map[string]string{ + "app.kubernetes.io/instance": "shop-production", + spec.LabelComponent: "db", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{{Ready: true}}, + }, + } + client := fake.NewSimpleClientset(sc, sts, pvc, pod) + client.PrependReactor("update", "persistentvolumeclaims", func(clienttesting.Action) (bool, runtime.Object, error) { + log.add("pvc-update") + return false, nil, nil + }) + client.PrependReactor("delete", "statefulsets", func(clienttesting.Action) (bool, runtime.Object, error) { + log.add("sts-delete") + return false, nil, nil + }) + return client } diff --git a/internal/cmd/deploy/deploy_test.go b/internal/cmd/deploy/deploy_test.go index 92d5405..7be0616 100644 --- a/internal/cmd/deploy/deploy_test.go +++ b/internal/cmd/deploy/deploy_test.go @@ -3,7 +3,8 @@ package deploy import ( "context" "errors" - "fmt" + "slices" + "sync" "testing" "time" @@ -24,17 +25,38 @@ import ( v1 "helm.sh/helm/v4/pkg/release/v1" ) +// deployCallLog records Helm and resize steps in order. A nil log is a no-op. +type deployCallLog struct { + mu sync.Mutex + calls []string +} + +func (l *deployCallLog) add(step string) { + if l == nil { + return + } + l.mu.Lock() + defer l.mu.Unlock() + l.calls = append(l.calls, step) +} + +func (l *deployCallLog) snapshot() []string { + if l == nil { + return nil + } + l.mu.Lock() + defer l.mu.Unlock() + return slices.Clone(l.calls) +} + // stubHelmClient implements [session.HelmClient] for unit tests. Only -// GetRelease and RenderManifests are wired; every other method panics if -// called unexpectedly. +// GetRelease, GetReleaseHistory, RenderManifests, and InstallApp are wired; +// every other method panics if called unexpectedly. type stubHelmClient struct { release *v1.Release releaseErr error - // renderResults is consumed in order across RenderManifests calls (Nth - // call can differ from the first); renderErr, if set, is returned on - // every call instead. - renderResults []*render.RenderResult + renderResult *render.RenderResult renderErr error renderCallCount int cleanupCalls int @@ -43,6 +65,8 @@ type stubHelmClient struct { installCallCount int lastCRDs []extras.RawFile lastSkipCRDs bool + + log *deployCallLog } func (s *stubHelmClient) IsReachable() error { return nil } @@ -51,19 +75,21 @@ func (s *stubHelmClient) InstallApp(_ context.Context, _ bool, _ *spec.ResolvedS s.installCallCount++ s.lastCRDs = crds s.lastSkipCRDs = skipCRDs + s.log.add("install") return s.installErr } func (s *stubHelmClient) RenderManifests(context.Context, *spec.ResolvedSpec, postrenderer.PostRenderer, []extras.RawFile) (*render.RenderResult, func(), error) { - if s.renderErr != nil { - return nil, func() { s.cleanupCalls++ }, s.renderErr - } - i := s.renderCallCount s.renderCallCount++ - if i >= len(s.renderResults) { - panic(fmt.Sprintf("unexpected RenderManifests call #%d: only %d result(s) configured", i+1, len(s.renderResults))) + s.log.add("render") + cleanup := func() { + s.cleanupCalls++ + s.log.add("cleanup") + } + if s.renderErr != nil { + return nil, cleanup, s.renderErr } - return s.renderResults[i], func() { s.cleanupCalls++ }, nil + return s.renderResult, cleanup, nil } func (s *stubHelmClient) RenderOffline(context.Context, *spec.ResolvedSpec, postrenderer.PostRenderer, []extras.RawFile) (*render.RenderResult, func(), error) { diff --git a/internal/cmd/deploy/doc.go b/internal/cmd/deploy/doc.go index 0996db2..6008ef5 100644 --- a/internal/cmd/deploy/doc.go +++ b/internal/cmd/deploy/doc.go @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package deploy implements the deployah deploy command, which renders a -// Deployah spec into a Helm chart and installs or upgrades the release on -// the target cluster. To preview changes without touching the cluster, use -// `deployah plan` instead. +// Package deploy implements the deployah deploy command. It validates a +// Deployah spec, runs deploy guards, and installs or upgrades the Helm +// release on the target cluster. To inspect changes without applying them, +// use `deployah plan`. package deploy diff --git a/internal/cmd/plan/plan.go b/internal/cmd/plan/plan.go index 1ece469..a174332 100644 --- a/internal/cmd/plan/plan.go +++ b/internal/cmd/plan/plan.go @@ -55,8 +55,8 @@ type Options struct { // Register adds the plan command to app. func Register(app *nabat.App) { app.MustCommand("plan", - nabat.WithDescription("Preview the changes a deploy would make"), - nabat.WithLongDescription("Render the chart for an environment and show what would change compared to the last successful release, without applying anything."), + nabat.WithDescription("Inspect changes for an environment"), + nabat.WithLongDescription("Render the chart for an environment and compare it with the last successful Helm release. With --drift, also compare the rendered manifests with live cluster state. Plan is read-only and never applies anything."), nabat.WithArg("environment", "", nabat.WithRequired(), nabat.WithUsage("Environment to plan for"), nabat.WithPrompt("Environment", "", nabat.WithHint("e.g. prod, staging"))), nabat.WithFlag("drift", false, nabat.WithUsage("Detect drift between the rendered manifests and the live cluster state (requires cluster access; not compatible with --offline)")), nabat.WithFlag("offline", false, nabat.WithUsage("Render and validate the chart without contacting the cluster")), @@ -67,7 +67,7 @@ func Register(app *nabat.App) { nabat.WithFlag("detailed-exitcode", false, nabat.WithUsage("Exit 2 when the plan has pending changes, 0 when it does not, 1 on error (for CI)")), nabat.WithValidation(validateOptions), nabat.WithExample(` -# Preview what a deploy would change +# Inspect changes for production deployah plan production # Render and validate the chart without touching the cluster diff --git a/internal/e2e/e2e_assert_test.go b/internal/e2e/e2e_assert_test.go index d70c47e..96dfc4d 100644 --- a/internal/e2e/e2e_assert_test.go +++ b/internal/e2e/e2e_assert_test.go @@ -119,7 +119,7 @@ func (s *E2ESuite) retryLogs(t *testing.T, dir string, args []string, contains s func stepArgs(project, env, ns string, step inttest.Step) []string { switch step.OpName() { case "deploy": - args := []string{"deploy", env, "--context", kindContext, "--yes", "--namespace", ns} + args := []string{"deploy", env, "--context", kindContext, "--namespace", ns} if step.Deploy != nil && step.Deploy.Spec != "" { args = append(args, "--spec", step.Deploy.Spec) } diff --git a/internal/e2e/e2e_crd_test.go b/internal/e2e/e2e_crd_test.go index aed31f0..a8e8a2a 100644 --- a/internal/e2e/e2e_crd_test.go +++ b/internal/e2e/e2e_crd_test.go @@ -86,7 +86,7 @@ func (s *E2ESuite) TestCRDLifecycle() { }) widget := hideExtrasManifest(t, dir, "widget.yaml") - runIn(t, dir, "deploy", "dev", "--context", kindContext, "--yes", + runIn(t, dir, "deploy", "dev", "--context", kindContext, "--namespace", ns) crd := waitCRDEstablished(t, ext, crdLifecycleName) assertCRDUserMetadata(t, crd) @@ -94,7 +94,7 @@ func (s *E2ESuite) TestCRDLifecycle() { require.NoError(t, os.WriteFile( filepath.Join(dir, ".deployah", "manifests", "widget.yaml"), []byte(widget), 0o600)) - runIn(t, dir, "deploy", "dev", "--context", kindContext, "--yes", + runIn(t, dir, "deploy", "dev", "--context", kindContext, "--namespace", ns) waitClusterResource(t, dyn, crdLifecycleGVR, crdLifecycleWidget) @@ -107,8 +107,8 @@ func (s *E2ESuite) TestCRDLifecycle() { require.NoError(t, os.WriteFile( filepath.Join(dir, ".deployah", "crds", "clusterwidget.yaml"), []byte(patched), 0o600)) - runIn(t, dir, "deploy", "dev", "--context", kindContext, "--yes", - "--namespace", ns, "--reapply") + runIn(t, dir, "deploy", "dev", "--context", kindContext, + "--namespace", ns) crd = getCRD(t, ext, crdLifecycleName) assert.Equal(t, "crd-lifecycle", crd.Labels["e2e-marker"], "Helm upgrade must not rewrite chart CRDs") @@ -178,7 +178,7 @@ func (s *E2ESuite) TestCRDNewlyAddedOnUpgrade() { }) hideExtrasManifest(t, dir, "widget.yaml") - runIn(t, dir, "deploy", "dev", "--context", kindContext, "--yes", + runIn(t, dir, "deploy", "dev", "--context", kindContext, "--namespace", ns) waitCRDEstablished(t, ext, crdLifecycleName) assertReleaseExists(t, dir, ns, "crd-lifecycle", "dev") @@ -186,8 +186,8 @@ func (s *E2ESuite) TestCRDNewlyAddedOnUpgrade() { require.NoError(t, os.WriteFile( filepath.Join(dir, ".deployah", "crds", "addedwidget.yaml"), []byte(crdAddedYAML), 0o600)) - runIn(t, dir, "deploy", "dev", "--context", kindContext, "--yes", - "--namespace", ns, "--reapply") + runIn(t, dir, "deploy", "dev", "--context", kindContext, + "--namespace", ns) _, err := ext.ApiextensionsV1().CustomResourceDefinitions().Get( t.Context(), crdAddedName, metav1.GetOptions{}) diff --git a/internal/e2e/e2e_wildcard_test.go b/internal/e2e/e2e_wildcard_test.go index b948034..fe21008 100644 --- a/internal/e2e/e2e_wildcard_test.go +++ b/internal/e2e/e2e_wildcard_test.go @@ -110,7 +110,7 @@ func (s *E2ESuite) TestWildcardInstances() { func (s *E2ESuite) deployWildcard(t *testing.T, dir, ns, env string) { t.Helper() - runIn(t, dir, "deploy", env, "--context", kindContext, "--yes", "--namespace", ns) + runIn(t, dir, "deploy", env, "--context", kindContext, "--namespace", ns) } func (s *E2ESuite) waitWildcardReady(t *testing.T, ns, original, release, slot string) { diff --git a/internal/helm/chart_lint_test.go b/internal/helm/chart_lint_test.go index 880832c..f76e5b8 100644 --- a/internal/helm/chart_lint_test.go +++ b/internal/helm/chart_lint_test.go @@ -23,12 +23,11 @@ import ( // nonDeterministicTemplateTokens are Go template / Sprig constructs whose // output can differ between two renders of the same chart values (e.g. a // live cluster lookup, or a random/CA-generating function), or that read -// Helm's upgrade-only template context. Any of these breaks the plan/apply -// determinism guarantee (docs/plan-proposal.md section 6): a chart template -// that uses one of them WILL make `deployah deploy` abort every time with -// "rendered manifests changed between plan and apply". This is exactly the -// bug the self-signed TLS template used to have (genCA/genSignedCert + -// lookup) before certs were moved to Go-side materialization. +// Helm's upgrade-only template context. A chart template that uses one of +// them makes `deployah plan` output differ from what deploy renders. This +// is exactly the bug the self-signed TLS template used to have +// (genCA/genSignedCert + lookup) before certs were moved to Go-side +// materialization. var nonDeterministicTemplateTokens = []string{ "lookup", ".Release.Revision", diff --git a/internal/helm/identity_test.go b/internal/helm/identity_test.go index 33cf96d..4890577 100644 --- a/internal/helm/identity_test.go +++ b/internal/helm/identity_test.go @@ -127,7 +127,6 @@ func TestRenderOffline_ChartAndReleaseAgree(t *testing.T) { assert.Equal(t, wantName, result.ReleaseName) assert.Equal(t, GenerateReleaseName(resolved.Spec.Project, resolved.Env.Original), result.ReleaseName) - assert.Nil(t, result.Previous) assert.False(t, result.IsUpgrade) dep := findRenderedDeployment(t, result.Manifest, "-api") diff --git a/internal/helm/preflight_test.go b/internal/helm/preflight_test.go index 4411e07..7c9b847 100644 --- a/internal/helm/preflight_test.go +++ b/internal/helm/preflight_test.go @@ -27,7 +27,6 @@ import ( "helm.sh/helm/v4/pkg/storage" "helm.sh/helm/v4/pkg/storage/driver" - "deployah.dev/deployah/internal/render" "deployah.dev/deployah/internal/spec" chartcommon "helm.sh/helm/v4/pkg/chart/common" @@ -191,6 +190,33 @@ func TestInstallApp_MissingReleaseIsInstall(t *testing.T) { v1rel, err := releaserToV1(rel) require.NoError(t, err) assert.Equal(t, applySSA, v1rel.ApplyMethod) + assert.Equal(t, "Install complete", v1rel.Info.Description) +} + +func TestInstallApp_ExistingReleaseIsUpgrade(t *testing.T) { + t.Parallel() + + c, cfg, resolved, releaseName := memoryHelmApp(t, "upgrade-app") + require.NoError(t, c.InstallApp(t.Context(), false, resolved, nil, nil, false)) + require.NoError(t, c.InstallApp(t.Context(), false, resolved, nil, nil, false)) + + rels, err := cfg.Releases.History(releaseName) + require.NoError(t, err) + require.Len(t, rels, 2) + + byVersion := map[int]*v1.Release{} + for _, rel := range rels { + v1rel, convErr := releaserToV1(rel) + require.NoError(t, convErr) + byVersion[v1rel.Version] = v1rel + } + require.Contains(t, byVersion, 1) + require.Contains(t, byVersion, 2) + assert.Equal(t, common.StatusSuperseded, byVersion[1].Info.Status) + assert.Equal(t, "Install complete", byVersion[1].Info.Description) + assert.Equal(t, common.StatusDeployed, byVersion[2].Info.Status) + assert.Equal(t, "Upgrade complete", byVersion[2].Info.Description) + assert.Equal(t, applySSA, byVersion[2].ApplyMethod) } func TestRenderManifests_MissingReleaseIsInstall(t *testing.T) { @@ -223,7 +249,6 @@ func TestRenderManifestsWithPrep_FreshInstall(t *testing.T) { assert.Equal(t, 1, prep.NextRevision) assert.False(t, result.IsUpgrade) assert.Equal(t, 1, result.Revision) - assert.Nil(t, result.Previous) } func TestRenderManifestsWithPrep_Upgrade(t *testing.T) { @@ -266,27 +291,8 @@ func TestRenderManifestsWithPrep_Upgrade(t *testing.T) { assert.Equal(t, 3, prep.NextRevision) assert.True(t, result.IsUpgrade) assert.Equal(t, 3, result.Revision) - require.NotNil(t, result.Previous) - assert.Equal(t, prep.Current.Manifest, result.Previous.Manifest) - assert.Equal(t, []*render.HookIntent{ - { - Name: "migrate", - Kind: "Job", - Path: "templates/migrate.yaml", - Manifest: "kind: Job\nmetadata:\n name: migrate\n", - Events: []v1.HookEvent{v1.HookPreUpgrade}, - Weight: 1, - DeletePolicies: []v1.HookDeletePolicy{v1.HookBeforeHookCreation}, - OutputLogPolicies: []v1.HookOutputLogPolicy{v1.HookOutputOnSucceeded}, - }, - { - Name: "smoke", - Kind: "Pod", - Manifest: "kind: Pod\nmetadata:\n name: smoke\n", - Events: []v1.HookEvent{v1.HookPostUpgrade}, - Weight: 2, - }, - }, result.Previous.Hooks) + assert.Equal(t, manifest, prep.Current.Manifest) + assert.Equal(t, hooks, prep.Current.Hooks) } func TestRenderManifestsWithPrep_NewestDiffersFromCurrent(t *testing.T) { @@ -314,10 +320,9 @@ func TestRenderManifestsWithPrep_NewestDiffersFromCurrent(t *testing.T) { assert.Equal(t, 5, prep.NextRevision) assert.True(t, result.IsUpgrade) assert.Equal(t, 5, result.Revision) - require.NotNil(t, result.Previous) - assert.Equal(t, currentManifest, result.Previous.Manifest) - assert.Equal(t, []*render.HookIntent{{Name: "current", Manifest: currentHook.Manifest}}, result.Previous.Hooks) - assert.NotEqual(t, newestManifest, result.Previous.Manifest) + assert.Equal(t, currentManifest, prep.Current.Manifest) + assert.Equal(t, []*v1.Hook{currentHook}, prep.Current.Hooks) + assert.NotEqual(t, newestManifest, prep.Current.Manifest) } func TestRenderManifestsWithPrep_FailedOnlyHistory(t *testing.T) { @@ -338,9 +343,8 @@ func TestRenderManifestsWithPrep_FailedOnlyHistory(t *testing.T) { assert.Equal(t, OperationUpgrade, prep.Operation) assert.Equal(t, 1, prep.Current.Version) assert.Equal(t, common.StatusFailed, prep.Current.Info.Status) - require.NotNil(t, result.Previous) - assert.Equal(t, manifest, result.Previous.Manifest) - assert.Equal(t, []*render.HookIntent{{Name: "failed", Kind: "Job", Manifest: hook.Manifest}}, result.Previous.Hooks) + assert.Equal(t, manifest, prep.Current.Manifest) + assert.Equal(t, []*v1.Hook{hook}, prep.Current.Hooks) } func TestRenderManifests_MatchesWithPrepResult(t *testing.T) { @@ -369,8 +373,6 @@ func TestRenderManifests_MatchesWithPrepResult(t *testing.T) { assert.Equal(t, withPrep.Revision, wrapped.Revision) assert.Equal(t, withPrep.ReleaseName, wrapped.ReleaseName) assert.Equal(t, withPrep.Namespace, wrapped.Namespace) - require.NotNil(t, withPrep.Previous) - assert.Equal(t, withPrep.Previous, wrapped.Previous) } func memoryHelmApp(t *testing.T, project string) (*Client, *action.Configuration, *spec.ResolvedSpec, string) { diff --git a/internal/helm/render.go b/internal/helm/render.go index eaa5c94..1bb6985 100644 --- a/internal/helm/render.go +++ b/internal/helm/render.go @@ -40,10 +40,8 @@ func (c *Client) RenderManifests(ctx context.Context, resolved *spec.ResolvedSpe } // RenderManifestsWithPrep renders [spec.ResolvedSpec] and returns the -// [ReleasePrep] for that install or upgrade. result.Previous is -// prep.Current's manifest and hooks when a current release exists. -// crds are copied into the chart. Cleanup is nil on error; the caller -// runs it once on success. +// [ReleasePrep] for that install or upgrade. crds are copied into the +// chart. Cleanup is nil on error; the caller runs it once on success. func (c *Client) RenderManifestsWithPrep(ctx context.Context, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, ReleasePrep, func(), error) { releaseName, labels, err := releaseIdentity(resolved) if err != nil { @@ -76,12 +74,6 @@ func (c *Client) RenderManifestsWithPrep(ctx context.Context, resolved *spec.Res return nil, ReleasePrep{}, nil, err } result.ChartPath = chartPath - if prep.Current != nil { - result.Previous = &render.ReleaseIntent{ - Manifest: prep.Current.Manifest, - Hooks: render.HookIntents(prep.Current.Hooks), - } - } return result, prep, cleanup, nil } diff --git a/internal/plan/build.go b/internal/plan/build.go index 790322e..00f6d19 100644 --- a/internal/plan/build.go +++ b/internal/plan/build.go @@ -38,19 +38,18 @@ type BuildClient interface { RenderManifests(ctx context.Context, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, func(), error) } -// BuildPlan renders [spec.ResolvedSpec] via client and diffs the result -// against the last successful release, returning the fully populated Plan -// (Header included) alongside the render result. It is the single -// render-diff-header pipeline shared by `deployah plan` and the plan -// `deployah deploy` shows before confirming. Project, environment, and -// chart content all come from resolved. +// BuildPlan renders resolved and compares that render with the last +// successful Helm release. It returns the Plan, header included, and the +// render result. deployah plan uses this. // -// The caller must invoke the returned cleanup func once done with -// result.ChartPath (same contract as [helm.Client.RenderManifests]). On -// error, cleanup is still returned when a chart was prepared and must be -// called. postRenderer, when non-nil, is forwarded to RenderManifests so -// extras appear in the diff. crds are written into the per-invocation -// chart copy so Helm sees the same files as apply. +// Project, environment, and chart content come from resolved. +// +// Call the returned cleanup once, after you are done with +// result.ChartPath. Call it even when BuildPlan returns an error, if a +// chart was prepared. This is the same contract as +// [helm.Client.RenderManifests]. A non-nil postRenderer is passed through +// so extra manifests show up in the diff. crds are copied into that chart +// so Helm sees the same files deploy applies. func BuildPlan(ctx context.Context, client BuildClient, clusterContext string, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*Plan, *render.RenderResult, func(), error) { if resolved == nil || resolved.Spec == nil { return nil, nil, func() {}, fmt.Errorf("plan requires resolved spec; call spec.Resolve first") diff --git a/internal/plan/diff.go b/internal/plan/diff.go index a96f273..6d538f4 100644 --- a/internal/plan/diff.go +++ b/internal/plan/diff.go @@ -147,7 +147,7 @@ func diffResource(prev, curr *yamlv3.Node) ([]FieldDiff, error) { // A pure reordering of an already-identical list (e.g. env // vars re-sorted by the template engine) has no effect on // the applied resource, so it is not a meaningful change to - // show in a deploy-confirmation preview. + // show in the plan. } } } diff --git a/internal/plan/doc.go b/internal/plan/doc.go index 95c5650..3a4f95a 100644 --- a/internal/plan/doc.go +++ b/internal/plan/doc.go @@ -12,29 +12,35 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package plan computes and renders a preview of the changes a deploy -// would make. [BuildPlan] diffs the rendered manifest against the last -// successful Helm release. [BuildSemanticPlan] renders Desired state, -// decides whether Helm runs by comparing the previous release with that -// render, and predicts writes only when Helm will run. +// Package plan computes and renders read-only deployment plans. Planning +// inspects release changes and their consequences. It does not decide +// whether `deployah deploy` runs Helm. +// +// [BuildPlan] diffs the rendered manifest against the last successful +// Helm release. [BuildSemanticPlan] renders Desired state, derives the +// plan's Helm release-intent transition by comparing the previous release +// with that render, and records write consequences only when the plan has +// a Helm transition. // // [ComputeDiff] is the older diff: it parses two rendered Kubernetes // manifests, matches resources by (apiVersion, kind, namespace, name), and // runs [github.com/homeport/dyff] field by field on resources present on // both sides. [Plan] is that model. [RenderText] prints it, and -// [NewJSONDocument] encodes it as JSON. [DeploymentIntent] holds the flags -// a deploy uses to decide what it may change. [BuildPlan] and -// [ComputeDiff] stay while callers move to the semantic plan. +// [NewJSONDocument] encodes it as JSON. [DeploymentIntent] records resize +// and hostname flags. It does not decide whether deploy runs Helm. +// [BuildPlan] and [ComputeDiff] stay while callers move to the semantic +// plan. // // [BuildSemanticPlan] renders with // [SemanticBuildClient.RenderManifestsWithPrep], then picks install, // upgrade, or none from the Helm operation and a comparison of the // previous release (prep.Current) with the rendered manifest and hooks. -// Live state and predictor output do not make that choice. If Helm will -// not run, Namespace and Helm write prediction are skipped, so the plan -// has no Helm writes. If Helm will install or upgrade, Namespace -// prediction and [deployah.dev/deployah/internal/predict.Predict] still -// produce those writes. Chart CRDs are passed through to Helm. The result +// Live state and predictor output do not make that choice. If the plan +// has no Helm transition, Namespace and Helm write prediction are +// skipped, so the plan has no Helm writes. If the plan's transition is +// install or upgrade, Namespace prediction and +// [deployah.dev/deployah/internal/predict.Predict] still produce those +// writes. Chart CRDs are passed through to Helm. The result // is a [deployah.dev/deployah/internal/plan/semantic.Plan]. Semantic types // live in plan/semantic. New rendering lives in plan/view. // diff --git a/internal/plan/helm_action.go b/internal/plan/helm_action.go index d9a7f8b..1588fec 100644 --- a/internal/plan/helm_action.go +++ b/internal/plan/helm_action.go @@ -20,7 +20,6 @@ import ( "deployah.dev/deployah/internal/helm" "deployah.dev/deployah/internal/plan/semantic" - "deployah.dev/deployah/internal/render" v1 "helm.sh/helm/v4/pkg/release/v1" ) @@ -48,37 +47,3 @@ func deriveHelmAction(prep helm.ReleasePrep, desiredManifest string, desiredHook return 0, fmt.Errorf("invalid helm operation %d", prep.Operation) } } - -// HelmActionFromRender picks install, upgrade, or none for result using -// the same release-intent comparison as [BuildSemanticPlan]. It returns -// an error when result is nil or an upgrade render has no Previous. -func HelmActionFromRender(result *render.RenderResult) (semantic.HelmAction, error) { - if result == nil { - return 0, errors.New("render result is required") - } - prep := helm.ReleasePrep{Operation: helm.OperationInstall} - if result.IsUpgrade { - if result.Previous == nil { - return 0, errors.New("upgrade render requires a previous release intent") - } - prep.Operation = helm.OperationUpgrade - prep.Current = &v1.Release{ - Manifest: result.Previous.Manifest, - Hooks: releaseHooks(result.Previous.Hooks), - } - } - return deriveHelmAction(prep, result.Manifest, result.Hooks) -} - -// releaseHooks rebuilds temporary Helm hooks from declarative intents. -// Nil entries stay nil. -func releaseHooks(hooks []*render.HookIntent) []*v1.Hook { - if hooks == nil { - return nil - } - out := make([]*v1.Hook, 0, len(hooks)) - for _, h := range hooks { - out = append(out, h.Hook()) - } - return out -} diff --git a/internal/plan/helm_action_test.go b/internal/plan/helm_action_test.go index d825216..06c208e 100644 --- a/internal/plan/helm_action_test.go +++ b/internal/plan/helm_action_test.go @@ -15,7 +15,6 @@ package plan import ( - "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -23,9 +22,7 @@ import ( "deployah.dev/deployah/internal/helm" "deployah.dev/deployah/internal/plan/semantic" - "deployah.dev/deployah/internal/render" "deployah.dev/deployah/internal/spec" - "deployah.dev/deployah/internal/testing/testpath" v1 "helm.sh/helm/v4/pkg/release/v1" ) @@ -98,88 +95,11 @@ func TestDeriveHelmAction_Errors(t *testing.T) { } } -func TestHelmActionFromRender(t *testing.T) { - t.Parallel() - same := configMapManifest("app", "same") - changed := configMapManifest("app", "new") - hookA := testHook("Job", "migrate", "migrate", "busybox", 1) - hookB := testHook("Job", "migrate", "migrate", "alpine", 1) - argsDir := filepath.Join(releaseIntentFixtures, "args-order") - hookDir := filepath.Join(releaseIntentFixtures, "hook-formatting") - argsPrev := testpath.ReadFile(t, argsDir, "previous.yaml") - argsNext := testpath.ReadFile(t, argsDir, "desired.yaml") - fmtPrev := testpath.ReadFile(t, hookDir, "previous.yaml") - fmtNext := testpath.ReadFile(t, hookDir, "desired.yaml") - fmtPrevHooks := readHookFixtures(t, filepath.Join(hookDir, "previous.hooks")) - fmtNextHooks := readHookFixtures(t, filepath.Join(hookDir, "desired.hooks")) - - tests := []struct { - name string - result *render.RenderResult - want semantic.HelmAction - }{ - {name: "install", result: &render.RenderResult{Manifest: changed}, want: semantic.HelmInstall}, - {name: "unchanged", result: upgradeRender(same, []*v1.Hook{hookA}, same, []*v1.Hook{hookA}), want: semantic.HelmNone}, - {name: "manifest change", result: upgradeRender(same, []*v1.Hook{hookA}, changed, []*v1.Hook{hookA}), want: semantic.HelmUpgrade}, - {name: "hook change", result: upgradeRender(same, []*v1.Hook{hookA}, same, []*v1.Hook{hookB}), want: semantic.HelmUpgrade}, - {name: "args reorder", result: upgradeRender(argsPrev, nil, argsNext, nil), want: semantic.HelmUpgrade}, - {name: "hook formatting", result: upgradeRender(fmtPrev, fmtPrevHooks, fmtNext, fmtNextHooks), want: semantic.HelmNone}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - got, err := HelmActionFromRender(tt.result) - require.NoError(t, err) - assert.Equal(t, tt.want, got) - }) - } -} - -func TestHelmActionFromRender_Errors(t *testing.T) { - t.Parallel() - tests := []struct { - name string - result *render.RenderResult - want string - }{ - {name: "nil result", want: "render result is required"}, - {name: "upgrade without previous", result: &render.RenderResult{IsUpgrade: true}, want: "upgrade render requires a previous release intent"}, - { - name: "nil previous hook", - result: &render.RenderResult{ - IsUpgrade: true, - Previous: &render.ReleaseIntent{Hooks: []*render.HookIntent{nil}}, - }, - want: "previous hook 1 is nil", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - _, err := HelmActionFromRender(tt.result) - require.Error(t, err) - assert.ErrorContains(t, err, tt.want) - }) - } -} - -func upgradeRender(previous string, previousHooks []*v1.Hook, desired string, desiredHooks []*v1.Hook) *render.RenderResult { - return &render.RenderResult{ - IsUpgrade: true, - Manifest: desired, - Hooks: desiredHooks, - Previous: &render.ReleaseIntent{ - Manifest: previous, - Hooks: render.HookIntents(previousHooks), - }, - } -} - func configMapManifest(name, data string) string { return "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: " + name + "\n namespace: prod\ndata:\n key: " + data + "\n" } -func TestApplyHelmWillRun_UnchangedHook(t *testing.T) { +func TestStampTaskWillRun_UnchangedHook(t *testing.T) { t.Parallel() unchanged := []semantic.TaskPlan{{ Name: "seed", @@ -199,7 +119,7 @@ func TestApplyHelmWillRun_UnchangedHook(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() tasks := append([]semantic.TaskPlan(nil), unchanged...) - applyHelmWillRun(tasks, tt.action) + stampTaskWillRun(tasks, tt.action) assert.Equal(t, tt.wantRun, tasks[0].WillRun) }) } @@ -216,6 +136,6 @@ func TestAssembleTasks_DoesNotStampWillRun(t *testing.T) { require.NoError(t, err) require.Len(t, tasks, 1) assert.False(t, tasks[0].WillRun) - applyHelmWillRun(tasks, semantic.HelmInstall) + stampTaskWillRun(tasks, semantic.HelmInstall) assert.True(t, tasks[0].WillRun) } diff --git a/internal/plan/intent.go b/internal/plan/intent.go index 3aee618..73d846e 100644 --- a/internal/plan/intent.go +++ b/internal/plan/intent.go @@ -21,8 +21,6 @@ package plan type DeploymentIntent struct { // ResizeVolumes enables persistent volume claim expansion. ResizeVolumes bool - // Reapply forces Helm to run even when the rendered spec is unchanged. - Reapply bool // ForceHostnameChange allows a hostname change that would otherwise be // blocked. ForceHostnameChange bool diff --git a/internal/plan/intent_test.go b/internal/plan/intent_test.go index 991ce39..2466e9c 100644 --- a/internal/plan/intent_test.go +++ b/internal/plan/intent_test.go @@ -25,7 +25,6 @@ func TestDefaultDeploymentIntent(t *testing.T) { got := DefaultDeploymentIntent() assert.False(t, got.ResizeVolumes) - assert.False(t, got.Reapply) assert.False(t, got.ForceHostnameChange) } @@ -41,7 +40,6 @@ func TestDeploymentIntent_ZeroValueMatchesDefault(t *testing.T) { // presentation field or CRD skip flag is added. type deploymentIntentFields struct { ResizeVolumes bool - Reapply bool ForceHostnameChange bool } @@ -52,6 +50,5 @@ func TestDeploymentIntent_BoolsDefaultFalse(t *testing.T) { got := deploymentIntentFields(DefaultDeploymentIntent()) assert.False(t, got.ResizeVolumes) - assert.False(t, got.Reapply) assert.False(t, got.ForceHostnameChange) } diff --git a/internal/plan/semantic/types.go b/internal/plan/semantic/types.go index 312ef3b..4a3b395 100644 --- a/internal/plan/semantic/types.go +++ b/internal/plan/semantic/types.go @@ -65,19 +65,23 @@ func actionRank(a Action) int { } } -// HelmAction is whether Helm will install, upgrade, or do neither. The -// zero value is invalid. +// HelmAction is the Helm release-intent transition a semantic plan +// describes. It does not control whether deployah deploy invokes Helm. +// The zero value is invalid. type HelmAction int const ( - // HelmNone means Helm will not execute. OriginHelm changes and - // changed or WillRun preDeploy/postDeploy tasks are invalid. + // HelmNone means the semantic plan contains no Helm release-intent + // transition. It does not control whether `deployah deploy` invokes + // Helm. OriginHelm changes and changed or WillRun preDeploy/postDeploy + // tasks are invalid. HelmNone HelmAction = iota + 1 - // HelmInstall is a fresh Helm install. It requires - // [Header.FreshInstall]. + // HelmInstall means the plan's transition is a fresh Helm install. It + // requires [Header.FreshInstall]. HelmInstall - // HelmUpgrade is a Helm upgrade. Zero resource consequences remain - // valid when release intent changed and Live already matches Desired. + // HelmUpgrade means the plan's transition is a Helm upgrade. Zero + // resource consequences remain valid when release intent changed and + // Live already matches Desired. HelmUpgrade ) @@ -365,9 +369,12 @@ func (a TaskAction) valid() bool { // Plan.Changes; previous hook documents are not emitted as Kubernetes // deletes. type TaskPlan struct { - Name string - Phase TaskPhase - Action TaskAction + Name string + Phase TaskPhase + Action TaskAction + // WillRun reports whether the task's hook takes part in the Helm + // transition the plan describes. It does not predict whether a later + // `deployah deploy` skips the hook. WillRun bool Definitions []HookDefinition Resources []ResourceRef diff --git a/internal/plan/semantic_build.go b/internal/plan/semantic_build.go index 4e938d5..47f19f4 100644 --- a/internal/plan/semantic_build.go +++ b/internal/plan/semantic_build.go @@ -71,10 +71,11 @@ type SemanticBuildInput struct { // history itself. PostRenderer, when non-nil, is forwarded once to the // render client. // -// Helm runs on a fresh install, or on an upgrade whose Previous release -// intent differs from the rendered Desired manifest or hooks. An unchanged -// release is [semantic.HelmNone] and runs no Namespace or Helm write -// prediction. When Helm will run, the install target Namespace is predicted +// The plan's HelmAction is [semantic.HelmInstall] on a fresh install, or +// [semantic.HelmUpgrade] when the Previous release intent differs from the +// rendered Desired manifest or hooks. An unchanged release is +// [semantic.HelmNone] and runs no Namespace or Helm write prediction. When +// the plan has a Helm transition, the install target Namespace is predicted // on cluster first, then Helm [predict.Predict] runs against a plan-local // wrapper so a same-deploy missing target namespace is not fatal. Chart CRD // files are forwarded to Helm only; they are not parsed into resource @@ -158,7 +159,7 @@ func BuildSemanticPlan( if err != nil { return semantic.Plan{}, nil, cleanup, fmt.Errorf("assemble semantic plan: %w", err) } - applyHelmWillRun(tasks, helmAction) + stampTaskWillRun(tasks, helmAction) p, err := semantic.New(header, helmAction, changes, tasks, diags) if err != nil { return semantic.Plan{}, nil, cleanup, fmt.Errorf("assemble semantic plan: %w", err) diff --git a/internal/plan/tasks.go b/internal/plan/tasks.go index 56db76a..018fa93 100644 --- a/internal/plan/tasks.go +++ b/internal/plan/tasks.go @@ -47,9 +47,9 @@ func assembleTasks( desiredHooks []*v1.Hook, changes []semantic.ResourceChange, ) ([]semantic.TaskPlan, error) { - // WillRun stays false. Callers stamp it with applyHelmWillRun after + // WillRun stays false. Callers stamp it with stampTaskWillRun after // deriveHelmAction. HelmAction is the source of truth for whether - // Helm executes. + // the plan has a Helm transition. if resolved == nil { return []semantic.TaskPlan{}, nil } @@ -272,8 +272,8 @@ func hookTask( }, nil } -func applyHelmWillRun(tasks []semantic.TaskPlan, helmAction semantic.HelmAction) { - helmWillRun := helmAction == semantic.HelmInstall || helmAction == semantic.HelmUpgrade +func stampTaskWillRun(tasks []semantic.TaskPlan, helmAction semantic.HelmAction) { + helmTransition := helmAction == semantic.HelmInstall || helmAction == semantic.HelmUpgrade for i, t := range tasks { if t.Phase == semantic.TaskSchedule { tasks[i].WillRun = false @@ -286,7 +286,7 @@ func applyHelmWillRun(tasks []semantic.TaskPlan, helmAction semantic.HelmAction) tasks[i].WillRun = false continue } - tasks[i].WillRun = helmWillRun + tasks[i].WillRun = helmTransition } } diff --git a/internal/plan/tasks_test.go b/internal/plan/tasks_test.go index da89878..df06d0d 100644 --- a/internal/plan/tasks_test.go +++ b/internal/plan/tasks_test.go @@ -42,7 +42,7 @@ func TestAssembleTasks_FreshInstallPrePost(t *testing.T) { testHook("Job", "smoke", "smoke", "v1", 2), }, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmInstall) + stampTaskWillRun(tasks, semantic.HelmInstall) byName := taskByName(t, tasks) require.Contains(t, byName, "migrate") require.Contains(t, byName, "smoke") @@ -65,7 +65,7 @@ func TestAssembleTasks_ManifestChangeUnchangedHookWillRun(t *testing.T) { prep := upgradePrep(map[string]any{"seed": map[string]any{"on": "preDeploy", "hookWeight": 1}}, []*v1.Hook{hook}, nil) tasks, err := assembleTasks(resolved, prep, []*v1.Hook{hook}, []semantic.ResourceChange{labeledCreate("api", "api")}) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, "seed") assert.Equal(t, semantic.TaskUnchanged, byName["seed"].Action) @@ -83,7 +83,7 @@ func TestAssembleTasks_HookDefinitionOnly(t *testing.T) { prep := upgradePrep(map[string]any{"migrate": map[string]any{"on": "preDeploy", "hookWeight": 1}}, []*v1.Hook{prev}, nil) tasks, err := assembleTasks(resolved, prep, []*v1.Hook{desired}, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, "migrate") assert.Equal(t, semantic.TaskUpdate, byName["migrate"].Action) @@ -104,7 +104,7 @@ func TestAssembleTasks_HookCreateAndDelete(t *testing.T) { }, []*v1.Hook{prev}, nil) tasks, err := assembleTasks(resolved, prep, []*v1.Hook{desired}, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, "smoke") require.Contains(t, byName, "old") @@ -131,7 +131,7 @@ func TestAssembleTasks_AllCurrentHooksRunOnHelmChange(t *testing.T) { changes := []semantic.ResourceChange{labeledCreate("api", "api")} tasks, err := assembleTasks(resolved, prep, []*v1.Hook{migrate, seed}, changes) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) assert.True(t, byName["migrate"].WillRun) assert.True(t, byName["seed"].WillRun) @@ -151,7 +151,7 @@ func TestAssembleTasks_NewestHooksIgnored(t *testing.T) { prep := upgradePrep(map[string]any{"migrate": map[string]any{"on": "preDeploy", "hookWeight": 1}}, []*v1.Hook{currentHook}, newest) tasks, err := assembleTasks(resolved, prep, []*v1.Hook{desired}, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmNone) + stampTaskWillRun(tasks, semantic.HelmNone) byName := taskByName(t, tasks) assert.Equal(t, semantic.TaskUnchanged, byName["migrate"].Action) assert.Empty(t, byName["migrate"].Definitions) @@ -169,7 +169,7 @@ func TestAssembleTasks_MultiResourceBundleOnlyChangedDefinition(t *testing.T) { prep := upgradePrep(map[string]any{"migrate": map[string]any{"on": "preDeploy", "hookWeight": 1}}, []*v1.Hook{prevCM, prevJob}, nil) tasks, err := assembleTasks(resolved, prep, []*v1.Hook{desiredCM, desiredJob}, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Len(t, byName["migrate"].Definitions, 1) assert.Equal(t, "migrate-env", byName["migrate"].Definitions[0].Resource.Name) @@ -201,7 +201,7 @@ func TestAssembleTasks_LeftoverHookWithoutConfig(t *testing.T) { prep := upgradePrep(map[string]any{}, []*v1.Hook{hook}, nil) tasks, err := assembleTasks(resolvedWithTasks(nil), prep, nil, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, "orphan") assert.Equal(t, semantic.TaskDelete, byName["orphan"].Action) @@ -237,7 +237,7 @@ spec: prep := upgradePrep(map[string]any{}, []*v1.Hook{hook}, nil) tasks, err := assembleTasks(resolvedWithTasks(nil), prep, nil, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, "orphan") assert.Equal(t, semantic.TaskDelete, byName["orphan"].Action) @@ -388,7 +388,7 @@ func TestAssembleTasks_HookChangeRunsAllCurrentHooks(t *testing.T) { prep := upgradePrep(tt.previous, tt.prev, nil) tasks, err := assembleTasks(resolvedWithTasks(tt.resolved), prep, tt.desired, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Len(t, byName, len(tt.want)) for name, want := range tt.want { @@ -413,7 +413,7 @@ func TestAssembleTasks_RemovedScheduleDoesNotCaptureComponent(t *testing.T) { changes := []semantic.ResourceChange{cron, comp} tasks, err := assembleTasks(resolved, prep, nil, changes) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, "cleanup") assert.Equal(t, semantic.TaskDelete, byName["cleanup"].Action) @@ -477,7 +477,7 @@ func TestAssembleTasks_TransitionToManual(t *testing.T) { prep := upgradePrep(tt.previous, tt.prevHooks, nil) tasks, err := assembleTasks(resolved, prep, nil, tt.changes) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, tt.task) got := byName[tt.task] @@ -528,7 +528,7 @@ func TestAssembleTasks_HookPhaseChange(t *testing.T) { }, []*v1.Hook{tt.prev}, nil) tasks, err := assembleTasks(resolved, prep, []*v1.Hook{tt.desired}, nil) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) got := taskByName(t, tasks)["migrate"] assert.Equal(t, tt.wantPhase, got.Phase) assert.Equal(t, semantic.TaskUpdate, got.Action) @@ -648,7 +648,7 @@ func TestAssembleTasks_HookScheduleTransition(t *testing.T) { prep := upgradePrep(tt.previous, tt.prevHooks, nil) tasks, err := assembleTasks(resolved, prep, tt.desired, tt.changes) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) p, err := semantic.New(semantic.Header{}, semantic.HelmUpgrade, tt.changes, tasks, nil) require.NoError(t, err) byName := taskByName(t, p.Tasks) @@ -678,7 +678,7 @@ func TestAssembleTasks_ScheduleReferencesAndOmitsUnchanged(t *testing.T) { changes := []semantic.ResourceChange{cron, labeledCreate("api", "api")} tasks, err := assembleTasks(resolved, prep, nil, changes) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, "cleanup") assert.NotContains(t, byName, "keep") @@ -734,7 +734,7 @@ func TestAssembleTasks_ScheduleProvenance(t *testing.T) { prep := upgradePrep(tt.previous, nil, nil) tasks, err := assembleTasks(resolvedWithTasks(tt.resolved), prep, nil, tt.changes) require.NoError(t, err) - applyHelmWillRun(tasks, semantic.HelmUpgrade) + stampTaskWillRun(tasks, semantic.HelmUpgrade) byName := taskByName(t, tasks) require.Contains(t, byName, "cleanup") got := byName["cleanup"] diff --git a/internal/render/result.go b/internal/render/result.go index 0a2137f..e881012 100644 --- a/internal/render/result.go +++ b/internal/render/result.go @@ -12,22 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package render holds the client-side chart render result shared by the -// Helm engine, session's [deployah.dev/deployah/internal/session.HelmClient], -// and plan/deploy callers. Keeping [RenderResult] here avoids a dependency -// cycle (session constructs helm clients; helm must not import session) -// while letting session expose render methods without importing helm. +// Package render holds the chart render that Helm, plan, and +// [deployah.dev/deployah/internal/session.HelmClient] share. The type +// lives here so session can return a render without importing helm, and +// helm does not have to import session to build a client. package render import ( - "slices" - v1 "helm.sh/helm/v4/pkg/release/v1" ) // RenderResult is the client-side render of a chart for one project and -// environment, used both to diff against the last release (`deployah plan`) -// and, on match, as input to the real install/upgrade (`deployah deploy`). +// environment. deployah plan diffs it against the last successful +// release. Deploy renders the same way before a volume resize, then +// drops the result, so a chart that does not render never gets resized. type RenderResult struct { // ReleaseName is the Helm release name computed for project/environment. ReleaseName string @@ -41,11 +39,6 @@ type RenderResult struct { Manifest string // Hooks are the Helm hooks declared by the chart for this release. Hooks []*v1.Hook - // Previous is the declarative Helm release this render upgrades from. - // It is nil on a fresh install. On an upgrade it is Helm's current - // upgrade baseline, not the newest revision and not the last - // successful release. - Previous *ReleaseIntent // IsUpgrade is true when an existing release was found and the render // used the upgrade action, so Revision is that release's version plus // one. It is false for a fresh install, where Revision is always 1. @@ -53,79 +46,3 @@ type RenderResult struct { // Revision is the release revision this render corresponds to. Revision int } - -// ReleaseIntent is the declarative part of a Helm release used to compare -// a previous release with the desired render. It has no runtime state such -// as status, revision, or hook execution. -type ReleaseIntent struct { - // Manifest is the rendered, "---"-concatenated Kubernetes YAML. - Manifest string - // Hooks are the chart hooks for this release, in release order. A nil - // entry is kept so comparison can fail closed. - Hooks []*HookIntent -} - -// HookIntent is the declarative part of one Helm hook. It omits execution -// state such as LastRun. -type HookIntent struct { - // Name is the hook name. - Name string - // Kind is the Kubernetes kind of the hook resource. - Kind string - // Path is the chart-relative path of the hook template. - Path string - // Manifest is the rendered hook YAML. - Manifest string - // Events are the Helm hook events, in release order. - Events []v1.HookEvent - // Weight is the hook sort weight. - Weight int - // DeletePolicies say when Helm deletes the hook resource. - DeletePolicies []v1.HookDeletePolicy - // OutputLogPolicies say when hook logs are copied to the main process. - OutputLogPolicies []v1.HookOutputLogPolicy -} - -// HookIntents copies the declarative fields of hooks, in order. A nil hook -// stays nil. Slice fields are cloned. LastRun is left behind. -func HookIntents(hooks []*v1.Hook) []*HookIntent { - if hooks == nil { - return nil - } - out := make([]*HookIntent, 0, len(hooks)) - for _, h := range hooks { - if h == nil { - out = append(out, nil) - continue - } - out = append(out, &HookIntent{ - Name: h.Name, - Kind: h.Kind, - Path: h.Path, - Manifest: h.Manifest, - Events: slices.Clone(h.Events), - Weight: h.Weight, - DeletePolicies: slices.Clone(h.DeletePolicies), - OutputLogPolicies: slices.Clone(h.OutputLogPolicies), - }) - } - return out -} - -// Hook returns a Helm hook with this intent's declarative fields. Slice -// fields are cloned. A nil intent returns nil. -func (h *HookIntent) Hook() *v1.Hook { - if h == nil { - return nil - } - return &v1.Hook{ - Name: h.Name, - Kind: h.Kind, - Path: h.Path, - Manifest: h.Manifest, - Events: slices.Clone(h.Events), - Weight: h.Weight, - DeletePolicies: slices.Clone(h.DeletePolicies), - OutputLogPolicies: slices.Clone(h.OutputLogPolicies), - } -} diff --git a/internal/render/result_test.go b/internal/render/result_test.go deleted file mode 100644 index 293c676..0000000 --- a/internal/render/result_test.go +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright 2026 The Deployah Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package render_test - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "deployah.dev/deployah/internal/render" - - v1 "helm.sh/helm/v4/pkg/release/v1" -) - -func TestHookIntents_NilEntryAndSliceCopy(t *testing.T) { - t.Parallel() - - events := []v1.HookEvent{v1.HookPreUpgrade} - policies := []v1.HookDeletePolicy{v1.HookBeforeHookCreation} - logs := []v1.HookOutputLogPolicy{v1.HookOutputOnSucceeded} - hooks := []*v1.Hook{ - nil, - { - Name: "migrate", - Events: events, - DeletePolicies: policies, - OutputLogPolicies: logs, - }, - } - - got := render.HookIntents(hooks) - require.Len(t, got, 2) - assert.Nil(t, got[0]) - require.NotNil(t, got[1]) - - events[0] = v1.HookPostUpgrade - policies[0] = v1.HookFailed - logs[0] = v1.HookOutputOnFailed - assert.Equal(t, []v1.HookEvent{v1.HookPreUpgrade}, got[1].Events) - assert.Equal(t, []v1.HookDeletePolicy{v1.HookBeforeHookCreation}, got[1].DeletePolicies) - assert.Equal(t, []v1.HookOutputLogPolicy{v1.HookOutputOnSucceeded}, got[1].OutputLogPolicies) - assert.Nil(t, render.HookIntents(nil)) - assert.Nil(t, (*render.HookIntent)(nil).Hook()) - - hook := got[1].Hook() - require.NotNil(t, hook) - got[1].Events[0] = v1.HookPostDelete - assert.Equal(t, []v1.HookEvent{v1.HookPreUpgrade}, hook.Events) -} - -func TestHookIntent_RoundTrip(t *testing.T) { - t.Parallel() - - ran := time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC) - src := &v1.Hook{ - Name: "migrate", - Kind: "Job", - Path: "templates/migrate.yaml", - Manifest: "kind: Job\nmetadata:\n name: migrate\n", - Events: []v1.HookEvent{v1.HookPreUpgrade, v1.HookPostUpgrade}, - LastRun: v1.HookExecution{ - StartedAt: ran, - CompletedAt: ran, - Phase: v1.HookPhaseSucceeded, - }, - Weight: 3, - DeletePolicies: []v1.HookDeletePolicy{v1.HookBeforeHookCreation, v1.HookSucceeded}, - OutputLogPolicies: []v1.HookOutputLogPolicy{v1.HookOutputOnSucceeded, v1.HookOutputOnFailed}, - } - - got := render.HookIntents([]*v1.Hook{src}) - require.Len(t, got, 1) - require.NotNil(t, got[0]) - - want := *src - want.LastRun = v1.HookExecution{} - assert.Equal(t, &want, got[0].Hook()) -} diff --git a/scenarios/crd-create-idempotent/e2e.yaml b/scenarios/crd-create-idempotent/e2e.yaml index a6e5aa3..bf092a1 100644 --- a/scenarios/crd-create-idempotent/e2e.yaml +++ b/scenarios/crd-create-idempotent/e2e.yaml @@ -21,7 +21,7 @@ steps: status: readyReplicas: 1 - deploy: {} - stderrContains: No changes + stderrContains: Deployed resources: - match: apiVersion: apiextensions.k8s.io/v1 diff --git a/scenarios/crd-skip-then-upgrade/e2e.yaml b/scenarios/crd-skip-then-upgrade/e2e.yaml index 22a8b8e..e80127c 100644 --- a/scenarios/crd-skip-then-upgrade/e2e.yaml +++ b/scenarios/crd-skip-then-upgrade/e2e.yaml @@ -18,8 +18,7 @@ steps: kind: CustomResourceDefinition metadata: name: skipwidgets.example.com - - deploy: - args: [--reapply] + - deploy: {} resources: - match: apiVersion: apps/v1 diff --git a/scenarios/plan-no-changes/plan-config.yaml b/scenarios/plan-no-changes/plan-config.yaml index b96b918..8e7abca 100644 --- a/scenarios/plan-no-changes/plan-config.yaml +++ b/scenarios/plan-no-changes/plan-config.yaml @@ -1,8 +1,7 @@ -# before.yaml and deployah.yaml are identical: the render is byte-for-byte -# the same resource set, so the plan must be empty. This also covers the -# "deploy twice" case (the second plan against an unchanged spec is -# empty); the deploy-time skip-on-no-op behavior itself is a separate -# integration concern tested elsewhere. +# Planning only. before.yaml and deployah.yaml are identical, so the +# render is the same resource set and the plan must be empty. A second +# plan against an unchanged spec is empty too. Deployment execution is +# independent of this plan result. freshInstall: false changes: []