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

engine: updateMode=synclet only if explicitly set with flag [8240] #3563

Merged
merged 5 commits into from
Jul 8, 2020
Merged

engine: updateMode=synclet only if explicitly set with flag [8240] #3563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
12a71c2
only use synclet with explicit flag (needs tests)
Jul 7, 2020
da404d3
fix liveupdate tests that expected synclet
Jul 7, 2020
dee0445
fix other tests, rm redundant ones
Jul 7, 2020
8324f4f
goimports
Jul 7, 2020
1b7306a
rm obsolete update mode
Jul 7, 2020
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
3 changes: 3 additions & 0 deletions internal/containerupdate/exec_updater.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func (cu *ExecUpdater) UpdateContainer(ctx context.Context, cInfo store.Containe
l := logger.Get(ctx)
w := logger.Get(ctx).Writer(logger.InfoLvl)

// delete files (if any)
if len(filesToDelete) > 0 {
err := cu.kCli.Exec(ctx,
cInfo.PodID, cInfo.ContainerName, cInfo.Namespace,
Expand All @@ -48,12 +49,14 @@ func (cu *ExecUpdater) UpdateContainer(ctx context.Context, cInfo store.Containe
}
}

// copy files to container
err := cu.kCli.Exec(ctx, cInfo.PodID, cInfo.ContainerName, cInfo.Namespace,
[]string{"tar", "-C", "/", "-x", "-f", "-"}, archiveToCopy, w, w)
if err != nil {
return err
}

// run commands
for i, c := range cmds {
l.Infof("[CMD %d/%d] %s", i+1, len(cmds), strings.Join(c.Argv, " "))
err := cu.kCli.Exec(ctx, cInfo.PodID, cInfo.ContainerName, cInfo.Namespace,
Expand Down
8 changes: 2 additions & 6 deletions internal/engine/build_and_deployer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,13 @@ func (composite *CompositeBuildAndDeployer) BuildAndDeploy(ctx context.Context,

func DefaultBuildOrder(lubad *LiveUpdateBuildAndDeployer, ibad *ImageBuildAndDeployer, dcbad *DockerComposeBuildAndDeployer,
ltbad *LocalTargetBuildAndDeployer, updMode buildcontrol.UpdateMode, env k8s.Env, runtime container.Runtime) BuildOrder {
if updMode == buildcontrol.UpdateModeImage || updMode == buildcontrol.UpdateModeNaive {
if updMode == buildcontrol.UpdateModeImage {
return BuildOrder{dcbad, ibad, ltbad}
}

if updMode == buildcontrol.UpdateModeSynclet || shouldUseSynclet(updMode, env, runtime) {
if updMode == buildcontrol.UpdateModeSynclet {
ibad.SetInjectSynclet(true)
}

return BuildOrder{lubad, dcbad, ibad, ltbad}
}

func shouldUseSynclet(updMode buildcontrol.UpdateMode, env k8s.Env, runtime container.Runtime) bool {
return updMode == buildcontrol.UpdateModeAuto && !env.UsesLocalDockerRegistry() && runtime == container.RuntimeDocker
}
132 changes: 120 additions & 12 deletions internal/engine/build_and_deployer_live_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/stretchr/testify/require"
"k8s.io/client-go/util/exec"

"github.com/tilt-dev/tilt/internal/engine/buildcontrol"

"github.com/tilt-dev/tilt/internal/k8s/testyaml"

"github.com/tilt-dev/tilt/internal/docker"
Expand Down Expand Up @@ -219,7 +221,7 @@ func TestLiveUpdateDockerBuildLocalContainerSameImgMultipleContainers(t *testing
}

func TestLiveUpdateDockerBuildSyncletSameImgMultipleContainers(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

m := NewSanchoLiveUpdateManifest(f)
Expand Down Expand Up @@ -292,7 +294,7 @@ func TestLiveUpdateDockerBuildLocalContainerDiffImgMultipleContainers(t *testing
}

func TestLiveUpdateDockerBuildSyncletDiffImgMultipleContainers(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

sanchoTarg := NewSanchoLiveUpdateImageTarget(f)
Expand Down Expand Up @@ -504,7 +506,7 @@ func TestLiveUpdateDockerContainerUserRunFailureDoesntFallBack(t *testing.T) {
// TODO(maia): make this test actually touch gRPC de/serialization,
// which is where most of this logic lives
func TestLiveUpdateSyncletUserRunFailureDoesntFallBack(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

f.docker.SetExecError(userFailureErrDocker)
Expand Down Expand Up @@ -777,7 +779,7 @@ func TestLiveUpdateRunTriggerLocalContainer(t *testing.T) {
}

func TestLiveUpdateRunTriggerSynclet(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

runs := []model.LiveUpdateRunStep{
Expand All @@ -803,10 +805,38 @@ func TestLiveUpdateRunTriggerSynclet(t *testing.T) {
runTestCase(t, f, tCase)
}

func TestLiveUpdateDockerBuildSynclet(t *testing.T) {
func TestLiveUpdateRunTriggerExec(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
defer f.TearDown()

runs := []model.LiveUpdateRunStep{
model.LiveUpdateRunStep{Command: model.ToUnixCmd("echo hello")},
model.LiveUpdateRunStep{Command: model.ToUnixCmd("echo a"), Triggers: f.NewPathSet("a.txt")}, // matches changed file
model.LiveUpdateRunStep{Command: model.ToUnixCmd("echo b"), Triggers: f.NewPathSet("b.txt")}, // does NOT match changed file
}
lu := assembleLiveUpdate(SanchoSyncSteps(f), runs, false, nil, f)
tCase := testCase{
manifest: manifestbuilder.New(f, "sancho").
WithK8sYAML(SanchoYAML).
WithImageTarget(NewSanchoDockerBuildImageTarget(f)).
WithLiveUpdate(lu).
Build(),
changedFiles: []string{"a.txt"},
expectDockerBuildCount: 0,
expectDockerPushCount: 0,
expectSyncletUpdateContainerCount: 0,
expectDockerCopyCount: 0,
expectDockerExecCount: 0, // one run's triggers don't match -- should only exec the other two.
expectDockerRestartCount: 0,
expectK8sExecCount: 3, // one copy, two runs (third run's triggers don't match so don't exec it)
}
runTestCase(t, f, tCase)
}

func TestLiveUpdateDockerBuildSynclet(t *testing.T) {
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

tCase := testCase{
manifest: NewSanchoLiveUpdateManifest(f),
changedFiles: []string{"a.txt"},
Expand All @@ -821,7 +851,7 @@ func TestLiveUpdateDockerBuildSynclet(t *testing.T) {
}

func TestLiveUpdateCustomBuildSynclet(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

lu := assembleLiveUpdate(SanchoSyncSteps(f), SanchoRunSteps, true, nil, f)
Expand All @@ -842,10 +872,33 @@ func TestLiveUpdateCustomBuildSynclet(t *testing.T) {
runTestCase(t, f, tCase)
}

func TestLiveUpdateHotReloadSynclet(t *testing.T) {
func TestLiveUpdateCustomBuildExec(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
defer f.TearDown()

lu := assembleLiveUpdate(SanchoSyncSteps(f), SanchoRunSteps, false, nil, f)
tCase := testCase{
manifest: manifestbuilder.New(f, "sancho").
WithK8sYAML(SanchoYAML).
WithImageTarget(NewSanchoCustomBuildImageTarget(f)).
WithLiveUpdate(lu).
Build(),
changedFiles: []string{"app/a.txt"},
expectDockerBuildCount: 0,
expectDockerPushCount: 0,
expectSyncletUpdateContainerCount: 0,
expectDockerCopyCount: 0,
expectDockerExecCount: 0,
expectDockerRestartCount: 0,
expectK8sExecCount: 2,
}
runTestCase(t, f, tCase)
}

func TestLiveUpdateHotReloadSynclet(t *testing.T) {
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

lu := assembleLiveUpdate(SanchoSyncSteps(f), SanchoRunSteps, false, nil, f)
tCase := testCase{
manifest: manifestbuilder.New(f, "sancho").
Expand Down Expand Up @@ -901,13 +954,14 @@ func TestLiveUpdateDockerBuildExec(t *testing.T) {
changedFiles: []string{"a.txt"},
expectDockerBuildCount: 0,
expectDockerPushCount: 0,
expectK8sExecCount: 2, // one tar archive, one run cmd
expectK8sExecCount: 2, // one tar archive, one run cmd
expectSyncletDeploy: false, // exec updater should not deploy synclet
}
runTestCase(t, f, tCase)
}

func TestDockerBuildWithoutLiveUpdateDoesNotDeploySynclet(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

tCase := testCase{
Expand All @@ -922,7 +976,7 @@ func TestDockerBuildWithoutLiveUpdateDoesNotDeploySynclet(t *testing.T) {
}

func TestLiveUpdateDockerBuildDeploysSynclet(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

tCase := testCase{
Expand Down Expand Up @@ -960,7 +1014,7 @@ func TestLiveUpdateLocalContainerFallBackOn(t *testing.T) {
}

func TestLiveUpdateSyncletFallBackOn(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

lu := assembleLiveUpdate(SanchoSyncSteps(f), SanchoRunSteps, true, []string{"a.txt"}, f)
Expand All @@ -983,6 +1037,29 @@ func TestLiveUpdateSyncletFallBackOn(t *testing.T) {
runTestCase(t, f, tCase)
}

func TestLiveUpdateExecFallBackOn(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
defer f.TearDown()

lu := assembleLiveUpdate(SanchoSyncSteps(f), SanchoRunSteps, false, []string{"a.txt"}, f)
tCase := testCase{
manifest: manifestbuilder.New(f, "sancho").
WithK8sYAML(SanchoYAML).
WithImageTarget(NewSanchoDockerBuildImageTarget(f)).
WithLiveUpdate(lu).
Build(),
changedFiles: []string{"a.txt"},
expectDockerBuildCount: 1, // we did a Docker build instead of an in-place update!
expectDockerPushCount: 1,
expectDockerCopyCount: 0,
expectDockerExecCount: 0,
expectDockerRestartCount: 0,
expectK8sDeploy: true, // because we fell back to image builder, we also did a k8s deploy
logsContain: []string{"Detected change to fall_back_on file", "a.txt"},
}
runTestCase(t, f, tCase)
}

func TestLiveUpdateLocalContainerChangedFileNotMatchingSyncFallsBack(t *testing.T) {
f := newBDFixture(t, k8s.EnvDockerDesktop, container.RuntimeDocker)
defer f.TearDown()
Expand Down Expand Up @@ -1015,7 +1092,7 @@ func TestLiveUpdateLocalContainerChangedFileNotMatchingSyncFallsBack(t *testing.
}

func TestLiveUpdateSyncletChangedFileNotMatchingSyncFallsBack(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
f := newBDFixtureWithUpdateMode(t, k8s.EnvGKE, container.RuntimeDocker, buildcontrol.UpdateModeSynclet)
defer f.TearDown()

steps := []model.LiveUpdateSyncStep{model.LiveUpdateSyncStep{
Expand Down Expand Up @@ -1046,6 +1123,37 @@ func TestLiveUpdateSyncletChangedFileNotMatchingSyncFallsBack(t *testing.T) {
runTestCase(t, f, tCase)
}

func TestLiveUpdateExecChangedFileNotMatchingSyncFallsBack(t *testing.T) {
f := newBDFixture(t, k8s.EnvGKE, container.RuntimeDocker)
defer f.TearDown()

steps := []model.LiveUpdateSyncStep{model.LiveUpdateSyncStep{
Source: f.JoinPath("specific/directory"),
Dest: "/go/src/github.com/tilt-dev/sancho",
}}

lu := assembleLiveUpdate(steps, SanchoRunSteps, false, []string{"a.txt"}, f)
tCase := testCase{
manifest: manifestbuilder.New(f, "sancho").
WithK8sYAML(SanchoYAML).
WithImageTarget(NewSanchoDockerBuildImageTarget(f)).
WithLiveUpdate(lu).
Build(),
changedFiles: []string{f.JoinPath("a.txt")}, // matches context but not sync'd directory

expectDockerBuildCount: 1, // we did a Docker build instead of an in-place update!
expectDockerPushCount: 1,
expectDockerCopyCount: 0,
expectDockerExecCount: 0,
expectDockerRestartCount: 0,
expectK8sDeploy: true, // because we fell back to image builder, we also did a k8s deploy

logsContain: []string{"Found file(s) not matching any sync", "a.txt"},
logsDontContain: []string{"unexpected error"},
}
runTestCase(t, f, tCase)
}

func TestLiveUpdateSomeFilesMatchSyncSomeDontFallsBack(t *testing.T) {
f := newBDFixture(t, k8s.EnvDockerDesktop, container.RuntimeDocker)
defer f.TearDown()
Expand Down
Loading