carry helm action into deploy plan

internal/cmd/deploy/deploy.go

@@ -16,6 +16,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"
@@ -70,12 +71,15 @@ deployah plan prod --offline`),
 }
 
 // deployPlan bundles the shown diff with the render that produced it, so
-// callers reuse one render instead of recomputing. cleanup releases the
-// chart temp dir behind result.ChartPath; runDeploy defers it.
+// 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
-	cleanup func()
+	diff       *planengine.Plan
+	result     *render.RenderResult
+	helmAction semantic.HelmAction
+	cleanup    func()
 }
 
 func runDeploy(c *nabat.Context) error {
@@ -285,7 +289,12 @@ func computePlan(c *nabat.Context, helmClient session.HelmClient, cluster *sessi
 		cleanup()
 		return nil, fmt.Errorf("%w%s", err, cmdopts.ClusterHint(err))
 	}
-	return &deployPlan{diff: p, result: result, cleanup: cleanup}, nil
+	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

internal/cmd/deploy/deploy_flow_test.go

@@ -21,19 +21,22 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"helm.sh/helm/v4/pkg/release/common"
 	"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/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"
+	v1 "helm.sh/helm/v4/pkg/release/v1"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -385,3 +388,115 @@ func TestDeployFlags_SkipCRDsAcceptedCRDsRemoved(t *testing.T) {
 	var opts Options
 	assert.False(t, opts.SkipCRDs)
 }
+
+func TestComputePlan_HelmAction(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)
+}
+
+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)
+}
+
+func deployResolved() *spec.ResolvedSpec {
+	return &spec.ResolvedSpec{
+		Spec: &spec.Spec{Project: "web"},
+		Env:  spec.EnvIdentity{Original: "production"},
+	}
+}
+
+func deployedRelease(manifest string) *v1.Release {
+	return &v1.Release{
+		Version:  2,
+		Manifest: manifest,
+		Info:     &v1.Info{Status: common.StatusDeployed},
+	}
+}
+
+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"
+}

internal/cmd/deploy/deploy_test.go

@@ -37,6 +37,7 @@ type stubHelmClient struct {
 	renderResults   []*render.RenderResult
 	renderErr       error
 	renderCallCount int
+	cleanupCalls    int
 
 	installErr       error
 	installCallCount int
@@ -55,14 +56,14 @@ func (s *stubHelmClient) InstallApp(_ context.Context, _ bool, _ *spec.ResolvedS
 
 func (s *stubHelmClient) RenderManifests(context.Context, *spec.ResolvedSpec, postrenderer.PostRenderer, []extras.RawFile) (*render.RenderResult, func(), error) {
 	if s.renderErr != nil {
-		return nil, func() {}, s.renderErr
+		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)))
 	}
-	return s.renderResults[i], func() {}, nil
+	return s.renderResults[i], func() { s.cleanupCalls++ }, nil
 }
 
 func (s *stubHelmClient) RenderOffline(context.Context, *spec.ResolvedSpec, postrenderer.PostRenderer, []extras.RawFile) (*render.RenderResult, func(), error) {

internal/helm/identity_test.go

@@ -127,6 +127,8 @@ 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")
 	assert.Equal(t, resolved.Spec.Project, dep.Labels[spec.LabelProject])

internal/helm/preflight_test.go

@@ -27,6 +27,7 @@ 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"
@@ -222,13 +223,36 @@ 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) {
 	t.Parallel()
 
 	c, cfg, resolved, releaseName := memoryHelmApp(t, "upgrade-prep")
-	seedRelease(t, cfg, releaseName, 2, common.StatusDeployed, applySSA)
+	ran := time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC)
+	manifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: app\n"
+	hooks := []*v1.Hook{
+		{
+			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,
+			LastRun:  v1.HookExecution{StartedAt: ran, CompletedAt: ran, Phase: v1.HookPhaseSucceeded},
+		},
+	}
+	seedReleaseIntent(t, cfg, releaseName, 2, common.StatusDeployed, applySSA, manifest, hooks)
 
 	result, prep, cleanup, err := c.RenderManifestsWithPrep(t.Context(), resolved, nil, nil)
 	if cleanup != nil {
@@ -242,14 +266,39 @@ 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)
 }
 
 func TestRenderManifestsWithPrep_NewestDiffersFromCurrent(t *testing.T) {
 	t.Parallel()
 
 	c, cfg, resolved, releaseName := memoryHelmApp(t, "newest-current")
-	seedRelease(t, cfg, releaseName, 3, common.StatusDeployed, applySSA)
-	seedRelease(t, cfg, releaseName, 4, common.StatusFailed, applySSA)
+	currentManifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: current\n"
+	newestManifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: newest\n"
+	currentHook := &v1.Hook{Name: "current", Manifest: "kind: Job\nmetadata:\n  name: current\n"}
+	newestHook := &v1.Hook{Name: "newest", Manifest: "kind: Job\nmetadata:\n  name: newest\n"}
+	seedReleaseIntent(t, cfg, releaseName, 3, common.StatusDeployed, applySSA, currentManifest, []*v1.Hook{currentHook})
+	seedReleaseIntent(t, cfg, releaseName, 4, common.StatusFailed, applySSA, newestManifest, []*v1.Hook{newestHook})
 
 	result, prep, cleanup, err := c.RenderManifestsWithPrep(t.Context(), resolved, nil, nil)
 	if cleanup != nil {
@@ -265,13 +314,42 @@ 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)
+}
+
+func TestRenderManifestsWithPrep_FailedOnlyHistory(t *testing.T) {
+	t.Parallel()
+
+	c, cfg, resolved, releaseName := memoryHelmApp(t, "failed-only")
+	manifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: failed\n"
+	hook := &v1.Hook{Name: "failed", Kind: "Job", Manifest: "kind: Job\nmetadata:\n  name: failed\n"}
+	seedReleaseIntent(t, cfg, releaseName, 1, common.StatusFailed, applySSA, manifest, []*v1.Hook{hook})
+
+	result, prep, cleanup, err := c.RenderManifestsWithPrep(t.Context(), resolved, nil, nil)
+	if cleanup != nil {
+		t.Cleanup(cleanup)
+	}
+	require.NoError(t, err)
+	require.NotNil(t, result)
+	require.NotNil(t, prep.Current)
+	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)
 }
 
 func TestRenderManifests_MatchesWithPrepResult(t *testing.T) {
 	t.Parallel()
 
 	c, cfg, resolved, releaseName := memoryHelmApp(t, "compat-render")
-	seedRelease(t, cfg, releaseName, 1, common.StatusDeployed, applySSA)
+	manifest := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: compat\n"
+	hook := &v1.Hook{Name: "compat", Manifest: "kind: Job\nmetadata:\n  name: compat\n"}
+	seedReleaseIntent(t, cfg, releaseName, 1, common.StatusDeployed, applySSA, manifest, []*v1.Hook{hook})
 
 	withPrep, prep, cleanup, err := c.RenderManifestsWithPrep(t.Context(), resolved, nil, nil)
 	if cleanup != nil {
@@ -291,6 +369,8 @@ 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) {
@@ -321,6 +401,11 @@ func memoryHelmApp(t *testing.T, project string) (*Client, *action.Configuration
 
 func seedRelease(t *testing.T, cfg *action.Configuration, name string, version int, status common.Status, applyMethod string) {
 	t.Helper()
+	seedReleaseIntent(t, cfg, name, version, status, applyMethod, "", nil)
+}
+
+func seedReleaseIntent(t *testing.T, cfg *action.Configuration, name string, version int, status common.Status, applyMethod, manifest string, hooks []*v1.Hook) {
+	t.Helper()
 
 	now := time.Now()
 	require.NoError(t, cfg.Releases.Create(&v1.Release{
@@ -343,6 +428,8 @@ func seedRelease(t *testing.T, cfg *action.Configuration, name string, version i
 		},
 		Version:     version,
 		Namespace:   "default",
+		Manifest:    manifest,
+		Hooks:       hooks,
 		ApplyMethod: applyMethod,
 	}))
 }