k8s_custom_deploy: add container name selector support (#5248)

In some instances, when using a custom deploy tool, you have no
underlying knowledge of the image name it'll produce, but there
is a predictable container name it'll use for the deployment, so
a `container_selector` argument allows providing the name of a
container in lieu of the image, which will be used to determine
which containers are targeted for Live Update operations.

internal/tiltfile/k8s_custom_deploy.go

@@ -3,13 +3,15 @@ package tiltfile
 import (
 	"fmt"
 
+	"github.com/docker/distribution/reference"
 	"github.com/pkg/errors"
 	"go.starlark.net/starlark"
 
 	"github.com/tilt-dev/tilt/internal/container"
 	"github.com/tilt-dev/tilt/internal/controllers/apis/liveupdate"
 	"github.com/tilt-dev/tilt/internal/tiltfile/starkit"
 	"github.com/tilt-dev/tilt/internal/tiltfile/value"
+	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
 	"github.com/tilt-dev/tilt/pkg/model"
 )
 
@@ -24,7 +26,7 @@ func (s *tiltfileState) k8sCustomDeploy(thread *starlark.Thread, fn *starlark.Bu
 	var applyCmdVal, applyCmdBatVal, applyCmdDirVal starlark.Value
 	var deleteCmdVal, deleteCmdBatVal, deleteCmdDirVal starlark.Value
 	var applyCmdEnv, deleteCmdEnv value.StringStringMap
-	var imageSelector string
+	var imageSelector, containerSelector string
 	var liveUpdateVal starlark.Value
 
 	deps := value.NewLocalPathListUnpacker(thread)
@@ -42,6 +44,7 @@ func (s *tiltfileState) k8sCustomDeploy(thread *starlark.Thread, fn *starlark.Bu
 		"delete_dir?", &deleteCmdDirVal,
 		"delete_env?", &deleteCmdEnv,
 		"delete_cmd_bat?", &deleteCmdBatVal,
+		"container_selector?", &containerSelector,
 	); err != nil {
 		return nil, err
 	}
@@ -77,13 +80,43 @@ func (s *tiltfileState) k8sCustomDeploy(thread *starlark.Thread, fn *starlark.Bu
 	}
 
 	if !liveupdate.IsEmptySpec(liveUpdate) {
-		if imageSelector == "" {
-			return nil, fmt.Errorf("k8s_custom_deploy: image_selector cannot be empty")
+		var ref reference.Named
+		var selectorCount int
+
+		if imageSelector != "" {
+			selectorCount++
+
+			// the ref attached to the image target will be inferred as the image selector
+			// for the LiveUpdateSpec by Manifest::InferLiveUpdateSelectors
+			ref, err = container.ParseNamed(imageSelector)
+			if err != nil {
+				return nil, fmt.Errorf("can't parse %q: %v", imageSelector, err)
+			}
 		}
 
-		ref, err := container.ParseNamed(imageSelector)
-		if err != nil {
-			return nil, fmt.Errorf("can't parse %q: %v", imageSelector, err)
+		if containerSelector != "" {
+			selectorCount++
+
+			// pre-populate the container name selector as this cannot be inferred from
+			// the image target by Manifest::InferLiveUpdateSelectors
+			liveUpdate.Selector.Kubernetes = &v1alpha1.LiveUpdateKubernetesSelector{
+				ContainerName: containerSelector,
+			}
+
+			// the image target needs a valid ref even though it'll never be
+			// built/used, so create one named after the manifest that won't
+			// collide with anything else
+			fakeImageName := fmt.Sprintf("k8s_custom_deploy:%s", name)
+			ref, err = container.ParseNamed(fakeImageName)
+			if err != nil {
+				return nil, fmt.Errorf("can't parse %q: %v", fakeImageName, err)
+			}
+		}
+
+		if selectorCount == 0 {
+			return nil, fmt.Errorf("k8s_custom_deploy: no Live Update selector specified")
+		} else if selectorCount > 1 {
+			return nil, fmt.Errorf("k8s_custom_deploy: cannot specify more than one Live Update selector")
 		}
 
 		img := &dockerImage{
@@ -99,6 +132,9 @@ func (s *tiltfileState) k8sCustomDeploy(thread *starlark.Thread, fn *starlark.Bu
 			skipsLocalDocker: true,
 			tiltfilePath:     starkit.CurrentExecPath(thread),
 		}
+		// N.B. even in the case that we're creating a fake image name, we need
+		// 	to reference it so that it can be "consumed" by this target to avoid
+		// 	producing warnings about unused image targets
 		res.imageRefs = append(res.imageRefs, ref)
 
 		if err := s.buildIndex.addImage(img); err != nil {

internal/tiltfile/k8s_custom_deploy_test.go

@@ -1 +1,107 @@
 package tiltfile
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
+	"github.com/tilt-dev/tilt/pkg/model"
+)
+
+func TestK8sCustomDeployLiveUpdateImageSelector(t *testing.T) {
+	f := newLiveUpdateFixture(t)
+	defer f.TearDown()
+
+	f.skipYAML = true
+	f.tiltfileCode = `
+default_registry('gcr.io/myrepo')
+k8s_custom_deploy('foo', 'apply', 'delete', deps=['foo'], image_selector='foo-img', live_update=%s)
+`
+	f.init()
+
+	f.load("foo")
+
+	m := f.assertNextManifest("foo", cb(image("foo-img"), f.expectedLU))
+	assert.True(t, m.ImageTargets[0].IsLiveUpdateOnly)
+	// this ref will never actually be used since the image isn't being built but the registry is applied here
+	assert.Equal(t, "gcr.io/myrepo/foo-img", m.ImageTargets[0].Refs.LocalRef().String())
+
+	require.NoError(t, m.InferLiveUpdateSelectors(), "Failed to infer Live Update selectors")
+	luSpec := m.ImageTargets[0].LiveUpdateSpec
+	require.NotNil(t, luSpec.Selector.Kubernetes)
+	assert.Empty(t, luSpec.Selector.Kubernetes.ContainerName)
+	// NO registry rewriting should be applied here because Tilt isn't actually building the image
+	assert.Equal(t, "foo-img", luSpec.Selector.Kubernetes.Image)
+}
+
+func TestK8sCustomDeployLiveUpdateContainerNameSelector(t *testing.T) {
+	f := newLiveUpdateFixture(t)
+	defer f.TearDown()
+
+	f.skipYAML = true
+	f.tiltfileCode = `
+k8s_custom_deploy('foo', 'apply', 'delete', deps=['foo'], container_selector='bar', live_update=%s)
+`
+	f.init()
+
+	f.load("foo")
+	f.expectedLU.Selector.Kubernetes = &v1alpha1.LiveUpdateKubernetesSelector{
+		ContainerName: "bar",
+	}
+
+	// NOTE: because there is no known image name, the manifest name is used to
+	// 	generate one since an image target without a ref is not valid
+	m := f.assertNextManifest("foo", cb(image("k8s_custom_deploy:foo"), f.expectedLU))
+	assert.True(t, m.ImageTargets[0].IsLiveUpdateOnly)
+
+	require.NoError(t, m.InferLiveUpdateSelectors(), "Failed to infer Live Update selectors")
+	luSpec := m.ImageTargets[0].LiveUpdateSpec
+	require.NotNil(t, luSpec.Selector.Kubernetes)
+	assert.Empty(t, luSpec.Selector.Kubernetes.Image)
+	// NO registry rewriting should be applied here because Tilt isn't actually building the image
+	assert.Equal(t, "bar", luSpec.Selector.Kubernetes.ContainerName)
+}
+
+func TestK8sCustomDeployNoLiveUpdate(t *testing.T) {
+	f := newFixture(t)
+	defer f.TearDown()
+
+	f.file("Tiltfile", `
+k8s_custom_deploy('foo',
+                  apply_cmd='apply',
+                  delete_cmd='delete',
+                  apply_dir='apply-dir',
+                  delete_dir='delete-dir',
+                  apply_env={'APPLY_KEY': '1'},
+                  delete_env={'DELETE_KEY': 'baz'},
+                  deps=['foo'])
+`)
+
+	f.load("foo")
+
+	m := f.assertNextManifest("foo")
+	assert.Empty(t, m.ImageTargets, "No image targets should have been created")
+
+	spec := m.K8sTarget().KubernetesApplySpec
+	assertK8sApplyCmdEqual(f,
+		model.ToHostCmdInDirWithEnv("apply", "apply-dir", []string{"APPLY_KEY=1"}),
+		spec.ApplyCmd)
+	assertK8sApplyCmdEqual(f,
+		model.ToHostCmdInDirWithEnv("delete", "delete-dir", []string{"DELETE_KEY=baz"}),
+		spec.DeleteCmd)
+}
+
+func assertK8sApplyCmdEqual(f *fixture, expected model.Cmd, actual *v1alpha1.KubernetesApplyCmd) bool {
+	t := f.t
+	t.Helper()
+	if !assert.NotNil(t, actual, "KubernetesApplyCmd was nil") {
+		return false
+	}
+	result := true
+	result = assert.Equal(t, expected.Argv, actual.Args, "Args were not equal") && result
+	result = assert.Equal(t, expected.Env, actual.Env, "Env was not equal") && result
+	result = assert.Equal(t, f.JoinPath(expected.Dir), actual.Dir, "Working dir was not equal") && result
+	return result
+}

internal/tiltfile/live_update_test.go

@@ -3,9 +3,11 @@ package tiltfile
 import (
 	"fmt"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
 	"github.com/tilt-dev/tilt/pkg/model"
@@ -178,13 +180,25 @@ func TestLiveUpdateOnlyCustomBuild(t *testing.T) {
 	f := newLiveUpdateFixture(t)
 	defer f.TearDown()
 
-	f.tiltfileCode = "custom_build('foo', ':', ['foo'], live_update=%s)"
+	f.tiltfileCode = `
+default_registry('gcr.io/myrepo')
+custom_build('foo', ':', ['foo'], live_update=%s)
+`
 	f.init()
 
 	f.load("foo")
 
 	m := f.assertNextManifest("foo", cb(image("foo"), f.expectedLU))
 	assert.True(t, m.ImageTargets[0].IsLiveUpdateOnly)
+	// this ref will never actually be used since the image isn't being built but the registry is applied here
+	assert.Equal(t, "gcr.io/myrepo/foo", m.ImageTargets[0].Refs.LocalRef().String())
+
+	require.NoError(t, m.InferLiveUpdateSelectors(), "Failed to infer Live Update selectors")
+	luSpec := m.ImageTargets[0].LiveUpdateSpec
+	require.NotNil(t, luSpec.Selector.Kubernetes)
+	assert.Empty(t, luSpec.Selector.Kubernetes.ContainerName)
+	// NO registry rewriting should be applied here because Tilt isn't actually building the image
+	assert.Equal(t, "foo", luSpec.Selector.Kubernetes.Image)
 }
 
 func TestLiveUpdateSyncFilesOutsideOfDockerBuildContext(t *testing.T) {
@@ -416,22 +430,29 @@ type liveUpdateFixture struct {
 	tiltfileCode  string
 	expectedImage string
 	expectedLU    v1alpha1.LiveUpdateSpec
+
+	skipYAML bool
 }
 
 func (f *liveUpdateFixture) init() {
 	f.dockerfile("foo/Dockerfile")
-	f.yaml("foo.yaml", deployment("foo", image(f.expectedImage)))
+
+	if !f.skipYAML {
+		f.yaml("foo.yaml", deployment("foo", image(f.expectedImage)))
+	}
 
 	luSteps := `[
     fall_back_on(['foo/i', 'foo/j']),
 	sync('foo/b', '/c'),
 	run('f', ['g', 'h']),
 ]`
 	codeToInsert := fmt.Sprintf(f.tiltfileCode, luSteps)
-	tiltfile := fmt.Sprintf(`
-k8s_yaml('foo.yaml')
-%s
-`, codeToInsert)
+
+	var tiltfile string
+	if !f.skipYAML {
+		tiltfile = `k8s_yaml('foo.yaml')`
+	}
+	tiltfile = strings.Join([]string{tiltfile, codeToInsert}, "\n")
 	f.file("Tiltfile", tiltfile)
 }
 

pkg/model/image_target.go

@@ -110,7 +110,7 @@ func (i ImageTarget) Validate() error {
 			return fmt.Errorf("[Validate] Image %q missing build path", confRef)
 		}
 	case CustomBuild:
-		if bd.Command.Empty() {
+		if !i.IsLiveUpdateOnly && bd.Command.Empty() {
 			return fmt.Errorf(
 				"[Validate] CustomBuild command must not be empty",
 			)

pkg/model/manifest.go

@@ -274,13 +274,29 @@ func (m *Manifest) InferLiveUpdateSelectors() error {
 
 		// TODO(nick): Also set docker-compose selectors once the model supports it.
 		if m.IsK8s() {
-			luSpec.Selector.Kubernetes = &v1alpha1.LiveUpdateKubernetesSelector{
-				Image:         reference.FamiliarName(iTarget.Refs.ClusterRef()),
-				ApplyName:     m.Name.String(),
-				DiscoveryName: m.Name.String(),
+			kSelector := luSpec.Selector.Kubernetes
+			if kSelector == nil {
+				kSelector = &v1alpha1.LiveUpdateKubernetesSelector{}
+				luSpec.Selector.Kubernetes = kSelector
 			}
-			if iTarget.IsLiveUpdateOnly {
-				luSpec.Selector.Kubernetes.Image = reference.FamiliarName(iTarget.Refs.WithoutRegistry().LocalRef())
+
+			if kSelector.ApplyName == "" {
+				kSelector.ApplyName = m.Name.String()
+			}
+			if kSelector.DiscoveryName == "" {
+				kSelector.DiscoveryName = m.Name.String()
+			}
+
+			// infer an image name from the ImageTarget if a container name selector was not specified
+			// (currently, this is always done except in some k8s_custom_deploy configurations)
+			if kSelector.ContainerName == "" {
+				var image string
+				if iTarget.IsLiveUpdateOnly {
+					image = reference.FamiliarName(iTarget.Refs.WithoutRegistry().LocalRef())
+				} else {
+					image = reference.FamiliarName(iTarget.Refs.ClusterRef())
+				}
+				kSelector.Image = image
 			}
 		}