Skip to content

Commit

Permalink
engine: add a ready condition to all uiresources (#5234)
Browse files Browse the repository at this point in the history
  • Loading branch information
nicks committed Nov 30, 2021
1 parent b996b83 commit c531e05
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 7 deletions.
17 changes: 16 additions & 1 deletion internal/cloud/io_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ func TestWriteSnapshotTo(t *testing.T) {
snapshot.View.UiResources = resources

now := time.Unix(1551202573, 0)
for _, r := range resources {
for i, cond := range r.Status.Conditions {
// Clear the transition timestamps so that the test is hermetic.
cond.LastTransitionTime = metav1.MicroTime{}
r.Status.Conditions[i] = cond
}
}

startTime, err := ptypes.TimestampProto(now)
require.NoError(t, err)
snapshot.View.TiltStartTime = startTime
Expand Down Expand Up @@ -70,7 +78,14 @@ func TestWriteSnapshotTo(t *testing.T) {
"status": {
"runtimeStatus": "not_applicable",
"updateStatus": "pending",
"order": 1
"order": 1,
"conditions": [
{
"type": "Ready",
"status": "False",
"reason": "UpdatePending"
}
]
}
}
]
Expand Down
23 changes: 23 additions & 0 deletions internal/engine/uiresource/subscriber.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ func (s *Subscriber) OnChange(ctx context.Context, st store.RStore, summary stor
continue
}

reconcileConditions(r.Status.Conditions, stored.Status.Conditions)

if !apicmp.DeepEqual(r.Status, stored.Status) {
// If the current version is different than what's stored, update it.
update := stored.DeepCopy()
Expand All @@ -85,4 +87,25 @@ func (s *Subscriber) OnChange(ctx context.Context, st store.RStore, summary stor
return nil
}

// Update the LastTransitionTime against the currently stored conditions.
func reconcileConditions(conds []v1alpha1.UIResourceCondition, stored []v1alpha1.UIResourceCondition) {
storedMap := make(map[v1alpha1.UIResourceConditionType]v1alpha1.UIResourceCondition, len(stored))
for _, c := range stored {
storedMap[c.Type] = c
}

for i, c := range conds {
existing, ok := storedMap[c.Type]
if !ok {
continue
}

// If the status hasn't changed, fall back to the previous transition time.
if existing.Status == c.Status {
c.LastTransitionTime = existing.LastTransitionTime
}
conds[i] = c
}
}

var _ store.Subscriber = &Subscriber{}
24 changes: 24 additions & 0 deletions internal/engine/uiresource/subscriber_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,28 @@ func TestUpdateTiltfile(t *testing.T) {
r = f.resource("(Tiltfile)")
require.NotNil(t, r)
assert.Equal(t, "3", r.ObjectMeta.ResourceVersion)
assert.Equal(t, "False", string(readyCondition(r).Status))

// Make sure OnChange is idempotent.
_ = f.sub.OnChange(f.ctx, f.store, store.LegacyChangeSummary())

r = f.resource("(Tiltfile)")
require.NotNil(t, r)
assert.Equal(t, "3", r.ObjectMeta.ResourceVersion)

f.store.WithState(func(es *store.EngineState) {
ms := es.TiltfileStates[model.MainTiltfileManifestName]
b := ms.CurrentBuild
b.FinishTime = time.Now()
ms.AddCompletedBuild(b)
ms.CurrentBuild = model.BuildRecord{}
})

_ = f.sub.OnChange(f.ctx, f.store, store.LegacyChangeSummary())
r = f.resource("(Tiltfile)")
require.NotNil(t, r)
assert.Equal(t, "4", r.ObjectMeta.ResourceVersion)
assert.Equal(t, "True", string(readyCondition(r).Status))
}

type fixture struct {
Expand Down Expand Up @@ -80,3 +95,12 @@ func (f *fixture) resource(name string) *v1alpha1.UIResource {
require.NoError(f.T(), err)
return r
}

func readyCondition(r *v1alpha1.UIResource) *v1alpha1.UIResourceCondition {
for _, c := range r.Status.Conditions {
if c.Type == v1alpha1.UIResourceReady {
return &c
}
}
return nil
}
56 changes: 50 additions & 6 deletions internal/hud/webview/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func ToUIResourceList(state store.EngineState, disableSources map[string][]v1alp
continue
}

r := TiltfileResourceProtoView(name, ms, state.LogStore)
r := TiltfileResource(name, ms, state.LogStore)
r.Status.Order = int32(len(ret) + 1)
ret = append(ret, r)
}
Expand Down Expand Up @@ -295,12 +295,55 @@ func toUIResource(mt *store.ManifestTarget, s store.EngineState, disableSources
if err != nil {
return nil, err
}

r.Status.Conditions = []v1alpha1.UIResourceCondition{
UIResourceReadyCondition(r.Status),
}
return r, nil
}

// The "Ready" condition is a cross-resource status report that's synthesized
// from the more type-specific fields of UIResource.
func UIResourceReadyCondition(r v1alpha1.UIResourceStatus) v1alpha1.UIResourceCondition {
c := v1alpha1.UIResourceCondition{
Type: v1alpha1.UIResourceReady,
Status: metav1.ConditionUnknown,

// LastTransitionTime will be computed by diffing against the current
// Condition. This doesn't really fit into the usual reconciler pattern,
// but is considered a worthwhile trade-off for the semantics we want, see discussion here:
// https://maelvls.dev/kubernetes-conditions/
LastTransitionTime: apis.NowMicro(),
}

if r.RuntimeStatus == v1alpha1.RuntimeStatusOK {
c.Status = metav1.ConditionTrue
return c
}

if r.RuntimeStatus == v1alpha1.RuntimeStatusNotApplicable && r.UpdateStatus == v1alpha1.UpdateStatusOK {
c.Status = metav1.ConditionTrue
return c
}

c.Status = metav1.ConditionFalse
if r.RuntimeStatus == v1alpha1.RuntimeStatusError {
c.Reason = "RuntimeError"
} else if r.UpdateStatus == v1alpha1.UpdateStatusError {
c.Reason = "UpdateError"
} else if r.UpdateStatus == v1alpha1.UpdateStatusOK && r.RuntimeStatus == v1alpha1.RuntimeStatusPending {
c.Reason = "RuntimePending"
} else if r.UpdateStatus == v1alpha1.UpdateStatusPending {
c.Reason = "UpdatePending"
} else {
c.Reason = "Unknown"
}
return c
}

// TODO(nick): We should build this from the Tiltfile in the apiserver,
// not the Tiltfile state in EngineState.
func TiltfileResourceProtoView(name model.ManifestName, ms *store.ManifestState, logStore *logstore.LogStore) *v1alpha1.UIResource {
func TiltfileResource(name model.ManifestName, ms *store.ManifestState, logStore *logstore.LogStore) *v1alpha1.UIResource {
ltfb := ms.LastBuild()
ctfb := ms.CurrentBuild

Expand All @@ -327,17 +370,18 @@ func TiltfileResourceProtoView(name model.ManifestName, ms *store.ManifestState,
} else {
tr.Status.LastDeployTime = finish
}

tr.Status.Conditions = []v1alpha1.UIResourceCondition{
UIResourceReadyCondition(tr.Status),
}

return tr
}

func populateResourceInfoView(mt *store.ManifestTarget, r *v1alpha1.UIResource) error {
r.Status.UpdateStatus = mt.UpdateStatus()
r.Status.RuntimeStatus = v1alpha1.RuntimeStatusNotApplicable

if mt.Manifest.PodReadinessMode() == model.PodReadinessIgnore {
return nil
}

if mt.Manifest.IsDC() {
dcState := mt.State.DCRuntimeState()
r.Status.RuntimeStatus = v1alpha1.RuntimeStatus(dcState.RuntimeStatus())
Expand Down
11 changes: 11 additions & 0 deletions internal/hud/webview/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ func TestReadinessCheckFailing(t *testing.T) {
rv, ok := findResource(m.Name, v)
require.True(t, ok)
require.Equal(t, v1alpha1.RuntimeStatusPending, v1alpha1.RuntimeStatus(rv.RuntimeStatus))
require.Equal(t, "False", string(readyCondition(rv).Status))
}

func TestLocalResource(t *testing.T) {
Expand All @@ -278,6 +279,7 @@ func TestLocalResource(t *testing.T) {
rs := r.Status
assert.Equal(t, "test", r.Name)
assert.Equal(t, v1alpha1.RuntimeStatusNotApplicable, rs.RuntimeStatus)
require.Equal(t, "False", string(readyCondition(rs).Status))
require.Len(t, rs.Specs, 1)
spec := rs.Specs[0]
require.Equal(t, v1alpha1.UIResourceTargetTypeLocal, spec.Type)
Expand Down Expand Up @@ -434,3 +436,12 @@ func lastBuild(r v1alpha1.UIResourceStatus) v1alpha1.UIBuildTerminated {

return r.BuildHistory[0]
}

func readyCondition(rs v1alpha1.UIResourceStatus) *v1alpha1.UIResourceCondition {
for _, c := range rs.Conditions {
if c.Type == v1alpha1.UIResourceReady {
return &c
}
}
return nil
}

0 comments on commit c531e05

Please sign in to comment.