diff --git a/internal/tiltfile/build_index.go b/internal/tiltfile/build_index.go index 8310c37ae2..550d6bb07e 100644 --- a/internal/tiltfile/build_index.go +++ b/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, "")) } diff --git a/internal/tiltfile/tiltfile_docker_compose_test.go b/internal/tiltfile/tiltfile_docker_compose_test.go index ea87f8c492..1b26229b3d 100644 --- a/internal/tiltfile/tiltfile_docker_compose_test.go +++ b/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 diff --git a/internal/tiltfile/tiltfile_state.go b/internal/tiltfile/tiltfile_state.go index ec8b9250e0..799e87473b 100644 --- a/internal/tiltfile/tiltfile_state.go +++ b/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 diff --git a/internal/tiltfile/tiltfile_test.go b/internal/tiltfile/tiltfile_test.go index 49ae1898bc..e058a7b42f 100644 --- a/internal/tiltfile/tiltfile_test.go +++ b/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 {