tiltfile: turn restart_container deprecation warning into an error [c…

…h7714] (#3551)

internal/tiltfile/live_update.go

@@ -15,14 +15,14 @@ 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"
+const fmtRestartContainerDeprecationError = "Found `restart_container()` LiveUpdate step in resource(s): [%s]. `restart_container()`  has been deprecated for k8s resources. We recommend the restart_process extension: https://github.com/windmilleng/tilt-extensions/tree/master/restart_process. For more information, see https://docs.tilt.dev/live_update_reference.html#restarting-your-process"
 
-func restartContainerDeprecationWarning(names []model.ManifestName) string {
+func restartContainerDeprecationError(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, ", "))
+	return fmt.Sprintf(fmtRestartContainerDeprecationError, strings.Join(strs, ", "))
 }
 
 // when adding a new type of `liveUpdateStep`, make sure that any tiltfile functions that create them also call

internal/tiltfile/live_update_test.go

@@ -310,7 +310,7 @@ 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) {
+func TestLiveUpdateRestartContainerDeprecationErrorK8s(t *testing.T) {
 	f := newFixture(t)
 	defer f.TearDown()
 
@@ -324,10 +324,10 @@ docker_build('gcr.io/foo', './foo',
 	restart_container(),
   ]
 )`)
-	f.loadAssertWarnings(restartContainerDeprecationWarning([]model.ManifestName{"foo"}))
+	f.loadErrString(restartContainerDeprecationError([]model.ManifestName{"foo"}))
 }
 
-func TestLiveUpdateRestartContainerDeprecationWarnK8sCustomBuild(t *testing.T) {
+func TestLiveUpdateRestartContainerDeprecationErrorK8sCustomBuild(t *testing.T) {
 	f := newFixture(t)
 	defer f.TearDown()
 
@@ -342,10 +342,10 @@ custom_build('gcr.io/foo', 'docker build -t $TAG foo', ['./foo'],
   ]
 )`)
 
-	f.loadAssertWarnings(restartContainerDeprecationWarning([]model.ManifestName{"foo"}))
+	f.loadErrString(restartContainerDeprecationError([]model.ManifestName{"foo"}))
 }
 
-func TestLiveUpdateRestartContainerDeprecationWarnMultiple(t *testing.T) {
+func TestLiveUpdateRestartContainerDeprecationErrorMultiple(t *testing.T) {
 	f := newFixture(t)
 	defer f.TearDown()
 
@@ -370,10 +370,10 @@ docker_build('gcr.io/d', './d',
   live_update=[sync('./d', '/')]
 )`)
 
-	f.loadAssertWarnings(restartContainerDeprecationWarning([]model.ManifestName{"a", "c"}))
+	f.loadErrString(restartContainerDeprecationError([]model.ManifestName{"a", "c"}))
 }
 
-func TestLiveUpdateNoRestartContainerDeprecationWarnK8sDockerCompose(t *testing.T) {
+func TestLiveUpdateNoRestartContainerDeprecationErrorK8sDockerCompose(t *testing.T) {
 	f := newFixture(t)
 	defer f.TearDown()
 	f.setupFoo()
@@ -387,7 +387,7 @@ 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
+	// Expect no deprecation error b/c restart_container() is still allowed on Docker Compose resources
 	f.load()
 	f.assertNextManifest("foo", db(image("gcr.io/foo")))
 }

internal/tiltfile/tiltfile_state.go

@@ -1115,7 +1115,11 @@ func (s *tiltfileState) translateK8s(resources []*k8sResource) ([]model.Manifest
 
 		result = append(result, m)
 	}
-	s.maybeWarnRestartContainerDeprecation(result)
+
+	err := maybeRestartContainerDeprecationError(result)
+	if err != nil {
+		return nil, err
+	}
 
 	return result, nil
 }
@@ -1225,22 +1229,23 @@ func (s *tiltfileState) validateLiveUpdate(iTarget model.ImageTarget, g model.Ta
 	return nil
 }
 
-func (s *tiltfileState) maybeWarnRestartContainerDeprecation(manifests []model.Manifest) {
-	var needsWarn []model.ManifestName
+func maybeRestartContainerDeprecationError(manifests []model.Manifest) error {
+	var needsError []model.ManifestName
 	for _, m := range manifests {
-		if needsRestartContainerDeprecationWarning(m) {
-			needsWarn = append(needsWarn, m.Name)
+		if needsRestartContainerDeprecationError(m) {
+			needsError = append(needsError, m.Name)
 		}
 	}
 
-	if len(needsWarn) > 0 {
-		s.logger.Warnf("%s", restartContainerDeprecationWarning(needsWarn))
+	if len(needsError) > 0 {
+		return fmt.Errorf("%s", restartContainerDeprecationError(needsError))
 	}
+	return nil
 }
-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.
+func needsRestartContainerDeprecationError(m model.Manifest) bool {
+	// 7/2/20: we've deprecated restart_container() in favor of the restart_process extension.
+	// If this is a k8s resource with a restart_container step, throw a deprecation error.
+	// (restart_container is still allowed for Docker Compose resources)
 	if !m.IsK8s() {
 		return false
 	}