diff --git a/internal/container/container.go b/internal/container/container.go index 2876359d66..30dfe3a1fb 100644 --- a/internal/container/container.go +++ b/internal/container/container.go @@ -73,11 +73,3 @@ func MustWithTag(name reference.Named, tag string) reference.NamedTagged { } return nt } - -func NewIDSet(ids ...ID) map[ID]bool { - result := make(map[ID]bool, len(ids)) - for _, id := range ids { - result[id] = true - } - return result -} diff --git a/internal/engine/buildcontrol/build_control.go b/internal/engine/buildcontrol/build_control.go index 6173d83ade..64a198e8e7 100644 --- a/internal/engine/buildcontrol/build_control.go +++ b/internal/engine/buildcontrol/build_control.go @@ -88,13 +88,6 @@ func NextTargetToBuild(state store.EngineState) (*store.ManifestTarget, HoldSet) return NextUnbuiltTargetToBuild(unbuilt), holds } - // Next prioritize builds that crashed and need a rebuilt to have up-to-date code. - for _, mt := range targets { - if mt.State.NeedsRebuildFromCrash { - return mt, holds - } - } - // Check to see if any targets are currently being successfully reconciled, // and so full rebuilt should be held back. This takes manual triggers into account. HoldLiveUpdateTargetsHandledByReconciler(state, targets, holds) @@ -180,7 +173,7 @@ func canReuseImageTargetHeuristic(spec model.TargetSpec, status store.BuildStatu } switch result.(type) { - case store.ImageBuildResult, store.LiveUpdateBuildResult: + case store.ImageBuildResult: return true } return false @@ -466,11 +459,6 @@ func FindTargetsNeedingAnyBuild(state store.EngineState) []*store.ManifestTarget continue } - if target.State.NeedsRebuildFromCrash { - result = append(result, target) - continue - } - if queue[target.Manifest.Name] { result = append(result, target) continue diff --git a/internal/engine/buildcontrol/build_control_test.go b/internal/engine/buildcontrol/build_control_test.go index 0aa2b46dec..46cf74010f 100644 --- a/internal/engine/buildcontrol/build_control_test.go +++ b/internal/engine/buildcontrol/build_control_test.go @@ -28,7 +28,7 @@ import ( func TestNextTargetToBuildDoesntReturnCurrentlyBuildingTarget(t *testing.T) { f := newTestFixture(t) - mt := f.manifestNeedingCrashRebuild() + mt := f.upsertK8sManifest("k8s1") f.st.UpsertManifestTarget(mt) // Verify this target is normally next-to-build @@ -741,22 +741,6 @@ func (f *testFixture) upsertLocalManifest(name model.ManifestName, opts ...manif return f.upsertManifest(b.WithLocalResource(fmt.Sprintf("exec-%s", name), nil).Build()) } -func (f *testFixture) manifestNeedingCrashRebuild() *store.ManifestTarget { - m := manifestbuilder.New(f, "needs-crash-rebuild"). - WithK8sYAML(testyaml.SanchoYAML). - Build() - mt := store.NewManifestTarget(m) - mt.State.BuildHistory = []model.BuildRecord{ - model.BuildRecord{ - StartTime: time.Now().Add(-5 * time.Second), - FinishTime: time.Now(), - }, - } - mt.State.NeedsRebuildFromCrash = true - mt.State.DisableState = v1alpha1.DisableStateEnabled - return mt -} - type manifestOption func(manifestbuilder.ManifestBuilder) manifestbuilder.ManifestBuilder func withResourceDeps(deps ...string) manifestOption { diff --git a/internal/engine/buildcontrol/image_build_and_deployer_test.go b/internal/engine/buildcontrol/image_build_and_deployer_test.go index 05878b7b84..37af863d6e 100644 --- a/internal/engine/buildcontrol/image_build_and_deployer_test.go +++ b/internal/engine/buildcontrol/image_build_and_deployer_test.go @@ -258,29 +258,6 @@ func TestImageIsClean(t *testing.T) { assert.Equal(t, 0, f.docker.PushCount) } -func TestImageIsDirtyAfterContainerBuild(t *testing.T) { - f := newIBDFixture(t, clusterid.ProductGKE) - - manifest := NewSanchoDockerBuildManifest(f) - iTargetID1 := manifest.ImageTargets[0].ID() - result1 := store.NewLiveUpdateBuildResult( - iTargetID1, - []container.ID{container.ID("12345")}) - - stateSet := store.BuildStateSet{ - iTargetID1: store.NewBuildState(result1, []string{}, nil), - } - _, err := f.BuildAndDeploy(BuildTargets(manifest), stateSet) - if err != nil { - t.Fatal(err) - } - - // Expect build + push; last result has a container ID, which implies that it was an in-place - // update, so the current state of this manifest is NOT reflected in an existing image. - assert.Equal(t, 1, f.docker.BuildCount) - assert.Equal(t, 1, f.docker.PushCount) -} - func TestMultiStageDockerBuild(t *testing.T) { f := newIBDFixture(t, clusterid.ProductGKE) diff --git a/internal/engine/buildcontroller.go b/internal/engine/buildcontroller.go index 5ff28d33f6..0b91057664 100644 --- a/internal/engine/buildcontroller.go +++ b/internal/engine/buildcontroller.go @@ -233,18 +233,16 @@ func buildStateSet(ctx context.Context, manifest model.Manifest, // // This will probably need to change as the mapping between containers and // manifests becomes many-to-one. - if !ms.NeedsRebuildFromCrash { - iTarget, ok := spec.(model.ImageTarget) - if ok { - selector := iTarget.LiveUpdateSpec.Selector - if manifest.IsK8s() && selector.Kubernetes != nil { - buildState.KubernetesSelector = selector.Kubernetes - buildState.KubernetesResource = kresource - } - - if manifest.IsDC() { - buildState.DockerComposeService = dcs - } + iTarget, ok := spec.(model.ImageTarget) + if ok { + selector := iTarget.LiveUpdateSpec.Selector + if manifest.IsK8s() && selector.Kubernetes != nil { + buildState.KubernetesSelector = selector.Kubernetes + buildState.KubernetesResource = kresource + } + + if manifest.IsDC() { + buildState.DockerComposeService = dcs } } result[id] = buildState diff --git a/internal/engine/buildcontroller_test.go b/internal/engine/buildcontroller_test.go index 785e23af36..ac4515c150 100644 --- a/internal/engine/buildcontroller_test.go +++ b/internal/engine/buildcontroller_test.go @@ -232,52 +232,6 @@ func TestBuildControllerImageBuildTrigger(t *testing.T) { } } -// Make sure we don't try display messages about live update after a full build trigger. -// https://github.com/tilt-dev/tilt/issues/3915 -func TestFullBuildTriggerClearsLiveUpdate(t *testing.T) { - f := newTestFixture(t) - mName := model.ManifestName("foobar") - - manifest := f.newManifest(mName.String()) - basePB := f.registerForDeployer(manifest) - opt := func(ia InitAction) InitAction { - ia.TerminalMode = store.TerminalModeStream - return ia - } - f.Start([]model.Manifest{manifest}, opt) - - f.nextCallComplete() - - f.podEvent(basePB.Build()) - f.WaitUntilManifestState("foobar loaded", "foobar", func(ms store.ManifestState) bool { - return ms.K8sRuntimeState().PodLen() == 1 - }) - f.WaitUntil("foobar k8sresource loaded", func(s store.EngineState) bool { - return s.KubernetesResources["foobar"] != nil && len(s.KubernetesResources["foobar"].FilteredPods) == 1 - }) - f.withManifestState("foobar", func(ms store.ManifestState) { - ms.LiveUpdatedContainerIDs["containerID"] = true - }) - - f.b.completeBuildsManually = true - f.store.Dispatch(server.AppendToTriggerQueueAction{Name: mName}) - f.WaitUntilManifestState("foobar building", "foobar", func(ms store.ManifestState) bool { - return ms.IsBuilding() - }) - - f.podEvent(basePB.WithDeletionTime(time.Now()).Build()) - f.WaitUntilManifestState("foobar deleting", "foobar", func(ms store.ManifestState) bool { - return ms.K8sRuntimeState().PodLen() == 0 - }) - assert.Contains(t, f.log.String(), "Initial Build") - f.WaitUntil("Trigger appears", func(st store.EngineState) bool { - return strings.Contains(f.log.String(), "Unknown Trigger") - }) - assert.NotContains(t, f.log.String(), "Detected a container change") - - f.completeBuildForManifest(manifest) -} - func TestBuildQueueOrdering(t *testing.T) { f := newTestFixture(t) diff --git a/internal/engine/upper_test.go b/internal/engine/upper_test.go index 43fa4cdd90..b1c7032886 100644 --- a/internal/engine/upper_test.go +++ b/internal/engine/upper_test.go @@ -222,10 +222,6 @@ type fakeBuildAndDeployer struct { buildCount int - // Set this to simulate a container update that returns the container IDs - // it updated. - nextLiveUpdateContainerIDs []container.ID - // Inject the container ID of the container started by Docker Compose. // If not set, we will auto-generate an ID. nextDockerComposeContainerID container.ID @@ -239,13 +235,6 @@ type fakeBuildAndDeployer struct { // Do not set this directly, use fixture.SetNextBuildError nextBuildError error - // Set this to simulate a live-update compile error - // - // This is slightly different than a compile error, because the containers are - // still running with the synced files. The build system returns a - // result with the container ID. - nextLiveUpdateCompileError error - buildLogOutput map[model.TargetID]string resultsByID store.BuildResultSet @@ -350,14 +339,7 @@ func (b *fakeBuildAndDeployer) BuildAndDeploy(ctx context.Context, st store.RSto return result, err } - containerIDs := b.nextLiveUpdateContainerIDs - if len(containerIDs) > 0 { - for k := range result { - result[k] = store.NewLiveUpdateBuildResult(k, containerIDs) - } - } - - if !call.dc().Empty() && len(b.nextLiveUpdateContainerIDs) == 0 { + if !call.dc().Empty() { dcContainerID := container.ID(fmt.Sprintf("dc-%s", path.Base(call.dc().ID().Name.String()))) if b.nextDockerComposeContainerID != "" { dcContainerID = b.nextDockerComposeContainerID @@ -383,16 +365,13 @@ func (b *fakeBuildAndDeployer) BuildAndDeploy(ctx context.Context, st store.RSto result[call.k8s().ID()] = nextK8sResult } - err = b.nextLiveUpdateCompileError - b.nextLiveUpdateCompileError = nil - b.nextLiveUpdateContainerIDs = nil b.nextDockerComposeContainerID = "" for key, val := range result { b.resultsByID[key] = val } - return result, err + return result, nil } func (b *fakeBuildAndDeployer) updateKubernetesApplyStatus(ctx context.Context, kTarg model.K8sTarget, iTargets []model.ImageTarget) error { @@ -3647,21 +3626,6 @@ func (f *testFixture) SetNextBuildError(err error) { f.store.RUnlockState() } -func (f *testFixture) SetNextLiveUpdateCompileError(err error, containerIDs []container.ID) { - f.WaitUntil("any in-flight builds have hit the buildAndDeployer", func(state store.EngineState) bool { - f.b.mu.Lock() - defer f.b.mu.Unlock() - return f.b.buildCount == state.BuildControllerStartCount - }) - - _ = f.store.RLockState() - f.b.mu.Lock() - f.b.nextLiveUpdateCompileError = err - f.b.nextLiveUpdateContainerIDs = containerIDs - f.b.mu.Unlock() - f.store.RUnlockState() -} - // Wait until the given view test passes. func (f *testFixture) WaitUntilHUD(msg string, isDone func(view.View) bool) { f.fakeHud().WaitUntil(f.T(), f.ctx, msg, isDone) diff --git a/internal/hud/buildstatus.go b/internal/hud/buildstatus.go index e21cdbda14..a14a84b1b9 100644 --- a/internal/hud/buildstatus.go +++ b/internal/hud/buildstatus.go @@ -39,12 +39,12 @@ func makeBuildStatus(res view.Resource, triggerMode model.TriggerMode) buildStat } } - if !res.CurrentBuild.Empty() && !res.CurrentBuild.Reason.IsCrashOnly() { + if !res.CurrentBuild.Empty() { status = "In prog." duration = time.Since(res.CurrentBuild.StartTime) edits = res.CurrentBuild.Edits reason = res.CurrentBuild.Reason - } else if !res.PendingBuildSince.IsZero() && !res.PendingBuildReason.IsCrashOnly() { + } else if !res.PendingBuildSince.IsZero() { status = "Pending" if triggerMode.AutoOnChange() { duration = time.Since(res.PendingBuildSince) diff --git a/internal/hud/edits.go b/internal/hud/edits.go index 0979dac75e..33eb9dc9af 100644 --- a/internal/hud/edits.go +++ b/internal/hud/edits.go @@ -87,8 +87,6 @@ func (esl *EditStatusLineComponent) Render(w rty.Writer, width, height int) erro if len(bs.edits) == 0 { if bs.reason.Has(model.BuildReasonFlagInit) { sb.Fg(cLightText).Text("FIRST BUILD ") - } else if bs.reason.Has(model.BuildReasonFlagCrash) { - sb.Fg(cLightText).Text("CRASH BUILD ") } } else { sb.Fg(cLightText).Text("EDITED FILES ") diff --git a/internal/hud/renderer.go b/internal/hud/renderer.go index 9b50cab2bb..465e78d548 100644 --- a/internal/hud/renderer.go +++ b/internal/hud/renderer.go @@ -11,7 +11,6 @@ import ( "github.com/tilt-dev/tilt/internal/hud/view" "github.com/tilt-dev/tilt/internal/rty" "github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1" - "github.com/tilt-dev/tilt/pkg/model" ) const defaultLogPaneHeight = 8 @@ -202,9 +201,6 @@ func isInError(res view.Resource) bool { func isCrashing(res view.Resource) bool { return (res.IsK8s() && res.K8sInfo().PodRestarts > 0) || - res.LastBuild().Reason.Has(model.BuildReasonFlagCrash) || - res.CurrentBuild.Reason.Has(model.BuildReasonFlagCrash) || - res.PendingBuildReason.Has(model.BuildReasonFlagCrash) || res.IsDC() && res.DockerComposeTarget().RuntimeStatus() == v1alpha1.RuntimeStatusError } diff --git a/internal/hud/renderer_test.go b/internal/hud/renderer_test.go index 5f8c427287..22309ac0fa 100644 --- a/internal/hud/renderer_test.go +++ b/internal/hud/renderer_test.go @@ -167,28 +167,6 @@ ERROR: ImageBuild: executor failed running [/bin/sh -c go install github.com/til rtf.run("all the data at once 50w", 50, 20, v, plainVs) rtf.run("all the data at once 10w", 10, 20, v, plainVs) - v = newView(view.Resource{ - Name: "abe vigoda", - LastDeployTime: ts, - BuildHistory: []model.BuildRecord{{ - Edits: []string{"main.go"}, - }}, - PendingBuildSince: ts, - CurrentBuild: model.BuildRecord{ - StartTime: ts, - Reason: model.BuildReasonFlagCrash, - }, - ResourceInfo: view.K8sResourceInfo{ - PodName: "vigoda-pod", - PodCreationTime: ts, - PodStatus: "Running", - RunStatus: v1alpha1.RuntimeStatusOK, - PodRestarts: 0, - }, - Endpoints: []string{"1.2.3.4:8080"}, - }) - rtf.run("crash rebuild", 70, 20, v, plainVs) - v = newView(view.Resource{ Name: "vigoda", LastDeployTime: ts, @@ -369,76 +347,6 @@ func TestPodPending(t *testing.T) { combinedStatus(v.Resources[0])) } -func TestCrashingPodInlineCrashLog(t *testing.T) { - rtf := newRendererTestFixture(t) - ts := time.Now().Add(-30 * time.Second) - - v := newView(view.Resource{ - Name: "vigoda", - Endpoints: []string{"1.2.3.4:8080"}, - BuildHistory: []model.BuildRecord{{ - SpanID: "vigoda:1", - StartTime: ts, - FinishTime: ts, - Reason: model.BuildReasonFlagCrash, - }}, - ResourceInfo: view.K8sResourceInfo{ - PodName: "vigoda-pod", - PodStatus: "Error", - RunStatus: v1alpha1.RuntimeStatusError, - SpanID: "vigoda:pod", - PodUpdateStartTime: ts, - PodCreationTime: ts.Add(-time.Minute), - }, - LastDeployTime: ts, - }) - - logStore := logstore.NewLogStore() - appendSpanLog(logStore, "vigoda", "vigoda:1", - "Building (1/2)\nBuilding (2/2)\n") - appendSpanLog(logStore, "vigoda", "vigoda:pod", - "Something's maybe wrong idk") - v.LogReader = logstore.NewReader(&sync.RWMutex{}, logStore) - - vs := fakeViewState(1, view.CollapseAuto) - rtf.run("crashing pod displays crash log inline if present", 70, 20, v, vs) -} - -func TestCrashingPodInlinePodLogIfNoCrashLog(t *testing.T) { - rtf := newRendererTestFixture(t) - ts := time.Now().Add(-30 * time.Second) - - v := newView(view.Resource{ - Name: "vigoda", - Endpoints: []string{"1.2.3.4:8080"}, - BuildHistory: []model.BuildRecord{{ - SpanID: "vigoda:1", - StartTime: ts, - FinishTime: ts, - Reason: model.BuildReasonFlagCrash, - }}, - ResourceInfo: view.K8sResourceInfo{ - PodName: "vigoda-pod", - PodStatus: "Error", - RunStatus: v1alpha1.RuntimeStatusError, - SpanID: "vigoda:pod", - PodUpdateStartTime: ts, - PodCreationTime: ts.Add(-time.Minute), - }, - LastDeployTime: ts, - }) - - logStore := logstore.NewLogStore() - appendSpanLog(logStore, "vigoda", "vigoda:1", - "Building (1/2)\nBuilding (2/2)\n") - appendSpanLog(logStore, "vigoda", "vigoda:pod", - "Something's maybe wrong idk") - v.LogReader = logstore.NewReader(&sync.RWMutex{}, logStore) - - vs := fakeViewState(1, view.CollapseAuto) - rtf.run("crashing pod displays pod log inline if no crash log if present", 70, 20, v, vs) -} - func TestNonCrashingPodNoInlineCrashLog(t *testing.T) { rtf := newRendererTestFixture(t) ts := time.Now().Add(-30 * time.Second) diff --git a/internal/hud/view/view.go b/internal/hud/view/view.go index 9167986fd6..20faf4f776 100644 --- a/internal/hud/view/view.go +++ b/internal/hud/view/view.go @@ -186,10 +186,7 @@ func (r Resource) DefaultCollapse() bool { autoExpand = autoExpand || r.LastBuild().Error != nil || - r.LastBuild().WarningCount > 0 || - r.LastBuild().Reason.Has(model.BuildReasonFlagCrash) || - r.CurrentBuild.Reason.Has(model.BuildReasonFlagCrash) || - r.PendingBuildReason.Has(model.BuildReasonFlagCrash) + r.LastBuild().WarningCount > 0 return !autoExpand } diff --git a/internal/hud/webview/convert_test.go b/internal/hud/webview/convert_test.go index 054f25e12c..9f3be86599 100644 --- a/internal/hud/webview/convert_test.go +++ b/internal/hud/webview/convert_test.go @@ -341,7 +341,7 @@ func TestBuildHistory(t *testing.T) { br3 := model.BuildRecord{ StartTime: time.Now().Add(-20 * time.Minute), FinishTime: time.Now().Add(-19 * time.Minute), - Reason: model.BuildReasonFlagCrash, + Reason: model.BuildReasonFlagChangedFiles, BuildTypes: []model.BuildType{model.BuildTypeImage, model.BuildTypeK8s}, } buildRecords := []model.BuildRecord{br1, br2, br3} diff --git a/internal/hud/webview/view.go b/internal/hud/webview/view.go index abea7b0e01..1eb3f2840a 100644 --- a/internal/hud/webview/view.go +++ b/internal/hud/webview/view.go @@ -80,7 +80,7 @@ func ToBuildTerminated(br model.BuildRecord, logStore *logstore.LogStore) v1alph Warnings: warnings, StartTime: metav1.NewMicroTime(br.StartTime), FinishTime: metav1.NewMicroTime(br.FinishTime), - IsCrashRebuild: br.Reason.IsCrashOnly(), + IsCrashRebuild: false, SpanID: string(br.SpanID), } } diff --git a/internal/store/build_result.go b/internal/store/build_result.go index 1aaa7b52b1..36509f8e6d 100644 --- a/internal/store/build_result.go +++ b/internal/store/build_result.go @@ -67,35 +67,6 @@ func NewImageBuildResultSingleRef(id model.TargetID, ref reference.NamedTagged) return NewImageBuildResult(id, ref, ref) } -// TODO(nick): In the old live-update system, keeping track of the LiveUpdateBuildResult -// allowed us to track when a container had crashed or replaced. -// -// In the reconciler-based live-update system, the reconciler keeps track of which -// containers it has live-updated internally and whether it can recover from a crash, -// then updates its status on whether its healthy. So tracking live updated containers -// separately becomes unnecessary. We can remove this code once the BuildAndDeploy -// live-updater is deleted. -type LiveUpdateBuildResult struct { - id model.TargetID - - // The ID of the container(s) that we live-updated in-place. - // - // The contents of the container have diverged from the image it's built on, - // so we need to keep track of that. - LiveUpdatedContainerIDs []container.ID -} - -func (r LiveUpdateBuildResult) TargetID() model.TargetID { return r.id } -func (r LiveUpdateBuildResult) BuildType() model.BuildType { return model.BuildTypeLiveUpdate } - -// For in-place container updates. -func NewLiveUpdateBuildResult(id model.TargetID, containerIDs []container.ID) LiveUpdateBuildResult { - return LiveUpdateBuildResult{ - id: id, - LiveUpdatedContainerIDs: containerIDs, - } -} - type DockerComposeBuildResult struct { id model.TargetID @@ -150,17 +121,6 @@ func ClusterImageRefFromBuildResult(r BuildResult) string { type BuildResultSet map[model.TargetID]BuildResult -func (set BuildResultSet) LiveUpdatedContainerIDs() []container.ID { - result := []container.ID{} - for _, r := range set { - r, ok := r.(LiveUpdateBuildResult) - if ok { - result = append(result, r.LiveUpdatedContainerIDs...) - } - } - return result -} - func (set BuildResultSet) ApplyFilter() *k8sconv.KubernetesApplyFilter { for _, r := range set { r, ok := r.(K8sBuildResult) @@ -196,38 +156,6 @@ func (set BuildResultSet) BuildTypes() []model.BuildType { return result } -// Returns a container ID iff it's the only container ID in the result set. -// If there are multiple container IDs, we have to give up. -func (set BuildResultSet) OneAndOnlyLiveUpdatedContainerID() container.ID { - var id container.ID - for _, br := range set { - result, ok := br.(LiveUpdateBuildResult) - if !ok { - continue - } - - if len(result.LiveUpdatedContainerIDs) == 0 { - continue - } - - if len(result.LiveUpdatedContainerIDs) > 1 { - return "" - } - - curID := result.LiveUpdatedContainerIDs[0] - if curID == "" { - continue - } - - if id != "" && curID != id { - return "" - } - - id = curID - } - return id -} - // A BuildResultSet that can only hold image build results. type ImageBuildResultSet map[model.TargetID]ImageBuildResult diff --git a/internal/store/build_result_test.go b/internal/store/build_result_test.go deleted file mode 100644 index 7dacc1229c..0000000000 --- a/internal/store/build_result_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package store - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/tilt-dev/tilt/internal/container" - "github.com/tilt-dev/tilt/pkg/model" -) - -func imageID(s string) model.TargetID { - return model.TargetID{ - Type: model.TargetTypeImage, - Name: model.TargetName(s), - } -} - -func TestOneAndOnlyLiveUpdatedContainerID(t *testing.T) { - set := BuildResultSet{ - imageID("a"): NewLiveUpdateBuildResult(imageID("a"), []container.ID{"cA"}), - imageID("b"): NewLiveUpdateBuildResult(imageID("b"), []container.ID{"cB"}), - } - assert.Equal(t, "", string(set.OneAndOnlyLiveUpdatedContainerID())) - - set = BuildResultSet{ - imageID("a"): NewLiveUpdateBuildResult(imageID("a"), []container.ID{"cA"}), - imageID("b"): NewLiveUpdateBuildResult(imageID("b"), []container.ID{"cA"}), - imageID("c"): NewLiveUpdateBuildResult(imageID("c"), []container.ID{""}), - } - assert.Equal(t, "cA", string(set.OneAndOnlyLiveUpdatedContainerID())) -} diff --git a/internal/store/buildcontrols/reducers.go b/internal/store/buildcontrols/reducers.go index ed7211db67..5464732fd5 100644 --- a/internal/store/buildcontrols/reducers.go +++ b/internal/store/buildcontrols/reducers.go @@ -10,7 +10,6 @@ import ( "github.com/tilt-dev/tilt/internal/engine/runtimelog" "github.com/tilt-dev/tilt/internal/k8s" "github.com/tilt-dev/tilt/internal/store" - "github.com/tilt-dev/tilt/internal/store/liveupdates" "github.com/tilt-dev/tilt/internal/timecmp" "github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1" "github.com/tilt-dev/tilt/pkg/logger" @@ -64,19 +63,6 @@ func HandleBuildStarted(ctx context.Context, state *store.EngineState, action Bu ms.RuntimeState = state } - // If this is a full build, we know all the containers will get replaced, - // so just reset them now. - // - // NOTE(nick): Currently, this addresses an issue where the full build deletes - // the deployment, which then starts killing pods, which we interpret as a - // crash. A better way to resolve this problem would be to watch for deletions - // directly. But it's still semantically correct to record that we intended to - // delete the containers. - if action.FullBuildTriggered { - // Reset all the container ids - ms.LiveUpdatedContainerIDs = container.NewIDSet() - } - state.RemoveFromTriggerQueue(mn) state.CurrentBuildSet[mn] = true } @@ -211,7 +197,6 @@ func HandleBuildCompleted(ctx context.Context, engineState *store.EngineState, c ms.AddCompletedBuild(bs) delete(ms.CurrentBuilds, cb.Source) - ms.NeedsRebuildFromCrash = false handleBuildResults(engineState, mt, bs, cb.Result) @@ -227,25 +212,6 @@ func HandleBuildCompleted(ctx context.Context, engineState *store.EngineState, c } } - // Track the container ids that have been live-updated whether the - // build succeeds or fails. - liveUpdateContainerIDs := cb.Result.LiveUpdatedContainerIDs() - if len(liveUpdateContainerIDs) == 0 { - // Assume this was an image build, and reset all the container ids - ms.LiveUpdatedContainerIDs = container.NewIDSet() - } else { - for _, cID := range liveUpdateContainerIDs { - ms.LiveUpdatedContainerIDs[cID] = true - } - - krs := ms.K8sRuntimeState() - bestPod := krs.MostRecentPod() - if timecmp.AfterOrEqual(bestPod.CreatedAt, bs.StartTime) || - timecmp.Equal(krs.UpdateStartTime[k8s.PodID(bestPod.Name)], bs.StartTime) { - liveupdates.CheckForContainerCrash(engineState, mn.String()) - } - } - manifest := mt.Manifest if manifest.IsK8s() { state := ms.K8sRuntimeState() diff --git a/internal/store/engine_state.go b/internal/store/engine_state.go index c79f11904c..eee7478192 100644 --- a/internal/store/engine_state.go +++ b/internal/store/engine_state.go @@ -8,7 +8,6 @@ import ( "github.com/tilt-dev/wmclient/pkg/analytics" tiltanalytics "github.com/tilt-dev/tilt/internal/analytics" - "github.com/tilt-dev/tilt/internal/container" "github.com/tilt-dev/tilt/internal/dockercompose" "github.com/tilt-dev/tilt/internal/k8s" "github.com/tilt-dev/tilt/internal/store/k8sconv" @@ -490,14 +489,6 @@ type ManifestState struct { // The last `BuildHistoryLimit` builds. The most recent build is first in the slice. BuildHistory []model.BuildRecord - // The container IDs that we've run a LiveUpdate on, if any. Their contents have - // diverged from the image they are built on. If these container don't appear on - // the pod, we've lost that state and need to rebuild. - LiveUpdatedContainerIDs map[container.ID]bool - - // We detected stale code and are currently doing an image build - NeedsRebuildFromCrash bool - // If this manifest was changed, which config files led to the most recent change in manifest definition ConfigFilesThatCausedChange []string @@ -555,11 +546,10 @@ func NewState() *EngineState { func NewManifestState(m model.Manifest) *ManifestState { mn := m.Name ms := &ManifestState{ - Name: mn, - BuildStatuses: make(map[model.TargetID]*BuildStatus), - LiveUpdatedContainerIDs: container.NewIDSet(), - DisableState: v1alpha1.DisableStatePending, - CurrentBuilds: make(map[string]model.BuildRecord), + Name: mn, + BuildStatuses: make(map[model.TargetID]*BuildStatus), + DisableState: v1alpha1.DisableStatePending, + CurrentBuilds: make(map[string]model.BuildRecord), } if m.IsK8s() { @@ -742,9 +732,6 @@ func (mt *ManifestTarget) NextBuildReason() model.BuildReason { if !mt.State.StartedFirstBuild() && mt.Manifest.TriggerMode.AutoInitial() { reason = reason.With(model.BuildReasonFlagInit) } - if mt.State.NeedsRebuildFromCrash { - reason = reason.With(model.BuildReasonFlagCrash) - } return reason } diff --git a/internal/store/kubernetesdiscoverys/reducers.go b/internal/store/kubernetesdiscoverys/reducers.go index 1f629bb427..c954677e0e 100644 --- a/internal/store/kubernetesdiscoverys/reducers.go +++ b/internal/store/kubernetesdiscoverys/reducers.go @@ -8,7 +8,6 @@ import ( "github.com/tilt-dev/tilt/internal/controllers/apicmp" "github.com/tilt-dev/tilt/internal/store" "github.com/tilt-dev/tilt/internal/store/k8sconv" - "github.com/tilt-dev/tilt/internal/store/liveupdates" "github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1" "github.com/tilt-dev/tilt/pkg/model" ) @@ -30,7 +29,6 @@ func HandleKubernetesDiscoveryUpsertAction(state *store.EngineState, action Kube !apicmp.DeepEqual(oldState.Spec, action.KubernetesDiscovery.Spec) if isChanged { RefreshKubernetesResource(state, n) - liveupdates.CheckForContainerCrash(state, n) } } @@ -41,7 +39,6 @@ func HandleKubernetesDiscoveryDeleteAction(state *store.EngineState, action Kube isChanged := oldState != nil if isChanged { RefreshKubernetesResource(state, action.Name) - liveupdates.CheckForContainerCrash(state, action.Name) } } diff --git a/internal/store/liveupdates/reducers.go b/internal/store/liveupdates/reducers.go index 4382e8eed7..b209df31c6 100644 --- a/internal/store/liveupdates/reducers.go +++ b/internal/store/liveupdates/reducers.go @@ -1,13 +1,7 @@ package liveupdates import ( - "fmt" - - "github.com/tilt-dev/tilt/internal/container" - "github.com/tilt-dev/tilt/internal/controllers/apis/liveupdate" "github.com/tilt-dev/tilt/internal/store" - "github.com/tilt-dev/tilt/pkg/logger" - "github.com/tilt-dev/tilt/pkg/model" ) func HandleLiveUpdateUpsertAction(state *store.EngineState, action LiveUpdateUpsertAction) { @@ -18,63 +12,3 @@ func HandleLiveUpdateUpsertAction(state *store.EngineState, action LiveUpdateUps func HandleLiveUpdateDeleteAction(state *store.EngineState, action LiveUpdateDeleteAction) { delete(state.LiveUpdates, action.Name) } - -// If a container crashes, and it's been live-updated in the past, -// then it needs to enter a special state to indicate that it -// needs to be rebuilt (because the file system has been reset to the original image). -// -// Eventually, this will be represented by a special state on the LiveUpdateStatus. -func CheckForContainerCrash(state *store.EngineState, name string) { - mt, ok := state.ManifestTargets[model.ManifestName(name)] - if !ok { - return - } - - ms := mt.State - if ms.NeedsRebuildFromCrash { - // We're already aware the pod is crashing. - return - } - - // In LiveUpdate V2, if a container crashes, the reconciler - // will wait for it to restart and re-sync the files that have - // changed since the last image build. - // - // If the container is in crash-rebuild mode, the reconciler will - // put it in an unrecoverable state. The next time we see a file change - // or trigger, we'll rebuild the image from scratch. - for _, iTarget := range mt.Manifest.ImageTargets { - if !liveupdate.IsEmptySpec(iTarget.LiveUpdateSpec) && iTarget.LiveUpdateReconciler { - return - } - } - - runningContainers := AllRunningContainers(mt, state) - if len(runningContainers) == 0 { - // If there are no running containers, it might mean the containers are - // being deleted. We don't need to intervene yet. - return - } - - hitList := make(map[container.ID]bool, len(ms.LiveUpdatedContainerIDs)) - for cID := range ms.LiveUpdatedContainerIDs { - hitList[cID] = true - } - for _, c := range runningContainers { - delete(hitList, c.ContainerID) - } - - if len(hitList) == 0 { - // The pod is what we expect it to be. - return - } - - // There are new running containers that don't match - // what we live-updated! - ms.NeedsRebuildFromCrash = true - ms.LiveUpdatedContainerIDs = container.NewIDSet() - - msg := fmt.Sprintf("Detected a container change for %s. We could be running stale code. Rebuilding and deploying a new image.", ms.Name) - le := store.NewLogAction(ms.Name, ms.LastBuild().SpanID, logger.WarnLvl, nil, []byte(msg+"\n")) - state.LogStore.Append(le, state.Secrets) -} diff --git a/internal/store/liveupdates/reducers_test.go b/internal/store/liveupdates/reducers_test.go deleted file mode 100644 index ac6578eeaa..0000000000 --- a/internal/store/liveupdates/reducers_test.go +++ /dev/null @@ -1,73 +0,0 @@ -package liveupdates - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/tilt-dev/tilt/internal/container" - "github.com/tilt-dev/tilt/internal/k8s/testyaml" - "github.com/tilt-dev/tilt/internal/store" - "github.com/tilt-dev/tilt/internal/store/k8sconv" - "github.com/tilt-dev/tilt/internal/testutils/manifestbuilder" - "github.com/tilt-dev/tilt/internal/testutils/tempdir" - "github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1" - "github.com/tilt-dev/tilt/pkg/model" -) - -var SanchoRef = container.MustParseSelector(testyaml.SanchoImage) - -func TestNeedsCrashRebuildLiveUpdateV2(t *testing.T) { - f := tempdir.NewTempDirFixture(t) - - iTarget := imageTarget() - iTarget.LiveUpdateReconciler = true - m := manifestbuilder.New(f, model.ManifestName("sancho")). - WithK8sYAML(testyaml.SanchoYAML). - WithImageTarget(iTarget). - Build() - s := store.NewState() - s.KubernetesResources["sancho"] = k8sResource() - - s.UpsertManifestTarget(store.NewManifestTarget(m)) - st, _ := s.ManifestState("sancho") - st.LiveUpdatedContainerIDs = map[container.ID]bool{"crash": true} - - CheckForContainerCrash(s, "sancho") - assert.False(t, st.NeedsRebuildFromCrash) -} - -func imageTarget() model.ImageTarget { - iTarget := model.MustNewImageTarget(SanchoRef) - iTarget.LiveUpdateReconciler = true - iTarget.LiveUpdateSpec = v1alpha1.LiveUpdateSpec{ - Selector: v1alpha1.LiveUpdateSelector{ - Kubernetes: &v1alpha1.LiveUpdateKubernetesSelector{ - Image: "sancho", - }, - }, - Syncs: []v1alpha1.LiveUpdateSync{{LocalPath: "/", ContainerPath: "/"}}, - } - return iTarget -} - -func k8sResource() *k8sconv.KubernetesResource { - return &k8sconv.KubernetesResource{ - FilteredPods: []v1alpha1.Pod{ - { - Name: "pod-1", - Namespace: "default", - Containers: []v1alpha1.Container{ - { - Name: "main", - ID: "main-id", - Image: "sancho", - State: v1alpha1.ContainerState{ - Running: &v1alpha1.ContainerStateRunning{}, - }, - }, - }, - }, - }, - } -} diff --git a/internal/testutils/podbuilder/podbuilder.go b/internal/testutils/podbuilder/podbuilder.go index 3ad4bb474a..0c06830cc9 100644 --- a/internal/testutils/podbuilder/podbuilder.go +++ b/internal/testutils/podbuilder/podbuilder.go @@ -32,14 +32,6 @@ func FakeContainerIDAtIndex(index int) container.ID { return container.ID(fmt.Sprintf("%s%s", fakeContainerID, indexSuffix)) } -func FakeContainerIDSet(size int) map[container.ID]bool { - result := container.NewIDSet() - for i := 0; i < size; i++ { - result[FakeContainerIDAtIndex(i)] = true - } - return result -} - // Builds Pod objects for testing // // The pod model should be internally well-formed (e.g., the containers diff --git a/pkg/model/build_reason.go b/pkg/model/build_reason.go index e47cb4d234..3f75e1a524 100644 --- a/pkg/model/build_reason.go +++ b/pkg/model/build_reason.go @@ -10,8 +10,16 @@ const ( BuildReasonFlagChangedFiles BuildReason = 1 << iota BuildReasonFlagConfig - // See comments on NeedsRebuildFromCrash - BuildReasonFlagCrash + // NOTE(nick): In live-update-v1, if a container had live-updated changed, + // then crashed, we would automatically replace it with a fresh image. + // This approach was difficult to reason about and sometimes led to infinite loops. + // Users complained that it was too aggressive about doing an image build. + // + // In live-update-v2, the reconciler keeps track of how to bring crashing + // containers up to date. Instead, we only kick off fresh image builds + // if there's a new file change / trigger but the container has been + // marked unrecoverable. So this build reason is obsolete. + BuildReasonFlagCrashDeprecated BuildReasonFlagInit @@ -58,20 +66,16 @@ func (r BuildReason) WithoutTriggers() BuildReason { return BuildReason(result) } -func (r BuildReason) IsCrashOnly() bool { - return r == BuildReasonFlagCrash -} - var translations = map[BuildReason]string{ - BuildReasonFlagChangedFiles: "Changed Files", - BuildReasonFlagConfig: "Config Changed", - BuildReasonFlagCrash: "Pod Crashed, Lost live_update Changes", - BuildReasonFlagInit: "Initial Build", - BuildReasonFlagTriggerWeb: "Web Trigger", - BuildReasonFlagTriggerCLI: "CLI Trigger", - BuildReasonFlagTriggerUnknown: "Unknown Trigger", - BuildReasonFlagTiltfileArgs: "Tilt Args", - BuildReasonFlagChangedDeps: "Dependency Updated", + BuildReasonFlagChangedFiles: "Changed Files", + BuildReasonFlagConfig: "Config Changed", + BuildReasonFlagCrashDeprecated: "Pod Crashed, Lost live_update Changes", + BuildReasonFlagInit: "Initial Build", + BuildReasonFlagTriggerWeb: "Web Trigger", + BuildReasonFlagTriggerCLI: "CLI Trigger", + BuildReasonFlagTriggerUnknown: "Unknown Trigger", + BuildReasonFlagTiltfileArgs: "Tilt Args", + BuildReasonFlagChangedDeps: "Dependency Updated", } var triggerBuildReasons = []BuildReason{ @@ -84,7 +88,7 @@ var allBuildReasons = []BuildReason{ BuildReasonFlagInit, BuildReasonFlagChangedFiles, BuildReasonFlagConfig, - BuildReasonFlagCrash, + BuildReasonFlagCrashDeprecated, BuildReasonFlagTriggerWeb, BuildReasonFlagTriggerCLI, BuildReasonFlagChangedDeps,