Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "tiltfile: deprecation warning for restart_container + k8s resource [ch5943] (#3249)" #3484

Merged
merged 1 commit into from
Jun 19, 2020
Merged

Revert "tiltfile: deprecation warning for restart_container + k8s resource [ch5943] (#3249)" #3484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions internal/tiltfile/live_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,6 @@ 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 {
Expand Down
84 changes: 2 additions & 82 deletions internal/tiltfile/live_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,88 +310,6 @@ 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

Expand All @@ -408,6 +326,7 @@ 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(`
Expand All @@ -434,6 +353,7 @@ func newLiveUpdateFixture(t *testing.T) *liveUpdateFixture {
Command: model.ToUnixCmd("f"),
Triggers: model.NewPathSet([]string{"g", "h"}, f.Path()),
},
model.LiveUpdateRestartContainerStep{},
)

f.expectedLU = model.LiveUpdate{
Expand Down
31 changes: 1 addition & 30 deletions internal/tiltfile/tiltfile_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,6 @@ func (s *tiltfileState) translateK8s(resources []*k8sResource) ([]model.Manifest

result = append(result, m)
}
s.maybeWarnRestartContainerDeprecation(result)

return result, nil
}
Expand Down Expand Up @@ -1206,38 +1205,10 @@ 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
return nil
}

// Grabs all image targets for the given references,
Expand Down