Repository: tilt-dev/tilt
Pull request #5139 (2021-11-05 14:49:37)

api: fix race during FileWatch cancellation
It's possible for the monitor to get cancelled while in the midst
of reporting an event, which will cause the API operations to fail.

We don't want to treat these as fatal errors: since the monitor
was cancelled, the failed update was stale anyway and we should
just silently ignore it.

milas: Yeah, I think the pattern where we update internal state and then queue a reconcile which has built-in retry semantics is much more robust.

FYI I had to apply a similar change in a2771e27e to the `Cmd` reconciler - in practice, it wasn't an issue since it was on the global context, which would only be canceled during shutdown, but it was causing tests to fail since the context gets canceled on test cleanup but a goroutine might still try to do an update after that 🙈 

Patch:
diff --git a/internal/container/container.go b/internal/container/container.go
index 2a7d3b3ef..2876359d6 100644
--- a/internal/container/container.go
+++ b/internal/container/container.go
@@ -2,7 +2,6 @@ package container
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/docker/distribution/reference"
 	"github.com/pkg/errors"
@@ -20,14 +19,6 @@ func (id ID) ShortStr() string {
 	return string(id)
 }
 
-func ShortStrs(ids []ID) string {
-	shortStrs := make([]string, len(ids))
-	for i, id := range ids {
-		shortStrs[i] = id.ShortStr()
-	}
-	return strings.Join(shortStrs, ", ")
-}
-
 func (n Name) String() string { return string(n) }
 
 func ParseNamed(s string) (reference.Named, error) {
diff --git a/internal/containerupdate/docker_container_updater.go b/internal/containerupdate/docker_container_updater.go
index ae44d1b53..1080604b9 100644
--- a/internal/containerupdate/docker_container_updater.go
+++ b/internal/containerupdate/docker_container_updater.go
@@ -62,12 +62,12 @@ func (cu *DockerUpdater) UpdateContainer(ctx context.Context, cInfo liveupdates.
 	}
 
 	if hotReload {
-		l.Debugf("Hot reload on, skipping container restart: %s", cInfo.ContainerID.ShortStr())
+		l.Debugf("Hot reload on, skipping container restart: %s", cInfo.DisplayName())
 		return nil
 	}
 
 	// Restart container so that entrypoint restarts with the updated files etc.
-	l.Debugf("Restarting container: %s", cInfo.ContainerID.ShortStr())
+	l.Debugf("Restarting container: %s", cInfo.DisplayName())
 	err = cu.dCli.ContainerRestartNoWait(ctx, cInfo.ContainerID.String())
 	if err != nil {
 		return errors.Wrap(err, "ContainerRestart")
diff --git a/internal/controllers/core/cmd/controller.go b/internal/controllers/core/cmd/controller.go
index 61824d075..163d12e72 100644
--- a/internal/controllers/core/cmd/controller.go
+++ b/internal/controllers/core/cmd/controller.go
@@ -473,7 +473,11 @@ func (c *Controller) updateStatus(name types.NamespacedName, update func(status
 
 	err = c.client.Status().Update(c.globalCtx, &cmd)
 	if err != nil && !apierrors.IsNotFound(err) {
-		c.st.Dispatch(store.NewErrorAction(fmt.Errorf("syncing to apiserver: %v", err)))
+		if c.globalCtx.Err() == nil {
+			// if the global context has been canceled, the controller is being torn down,
+			// so don't propagate a store error
+			c.st.Dispatch(store.NewErrorAction(fmt.Errorf("syncing to apiserver: %v", err)))
+		}
 		return
 	}
 
diff --git a/internal/controllers/core/filewatch/controller.go b/internal/controllers/core/filewatch/controller.go
index 6ff9ea660..15de61ffd 100644
--- a/internal/controllers/core/filewatch/controller.go
+++ b/internal/controllers/core/filewatch/controller.go
@@ -224,13 +224,21 @@ func (c *Controller) dispatchFileChangesLoop(ctx context.Context, st store.RStor
 			}
 		case <-ctx.Done():
 			return
-
 		case fsEvents, ok := <-eventsCh:
 			if !ok {
 				return
 			}
 			if err := w.recordEvent(ctx, c.Client, st, fsEvents); err != nil {
-				st.Dispatch(store.NewErrorAction(err))
+				if ctx.Err() == nil {
+					// there's an unavoidable race here - the context might have
+					// been canceled while we were recording the event, which will
+					// cause a failure, so we just ignore _any_ errors in this case
+					// (even if it was a non-context related error, this watcher is
+					// being disposed of, so it's no longer relevant)
+					st.Dispatch(store.NewErrorAction(err))
+				} else {
+					logger.Get(ctx).Debugf("Ignored stale error for %q: %v", w.name, err)
+				}
 				return
 			}
 		}
diff --git a/internal/controllers/core/filewatch/fsevent/fake_watcher.go b/internal/controllers/core/filewatch/fsevent/fake_watcher.go
index 5b3a47d73..f5cd81b3c 100644
--- a/internal/controllers/core/filewatch/fsevent/fake_watcher.go
+++ b/internal/controllers/core/filewatch/fsevent/fake_watcher.go
@@ -3,6 +3,7 @@ package fsevent
 import (
 	"path/filepath"
 	"sync"
+	"sync/atomic"
 
 	"github.com/tilt-dev/tilt/internal/ospath"
 	"github.com/tilt-dev/tilt/internal/watch"
@@ -20,7 +21,10 @@ type FakeMultiWatcher struct {
 }
 
 func NewFakeMultiWatcher() *FakeMultiWatcher {
-	r := &FakeMultiWatcher{Events: make(chan watch.FileEvent, 20), Errors: make(chan error, 20)}
+	r := &FakeMultiWatcher{
+		Events: make(chan watch.FileEvent, 20),
+		Errors: make(chan error, 20),
+	}
 	go r.loop()
 	return r
 }
@@ -92,6 +96,8 @@ type FakeWatcher struct {
 	outboundCh chan watch.FileEvent
 	errorCh    chan error
 
+	eventCount uint64
+
 	paths  []string
 	ignore watch.PathMatcher
 }
@@ -141,6 +147,14 @@ func (w *FakeWatcher) Events() chan watch.FileEvent {
 	return w.outboundCh
 }
 
+func (w *FakeWatcher) TotalEventCount() uint64 {
+	return atomic.LoadUint64(&w.eventCount)
+}
+
+func (w *FakeWatcher) QueuedCount() int {
+	return len(w.outboundCh)
+}
+
 func (w *FakeWatcher) loop() {
 	var q []watch.FileEvent
 	for {
@@ -154,6 +168,7 @@ func (w *FakeWatcher) loop() {
 		} else {
 			e := q[0]
 			w.outboundCh <- e
+			atomic.AddUint64(&w.eventCount, 1)
 			q = q[1:]
 		}
 	}
diff --git a/internal/controllers/core/liveupdate/reconciler.go b/internal/controllers/core/liveupdate/reconciler.go
index 847216ae8..c17952e14 100644
--- a/internal/controllers/core/liveupdate/reconciler.go
+++ b/internal/controllers/core/liveupdate/reconciler.go
@@ -819,7 +819,7 @@ func (r *Reconciler) applyInternal(
 	cu := r.containerUpdater(input)
 	l := logger.Get(ctx)
 	containers := input.Containers
-	cIDStr := container.ShortStrs(liveupdates.IDsForContainers(containers))
+	names := liveupdates.ContainerDisplayNames(containers)
 	suffix := ""
 	if len(containers) != 1 {
 		suffix = "(s)"
@@ -848,14 +848,14 @@ func (r *Reconciler) applyInternal(
 	}
 
 	if len(toRemove) > 0 {
-		l.Infof("Will delete %d file(s) from container%s: %s", len(toRemove), suffix, cIDStr)
+		l.Infof("Will delete %d file(s) from container%s: %s", len(toRemove), suffix, names)
 		for _, pm := range toRemove {
 			l.Infof("- '%s' (matched local path: '%s')", pm.ContainerPath, pm.LocalPath)
 		}
 	}
 
 	if len(toArchive) > 0 {
-		l.Infof("Will copy %d file(s) to container%s: %s", len(toArchive), suffix, cIDStr)
+		l.Infof("Will copy %d file(s) to container%s: %s", len(toArchive), suffix, names)
 		for _, pm := range toArchive {
 			l.Infof("- %s", pm.PrettyStr())
 		}
@@ -885,7 +885,7 @@ func (r *Reconciler) applyInternal(
 				// Keep running updates -- we want all containers to have the same files on them
 				// even if the Runs don't succeed
 				logger.Get(ctx).Infof("  → Failed to update container %s: run step %q failed with exit code: %d",
-					cInfo.ContainerID.ShortStr(), runFail.Cmd.String(), runFail.ExitCode)
+					cInfo.DisplayName(), runFail.Cmd.String(), runFail.ExitCode)
 				cStatus.LastExecError = err.Error()
 				lastExecErrorStatus = &cStatus
 			} else {
@@ -898,7 +898,7 @@ func (r *Reconciler) applyInternal(
 				return result
 			}
 		} else {
-			logger.Get(ctx).Infof("  → Container %s updated!", cInfo.ContainerID.ShortStr())
+			logger.Get(ctx).Infof("  → Container %s updated!", cInfo.DisplayName())
 			if lastExecErrorStatus != nil {
 				// This build succeeded, but previously at least one failed due to user error.
 				// We may have inconsistent state--bail, and fall back to full build.
diff --git a/internal/controllers/fake/client.go b/internal/controllers/fake/client.go
index ba101f7dd..82c128a18 100644
--- a/internal/controllers/fake/client.go
+++ b/internal/controllers/fake/client.go
@@ -18,6 +18,12 @@ type fakeTiltClient struct {
 //
 // We simulate this behavior to catch this class of bug.
 func (f fakeTiltClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
+	if ctxErr := ctx.Err(); ctxErr != nil {
+		// controller-runtime fake ignores context; check it here to allow controllers to test
+		// handling of cancellation related errors
+		return ctxErr
+	}
+
 	err := f.Client.Update(ctx, obj, opts...)
 	if err != nil {
 		return err
@@ -39,6 +45,12 @@ type fakeStatusWriter struct {
 }
 
 func (f fakeStatusWriter) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
+	if ctxErr := ctx.Err(); ctxErr != nil {
+		// controller-runtime fake ignores context; check it here to allow controllers to test
+		// handling of cancellation related errors
+		return ctxErr
+	}
+
 	err := f.StatusWriter.Update(ctx, obj, opts...)
 	if err != nil {
 		return err
diff --git a/internal/engine/buildcontrol/live_update_build_and_deployer.go b/internal/engine/buildcontrol/live_update_build_and_deployer.go
index 2cf76d6e1..fa751e643 100644
--- a/internal/engine/buildcontrol/live_update_build_and_deployer.go
+++ b/internal/engine/buildcontrol/live_update_build_and_deployer.go
@@ -9,7 +9,6 @@ import (
 
 	"github.com/tilt-dev/tilt/internal/analytics"
 	"github.com/tilt-dev/tilt/internal/build"
-	"github.com/tilt-dev/tilt/internal/container"
 	"github.com/tilt-dev/tilt/internal/controllers/apis/liveupdate"
 	ctrlliveupdate "github.com/tilt-dev/tilt/internal/controllers/core/liveupdate"
 	"github.com/tilt-dev/tilt/internal/ospath"
@@ -108,12 +107,12 @@ func (lubad *LiveUpdateBuildAndDeployer) buildAndDeploy(ctx context.Context, ps
 	}()
 
 	containers := info.Containers
-	cIDStr := container.ShortStrs(liveupdates.IDsForContainers(containers))
+	names := liveupdates.ContainerDisplayNames(containers)
 	suffix := ""
 	if len(containers) != 1 {
 		suffix = "(s)"
 	}
-	ps.StartBuildStep(ctx, "Updating container%s: %s", suffix, cIDStr)
+	ps.StartBuildStep(ctx, "Updating container%s: %s", suffix, names)
 
 	status, err := lubad.luReconciler.ForceApply(
 		ctx,
diff --git a/internal/k8s/target.go b/internal/k8s/target.go
index 727055649..d5108737e 100644
--- a/internal/k8s/target.go
+++ b/internal/k8s/target.go
@@ -90,7 +90,7 @@ func NewTarget(
 		Name:                name,
 		PodReadinessMode:    podReadinessMode,
 		Links:               links,
-	}.WithDependencyIDs(dependencyIDs, model.ToLiveUpdateOnlyMap(imageTargets)).
+	}.WithImageDependencies(dependencyIDs, model.ToLiveUpdateOnlyMap(imageTargets)).
 		WithRefInjectCounts(refInjectCounts), nil
 }
 
diff --git a/internal/store/liveupdates/containers.go b/internal/store/liveupdates/containers.go
index b8a2c2439..42dc223a9 100644
--- a/internal/store/liveupdates/containers.go
+++ b/internal/store/liveupdates/containers.go
@@ -119,6 +119,17 @@ func (c Container) Empty() bool {
 	return c == Container{}
 }
 
+func (c Container) DisplayName() string {
+	if c.PodID == "" {
+		if c.ContainerName == "" {
+			return c.ContainerID.ShortStr()
+		}
+		return c.ContainerName.String()
+	}
+
+	return fmt.Sprintf("%s/%s", c.PodID, c.ContainerName)
+}
+
 func IDsForContainers(infos []Container) []container.ID {
 	ids := make([]container.ID, len(infos))
 	for i, info := range infos {
diff --git a/internal/store/liveupdates/string.go b/internal/store/liveupdates/string.go
new file mode 100644
index 000000000..eebc65521
--- /dev/null
+++ b/internal/store/liveupdates/string.go
@@ -0,0 +1,9 @@
+package liveupdates
+
+func ContainerDisplayNames(containers []Container) []string {
+	names := make([]string, 0, len(containers))
+	for _, c := range containers {
+		names = append(names, c.DisplayName())
+	}
+	return names
+}
diff --git a/pkg/model/k8s_target.go b/pkg/model/k8s_target.go
index 79c826a68..a4368ad08 100644
--- a/pkg/model/k8s_target.go
+++ b/pkg/model/k8s_target.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"reflect"
 
+	"github.com/tilt-dev/tilt/internal/sliceutils"
 	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
 )
 
@@ -44,7 +45,18 @@ type K8sTarget struct {
 	// in addition to any port forwards/LB endpoints)
 	Links []Link
 
-	depIDs []TargetID
+	imageDeps []TargetID
+
+	// pathDependencies are files required by this target.
+	//
+	// For Tiltfile-based, YAML-driven (i.e. `k8s_yaml()`) resources, this is
+	// NOT used because it's not sufficient to reload the YAML and re-deploy;
+	// there is a lot of post-Tiltfile-load logic for resource assembly, image
+	// locator injection, etc. As a result, these resources have their YAML
+	// files registered as "config files", which cause the Tiltfile to be
+	// re-evaluated.
+	pathDependencies []string
+	localRepos       []LocalGitRepo
 }
 
 func NewK8sTargetForTesting(yaml string) K8sTarget {
@@ -57,7 +69,7 @@ func NewK8sTargetForTesting(yaml string) K8sTarget {
 func (k8s K8sTarget) Empty() bool { return reflect.DeepEqual(k8s, K8sTarget{}) }
 
 func (k8s K8sTarget) DependencyIDs() []TargetID {
-	return append([]TargetID{}, k8s.depIDs...)
+	return append([]TargetID{}, k8s.imageDeps...)
 }
 
 func (k8s K8sTarget) RefInjectCounts() map[string]int {
@@ -83,6 +95,29 @@ func (k8s K8sTarget) ID() TargetID {
 	}
 }
 
+// LocalRepos is part of the WatchableTarget interface.
+func (k8s K8sTarget) LocalRepos() []LocalGitRepo {
+	return k8s.localRepos
+}
+
+// Dockerignores is part of the WatchableTarget interface.
+func (k8s K8sTarget) Dockerignores() []Dockerignore {
+	return nil
+}
+
+// IgnoredLocalDirectories is part of the WatchableTarget interface.
+func (k8s K8sTarget) IgnoredLocalDirectories() []string {
+	return nil
+}
+
+// Dependencies are files required by this target.
+//
+// Part of the WatchableTarget interface.
+func (k8s K8sTarget) Dependencies() []string {
+	// sorting/de-duping guaranteed by setter
+	return k8s.pathDependencies
+}
+
 // Track which objects this target depends on inside the manifest.
 //
 // We're disentangling ImageTarget and live updates -
@@ -91,14 +126,14 @@ func (k8s K8sTarget) ID() TargetID {
 //
 // KubernetesApplySpec only depends on ImageTargets with an Image build.
 //
-// The depIDs field depends on ImageTargets that have image builds OR have live
+// The imageDeps field depends on ImageTargets that have image builds OR have live
 // updates.
 //
 // ids: a list of the images we directly depend on.
 // isLiveUpdateOnly: a map of images that are live-update-only
-func (k8s K8sTarget) WithDependencyIDs(ids []TargetID, isLiveUpdateOnly map[TargetID]bool) K8sTarget {
+func (k8s K8sTarget) WithImageDependencies(ids []TargetID, isLiveUpdateOnly map[TargetID]bool) K8sTarget {
 	ids = DedupeTargetIDs(ids)
-	k8s.depIDs = ids
+	k8s.imageDeps = ids
 
 	k8s.ImageMaps = make([]string, 0, len(ids))
 
@@ -115,6 +150,13 @@ func (k8s K8sTarget) WithDependencyIDs(ids []TargetID, isLiveUpdateOnly map[Targ
 	return k8s
 }
 
+// WithPathDependencies registers paths that this K8sTarget depends on.
+func (k8s K8sTarget) WithPathDependencies(paths []string, localRepos []LocalGitRepo) K8sTarget {
+	k8s.pathDependencies = sliceutils.DedupedAndSorted(paths)
+	k8s.localRepos = localRepos
+	return k8s
+}
+
 func (k8s K8sTarget) WithRefInjectCounts(ric map[string]int) K8sTarget {
 	k8s.refInjectCounts = ric
 	return k8s


Test patch:
diff --git a/internal/controllers/core/filewatch/controller_test.go b/internal/controllers/core/filewatch/controller_test.go
index 1cfb4d91c..6a8cc1322 100644
--- a/internal/controllers/core/filewatch/controller_test.go
+++ b/internal/controllers/core/filewatch/controller_test.go
@@ -6,6 +6,7 @@ import (
 	"reflect"
 	"runtime"
 	"strconv"
+	"strings"
 	"testing"
 	"time"
 
@@ -348,3 +349,42 @@ func TestController_Disable_Ignores_File_Changes(t *testing.T) {
 	f.MustGet(key, &fwAfterDisable)
 	require.Equal(t, 0, len(fwAfterDisable.Status.FileEvents))
 }
+
+func TestController_IgnoreErrorsOnCancel(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test in short mode")
+	}
+
+	f := newFixture(t)
+	key, _ := f.CreateSimpleFileWatch()
+
+	tw := f.controller.targetWatches[key]
+	require.NotNil(t, tw, "Watcher never created")
+
+	// HACK(milas): this is seriously misusing the internals of this controller to
+	// 	force the synchronization so we can cancel the context at the right time
+	tw.mu.Lock()
+
+	watcher := tw.notify.(*fsevent.FakeWatcher)
+	require.Zero(t, watcher.TotalEventCount(), "No events should have been seen yet")
+
+	f.ChangeFile("a", "1")
+
+	require.Eventually(t, func() bool {
+		return watcher.TotalEventCount() == 1
+	}, time.Second, 20*time.Millisecond, "Event never seen")
+
+	require.Eventually(t, func() bool {
+		return watcher.QueuedCount() == 0
+	}, time.Second, 20*time.Millisecond, "Event never consumed")
+
+	// at this point, the watcher is blocked trying to report the event, so
+	// we'll cancel the context on it, simulating a race, to force a failure
+	tw.cancel()
+	tw.mu.Unlock()
+
+	require.Eventually(t, func() bool {
+		return strings.Contains(f.Stdout(),
+			`Ignored stale error for "TestController_IgnoreErrorsOnCancel/test-file-watch": apiserver update status error: context canceled`)
+	}, time.Second, 10*time.Millisecond, "Error ignored log message never seen")
+}
diff --git a/internal/engine/buildcontrol/image_build_and_deployer_test.go b/internal/engine/buildcontrol/image_build_and_deployer_test.go
index 7a4558ef6..241ec98ad 100644
--- a/internal/engine/buildcontrol/image_build_and_deployer_test.go
+++ b/internal/engine/buildcontrol/image_build_and_deployer_test.go
@@ -45,7 +45,7 @@ func TestDeployTwinImages(t *testing.T) {
 
 	sancho := NewSanchoDockerBuildManifest(f)
 	newK8sTarget := k8s.MustTarget("sancho", yaml.ConcatYAML(SanchoYAML, SanchoTwinYAML)).
-		WithDependencyIDs(sancho.K8sTarget().DependencyIDs(), nil)
+		WithImageDependencies(sancho.K8sTarget().DependencyIDs(), nil)
 	manifest := sancho.WithDeployTarget(newK8sTarget)
 	result, err := f.BuildAndDeploy(BuildTargets(manifest), store.BuildStateSet{})
 	if err != nil {
@@ -129,7 +129,7 @@ func TestDeployPodWithMultipleImages(t *testing.T) {
 	iTarget1 := NewSanchoDockerBuildImageTarget(f)
 	iTarget2 := NewSanchoSidecarDockerBuildImageTarget(f)
 	kTarget := k8s.MustTarget("sancho", testyaml.SanchoSidecarYAML).
-		WithDependencyIDs([]model.TargetID{iTarget1.ID(), iTarget2.ID()}, nil)
+		WithImageDependencies([]model.TargetID{iTarget1.ID(), iTarget2.ID()}, nil)
 	targets := []model.TargetSpec{iTarget1, iTarget2, kTarget}
 
 	result, err := f.BuildAndDeploy(targets, store.BuildStateSet{})
@@ -160,7 +160,7 @@ func TestDeployPodWithMultipleLiveUpdateImages(t *testing.T) {
 	iTarget2 := NewSanchoSidecarLiveUpdateImageTarget(f)
 
 	kTarget := k8s.MustTarget("sancho", testyaml.SanchoSidecarYAML).
-		WithDependencyIDs([]model.TargetID{iTarget1.ID(), iTarget2.ID()}, nil)
+		WithImageDependencies([]model.TargetID{iTarget1.ID(), iTarget2.ID()}, nil)
 	targets := []model.TargetSpec{iTarget1, iTarget2, kTarget}
 
 	result, err := f.BuildAndDeploy(targets, store.BuildStateSet{})
@@ -234,7 +234,7 @@ func TestStatefulSetPodManagementPolicy(t *testing.T) {
 	_, err = f.BuildAndDeploy(
 		[]model.TargetSpec{
 			iTarget,
-			kTarget.WithDependencyIDs([]model.TargetID{iTarget.ID()}, nil),
+			kTarget.WithImageDependencies([]model.TargetID{iTarget.ID()}, nil),
 		},
 		store.BuildStateSet{})
 	if err != nil {
@@ -826,7 +826,7 @@ func TestInjectOverrideCommandsMultipleImages(t *testing.T) {
 	iTarget1 := NewSanchoDockerBuildImageTarget(f).WithOverrideCommand(cmd1)
 	iTarget2 := NewSanchoSidecarDockerBuildImageTarget(f).WithOverrideCommand(cmd2)
 	kTarget := k8s.MustTarget("sancho", testyaml.SanchoSidecarYAML).
-		WithDependencyIDs([]model.TargetID{iTarget1.ID(), iTarget2.ID()}, nil)
+		WithImageDependencies([]model.TargetID{iTarget1.ID(), iTarget2.ID()}, nil)
 	targets := []model.TargetSpec{iTarget1, iTarget2, kTarget}
 
 	_, err := f.BuildAndDeploy(targets, store.BuildStateSet{})
diff --git a/internal/engine/buildcontrol/testdata_test.go b/internal/engine/buildcontrol/testdata_test.go
index c5df9a441..c6b0e6ef8 100644
--- a/internal/engine/buildcontrol/testdata_test.go
+++ b/internal/engine/buildcontrol/testdata_test.go
@@ -164,7 +164,7 @@ func NewSanchoLiveUpdateMultiStageManifest(fixture Fixture) model.Manifest {
 		WithDependencyIDs([]model.TargetID{baseImage.ID()})
 
 	kTarget := k8s.MustTarget("sancho", SanchoYAML).
-		WithDependencyIDs([]model.TargetID{srcImage.ID()}, nil)
+		WithImageDependencies([]model.TargetID{srcImage.ID()}, nil)
 
 	return model.Manifest{Name: "sancho"}.
 		WithImageTargets([]model.ImageTarget{baseImage, srcImage}).
diff --git a/internal/engine/buildcontroller_test.go b/internal/engine/buildcontroller_test.go
index 721e0b73c..3a848f6e8 100644
--- a/internal/engine/buildcontroller_test.go
+++ b/internal/engine/buildcontroller_test.go
@@ -17,6 +17,7 @@ import (
 	"github.com/tilt-dev/tilt/internal/container"
 	"github.com/tilt-dev/tilt/internal/engine/buildcontrol"
 	"github.com/tilt-dev/tilt/internal/hud/server"
+	"github.com/tilt-dev/tilt/internal/k8s"
 	"github.com/tilt-dev/tilt/internal/k8s/testyaml"
 	"github.com/tilt-dev/tilt/internal/store"
 	"github.com/tilt-dev/tilt/internal/store/liveupdates"
@@ -1660,6 +1661,35 @@ func TestDisablingCancelsBuild(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestBuildControllerK8sFileDependencies(t *testing.T) {
+	f := newTestFixture(t)
+	defer f.TearDown()
+
+	kt := k8s.MustTarget("fe", testyaml.SanchoYAML).
+		WithPathDependencies(
+			[]string{f.JoinPath("k8s-dep")},
+			[]model.LocalGitRepo{
+				{LocalPath: f.JoinPath("k8s-dep")},
+			})
+	m := model.Manifest{Name: "fe"}.WithDeployTarget(kt)
+
+	f.Start([]model.Manifest{m})
+
+	call := f.nextCall()
+	assert.Empty(t, call.k8sState().FilesChanged())
+
+	// path dependency is on ./k8s-dep/** with a local repo of ./k8s-dep/.git/** (ignored)
+	f.fsWatcher.Events <- watch.NewFileEvent(f.JoinPath("k8s-dep", "file"))
+	f.fsWatcher.Events <- watch.NewFileEvent(f.JoinPath("k8s-dep", ".git", "file"))
+
+	call = f.nextCall()
+	assert.Equal(t, []string{f.JoinPath("k8s-dep", "file")}, call.k8sState().FilesChanged())
+
+	err := f.Stop()
+	assert.NoError(t, err)
+	f.assertAllBuildsConsumed()
+}
+
 func (f *testFixture) waitUntilManifestBuilding(name model.ManifestName) {
 	f.t.Helper()
 	msg := fmt.Sprintf("manifest %q is building", name)
diff --git a/internal/engine/testdata_test.go b/internal/engine/testdata_test.go
index b00b0e6fe..17b579ea9 100644
--- a/internal/engine/testdata_test.go
+++ b/internal/engine/testdata_test.go
@@ -164,7 +164,7 @@ func NewSanchoLiveUpdateMultiStageManifest(fixture Fixture) model.Manifest {
 		WithDependencyIDs([]model.TargetID{baseImage.ID()})
 
 	kTarget := k8s.MustTarget("sancho", SanchoYAML).
-		WithDependencyIDs([]model.TargetID{srcImage.ID()}, nil)
+		WithImageDependencies([]model.TargetID{srcImage.ID()}, nil)
 
 	return model.Manifest{Name: "sancho"}.
 		WithImageTargets([]model.ImageTarget{baseImage, srcImage}).
diff --git a/internal/engine/upper_test.go b/internal/engine/upper_test.go
index 87887eae1..84ed4b9d9 100644
--- a/internal/engine/upper_test.go
+++ b/internal/engine/upper_test.go
@@ -3958,6 +3958,7 @@ type testFixture struct {
 	onchangeCh            chan bool
 	sessionController     *session.Controller
 	localServerController *local.ServerController
+	execer                *localexec.FakeExecer
 }
 
 type fixtureOptions struct {
@@ -4011,7 +4012,7 @@ func newTestFixture(t *testing.T, options ...fixtureOptions) *testFixture {
 	k8sContextExt := k8scontext.NewPlugin("fake-context", env)
 	versionExt := version.NewPlugin(model.TiltBuild{Version: "0.5.0"})
 	configExt := config.NewPlugin("up")
-	execer := localexec.NewProcessExecer(localexec.EmptyEnv())
+	execer := localexec.NewFakeExecer(t)
 	realTFL := tiltfile.ProvideTiltfileLoader(ta, b.kClient, k8sContextExt, versionExt, configExt, fakeDcc, "localhost", execer, feature.MainDefaults, env)
 	tfl := tiltfile.NewFakeTiltfileLoader()
 	buildSource := ctrltiltfile.NewBuildSource()
@@ -4120,6 +4121,7 @@ func newTestFixture(t *testing.T, options ...fixtureOptions) *testFixture {
 		sessionController:     sessionController,
 		localServerController: lsc,
 		engineMode:            engineMode,
+		execer:                execer,
 	}
 
 	ret.disableEnvAnalyticsOpt()
diff --git a/internal/testutils/manifestbuilder/assemble.go b/internal/testutils/manifestbuilder/assemble.go
index 2ec270fb9..4805361a1 100644
--- a/internal/testutils/manifestbuilder/assemble.go
+++ b/internal/testutils/manifestbuilder/assemble.go
@@ -23,7 +23,7 @@ func assembleK8s(m model.Manifest, k model.K8sTarget, iTargets ...model.ImageTar
 		}
 		ids = append(ids, iTarget.ID())
 	}
-	k = k.WithDependencyIDs(ids, model.ToLiveUpdateOnlyMap(iTargets))
+	k = k.WithImageDependencies(ids, model.ToLiveUpdateOnlyMap(iTargets))
 	return m.
 		WithImageTargets(iTargets).
 		WithDeployTarget(k)
diff --git a/pkg/model/target_test.go b/pkg/model/target_test.go
index be91de9ca..546e6e49a 100644
--- a/pkg/model/target_test.go
+++ b/pkg/model/target_test.go
@@ -114,5 +114,5 @@ func newK8sTarget(name string, deps ...string) K8sTarget {
 	for i, dep := range deps {
 		depIDs[i] = ImageID(container.MustParseSelector(dep))
 	}
-	return K8sTarget{Name: TargetName(name)}.WithDependencyIDs(depIDs, nil)
+	return K8sTarget{Name: TargetName(name)}.WithImageDependencies(depIDs, nil)
 }