Move revision condition manipulation to _types.go

docs/spec/errors.md

@@ -59,11 +59,9 @@ Type names should be chosen such that these interpretations are clear:
 * `BuildSucceeded` works because `True` = success and `False` = failure.
 * `BuildCompleted` does not, because `False` could mean "in-progress".
 
-Conditions may also be omitted entirely if reconciliation has been
-skipped. When all conditions have succeeded, the "happy state"
-should clear other conditions for output legibility. Until the
-"happy state" is set, conditions should be persisted for the
-benefit of UI tools representing progress on the outcome.
+Conditions may also be omitted entirely if it doesn't pertain to the
+resource at hand.  e.g. Revisions that take pre-build containers may
+elide Build-related conditions.
 
 Conditions with a status of `False` will also supply additional details
 about the failure in [the "Reason" and "Message" sections](#condition-reason-and-message).

pkg/activator/revision_test.go

@@ -136,10 +136,8 @@ func TestActiveEndpoint_Reserve_WaitsForReady(t *testing.T) {
 	}
 
 	rev, _ := kna.ServingV1alpha1().Revisions(testNamespace).Get(testRevision, metav1.GetOptions{})
-	rev.Status.SetCondition(&v1alpha1.RevisionCondition{
-		Type:   v1alpha1.RevisionConditionReady,
-		Status: corev1.ConditionTrue,
-	})
+	rev.Status.MarkContainerHealthy()
+	rev.Status.MarkResourcesAvailable()
 	kna.ServingV1alpha1().Revisions(testNamespace).Update(rev)
 
 	time.Sleep(3 * time.Second)
@@ -250,17 +248,12 @@ func (b *revisionBuilder) withServingState(servingState v1alpha1.RevisionServing
 }
 
 func (b *revisionBuilder) withReady(ready bool) *revisionBuilder {
-	var status corev1.ConditionStatus
 	if ready {
-		status = corev1.ConditionTrue
+		b.revision.Status.MarkContainerHealthy()
+		b.revision.Status.MarkResourcesAvailable()
 	} else {
-		status = corev1.ConditionFalse
+		b.revision.Status.MarkContainerMissing("reasonz")
 	}
-	new := &v1alpha1.RevisionCondition{
-		Type:   v1alpha1.RevisionConditionReady,
-		Status: status,
-	}
-	b.revision.Status.SetCondition(new)
 	return b
 }
 

pkg/apis/serving/v1alpha1/revision_types.go

@@ -21,8 +21,9 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1"
 )
 
 // +genclient
@@ -246,7 +247,7 @@ func (rs *RevisionStatus) GetCondition(t RevisionConditionType) *RevisionConditi
 	return nil
 }
 
-func (rs *RevisionStatus) SetCondition(new *RevisionCondition) {
+func (rs *RevisionStatus) setCondition(new *RevisionCondition) {
 	if new == nil {
 		return
 	}
@@ -272,3 +273,149 @@ func (rs *RevisionStatus) RemoveCondition(t RevisionConditionType) {
 	}
 	rs.Conditions = conditions
 }
+
+func (rs *RevisionStatus) InitializeConditions() {
+	// We don't include BuildSucceeded here because it could confuse users if
+	// no `buildName` was specified.
+	rct := []RevisionConditionType{RevisionConditionResourcesAvailable,
+		RevisionConditionContainerHealthy, RevisionConditionReady}
+	for _, cond := range rct {
+		if rc := rs.GetCondition(cond); rc == nil {
+			rs.setCondition(&RevisionCondition{
+				Type:   cond,
+				Status: corev1.ConditionUnknown,
+			})
+		}
+	}
+}
+
+func (rs *RevisionStatus) InitializeBuildCondition() {
+	if rc := rs.GetCondition(RevisionConditionBuildSucceeded); rc == nil {
+		rs.setCondition(&RevisionCondition{
+			Type:   RevisionConditionBuildSucceeded,
+			Status: corev1.ConditionUnknown,
+		})
+	}
+}
+
+func (rs *RevisionStatus) MarkBuilding() {
+	for _, cond := range []RevisionConditionType{RevisionConditionBuildSucceeded, RevisionConditionReady} {
+		rs.setCondition(&RevisionCondition{
+			Type:   cond,
+			Status: corev1.ConditionUnknown,
+			Reason: "Building",
+		})
+	}
+}
+
+func (rs *RevisionStatus) MarkBuildSucceeded() {
+	rs.setCondition(&RevisionCondition{
+		Type:   RevisionConditionBuildSucceeded,
+		Status: corev1.ConditionTrue,
+	})
+	// Clear "Reason: Building".  There is a risk this could reset a "Ready: False" state,
+	// but not as things exist today.
+	rs.setCondition(&RevisionCondition{
+		Type:   RevisionConditionReady,
+		Status: corev1.ConditionUnknown,
+	})
+}
+
+func (rs *RevisionStatus) MarkBuildFailed(bc *buildv1alpha1.BuildCondition) {
+	for _, cond := range []RevisionConditionType{RevisionConditionBuildSucceeded, RevisionConditionReady} {
+		rs.setCondition(&RevisionCondition{
+			Type:    cond,
+			Status:  corev1.ConditionFalse,
+			Reason:  bc.Reason,
+			Message: bc.Message,
+		})
+	}
+}
+
+func (rs *RevisionStatus) MarkDeploying(reason string) {
+	rct := []RevisionConditionType{RevisionConditionResourcesAvailable,
+		RevisionConditionContainerHealthy, RevisionConditionReady}
+	for _, cond := range rct {
+		rs.setCondition(&RevisionCondition{
+			Type:   cond,
+			Status: corev1.ConditionUnknown,
+			Reason: reason,
+		})
+	}
+}
+
+func (rs *RevisionStatus) MarkServiceTimeout() {
+	for _, cond := range []RevisionConditionType{RevisionConditionResourcesAvailable, RevisionConditionReady} {
+		rs.setCondition(&RevisionCondition{
+			Type:    cond,
+			Status:  corev1.ConditionFalse,
+			Reason:  "ServiceTimeout",
+			Message: "Timed out waiting for a service endpoint to become ready",
+		})
+	}
+}
+
+func (rs *RevisionStatus) MarkProgressDeadlineExceeded(message string) {
+	for _, cond := range []RevisionConditionType{RevisionConditionResourcesAvailable, RevisionConditionReady} {
+		rs.setCondition(&RevisionCondition{
+			Type:    cond,
+			Status:  corev1.ConditionFalse,
+			Reason:  "ProgressDeadlineExceeded",
+			Message: message,
+		})
+	}
+}
+
+func (rs *RevisionStatus) MarkContainerHealthy() {
+	rs.setCondition(&RevisionCondition{
+		Type:   RevisionConditionContainerHealthy,
+		Status: corev1.ConditionTrue,
+	})
+	rs.checkAndMarkReady()
+}
+
+func (rs *RevisionStatus) MarkResourcesAvailable() {
+	rs.setCondition(&RevisionCondition{
+		Type:   RevisionConditionResourcesAvailable,
+		Status: corev1.ConditionTrue,
+	})
+	rs.checkAndMarkReady()
+}
+
+func (rs *RevisionStatus) MarkInactive() {
+	rs.setCondition(&RevisionCondition{
+		Type:   RevisionConditionReady,
+		Status: corev1.ConditionFalse,
+		Reason: "Inactive",
+	})
+}
+
+func (rs *RevisionStatus) MarkContainerMissing(message string) {
+	for _, cond := range []RevisionConditionType{RevisionConditionContainerHealthy, RevisionConditionReady} {
+		rs.setCondition(&RevisionCondition{
+			Type:    cond,
+			Status:  corev1.ConditionFalse,
+			Reason:  "ContainerMissing",
+			Message: message,
+		})
+	}
+}
+
+func (rs *RevisionStatus) checkAndMarkReady() {
+	ra := rs.GetCondition(RevisionConditionResourcesAvailable)
+	if ra == nil || ra.Status != corev1.ConditionTrue {
+		return
+	}
+	ch := rs.GetCondition(RevisionConditionContainerHealthy)
+	if ch == nil || ch.Status != corev1.ConditionTrue {
+		return
+	}
+	rs.markReady()
+}
+
+func (rs *RevisionStatus) markReady() {
+	rs.setCondition(&RevisionCondition{
+		Type:   RevisionConditionReady,
+		Status: corev1.ConditionTrue,
+	})
+}