Revert "Revert "tiltfile: deprecation warning for restart_container +…

… k8s resource [ch5943] (#3249)" (#3484)"

This reverts commit fb8fc10.

internal/tiltfile/live_update.go

@@ -15,6 +15,16 @@ import (
 	"github.com/tilt-dev/tilt/pkg/model"
 )
 
+const fmtRestartContainerDeprecationWarning = "Found `restart_container()` LiveUpdate step in resource(s): [%s]. `restart_container()`  will soon be deprecated. For recommended ways to restart your process, see https://docs.tilt.dev/live_update_reference.html#restarting-your-process"
+
+func restartContainerDeprecationWarning(names []model.ManifestName) string {
+	strs := make([]string, len(names))
+	for i, n := range names {
+		strs[i] = n.String()
+	}
+	return fmt.Sprintf(fmtRestartContainerDeprecationWarning, strings.Join(strs, ", "))
+}
+
 // when adding a new type of `liveUpdateStep`, make sure that any tiltfile functions that create them also call
 // `s.recordLiveUpdateStep`
 type liveUpdateStep interface {

internal/tiltfile/live_update_test.go

@@ -310,6 +310,88 @@ custom_build('gcr.io/foo', 'docker build -t $TAG foo', ['./foo'],
 	f.loadErrString("fall_back_on", f.JoinPath("bar"), f.JoinPath("foo"), "child", "any watched filepaths")
 }
 
+func TestLiveUpdateRestartContainerDeprecationWarnK8s(t *testing.T) {
+	f := newFixture(t)
+	defer f.TearDown()
+
+	f.setupFoo()
+
+	f.file("Tiltfile", `
+k8s_yaml('foo.yaml')
+docker_build('gcr.io/foo', './foo',
+  live_update=[
+    sync('foo/bar', '/baz'),
+	restart_container(),
+  ]
+)`)
+	f.loadAssertWarnings(restartContainerDeprecationWarning([]model.ManifestName{"foo"}))
+}
+
+func TestLiveUpdateRestartContainerDeprecationWarnK8sCustomBuild(t *testing.T) {
+	f := newFixture(t)
+	defer f.TearDown()
+
+	f.setupFoo()
+
+	f.file("Tiltfile", `
+k8s_yaml('foo.yaml')
+custom_build('gcr.io/foo', 'docker build -t $TAG foo', ['./foo'],
+  live_update=[
+    sync('foo/bar', '/baz'),
+	restart_container(),
+  ]
+)`)
+
+	f.loadAssertWarnings(restartContainerDeprecationWarning([]model.ManifestName{"foo"}))
+}
+
+func TestLiveUpdateRestartContainerDeprecationWarnMultiple(t *testing.T) {
+	f := newFixture(t)
+	defer f.TearDown()
+
+	f.setupExpand()
+
+	f.file("Tiltfile", `
+k8s_yaml('all.yaml')
+docker_build('gcr.io/a', './a',
+  live_update=[
+    sync('./a', '/'),
+	restart_container(),
+  ]
+)
+docker_build('gcr.io/b', './b')
+docker_build('gcr.io/c', './c',
+  live_update=[
+    sync('./c', '/'),
+	restart_container(),
+  ]
+)
+docker_build('gcr.io/d', './d',
+  live_update=[sync('./d', '/')]
+)`)
+
+	f.loadAssertWarnings(restartContainerDeprecationWarning([]model.ManifestName{"a", "c"}))
+}
+
+func TestLiveUpdateNoRestartContainerDeprecationWarnK8sDockerCompose(t *testing.T) {
+	f := newFixture(t)
+	defer f.TearDown()
+	f.setupFoo()
+	f.file("docker-compose.yml", `version: '3'
+services:
+  foo:
+    image: gcr.io/foo
+`)
+	f.file("Tiltfile", `
+docker_build('gcr.io/foo', 'foo')
+docker_compose('docker-compose.yml')
+`)
+
+	// Expect no deprecation warning b/c restart_container() is still allowed on Docker Compose resources
+	f.load()
+	f.assertNextManifest("foo", db(image("gcr.io/foo")))
+}
+
 type liveUpdateFixture struct {
 	*fixture
 
@@ -326,7 +408,6 @@ func (f *liveUpdateFixture) init() {
     fall_back_on(['foo/i', 'foo/j']),
 	sync('foo/b', '/c'),
 	run('f', ['g', 'h']),
-	restart_container(),
 ]`
 	codeToInsert := fmt.Sprintf(f.tiltfileCode, luSteps)
 	tiltfile := fmt.Sprintf(`
@@ -353,7 +434,6 @@ func newLiveUpdateFixture(t *testing.T) *liveUpdateFixture {
 			Command:  model.ToUnixCmd("f"),
 			Triggers: model.NewPathSet([]string{"g", "h"}, f.Path()),
 		},
-		model.LiveUpdateRestartContainerStep{},
 	)
 
 	f.expectedLU = model.LiveUpdate{

internal/tiltfile/tiltfile_state.go

@@ -1104,6 +1104,7 @@ func (s *tiltfileState) translateK8s(resources []*k8sResource) ([]model.Manifest
 
 		result = append(result, m)
 	}
+	s.maybeWarnRestartContainerDeprecation(result)
 
 	return result, nil
 }
@@ -1208,12 +1209,40 @@ func (s *tiltfileState) validateLiveUpdate(iTarget model.ImageTarget, g model.Ta
 			return fmt.Errorf("fall_back_on paths '%s' is not a child of any watched filepaths (%v)",
 				path, watchedPaths)
 		}
-
 	}
 
 	return nil
 }
 
+func (s *tiltfileState) maybeWarnRestartContainerDeprecation(manifests []model.Manifest) {
+	var needsWarn []model.ManifestName
+	for _, m := range manifests {
+		if needsRestartContainerDeprecationWarning(m) {
+			needsWarn = append(needsWarn, m.Name)
+		}
+	}
+
+	if len(needsWarn) > 0 {
+		s.logger.Warnf("%s", restartContainerDeprecationWarning(needsWarn))
+	}
+}
+func needsRestartContainerDeprecationWarning(m model.Manifest) bool {
+	// 6/8/20: we're in the process of deprecating restart_container() in favor of the
+	// restart_process extension. If this is a k8s resource with a restart_container
+	// step, give a deprecation warning.
+	if !m.IsK8s() {
+		return false
+	}
+
+	for _, iTarg := range m.ImageTargets {
+		if iTarg.LiveUpdateInfo().ShouldRestart() {
+			return true
+		}
+	}
+
+	return false
+}
+
 // Grabs all image targets for the given references,
 // as well as any of their transitive dependencies.
 func (s *tiltfileState) imgTargetsForDependencyIDs(ids []model.TargetID, reg container.Registry) ([]model.ImageTarget, error) {