tiltfile: improve the unmatched image warning message. Fixes #3410 (#…

…3430)

internal/tiltfile/build_index.go

@@ -89,27 +89,33 @@ func (idx *buildIndex) findBuilderForConsumedImage(ref reference.Named) *dockerI
 	return nil
 }
 
-func (idx *buildIndex) assertAllMatched() error {
+func (idx *buildIndex) unmatchedImages() []*dockerImage {
+	unmatchedImages := make([]*dockerImage, 0)
 	for _, image := range idx.images {
 		if !image.matched {
-			bagSizes := []int{2, 3, 4}
-			cm := closestmatch.New(idx.consumedImageNames, bagSizes)
-			matchLines := []string{}
-			for i, match := range cm.ClosestN(image.configurationRef.RefFamiliarName(), 3) {
-				// If there are no matches, the closestmatch library sometimes returns
-				// an empty string
-				if match == "" {
-					break
-				}
-				if i == 0 {
-					matchLines = append(matchLines, "Did you mean…\n")
-				}
-				matchLines = append(matchLines, fmt.Sprintf("    - %s\n", match))
-			}
+			unmatchedImages = append(unmatchedImages, image)
+		}
+	}
+	return unmatchedImages
+}
 
-			return fmt.Errorf("Image not used in any deploy config:\n    ✕ %v\n%sSkipping this image build",
-				container.FamiliarString(image.configurationRef), strings.Join(matchLines, ""))
+func (idx *buildIndex) unmatchedImageWarning(image *dockerImage, configType string) error {
+
+	bagSizes := []int{2, 3, 4}
+	cm := closestmatch.New(idx.consumedImageNames, bagSizes)
+	matchLines := []string{}
+	for i, match := range cm.ClosestN(image.configurationRef.RefFamiliarName(), 3) {
+		// If there are no matches, the closestmatch library sometimes returns
+		// an empty string
+		if match == "" {
+			break
+		}
+		if i == 0 {
+			matchLines = append(matchLines, "Did you mean…\n")
 		}
+		matchLines = append(matchLines, fmt.Sprintf("    - %s\n", match))
 	}
-	return nil
+
+	return fmt.Errorf("Image not used in any %s config:\n    ✕ %v\n%sSkipping this image build",
+		configType, container.FamiliarString(image.configurationRef), strings.Join(matchLines, ""))
 }

internal/tiltfile/tiltfile_docker_compose_test.go

@@ -507,7 +507,7 @@ services:
 docker_build('gcr.typo.io/foo', 'foo')
 docker_compose('docker-compose.yml')
 `)
-	f.loadAssertWarnings(`Image not used in any deploy config:
+	f.loadAssertWarnings(`Image not used in any Docker Compose config:
     ✕ gcr.typo.io/foo
 Did you mean…
     - gcr.io/foo

internal/tiltfile/tiltfile_state.go

@@ -40,6 +40,12 @@ import (
 	"github.com/tilt-dev/tilt/pkg/model"
 )
 
+var unmatchedImageNoConfigsWarning = "No Kubernetes or Docker Compose configs found.\n" +
+	"Skipping all image builds until we have a place to deploy them"
+var unmatchedImageAllUnresourcedWarning = "No Kubernetes configs with images found.\n" +
+	"If you are using CRDs, add k8s_kind() to tell Tilt how to find images.\n" +
+	"https://docs.tilt.dev/api.html#api.k8s_kind"
+
 type resourceSet struct {
 	dc  dcResourceSet // currently only support one d-c.yml
 	k8s []*k8sResource
@@ -524,7 +530,7 @@ func (s *tiltfileState) assemble() (resourceSet, []k8s.K8sEntity, error) {
 			"resources/entities and docker-compose resources")
 	}
 
-	err = s.buildIndex.assertAllMatched()
+	err = s.assertAllImagesMatched()
 	if err != nil {
 		s.logger.Warnf("%s", err.Error())
 	}
@@ -535,6 +541,42 @@ func (s *tiltfileState) assemble() (resourceSet, []k8s.K8sEntity, error) {
 	}, s.k8sUnresourced, nil
 }
 
+// Emit an error if there are unmatches images.
+//
+// There are 4 mistakes people commonly make if they
+// have unmatched images:
+// 1) They didn't include any Kubernetes or Docker Compose configs at all.
+// 2) They included Kubernetes configs, but they're custom resources
+//    and Tilt can't infer the image.
+// 3) They typo'd the image name, and need help finding the right name.
+// 4) The tooling they're using to generating the k8s resources
+//    isn't generating what they expect.
+//
+// This function intends to help with cases (1)-(3).
+// Long-term, we want to have better tooling to help with (4),
+// like being able to see k8s resources as they move thru
+// the build system.
+func (s *tiltfileState) assertAllImagesMatched() error {
+	unmatchedImages := s.buildIndex.unmatchedImages()
+	if len(unmatchedImages) == 0 {
+		return nil
+	}
+
+	if len(s.dc.services) == 0 && len(s.k8s) == 0 && len(s.k8sUnresourced) == 0 {
+		return fmt.Errorf(unmatchedImageNoConfigsWarning)
+	}
+
+	if len(s.k8s) == 0 && len(s.k8sUnresourced) != 0 {
+		return fmt.Errorf(unmatchedImageAllUnresourcedWarning)
+	}
+
+	configType := "Kubernetes"
+	if len(s.dc.services) > 0 {
+		configType = "Docker Compose"
+	}
+	return s.buildIndex.unmatchedImageWarning(unmatchedImages[0], configType)
+}
+
 func (s *tiltfileState) assembleImages() error {
 	for _, imageBuilder := range s.buildIndex.images {
 		var err error

internal/tiltfile/tiltfile_test.go

@@ -1468,6 +1468,19 @@ k8s_yaml('foo.yaml')
 	)
 }
 
+func TestDockerBuildButK8sMissing(t *testing.T) {
+	f := newFixture(t)
+	defer f.TearDown()
+
+	f.gitInit("")
+	f.file("Dockerfile", "FROM golang:1.10")
+	f.file("Tiltfile", `
+docker_build('gcr.io/foo:stable', '.')
+`)
+
+	f.loadAssertWarnings(unmatchedImageNoConfigsWarning)
+}
+
 func TestDockerBuildButK8sMissingTag(t *testing.T) {
 	f := newFixture(t)
 	defer f.TearDown()
@@ -1480,7 +1493,7 @@ docker_build('gcr.io/foo:stable', '.')
 k8s_yaml('foo.yaml')
 `)
 
-	w := unusedImageWarning("gcr.io/foo:stable", []string{"gcr.io/foo"})
+	w := unusedImageWarning("gcr.io/foo:stable", []string{"gcr.io/foo"}, "Kubernetes")
 	f.loadAssertWarnings(w)
 }
 
@@ -1496,7 +1509,7 @@ docker_build('gcr.io/foo:stable', '.')
 k8s_yaml('foo.yaml')
 `)
 
-	w := unusedImageWarning("gcr.io/foo:stable", []string{"gcr.io/foo"})
+	w := unusedImageWarning("gcr.io/foo:stable", []string{"gcr.io/foo"}, "Kubernetes")
 	f.loadAssertWarnings(w)
 }
 
@@ -1825,7 +1838,7 @@ docker_build('gcr.typo.io/foo', 'foo')
 k8s_yaml('foo.yaml')
 `)
 
-	w := unusedImageWarning("gcr.typo.io/foo", []string{"gcr.io/foo"})
+	w := unusedImageWarning("gcr.typo.io/foo", []string{"gcr.io/foo"}, "Kubernetes")
 	f.loadAssertWarnings(w)
 }
 
@@ -2202,8 +2215,7 @@ k8s_kind(%s)
 					t.Fatal("invalid test: cannot expect image without expecting workload")
 				}
 				if test.expectedError == "" {
-					w := unusedImageWarning("test/mycrd-env", []string{})
-					f.loadAssertWarnings(w)
+					f.loadAssertWarnings(unmatchedImageAllUnresourcedWarning)
 				} else {
 					f.loadErrString(test.expectedError)
 				}
@@ -2323,7 +2335,7 @@ func TestExtraImageLocationDeploymentEnvVarDoesNotMatchIfNotSpecified(t *testing
 docker_build('gcr.io/foo', 'foo')
 docker_build('gcr.io/foo-fetcher', 'foo-fetcher')
 	`)
-	f.loadAssertWarnings(unusedImageWarning("gcr.io/foo-fetcher", []string{"gcr.io/foo"}))
+	f.loadAssertWarnings(unusedImageWarning("gcr.io/foo-fetcher", []string{"gcr.io/foo"}, "Kubernetes"))
 	f.assertNextManifest("foo",
 		db(
 			image("gcr.io/foo"),
@@ -2385,7 +2397,7 @@ k8s_image_json_path("{.spec.template.spec.containers[*].env[?(@.name=='FETCHER_I
 				)
 			} else {
 				if test.expectedError == "" {
-					w := unusedImageWarning("gcr.io/foo-fetcher", []string{"gcr.io/foo"})
+					w := unusedImageWarning("gcr.io/foo-fetcher", []string{"gcr.io/foo"}, "Kubernetes")
 					f.loadAssertWarnings(w)
 				} else {
 					f.loadErrString(test.expectedError)
@@ -5179,8 +5191,8 @@ func (f *fixture) loadAllowWarnings(args ...string) {
 	f.loadResult = tlr
 }
 
-func unusedImageWarning(unusedImage string, suggestedImages []string) string {
-	ret := fmt.Sprintf("Image not used in any deploy config:\n    ✕ %s", unusedImage)
+func unusedImageWarning(unusedImage string, suggestedImages []string, configType string) string {
+	ret := fmt.Sprintf("Image not used in any %s config:\n    ✕ %s", configType, unusedImage)
 	if len(suggestedImages) > 0 {
 		ret = ret + "\nDid you mean…"
 		for _, s := range suggestedImages {