feat: limit resource deletion on trigger

Currently if `tilt trigger` is run on a resource with RBACs, admission webhooks or ServiceAccounts
those are removed causing cascading failures on other resources with `resource_deps` to them.

This commit changes the `tilt trigger` logic to not call ForceDelete on this kind of resources while
keeping for example `Job`s and `Pod`s in the scope of ForceDelete for faster development loops.

Signed-off-by: Jaakko Sirén <jaakko@craci.com>

Author: Jaakkonen

internal/controllers/core/kubernetesapply/reconciler.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"sync"
+	"time"
 
 	"github.com/pkg/errors"
 	batchv1 "k8s.io/api/batch/v1"
@@ -851,6 +852,91 @@ func (r *Reconciler) ForceDelete(ctx context.Context, nn types.NamespacedName,
 	return nil
 }
 
+// triggerRestartSet is the set of workload-controller GVKs whose pod
+// template we stamp with kubectl.kubernetes.io/restartedAt on trigger
+// to force a rolling restart — the same mechanism `kubectl rollout
+// restart` uses.
+var triggerRestartSet = map[schema.GroupKind]bool{
+	{Group: "apps", Kind: "Deployment"}:  true,
+	{Group: "apps", Kind: "StatefulSet"}: true,
+	{Group: "apps", Kind: "DaemonSet"}:   true,
+}
+
+// triggerRecreateSet is the set of GVKs whose spec is immutable or
+// whose "run" IS the object (Jobs, standalone Pods). Trigger on a
+// resource containing these force-deletes the specific entity; the
+// apply pass then recreates it.
+var triggerRecreateSet = map[schema.GroupKind]bool{
+	{Group: "batch", Kind: "Job"}: true,
+	{Group: "", Kind: "Pod"}:      true,
+}
+
+// PrepareTriggerEntities partitions the rendered entities of a
+// KubernetesApply for a `tilt trigger` / force-refresh apply pass.
+// Workload controllers receive a kubectl.kubernetes.io/restartedAt
+// stamp on their pod template so the apply triggers a rolling
+// restart; Jobs and standalone Pods are returned in toRecreate for
+// per-entity force-delete before apply; everything else passes
+// through unchanged. Ordering is preserved.
+func PrepareTriggerEntities(entities []k8s.K8sEntity, now time.Time) (toApply []k8s.K8sEntity, toRecreate []k8s.K8sEntity, err error) {
+	stamp := now.UTC().Format(time.RFC3339)
+	toApply = make([]k8s.K8sEntity, 0, len(entities))
+	for _, e := range entities {
+		gk := e.GVK().GroupKind()
+		switch {
+		case triggerRecreateSet[gk]:
+			toRecreate = append(toRecreate, e)
+			toApply = append(toApply, e)
+		case triggerRestartSet[gk]:
+			stamped, rerr := withPodTemplateRestartedAt(e, stamp)
+			if rerr != nil {
+				return nil, nil, fmt.Errorf("stamping restartedAt on %s/%s: %v",
+					e.GVK().Kind, e.Name(), rerr)
+			}
+			toApply = append(toApply, stamped)
+		default:
+			toApply = append(toApply, e)
+		}
+	}
+	return toApply, toRecreate, nil
+}
+
+func withPodTemplateRestartedAt(e k8s.K8sEntity, stamp string) (k8s.K8sEntity, error) {
+	const annKey = "kubectl.kubernetes.io/restartedAt"
+	out := e.DeepCopy()
+	templates, err := k8s.ExtractPodTemplateSpec(out)
+	if err != nil {
+		return k8s.K8sEntity{}, err
+	}
+	if len(templates) == 0 {
+		return k8s.K8sEntity{}, fmt.Errorf("no pod template on %s/%s", e.GVK().Kind, e.Name())
+	}
+	for _, t := range templates {
+		if t.Annotations == nil {
+			t.Annotations = map[string]string{}
+		}
+		t.Annotations[annKey] = stamp
+	}
+	return out, nil
+}
+
+// ForceDeleteEntities deletes a pre-filtered list of entities in
+// reverse dependency order. Used by the trigger path to recreate Jobs
+// and standalone Pods after PrepareTriggerEntities has partitioned the
+// rendered set.
+func (r *Reconciler) ForceDeleteEntities(ctx context.Context, nn types.NamespacedName,
+	cluster *v1alpha1.Cluster, entities []k8s.K8sEntity, reason string) error {
+	if len(entities) == 0 {
+		return nil
+	}
+	r.recordDelete(nn)
+	r.bestEffortDelete(ctx, nn, deleteSpec{
+		cluster:  cluster,
+		entities: k8s.ReverseSortedEntities(entities),
+	}, reason)
+	return nil
+}
+
 // Update the status if necessary.
 func (r *Reconciler) maybeUpdateStatus(ctx context.Context, nn types.NamespacedName, obj *v1alpha1.KubernetesApply) (*v1alpha1.KubernetesApply, error) {
 	newStatus := v1alpha1.KubernetesApplyStatus{}

internal/controllers/core/kubernetesapply/reconciler_test.go

@@ -689,6 +689,53 @@ func TestForceDeleteWithCmd(t *testing.T) {
 	}
 }
 
+func TestPrepareTriggerEntitiesStampsWorkloadController(t *testing.T) {
+	entities, err := k8s.ParseYAMLFromString(testyaml.SanchoYAML)
+	require.NoError(t, err)
+
+	stamp := time.Date(2024, 1, 2, 3, 4, 5, 0, time.UTC)
+	toApply, toRecreate, err := PrepareTriggerEntities(entities, stamp)
+	require.NoError(t, err)
+	require.Empty(t, toRecreate)
+	require.Len(t, toApply, 1)
+
+	out, err := k8s.SerializeSpecYAML(toApply)
+	require.NoError(t, err)
+	assert.Contains(t, out, "kubectl.kubernetes.io/restartedAt: \"2024-01-02T03:04:05Z\"")
+}
+
+func TestPrepareTriggerEntitiesRecreatesJob(t *testing.T) {
+	entities, err := k8s.ParseYAMLFromString(testyaml.JobYAML)
+	require.NoError(t, err)
+
+	toApply, toRecreate, err := PrepareTriggerEntities(entities, time.Now())
+	require.NoError(t, err)
+	require.Len(t, toApply, 1)
+	require.Len(t, toRecreate, 1)
+	assert.Equal(t, "Job", toRecreate[0].GVK().Kind)
+}
+
+func TestPrepareTriggerEntitiesPassesThroughConfigMap(t *testing.T) {
+	yaml := `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: untouched
+data:
+  hello: world
+`
+	entities, err := k8s.ParseYAMLFromString(yaml)
+	require.NoError(t, err)
+
+	toApply, toRecreate, err := PrepareTriggerEntities(entities, time.Now())
+	require.NoError(t, err)
+	require.Empty(t, toRecreate)
+	require.Len(t, toApply, 1)
+
+	out, err := k8s.SerializeSpecYAML(toApply)
+	require.NoError(t, err)
+	assert.NotContains(t, out, "restartedAt")
+}
+
 func TestDisableByConfigmap(t *testing.T) {
 	f := newFixture(t)
 	ka := v1alpha1.KubernetesApply{

internal/engine/buildcontrol/image_build_and_deployer.go

@@ -13,6 +13,7 @@ import (
 	"github.com/tilt-dev/tilt/internal/controllers/core/cmdimage"
 	"github.com/tilt-dev/tilt/internal/controllers/core/dockerimage"
 	"github.com/tilt-dev/tilt/internal/controllers/core/kubernetesapply"
+	"github.com/tilt-dev/tilt/internal/k8s"
 	"github.com/tilt-dev/tilt/internal/store"
 	"github.com/tilt-dev/tilt/internal/store/k8sconv"
 	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
@@ -81,17 +82,17 @@ func (ibd *ImageBuildAndDeployer) BuildAndDeploy(ctx context.Context, st store.R
 		numStages++
 	}
 
-	hasDeleteStep := stateSet.FullBuildTriggered()
-	if hasDeleteStep {
+	hasForceUpdateStep := stateSet.FullBuildTriggered()
+	if hasForceUpdateStep {
 		numStages++
 	}
 
 	ps := build.NewPipelineState(ctx, numStages, ibd.clock)
 	defer func() { ps.End(ctx, err) }()
 
-	if hasDeleteStep {
+	if hasForceUpdateStep {
 		ps.StartPipelineStep(ctx, "Force update")
-		err = ibd.delete(ps.AttachLogger(ctx), kTarget, kCluster)
+		kTarget, err = ibd.prepareForceUpdate(ps.AttachLogger(ctx), kTarget, kCluster)
 		if err != nil {
 			return store.BuildResultSet{}, WrapDontFallBackError(err)
 		}
@@ -197,9 +198,42 @@ func (ibd *ImageBuildAndDeployer) deploy(
 	return store.NewK8sDeployResult(kTargetID, filter), nil
 }
 
-// Delete all the resources in the Kubernetes target, to ensure that they restart when
-// we re-apply them.
-func (ibd *ImageBuildAndDeployer) delete(ctx context.Context, k8sTarget model.K8sTarget, cluster *v1alpha1.Cluster) error {
-	kTargetNN := types.NamespacedName{Name: k8sTarget.ID().Name.String()}
-	return ibd.r.ForceDelete(ctx, kTargetNN, k8sTarget.KubernetesApplySpec, cluster, "force update")
+// prepareForceUpdate rewrites the target's rendered YAML for a
+// `tilt trigger` / force-refresh apply pass. Workload controllers get
+// a kubectl.kubernetes.io/restartedAt stamp so the apply triggers a
+// rolling restart; Jobs and standalone Pods are force-deleted here and
+// recreated by the apply; everything else passes through so the apply
+// pipeline updates it in place. Specs with a custom DeleteCmd keep the
+// legacy blanket-delete path via Reconciler.ForceDelete.
+func (ibd *ImageBuildAndDeployer) prepareForceUpdate(ctx context.Context, kTarget model.K8sTarget, cluster *v1alpha1.Cluster) (model.K8sTarget, error) {
+	spec := kTarget.KubernetesApplySpec
+	kTargetNN := types.NamespacedName{Name: kTarget.ID().Name.String()}
+
+	if spec.YAML == "" {
+		if spec.DeleteCmd != nil {
+			return kTarget, ibd.r.ForceDelete(ctx, kTargetNN, spec, cluster, "force update")
+		}
+		return kTarget, nil
+	}
+
+	entities, err := k8s.ParseYAMLFromString(spec.YAML)
+	if err != nil {
+		return kTarget, fmt.Errorf("force update: parsing YAML: %v", err)
+	}
+
+	toApply, toRecreate, err := kubernetesapply.PrepareTriggerEntities(entities, ibd.clock.Now())
+	if err != nil {
+		return kTarget, fmt.Errorf("force update: preparing entities: %v", err)
+	}
+
+	if err := ibd.r.ForceDeleteEntities(ctx, kTargetNN, cluster, toRecreate, "force update (recreate)"); err != nil {
+		return kTarget, fmt.Errorf("force update: deleting ephemeral entities: %v", err)
+	}
+
+	rewritten, err := k8s.SerializeSpecYAML(toApply)
+	if err != nil {
+		return kTarget, fmt.Errorf("force update: serializing rewritten YAML: %v", err)
+	}
+	kTarget.KubernetesApplySpec.YAML = rewritten
+	return kTarget, nil
 }

internal/engine/buildcontrol/image_build_and_deployer_test.go

@@ -74,8 +74,13 @@ func TestForceUpdateK8s(t *testing.T) {
 	_, err := f.BuildAndDeploy(BuildTargets(m), stateSet)
 	require.NoError(t, err)
 
-	// A force rebuild should delete the old resources.
-	assert.Equal(t, 1, strings.Count(f.k8s.DeletedYaml, "Deployment"))
+	// A force update stamps a restartedAt annotation on the pod template
+	// rather than deleting the Deployment; the Deployment controller then
+	// rolls its pods on the next apply.
+	assert.Equal(t, 0, strings.Count(f.k8s.DeletedYaml, "Deployment"))
+	// Fake clock is fixed at 2019-01-01 01:01:01 UTC; the stamp is RFC3339.
+	assert.Contains(t, f.k8s.Yaml,
+		"kubectl.kubernetes.io/restartedAt: \"2019-01-01T01:01:01Z\"")
 }
 
 func TestForceUpdateDoesNotDeleteNamespace(t *testing.T) {
@@ -105,17 +110,19 @@ metadata:
 	_, err := f.BuildAndDeploy(BuildTargets(m), stateSet)
 	require.NoError(t, err)
 
-	// A force rebuild should delete the ConfigMap but not the Namespace.
-	assert.Equal(t, 1, strings.Count(f.k8s.DeletedYaml, "kind: ConfigMap"))
+	// Force update no longer deletes non-workload kinds: the ConfigMap
+	// updates in place via the apply pipeline and the Namespace was
+	// already preserved by ForceDelete's filter.
+	assert.Equal(t, 0, strings.Count(f.k8s.DeletedYaml, "kind: ConfigMap"))
 	assert.Equal(t, 0, strings.Count(f.k8s.DeletedYaml, "kind: Namespace"))
 }
 
-func TestDeleteShouldHappenInReverseOrder(t *testing.T) {
+func TestForceDeleteHappensInReverseOrder(t *testing.T) {
 	f := newIBDFixture(t, clusterid.ProductGKE)
 
 	m := newK8sMultiEntityManifest("sancho")
-
-	err := f.ibd.delete(f.ctx, m.K8sTarget(), nil)
+	nn := ktypes.NamespacedName{Name: m.K8sTarget().ID().Name.String()}
+	err := f.ibd.r.ForceDelete(f.ctx, nn, m.K8sTarget().KubernetesApplySpec, nil, "test")
 	require.NoError(t, err)
 
 	assert.Regexp(t, "(?s)name: sancho-deployment.*name: sancho-pvc", f.k8s.DeletedYaml) // pvc comes after deployment