dockercompose: fix spurious full rebuild instead of Live Update (#5242)

Occasionally, when using Live Update with a Docker Compose project,
a file change would trigger a full rebuild instead of Live Update
despite matching a sync path.

The root cause here is that the Docker Compose target watched files
in addition to the image target, so there was a race: if the Docker
Compose target saw the file but the image target hadn't yet, the
Docker Compose builder would take over and do a full rebuild. This
has probably been exacerbated by the API transition as there's now
greater potential for timing-related issues like this to manifest.

The reason the Docker Compose target watches for files is because
in some cases, there IS no image target for a Docker Compose service.
If there's no `docker_build` or `custom_build` configured, and the
Docker Compose service has a `build:` section in its YAML, the Docker
Compose build and deployer watched for file changes and then triggered
a build via passing the `--build` flag to the `docker-compose up`
call.

To address this, the Docker Compose target now behaves more like a
Kubernetes target. It does not watch for any files. Instead, image
targets are created for both services that will be managed by Tilt
(via `docker_build` or `custom_build`) as well as those that will
be built by Docker Compose. (NOTE: If a Docker Compose service has
no `build:` section and there's no corresponding `docker_build` or
`custom_build` call for it, there will be no target, as no image
builds will occur. This is commonly seen for infra deps that are
pulled from a registry.)

The image targets for Docker Compose managed builds have a new
`BuildDetails` type of `DockerComposeBuild` to go along with the
pre-existing `DockerBuild` and `CustomBuild` types. This ensures
that all file changes are tie to the image, so it will pass through
the `LiveUpdateBuildAndDeployer` first, and fallback to the
`DockerComposeBuildAndDeployer` if not Live Update-eligible. (This
is always the case for `DockerComposeBuild` targets currently!)
The `DockerComposeBuildAndDeployer` looks at the image targets and
either builds the set of Tilt-managed images for the service OR
delegates to `docker-compose up` by including the `--build` flag.

The `DockerComposeBuild` targets are created during Docker Compose
assembly during Tiltfile loading if no Tilt-managed builder exists.
This is kinda icky - it'd probably be nicer to create these as
stubs when `docker_compose()` is called and allow `docker_build`
and `custom_build` to override them, but that's a bigger yak shave
than practical for this fix.

internal/controllers/core/tiltfile/filewatch.go

@@ -32,7 +32,6 @@ type WatchableTarget interface {
 
 var _ WatchableTarget = model.ImageTarget{}
 var _ WatchableTarget = model.LocalTarget{}
-var _ WatchableTarget = model.DockerComposeTarget{}
 
 func specForTarget(t WatchableTarget, globalIgnores []model.Dockerignore) *v1alpha1.FileWatchSpec {
 	watchedPaths := append([]string(nil), t.Dependencies()...)

internal/controllers/core/tiltfile/filewatch_test.go

@@ -31,9 +31,11 @@ func TestFileWatch_basic(t *testing.T) {
 	f := newFWFixture(t)
 	defer f.TearDown()
 
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target)
+	target := model.LocalTarget{
+		Name: "foo",
+		Deps: []string{"."},
+	}
+	f.SetManifestLocalTarget(target)
 
 	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{WatchedPaths: []string{"."}})
 }
@@ -42,10 +44,11 @@ func TestFileWatch_localRepo(t *testing.T) {
 	f := newFWFixture(t)
 	defer f.TearDown()
 
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithBuildPath(".").
-		WithRepos([]model.LocalGitRepo{model.LocalGitRepo{LocalPath: "."}})
-	f.SetManifestDCTarget(target)
+	target := model.LocalTarget{
+		Name: "foo",
+		Deps: []string{"."},
+	}.WithRepos([]model.LocalGitRepo{{LocalPath: "."}})
+	f.SetManifestLocalTarget(target)
 
 	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{
 		WatchedPaths: []string{"."},
@@ -61,41 +64,27 @@ func TestFileWatch_disabledOnCIMode(t *testing.T) {
 
 	f.inputs.EngineMode = store.EngineModeCI
 
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target)
+	target := model.LocalTarget{
+		Name: "foo",
+		Deps: []string{"."},
+	}
+	f.SetManifestLocalTarget(target)
 	m := model.Manifest{Name: "foo"}.WithDeployTarget(target)
 	f.SetManifest(m)
 
 	actualSet := ToFileWatchObjects(f.inputs, make(disableSourceMap))
 	assert.Empty(t, actualSet)
 }
 
-func TestFileWatch_IgnoredLocalDirectories(t *testing.T) {
-	f := newFWFixture(t)
-	defer f.TearDown()
-
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithIgnoredLocalDirectories([]string{"bar"}).
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target)
-
-	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{
-		WatchedPaths: []string{"."},
-		Ignores: []v1alpha1.IgnoreDef{
-			{BasePath: "bar"},
-		},
-	})
-}
-
 func TestFileWatch_Dockerignore(t *testing.T) {
 	f := newFWFixture(t)
 	defer f.TearDown()
 
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithDockerignores([]model.Dockerignore{{LocalPath: ".", Patterns: []string{"bar"}}}).
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target)
+	target := model.LocalTarget{
+		Name: "foo",
+		Deps: []string{"."},
+	}.WithIgnores([]model.Dockerignore{{LocalPath: ".", Patterns: []string{"bar"}}})
+	f.SetManifestLocalTarget(target)
 
 	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{
 		WatchedPaths: []string{"."},
@@ -129,42 +118,6 @@ func TestFileWatch_IgnoreOutputsImageRefs(t *testing.T) {
 	})
 }
 
-func TestFileWatch_WatchesReappliedOnDockerComposeSyncChange(t *testing.T) {
-	f := newFWFixture(t)
-	defer f.TearDown()
-
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target.WithIgnoredLocalDirectories([]string{"bar"}))
-	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{
-		WatchedPaths: []string{"."},
-		Ignores: []v1alpha1.IgnoreDef{
-			{BasePath: "bar"},
-		},
-	})
-
-	f.SetManifestDCTarget(target)
-	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{WatchedPaths: []string{"."}})
-}
-
-func TestFileWatch_WatchesReappliedOnDockerIgnoreChange(t *testing.T) {
-	f := newFWFixture(t)
-	defer f.TearDown()
-
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target.WithDockerignores([]model.Dockerignore{{LocalPath: ".", Patterns: []string{"bar"}}}))
-	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{
-		WatchedPaths: []string{"."},
-		Ignores: []v1alpha1.IgnoreDef{
-			{BasePath: ".", Patterns: []string{"bar"}},
-		},
-	})
-
-	f.SetManifestDCTarget(target)
-	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{WatchedPaths: []string{"."}})
-}
-
 func TestFileWatch_ConfigFiles(t *testing.T) {
 	f := newFWFixture(t)
 	defer f.TearDown()
@@ -185,9 +138,11 @@ func TestFileWatch_IgnoreTiltIgnore(t *testing.T) {
 	f := newFWFixture(t)
 	defer f.TearDown()
 
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target)
+	target := model.LocalTarget{
+		Name: "foo",
+		Deps: []string{"."},
+	}
+	f.SetManifestLocalTarget(target)
 	f.SetTiltIgnoreContents("**/foo")
 	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{
 		WatchedPaths: []string{"."},
@@ -201,9 +156,11 @@ func TestFileWatch_IgnoreWatchSettings(t *testing.T) {
 	f := newFWFixture(t)
 	defer f.TearDown()
 
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target)
+	target := model.LocalTarget{
+		Name: "foo",
+		Deps: []string{"."},
+	}
+	f.SetManifestLocalTarget(target)
 
 	f.inputs.WatchSettings.Ignores = append(f.inputs.WatchSettings.Ignores, model.Dockerignore{
 		LocalPath: f.Path(),
@@ -222,9 +179,11 @@ func TestFileWatch_PickUpTiltIgnoreChanges(t *testing.T) {
 	f := newFWFixture(t)
 	defer f.TearDown()
 
-	target := model.DockerComposeTarget{Name: "foo"}.
-		WithBuildPath(".")
-	f.SetManifestDCTarget(target)
+	target := model.LocalTarget{
+		Name: "foo",
+		Deps: []string{"."},
+	}
+	f.SetManifestLocalTarget(target)
 	f.SetTiltIgnoreContents("**/foo")
 	f.RequireFileWatchSpecEqual(target.ID(), v1alpha1.FileWatchSpec{
 		WatchedPaths: []string{"."},
@@ -277,11 +236,11 @@ func newFWFixture(t *testing.T) *fwFixture {
 
 type fileWatchDiffer struct {
 	expected v1alpha1.FileWatchSpec
-	actual   *v1alpha1.FileWatchSpec
+	actual   v1alpha1.FileWatchSpec
 }
 
 func (f fileWatchDiffer) String() string {
-	return cmp.Diff(&f.expected, f.actual)
+	return cmp.Diff(f.expected, f.actual)
 }
 
 func (f *fwFixture) RequireFileWatchSpecEqual(targetID model.TargetID, spec v1alpha1.FileWatchSpec) {
@@ -290,11 +249,11 @@ func (f *fwFixture) RequireFileWatchSpecEqual(targetID model.TargetID, spec v1al
 	actualSet := ToFileWatchObjects(f.inputs, make(disableSourceMap))
 	actual, ok := actualSet[apis.SanitizeName(targetID.String())]
 	require.True(f.T(), ok, "No filewatch found for %s", targetID)
-	fwd := &fileWatchDiffer{expected: spec}
+	fwd := &fileWatchDiffer{expected: spec, actual: actual.GetSpec().(v1alpha1.FileWatchSpec)}
 	require.True(f.T(), equality.Semantic.DeepEqual(actual.GetSpec(), spec), "FileWatch spec was not equal: %v", fwd)
 }
 
-func (f *fwFixture) SetManifestDCTarget(target model.DockerComposeTarget) {
+func (f *fwFixture) SetManifestLocalTarget(target model.LocalTarget) {
 	m := model.Manifest{Name: "foo"}.WithDeployTarget(target)
 	f.SetManifest(m)
 }

internal/dockercompose/fake_client.go

@@ -5,6 +5,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -148,15 +150,30 @@ func (c *FakeDCClient) Project(_ context.Context, m model.DockerComposeProject)
 		return nil, err
 	}
 
+	workDir := c.WorkDir
+	projectNameOpt := func(opt *loader.Options) {
+		if workDir != "" {
+			name := filepath.Base(workDir)
+			// normalization logic from https://github.com/compose-spec/compose-go/blob/c39f6e771fe5034fe1bec40ba5f0285ec60f5efe/cli/options.go#L366-L371
+			r := regexp.MustCompile("[a-z0-9_-]")
+			name = strings.ToLower(name)
+			name = strings.Join(r.FindAllString(name, -1), "")
+			name = strings.TrimLeft(name, "_-")
+			opt.Name = name
+		} else {
+			opt.Name = "fakedc"
+		}
+	}
+
 	p, err := loader.Load(types.ConfigDetails{
-		WorkingDir: c.WorkDir,
+		WorkingDir: workDir,
 		ConfigFiles: []types.ConfigFile{
 			{
 				Content: []byte(c.ConfigOutput),
 			},
 		},
 		Environment: opts.Environment,
-	}, dcLoaderOption)
+	}, dcLoaderOption, projectNameOpt)
 	return p, err
 }