Skip to content

Commit

Permalink
engine: forcing an update should also force a pod update. Fixes #1928 (
Browse files Browse the repository at this point in the history
  • Loading branch information
nicks committed Jun 8, 2020
1 parent 4ec0ab5 commit 0ba1a83
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 12 deletions.
2 changes: 1 addition & 1 deletion internal/engine/buildcontroller.go
Expand Up @@ -213,7 +213,7 @@ func buildStateSet(ctx context.Context, manifest model.Manifest, specs []model.T
isImageBuildTrigger := reason.HasTrigger() && !isLiveUpdateEligibleTrigger
if isImageBuildTrigger {
for k, v := range result {
result[k] = v.WithImageBuildTriggered(true)
result[k] = v.WithFullBuildTriggered(true)
}
}

Expand Down
6 changes: 3 additions & 3 deletions internal/engine/buildcontroller_test.go
Expand Up @@ -337,7 +337,7 @@ func TestBuildControllerCrashRebuild(t *testing.T) {
f.podEvent(pb.WithContainerID("funnyContainerID").Build(), manifest.Name)
call = f.nextCall()
assert.True(t, call.oneState().OneContainerInfo().Empty())
assert.False(t, call.oneState().ImageBuildTriggered)
assert.False(t, call.oneState().FullBuildTriggered)
f.waitForCompletedBuildCount(3)

f.withManifestState("fe", func(ms store.ManifestState) {
Expand Down Expand Up @@ -556,7 +556,7 @@ func TestBuildControllerImageBuildTrigger(t *testing.T) {
call := f.nextCallComplete()
state := call.oneState()
assert.Equal(t, expectedFiles, state.FilesChanged())
assert.Equal(t, tc.expectedImageBuild, state.ImageBuildTriggered)
assert.Equal(t, tc.expectedImageBuild, state.FullBuildTriggered)

f.WaitUntil("manifest removed from queue", func(st store.EngineState) bool {
for _, mn := range st.TriggerQueue {
Expand Down Expand Up @@ -594,7 +594,7 @@ func TestBuildControllerManualTriggerWithFileChangesSinceLastSuccessfulBuildButB
call := f.nextCallComplete()
state := call.oneState()
assert.Equal(t, []string{}, state.FilesChanged())
assert.True(t, state.ImageBuildTriggered)
assert.True(t, state.FullBuildTriggered)

f.WaitUntil("manifest removed from queue", func(st store.EngineState) bool {
for _, mn := range st.TriggerQueue {
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/extractors.go
Expand Up @@ -48,7 +48,7 @@ func extractImageTargetsForLiveUpdates(specs []model.TargetSpec, stateSet store.
return nil, buildcontrol.SilentRedirectToNextBuilderf("In-place build does not support initial deploy")
}

if state.ImageBuildTriggered {
if state.FullBuildTriggered {
return nil, buildcontrol.SilentRedirectToNextBuilderf("Force update (triggered manually, not automatically, with no dirty files)")
}

Expand Down
23 changes: 23 additions & 0 deletions internal/engine/image_build_and_deployer.go
Expand Up @@ -132,9 +132,23 @@ func (ibd *ImageBuildAndDeployer) BuildAndDeploy(ctx context.Context, st store.R
numStages++
}

hasDeleteStep := stateSet.FullBuildTriggered()
if hasDeleteStep {
numStages++
}

ps := build.NewPipelineState(ctx, numStages, ibd.clock)
defer func() { ps.End(ctx, err) }()

if hasDeleteStep {
ps.StartPipelineStep(ctx, "Force update")
err = ibd.delete(ctx, kTarget)
if err != nil {
return store.BuildResultSet{}, buildcontrol.WrapDontFallBackError(err)
}
ps.EndPipelineStep(ctx)
}

if hasReusedStep {
ps.StartPipelineStep(ctx, "Loading cached images")
for _, result := range reused {
Expand Down Expand Up @@ -297,6 +311,15 @@ func (ibd *ImageBuildAndDeployer) indentLogger(ctx context.Context) context.Cont
return logger.WithLogger(ctx, newL)
}

func (ibd *ImageBuildAndDeployer) delete(ctx context.Context, k8sTarget model.K8sTarget) error {
entities, err := k8s.ParseYAMLFromString(k8sTarget.YAML)
if err != nil {
return err
}

return ibd.k8sClient.Delete(ctx, entities)
}

func (ibd *ImageBuildAndDeployer) createEntitiesToDeploy(ctx context.Context,
iTargetMap map[model.TargetID]model.ImageTarget, k8sTarget model.K8sTarget,
results store.BuildResultSet, needsSynclet bool) ([]k8s.K8sEntity, error) {
Expand Down
17 changes: 17 additions & 0 deletions internal/engine/image_build_and_deployer_test.go
Expand Up @@ -51,6 +51,23 @@ func TestDeployTwinImages(t *testing.T) {
"Expected image to update twice in YAML: %s", f.k8s.Yaml)
}

func TestForceUpdate(t *testing.T) {
f := newIBDFixture(t, k8s.EnvGKE)
defer f.TearDown()

m := NewSanchoDockerBuildManifest(f)

iTargetID1 := m.ImageTargets[0].ID()
stateSet := store.BuildStateSet{
iTargetID1: store.BuildState{FullBuildTriggered: true},
}
_, err := f.ibd.BuildAndDeploy(f.ctx, f.st, buildTargets(m), stateSet)
require.NoError(t, err)

// A force rebuild should delete the old resources.
assert.Equal(t, 1, strings.Count(f.k8s.DeletedYaml, "Deployment"))
}

func TestDeployPodWithMultipleImages(t *testing.T) {
f := newIBDFixture(t, k8s.EnvGKE)
defer f.TearDown()
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/live_update_build_and_deployer_test.go
Expand Up @@ -334,7 +334,7 @@ func TestSkipLiveUpdateIfForceUpdate(t *testing.T) {
state := store.BuildState{
LastSuccessfulResult: alreadyBuilt,
RunningContainers: []store.ContainerInfo{cInfo},
ImageBuildTriggered: true, // should make us skip LiveUpdate
FullBuildTriggered: true, // should make us skip LiveUpdate
}

stateSet := store.BuildStateSet{m.ImageTargetAt(0).ID(): state}
Expand Down
3 changes: 2 additions & 1 deletion internal/k8s/client.go
Expand Up @@ -370,8 +370,9 @@ func maybeShouldTryReplaceReason(stderr string) (string, bool) {
// behavior for our use cases.
func (k K8sClient) Delete(ctx context.Context, entities []K8sEntity) error {
l := logger.Get(ctx)
l.Infof("Deleting via kubectl:")
for _, e := range entities {
l.Infof("Deleting via kubectl: %s/%s\n", e.GVK().Kind, e.Name())
l.Infof("%s/%s", e.GVK().Kind, e.Name())
}

_, stderr, err := k.actOnEntities(ctx, []string{"delete", "--ignore-not-found"}, entities)
Expand Down
17 changes: 13 additions & 4 deletions internal/store/build_result.go
Expand Up @@ -289,7 +289,7 @@ type BuildState struct {
//
// This field indicates case 1 || case 2 -- i.e. that we should skip
// live_update, and force an image build (even if there are no changed files)
ImageBuildTriggered bool
FullBuildTriggered bool

RunningContainers []ContainerInfo

Expand Down Expand Up @@ -318,8 +318,8 @@ func (b BuildState) WithRunningContainerError(err error) BuildState {
return b
}

func (b BuildState) WithImageBuildTriggered(isImageBuildTrigger bool) BuildState {
b.ImageBuildTriggered = isImageBuildTrigger
func (b BuildState) WithFullBuildTriggered(isImageBuildTrigger bool) BuildState {
b.FullBuildTriggered = isImageBuildTrigger
return b
}

Expand Down Expand Up @@ -366,11 +366,20 @@ func (b BuildState) HasLastSuccessfulResult() bool {
func (b BuildState) NeedsImageBuild() bool {
lastBuildWasImgBuild := b.LastSuccessfulResult != nil &&
b.LastSuccessfulResult.BuildType() == model.BuildTypeImage
return !lastBuildWasImgBuild || len(b.FilesChangedSet) > 0 || b.ImageBuildTriggered
return !lastBuildWasImgBuild || len(b.FilesChangedSet) > 0 || b.FullBuildTriggered
}

type BuildStateSet map[model.TargetID]BuildState

func (set BuildStateSet) FullBuildTriggered() bool {
for _, state := range set {
if state.FullBuildTriggered {
return true
}
}
return false
}

func (set BuildStateSet) Empty() bool {
return len(set) == 0
}
Expand Down
2 changes: 1 addition & 1 deletion web/src/SidebarTriggerButton.tsx
Expand Up @@ -23,7 +23,7 @@ export const TriggerButtonTooltip = {
"This manual resource has pending file changes; click to trigger an update.",
UpdateInProgOrPending:
"Cannot trigger an update while resource is updating or update is pending.",
ClickToForce: "Force a rebuild/update for this resource.",
ClickToForce: "Force an update for this resource.",
CannotTriggerTiltfile: "Cannot trigger an update to the Tiltfile.",
}

Expand Down

0 comments on commit 0ba1a83

Please sign in to comment.