From d4ba52be1f58f2d557cad3ebeba756fff86f9622 Mon Sep 17 00:00:00 2001
From: Nick Santos <nicholas.j.santos@gmail.com>
Date: Tue, 7 Jul 2020 13:20:49 -0400
Subject: [PATCH] tiltfile: resources with extra pod selectors or images should
 be considered workloads (#3561)

---
 internal/tiltfile/k8s.go            | 16 ++++++++--------
 internal/tiltfile/tiltfile_state.go | 11 ++++++++---
 internal/tiltfile/tiltfile_test.go  |  9 ++++++---
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/internal/tiltfile/k8s.go b/internal/tiltfile/k8s.go
index 1644b996e3..f74f079a2c 100644
--- a/internal/tiltfile/k8s.go
+++ b/internal/tiltfile/k8s.go
@@ -55,7 +55,7 @@ type k8sResource struct {
 
 	resourceDeps []string
 
-	nonWorkload bool
+	manuallyGrouped bool
 }
 
 const deprecatedResourceAssemblyV1Warning = "This Tiltfile is using k8s resource assembly version 1, which has been " +
@@ -74,7 +74,7 @@ type k8sResourceOptions struct {
 	tiltfilePosition  syntax.Position
 	resourceDeps      []string
 	objects           []string
-	nonWorkload       bool
+	manuallyGrouped   bool
 }
 
 func (r *k8sResource) addRefSelector(selector container.RefSelector) {
@@ -417,11 +417,11 @@ func (s *tiltfileState) k8sResourceV2(thread *starlark.Thread, fn *starlark.Buil
 	}
 
 	resourceName := workload
-	needsToHaveObjects := false
+	manuallyGrouped := false
 	if workload == "" {
 		resourceName = newName
-		// If a resource doesn't specify a workload then it needs to have objects to be valid
-		needsToHaveObjects = true
+		// If a resource doesn't specify an existing workload then it needs to have objects to be valid
+		manuallyGrouped = true
 	}
 
 	portForwards, err := convertPortForwards(portForwardsVal)
@@ -448,11 +448,11 @@ func (s *tiltfileState) k8sResourceV2(thread *starlark.Thread, fn *starlark.Buil
 		return nil, errors.Wrapf(err, "%s: resource_deps", fn.Name())
 	}
 
-	if needsToHaveObjects && len(objects) == 0 {
+	if manuallyGrouped && len(objects) == 0 {
 		return nil, fmt.Errorf("k8s_resource doesn't specify a workload or any objects. All non-workload resources must specify 1 or more objects")
 	}
 
-	if needsToHaveObjects && len(objects) > 0 && newName == "" {
+	if manuallyGrouped && len(objects) > 0 && newName == "" {
 		return nil, fmt.Errorf("k8s_resource has only non-workload objects but doesn't provide a new_name")
 	}
 
@@ -465,7 +465,7 @@ func (s *tiltfileState) k8sResourceV2(thread *starlark.Thread, fn *starlark.Buil
 		autoInit:          autoInit,
 		resourceDeps:      resourceDeps,
 		objects:           objects,
-		nonWorkload:       needsToHaveObjects,
+		manuallyGrouped:   manuallyGrouped,
 	}
 
 	return starlark.None, nil
diff --git a/internal/tiltfile/tiltfile_state.go b/internal/tiltfile/tiltfile_state.go
index 120d74fbce..beb7662008 100644
--- a/internal/tiltfile/tiltfile_state.go
+++ b/internal/tiltfile/tiltfile_state.go
@@ -675,12 +675,12 @@ func (s *tiltfileState) assembleK8sV2() error {
 	}
 
 	for workload, opts := range s.k8sResourceOptions {
-		if opts.nonWorkload {
+		if opts.manuallyGrouped {
 			r, err := s.makeK8sResource(opts.newName)
 			if err != nil {
 				return err
 			}
-			r.nonWorkload = true
+			r.manuallyGrouped = true
 			s.k8sByName[opts.newName] = r
 		}
 		if r, ok := s.k8sByName[workload]; ok {
@@ -1098,8 +1098,13 @@ func (s *tiltfileState) translateK8s(resources []*k8sResource) ([]model.Manifest
 			ResourceDependencies: mds,
 		}
 
+		nonWorkload := false
+		if r.manuallyGrouped {
+			nonWorkload = len(r.extraPodSelectors) == 0 && len(r.dependencyIDs) == 0
+		}
+
 		k8sTarget, err := k8s.NewTarget(mn.TargetName(), r.entities, s.defaultedPortForwards(r.portForwards),
-			r.extraPodSelectors, r.dependencyIDs, r.imageRefMap, r.nonWorkload, locators)
+			r.extraPodSelectors, r.dependencyIDs, r.imageRefMap, nonWorkload, locators)
 		if err != nil {
 			return nil, err
 		}
diff --git a/internal/tiltfile/tiltfile_test.go b/internal/tiltfile/tiltfile_test.go
index e7d3df634b..85985cb894 100644
--- a/internal/tiltfile/tiltfile_test.go
+++ b/internal/tiltfile/tiltfile_test.go
@@ -1447,7 +1447,8 @@ func TestExtraPodSelectors(t *testing.T) {
 	f.load()
 
 	f.assertNextManifest("foo",
-		extraPodSelectors(labels.Set{"foo": "bar", "baz": "qux"}, labels.Set{"quux": "corge"}))
+		extraPodSelectors(labels.Set{"foo": "bar", "baz": "qux"}, labels.Set{"quux": "corge"}),
+		nonWorkload(false))
 }
 
 func TestExtraPodSelectorsNotList(t *testing.T) {
@@ -1465,7 +1466,8 @@ func TestExtraPodSelectorsDict(t *testing.T) {
 	f.setupExtraPodSelectors("{'foo': 'bar'}")
 	f.load()
 	f.assertNextManifest("foo",
-		extraPodSelectors(labels.Set{"foo": "bar"}))
+		extraPodSelectors(labels.Set{"foo": "bar"}),
+		nonWorkload(false))
 }
 
 func TestExtraPodSelectorsElementNotDict(t *testing.T) {
@@ -2166,7 +2168,8 @@ docker_build('tilt.dev/frontend', '.')
 `)
 
 	f.load()
-	m := f.assertNextManifest("um")
+	m := f.assertNextManifest("um",
+		nonWorkload(false))
 	assert.Equal(t, "tilt.dev/frontend",
 		m.ImageTargets[0].Refs.LocalRef().String())
 }