engine: fix a bug where resource_deps on non-workloads didn't work co…

…rrectly (#3574)

internal/engine/buildcontrol/build_control_test.go

@@ -61,6 +61,55 @@ func TestCurrentlyBuildingUncategorizedDisablesOtherK8sTargets(t *testing.T) {
 	f.assertNoTargetNextToBuild()
 }
 
+func TestK8sDependsOnLocal(t *testing.T) {
+	f := newTestFixture(t)
+	defer f.TearDown()
+
+	k8s1 := f.upsertK8sManifest("k8s1", withResourceDeps("local1"))
+	k8s2 := f.upsertK8sManifest("k8s2")
+	local1 := f.upsertLocalManifest("local1")
+
+	f.assertNextTargetToBuild("local1")
+
+	local1.State.AddCompletedBuild(model.BuildRecord{
+		StartTime:  time.Now(),
+		FinishTime: time.Now(),
+	})
+	local1.State.RuntimeState = store.LocalRuntimeState{HasSucceededAtLeastOnce: true}
+
+	f.assertNextTargetToBuild("k8s1")
+	k8s1.State.CurrentBuild = model.BuildRecord{StartTime: time.Now()}
+	f.assertNextTargetToBuild("k8s2")
+
+	_ = k8s2
+}
+
+func TestLocalDependsOnNonWorkloadK8s(t *testing.T) {
+	f := newTestFixture(t)
+	defer f.TearDown()
+
+	local1 := f.upsertLocalManifest("local1", withResourceDeps("k8s1"))
+	k8s1 := f.upsertK8sManifest("k8s1", withK8sNonWorkload())
+	k8s2 := f.upsertK8sManifest("k8s2", withK8sNonWorkload())
+
+	f.assertNextTargetToBuild("k8s1")
+
+	k8s1.State.AddCompletedBuild(model.BuildRecord{
+		StartTime:  time.Now(),
+		FinishTime: time.Now(),
+	})
+	k8s1.State.RuntimeState = store.K8sRuntimeState{NonWorkload: true, HasEverDeployedSuccessfully: true}
+
+	f.assertNextTargetToBuild("local1")
+	local1.State.AddCompletedBuild(model.BuildRecord{
+		StartTime:  time.Now(),
+		FinishTime: time.Now(),
+	})
+	f.assertNextTargetToBuild("k8s2")
+
+	_ = k8s2
+}
+
 func TestCurrentlyBuildingLocalResourceDisablesK8sScheduling(t *testing.T) {
 	f := newTestFixture(t)
 	defer f.TearDown()
@@ -173,16 +222,20 @@ func (f *testFixture) upsertManifest(m model.Manifest) *store.ManifestTarget {
 	return mt
 }
 
-func (f *testFixture) upsertK8sManifest(name model.ManifestName) *store.ManifestTarget {
-	return f.upsertManifest(manifestbuilder.New(f, name).
-		WithK8sYAML(testyaml.SanchoYAML).
-		Build())
+func (f *testFixture) upsertK8sManifest(name model.ManifestName, opts ...manifestOption) *store.ManifestTarget {
+	b := manifestbuilder.New(f, name)
+	for _, o := range opts {
+		b = o(b)
+	}
+	return f.upsertManifest(b.WithK8sYAML(testyaml.SanchoYAML).Build())
 }
 
-func (f *testFixture) upsertLocalManifest(name model.ManifestName) *store.ManifestTarget {
-	return f.upsertManifest(manifestbuilder.New(f, name).
-		WithLocalResource(fmt.Sprintf("exec-%s", name), nil).
-		Build())
+func (f *testFixture) upsertLocalManifest(name model.ManifestName, opts ...manifestOption) *store.ManifestTarget {
+	b := manifestbuilder.New(f, name)
+	for _, o := range opts {
+		b = o(b)
+	}
+	return f.upsertManifest(b.WithLocalResource(fmt.Sprintf("exec-%s", name), nil).Build())
 }
 
 func (f *testFixture) manifestNeedingCrashRebuild() *store.ManifestTarget {
@@ -199,3 +252,16 @@ func (f *testFixture) manifestNeedingCrashRebuild() *store.ManifestTarget {
 	mt.State.NeedsRebuildFromCrash = true
 	return mt
 }
+
+type manifestOption func(manifestbuilder.ManifestBuilder) manifestbuilder.ManifestBuilder
+
+func withResourceDeps(deps ...string) manifestOption {
+	return manifestOption(func(m manifestbuilder.ManifestBuilder) manifestbuilder.ManifestBuilder {
+		return m.WithResourceDeps(deps...)
+	})
+}
+func withK8sNonWorkload() manifestOption {
+	return manifestOption(func(m manifestbuilder.ManifestBuilder) manifestbuilder.ManifestBuilder {
+		return m.WithK8sNonWorkload()
+	})
+}

internal/engine/buildcontroller_test.go

@@ -1368,6 +1368,33 @@ func TestManifestsWithTwoCommonAncestors(t *testing.T) {
 	f.assertAllBuildsConsumed()
 }
 
+func TestLocalDependsOnNonWorkloadK8s(t *testing.T) {
+	f := newTestFixture(t)
+	defer f.TearDown()
+
+	local1 := manifestbuilder.New(f, "local").
+		WithLocalResource("exec-local", nil).
+		WithResourceDeps("k8s1").
+		Build()
+	k8s1 := manifestbuilder.New(f, "k8s1").
+		WithK8sYAML(testyaml.SanchoYAML).
+		WithK8sNonWorkload().
+		Build()
+	f.Start([]model.Manifest{local1, k8s1})
+
+	f.waitForCompletedBuildCount(2)
+
+	call := f.nextCall("k8s1 build")
+	assert.Equal(t, k8s1.K8sTarget(), call.k8s())
+
+	call = f.nextCall("local build")
+	assert.Equal(t, local1.LocalTarget(), call.local())
+
+	err := f.Stop()
+	assert.NoError(t, err)
+	f.assertAllBuildsConsumed()
+}
+
 func TestManifestsWithCommonAncestorAndTrigger(t *testing.T) {
 	f := newTestFixture(t)
 	m1, m2 := NewManifestsWithCommonAncestor(f)

internal/engine/exit/controller_test.go

@@ -106,7 +106,7 @@ func TestExitControlCIFirstRuntimeFailure(t *testing.T) {
 
 	f.store.WithState(func(state *store.EngineState) {
 		mt := state.ManifestTargets["fe"]
-		mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest, store.Pod{
+		mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest, store.Pod{
 			PodID:  "pod-a",
 			Status: "ErrImagePull",
 			Containers: []store.Container{
@@ -142,15 +142,15 @@ func TestExitControlCISuccess(t *testing.T) {
 			StartTime:  time.Now(),
 			FinishTime: time.Now(),
 		})
-		state.ManifestTargets["fe"].State.RuntimeState = store.NewK8sRuntimeState(m, readyPod("pod-a"))
+		state.ManifestTargets["fe"].State.RuntimeState = store.NewK8sRuntimeStateWithPods(m, readyPod("pod-a"))
 	})
 
 	f.c.OnChange(f.ctx, f.store)
 	assert.False(t, f.store.exitSignal)
 
 	f.store.WithState(func(state *store.EngineState) {
 		mt := state.ManifestTargets["fe2"]
-		mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest, readyPod("pod-b"))
+		mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest, readyPod("pod-b"))
 	})
 
 	// Verify that if one pod fails with an error, it fails immediately.
@@ -171,15 +171,15 @@ func TestExitControlCIJobSuccess(t *testing.T) {
 			StartTime:  time.Now(),
 			FinishTime: time.Now(),
 		})
-		state.ManifestTargets["fe"].State.RuntimeState = store.NewK8sRuntimeState(m, readyPod("pod-a"))
+		state.ManifestTargets["fe"].State.RuntimeState = store.NewK8sRuntimeStateWithPods(m, readyPod("pod-a"))
 	})
 
 	f.c.OnChange(f.ctx, f.store)
 	assert.False(t, f.store.exitSignal)
 
 	f.store.WithState(func(state *store.EngineState) {
 		mt := state.ManifestTargets["fe"]
-		mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest, successPod("pod-a"))
+		mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest, successPod("pod-a"))
 	})
 
 	// Verify that if one pod fails with an error, it fails immediately.
@@ -201,7 +201,7 @@ func TestExitControlCIDontBlockOnAutoInitFalse(t *testing.T) {
 			StartTime:  time.Now(),
 			FinishTime: time.Now(),
 		})
-		mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest, readyPod("pod-a"))
+		mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest, readyPod("pod-a"))
 
 		manifestAutoInitFalse := manifestbuilder.New(f, "local").WithLocalResource("echo hi", nil).
 			WithTriggerMode(model.TriggerModeManualIncludingInitial).Build()
@@ -213,7 +213,7 @@ func TestExitControlCIDontBlockOnAutoInitFalse(t *testing.T) {
 
 	f.store.WithState(func(state *store.EngineState) {
 		mt := state.ManifestTargets["fe"]
-		mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest, successPod("pod-a"))
+		mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest, successPod("pod-a"))
 	})
 
 	// Verify that if one pod fails with an error, it fails immediately.

internal/engine/portforward/controller_test.go

@@ -42,7 +42,7 @@ func TestPortForward(t *testing.T) {
 
 	state = f.st.LockMutableStateForTesting()
 	mt := state.ManifestTargets["fe"]
-	mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest,
+	mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest,
 		store.Pod{PodID: "pod-id", Phase: v1.PodRunning})
 	f.st.UnlockMutableState()
 
@@ -53,7 +53,7 @@ func TestPortForward(t *testing.T) {
 
 	state = f.st.LockMutableStateForTesting()
 	mt = state.ManifestTargets["fe"]
-	mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest,
+	mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest,
 		store.Pod{PodID: "pod-id2", Phase: v1.PodRunning})
 	f.st.UnlockMutableState()
 
@@ -63,7 +63,7 @@ func TestPortForward(t *testing.T) {
 
 	state = f.st.LockMutableStateForTesting()
 	mt = state.ManifestTargets["fe"]
-	mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest, store.Pod{PodID: "pod-id2", Phase: v1.PodPending})
+	mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest, store.Pod{PodID: "pod-id2", Phase: v1.PodPending})
 	f.st.UnlockMutableState()
 
 	f.onChange()
@@ -91,7 +91,7 @@ func TestPortForwardAutoDiscovery(t *testing.T) {
 	state.UpsertManifestTarget(store.NewManifestTarget(m))
 
 	mt := state.ManifestTargets["fe"]
-	mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest,
+	mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest,
 		store.Pod{PodID: "pod-id", Phase: v1.PodRunning})
 	f.st.UnlockMutableState()
 
@@ -126,7 +126,7 @@ func TestPortForwardAutoDiscovery2(t *testing.T) {
 	state.UpsertManifestTarget(store.NewManifestTarget(m))
 
 	mt := state.ManifestTargets["fe"]
-	mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest, store.Pod{
+	mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest, store.Pod{
 		PodID: "pod-id",
 		Phase: v1.PodRunning,
 		Containers: []store.Container{
@@ -155,7 +155,7 @@ func TestPortForwardChangePort(t *testing.T) {
 	})
 	state.UpsertManifestTarget(store.NewManifestTarget(m))
 	mt := state.ManifestTargets["fe"]
-	mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest, store.Pod{PodID: "pod-id", Phase: v1.PodRunning})
+	mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest, store.Pod{PodID: "pod-id", Phase: v1.PodRunning})
 	f.st.UnlockMutableState()
 
 	f.onChange()
@@ -190,7 +190,7 @@ func TestPortForwardRestart(t *testing.T) {
 	})
 	state.UpsertManifestTarget(store.NewManifestTarget(m))
 	mt := state.ManifestTargets["fe"]
-	mt.State.RuntimeState = store.NewK8sRuntimeState(mt.Manifest,
+	mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(mt.Manifest,
 		store.Pod{PodID: "pod-id", Phase: v1.PodRunning})
 	f.st.UnlockMutableState()
 

internal/engine/upper.go

@@ -393,19 +393,22 @@ func handleBuildCompleted(ctx context.Context, engineState *store.EngineState, c
 
 	manifest := mt.Manifest
 	if manifest.IsK8s() {
+		state := ms.K8sRuntimeState()
 		deployedUIDSet := cb.Result.DeployedUIDSet()
 		if len(deployedUIDSet) > 0 {
-			state := ms.K8sRuntimeState()
 			state.DeployedUIDSet = deployedUIDSet
-			ms.RuntimeState = state
 		}
 
 		deployedPodTemplateSpecHashSet := cb.Result.DeployedPodTemplateSpecHashes()
 		if len(deployedPodTemplateSpecHashSet) > 0 {
-			state := ms.K8sRuntimeState()
 			state.DeployedPodTemplateSpecHashSet = deployedPodTemplateSpecHashSet
-			ms.RuntimeState = state
 		}
+
+		if err == nil {
+			state.HasEverDeployedSuccessfully = true
+		}
+
+		ms.RuntimeState = state
 	}
 
 	if mt.Manifest.IsDC() {
@@ -437,10 +440,12 @@ func handleBuildCompleted(ctx context.Context, engineState *store.EngineState, c
 	}
 
 	if mt.Manifest.IsLocal() {
-		ms.RuntimeState = store.LocalRuntimeState{
-			Status:                  model.RuntimeStatusNotApplicable,
-			HasSucceededAtLeastOnce: err == nil,
+		lrs := ms.LocalRuntimeState()
+		lrs.Status = model.RuntimeStatusNotApplicable
+		if err == nil {
+			lrs.HasSucceededAtLeastOnce = true
 		}
+		ms.RuntimeState = lrs
 	}
 }
 

internal/store/engine_state_test.go

@@ -81,8 +81,12 @@ func TestRuntimeStateNonWorkload(t *testing.T) {
 		WithK8sYAML(testyaml.SecretYaml).
 		Build()
 	state := newState([]model.Manifest{m})
-	assert.Equal(t, model.RuntimeStatusOK,
-		state.ManifestTargets[m.Name].State.K8sRuntimeState().RuntimeStatus())
+	runtimeState := state.ManifestTargets[m.Name].State.K8sRuntimeState()
+	assert.Equal(t, model.RuntimeStatusPending, runtimeState.RuntimeStatus())
+
+	runtimeState.HasEverDeployedSuccessfully = true
+
+	assert.Equal(t, model.RuntimeStatusOK, runtimeState.RuntimeStatus())
 }
 
 func TestStateToViewUnresourcedYAMLManifest(t *testing.T) {
@@ -126,7 +130,7 @@ func TestMostRecentPod(t *testing.T) {
 	podB := Pod{PodID: "pod-b", StartedAt: time.Now().Add(time.Minute)}
 	podC := Pod{PodID: "pod-c", StartedAt: time.Now().Add(-time.Minute)}
 	m := model.Manifest{Name: "fe"}
-	podSet := NewK8sRuntimeState(m, podA, podB, podC)
+	podSet := NewK8sRuntimeStateWithPods(m, podA, podB, podC)
 	assert.Equal(t, "pod-b", podSet.MostRecentPod().PodID.String())
 }
 

internal/store/runtime_state.go

@@ -72,7 +72,8 @@ type K8sRuntimeState struct {
 	DeployedUIDSet                 UIDSet                 // for the most recent successful deploy
 	DeployedPodTemplateSpecHashSet PodTemplateSpecHashSet // for the most recent successful deploy
 
-	LastReadyOrSucceededTime time.Time
+	LastReadyOrSucceededTime    time.Time
+	HasEverDeployedSuccessfully bool
 
 	NonWorkload bool
 }
@@ -81,16 +82,20 @@ func (K8sRuntimeState) RuntimeState() {}
 
 var _ RuntimeState = K8sRuntimeState{}
 
-func NewK8sRuntimeState(m model.Manifest, pods ...Pod) K8sRuntimeState {
-
-	podMap := make(map[k8s.PodID]*Pod, len(pods))
+func NewK8sRuntimeStateWithPods(m model.Manifest, pods ...Pod) K8sRuntimeState {
+	state := NewK8sRuntimeState(m)
 	for _, pod := range pods {
 		p := pod
-		podMap[p.PodID] = &p
+		state.Pods[p.PodID] = &p
 	}
+	state.HasEverDeployedSuccessfully = len(pods) > 0
+	return state
+}
+
+func NewK8sRuntimeState(m model.Manifest) K8sRuntimeState {
 	return K8sRuntimeState{
 		NonWorkload:                    m.K8sTarget().NonWorkload,
-		Pods:                           podMap,
+		Pods:                           make(map[k8s.PodID]*Pod),
 		LBs:                            make(map[k8s.ServiceName]*url.URL),
 		DeployedUIDSet:                 NewUIDSet(),
 		DeployedPodTemplateSpecHashSet: NewPodTemplateSpecHashSet(),
@@ -107,6 +112,10 @@ func (s K8sRuntimeState) RuntimeStatusError() error {
 }
 
 func (s K8sRuntimeState) RuntimeStatus() model.RuntimeStatus {
+	if !s.HasEverDeployedSuccessfully {
+		return model.RuntimeStatusPending
+	}
+
 	if s.NonWorkload {
 		return model.RuntimeStatusOK
 	}
@@ -137,6 +146,9 @@ func (s K8sRuntimeState) RuntimeStatus() model.RuntimeStatus {
 }
 
 func (s K8sRuntimeState) HasEverBeenReadyOrSucceeded() bool {
+	if !s.HasEverDeployedSuccessfully {
+		return false
+	}
 	if s.NonWorkload {
 		return true
 	}

internal/testutils/manifestutils/manifest.go

@@ -7,6 +7,6 @@ import (
 
 func NewManifestTargetWithPod(m model.Manifest, pod store.Pod) *store.ManifestTarget {
 	mt := store.NewManifestTarget(m)
-	mt.State.RuntimeState = store.NewK8sRuntimeState(m, pod)
+	mt.State.RuntimeState = store.NewK8sRuntimeStateWithPods(m, pod)
 	return mt
 }