buildcontrol: restart should do a full delete of the resource

internal/controllers/core/dockercomposeservice/reconciler.go

@@ -26,6 +26,7 @@ import (
 	"github.com/tilt-dev/tilt/internal/controllers/indexer"
 	"github.com/tilt-dev/tilt/internal/docker"
 	"github.com/tilt-dev/tilt/internal/dockercompose"
+	"github.com/tilt-dev/tilt/internal/filteredwriter"
 	"github.com/tilt-dev/tilt/internal/store"
 	"github.com/tilt-dev/tilt/internal/store/dockercomposeservices"
 	"github.com/tilt-dev/tilt/pkg/apis"
@@ -306,6 +307,28 @@ func (r *Reconciler) recordSpecAndDisableStatus(
 	result.Status = *update
 }
 
+// A helper that deletes the Docker Compose service, even if they haven't been applied yet.
+//
+// Primarily intended so that the build controller can do force restarts.
+func (r *Reconciler) ForceDelete(
+	ctx context.Context,
+	nn types.NamespacedName,
+	spec v1alpha1.DockerComposeServiceSpec,
+	reason string) error {
+	out := logger.Get(ctx).Writer(logger.InfoLvl)
+	out = filteredwriter.New(out, func(s string) bool {
+		// https://app.shortcut.com/windmill/story/13147/docker-compose-down-messages-for-disabled-resources-may-be-confusing
+		return strings.HasPrefix(s, "Going to remove")
+	})
+	err := r.dcc.Rm(ctx, []v1alpha1.DockerComposeServiceSpec{spec}, out, out)
+	if err != nil {
+		logger.Get(ctx).Errorf("Error %s: %v", reason, err)
+	}
+	r.clearResult(nn)
+	r.requeuer.Add(nn)
+	return nil
+}
+
 // Apply the DockerCompose service spec, unconditionally,
 // and requeue the reconciler so that it updates the apiserver.
 //

internal/controllers/core/dockercomposeservice/reconciler_test.go

@@ -188,6 +188,30 @@ func TestContainerEvent(t *testing.T) {
 		s.ManifestTargets["fe"].State.DCRuntimeState().ContainerState.Status)
 }
 
+func TestForceDelete(t *testing.T) {
+	f := newFixture(t)
+	nn := types.NamespacedName{Name: "fe"}
+	obj := v1alpha1.DockerComposeService{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "fe",
+			Annotations: map[string]string{
+				v1alpha1.AnnotationManifest: "fe",
+			},
+		},
+		Spec: v1alpha1.DockerComposeServiceSpec{
+			Service: "fe",
+			Project: v1alpha1.DockerComposeProject{
+				YAML: "fake-yaml",
+			},
+		},
+	}
+	f.Create(&obj)
+
+	err := f.r.ForceDelete(f.Context(), nn, obj.Spec, "testing")
+	assert.Nil(f.T(), err)
+	assert.Equal(f.T(), f.dcc.RmCalls()[0].Specs[0].Service, "fe")
+}
+
 type fixture struct {
 	*fake.ControllerFixture
 	r   *Reconciler

internal/controllers/core/kubernetesapply/reconciler.go

@@ -824,12 +824,6 @@ func (r *Reconciler) garbageCollect(nn types.NamespacedName, isDeleting bool) de
 //
 // Namespaces are not deleted by default. Similar to `tilt down`, deleting namespaces
 // is likely to be more destructive than most users want from this operation.
-//
-// TODO(nick): Delete operations aren't currently reflected in the KubernetesApplyStatus
-// in any meaningful way. ForceDelete() deletes our internal bookkeeping, but
-// doesn't otherwise change the KubernetesApplyStatus. We should probably change
-// this, but that's lower priority than a bigger level-based refactor
-// and getting this to be pure API objects.
 func (r *Reconciler) ForceDelete(ctx context.Context, nn types.NamespacedName,
 	spec v1alpha1.KubernetesApplySpec,
 	cluster *v1alpha1.Cluster,

internal/engine/buildcontrol/docker_compose_build_and_deployer.go

@@ -116,6 +116,8 @@ func (bd *DockerComposeBuildAndDeployer) BuildAndDeploy(ctx context.Context, st
 		})
 	}()
 
+	dcTarget := plan.dockerComposeTarget
+	dcTargetNN := types.NamespacedName{Name: dcTarget.ID().Name.String()}
 	ctx = docker.WithOrchestrator(ctx, model.OrchestratorDC)
 
 	iTargets := plan.tiltManagedImageTargets
@@ -128,6 +130,11 @@ func (bd *DockerComposeBuildAndDeployer) BuildAndDeploy(ctx context.Context, st
 	// a Tilt-built image OR might build+launch a Docker Compose-managed image)
 	numStages := q.CountBuilds() + 1
 
+	hasDeleteStep := currentState.FullBuildTriggered()
+	if hasDeleteStep {
+		numStages++
+	}
+
 	reused := q.ReusedResults()
 	hasReusedStep := len(reused) > 0
 	if hasReusedStep {
@@ -137,6 +144,15 @@ func (bd *DockerComposeBuildAndDeployer) BuildAndDeploy(ctx context.Context, st
 	ps := build.NewPipelineState(ctx, numStages, bd.clock)
 	defer func() { ps.End(ctx, err) }()
 
+	if hasDeleteStep {
+		ps.StartPipelineStep(ctx, "Force update")
+		err = bd.dcsr.ForceDelete(ps.AttachLogger(ctx), dcTargetNN, dcTarget.Spec, "force update")
+		if err != nil {
+			return store.BuildResultSet{}, WrapDontFallBackError(err)
+		}
+		ps.EndPipelineStep(ctx)
+	}
+
 	if hasReusedStep {
 		ps.StartPipelineStep(ctx, "Loading cached images")
 		for _, result := range reused {
@@ -194,8 +210,6 @@ func (bd *DockerComposeBuildAndDeployer) BuildAndDeploy(ctx context.Context, st
 	}
 	ps.StartPipelineStep(ctx, stepName)
 
-	dcTarget := plan.dockerComposeTarget
-	dcTargetNN := types.NamespacedName{Name: dcTarget.ID().Name.String()}
 	status := bd.dcsr.ForceApply(ctx, dcTargetNN, dcTarget.Spec, imageMapSet, dcManagedBuild)
 	ps.EndPipelineStep(ctx)
 	if status.ApplyError != "" {

internal/engine/buildcontrol/docker_compose_build_and_deployer_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/jonboulle/clockwork"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ktypes "k8s.io/apimachinery/pkg/types"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -168,6 +169,22 @@ ENTRYPOINT /go/bin/sancho
 	testutils.AssertFileInTar(t, tar.NewReader(f.dCli.BuildContext), expected)
 }
 
+func TestForceUpdateDC(t *testing.T) {
+	f := newDCBDFixture(t)
+
+	m := manifestbuilder.New(f, "fe").WithDockerCompose().Build()
+	iTargetID1 := m.ImageTargets[0].ID()
+	stateSet := store.BuildStateSet{
+		iTargetID1: store.BuildState{FullBuildTriggered: true},
+	}
+
+	_, err := f.BuildAndDeploy(BuildTargets(m), stateSet)
+	require.NoError(t, err)
+
+	// A force rebuild should delete the old resources.
+	assert.Equal(t, 1, len(f.dcCli.RmCalls()))
+}
+
 type dcbdFixture struct {
 	*tempdir.TempDirFixture
 	ctx        context.Context

internal/engine/buildcontrol/image_build_and_deployer_test.go

@@ -61,7 +61,7 @@ func TestDeployTwinImages(t *testing.T) {
 		"Expected image to update twice in YAML: %s", f.k8s.Yaml)
 }
 
-func TestForceUpdate(t *testing.T) {
+func TestForceUpdateK8s(t *testing.T) {
 	f := newIBDFixture(t, clusterid.ProductGKE)
 
 	m := NewSanchoDockerBuildManifest(f)