Move configuration condition manipulation into _types.go

pkg/apis/serving/v1alpha1/configuration_types.go

@@ -18,6 +18,7 @@ package v1alpha1
 
 import (
 	"encoding/json"
+	"fmt"
 
 	build "github.com/knative/build/pkg/apis/build/v1alpha1"
 
@@ -146,8 +147,8 @@ func (r *Configuration) GetSpecJSON() ([]byte, error) {
 
 // IsReady looks at the conditions on the ConfigurationStatus.
 // ConfigurationConditionReady returns true if ConditionStatus is True
-func (configStatus *ConfigurationStatus) IsReady() bool {
-	if c := configStatus.GetCondition(ConfigurationConditionReady); c != nil {
+func (cs *ConfigurationStatus) IsReady() bool {
+	if c := cs.GetCondition(ConfigurationConditionReady); c != nil {
 		return c.Status == corev1.ConditionTrue
 	}
 	return false
@@ -156,9 +157,9 @@ func (configStatus *ConfigurationStatus) IsReady() bool {
 // IsLatestReadyRevisionNameUpToDate returns true if the Configuration is ready
 // and LatestCreateRevisionName is equal to LatestReadyRevisionName. Otherwise
 // it returns false.
-func (configStatus *ConfigurationStatus) IsLatestReadyRevisionNameUpToDate() bool {
-	return configStatus.IsReady() &&
-		configStatus.LatestCreatedRevisionName == configStatus.LatestReadyRevisionName
+func (cs *ConfigurationStatus) IsLatestReadyRevisionNameUpToDate() bool {
+	return cs.IsReady() &&
+		cs.LatestCreatedRevisionName == cs.LatestReadyRevisionName
 }
 
 func (config *ConfigurationStatus) GetCondition(t ConfigurationConditionType) *ConfigurationCondition {
@@ -170,28 +171,75 @@ func (config *ConfigurationStatus) GetCondition(t ConfigurationConditionType) *C
 	return nil
 }
 
-func (configStatus *ConfigurationStatus) SetCondition(new *ConfigurationCondition) {
+func (cs *ConfigurationStatus) setCondition(new *ConfigurationCondition) {
 	if new == nil {
 		return
 	}
 
 	t := new.Type
 	var conditions []ConfigurationCondition
-	for _, cond := range configStatus.Conditions {
+	for _, cond := range cs.Conditions {
 		if cond.Type != t {
 			conditions = append(conditions, cond)
 		}
 	}
 	conditions = append(conditions, *new)
-	configStatus.Conditions = conditions
+	cs.Conditions = conditions
 }
 
-func (configStatus *ConfigurationStatus) RemoveCondition(t ConfigurationConditionType) {
+func (cs *ConfigurationStatus) RemoveCondition(t ConfigurationConditionType) {
 	var conditions []ConfigurationCondition
-	for _, cond := range configStatus.Conditions {
+	for _, cond := range cs.Conditions {
 		if cond.Type != t {
 			conditions = append(conditions, cond)
 		}
 	}
-	configStatus.Conditions = conditions
+	cs.Conditions = conditions
+}
+
+func (cs *ConfigurationStatus) InitializeConditions() {
+	rct := []ConfigurationConditionType{
+		ConfigurationConditionLatestRevisionReady, ConfigurationConditionReady}
+	for _, cond := range rct {
+		if rc := cs.GetCondition(cond); rc == nil {
+			cs.setCondition(&ConfigurationCondition{
+				Type:   cond,
+				Status: corev1.ConditionUnknown,
+			})
+		}
+	}
+}
+
+func (cs *ConfigurationStatus) SetLatestCreatedRevisionName(name string) {
+	cs.LatestCreatedRevisionName = name
+	cs.setCondition(&ConfigurationCondition{
+		Type:   ConfigurationConditionLatestRevisionReady,
+		Status: corev1.ConditionUnknown,
+	})
+}
+
+func (cs *ConfigurationStatus) SetLatestReadyRevisionName(name string) {
+	cs.LatestReadyRevisionName = name
+	for _, cond := range []ConfigurationConditionType{
+		ConfigurationConditionLatestRevisionReady, ConfigurationConditionReady} {
+		cs.setCondition(&ConfigurationCondition{
+			Type:   cond,
+			Status: corev1.ConditionTrue,
+		})
+	}
+}
+
+func (cs *ConfigurationStatus) MarkLatestCreatedFailed(name, message string) {
+	cct := []ConfigurationConditionType{ConfigurationConditionLatestRevisionReady}
+	if cs.LatestReadyRevisionName == "" {
+		cct = append(cct, ConfigurationConditionReady)
+	}
+	for _, cond := range cct {
+		cs.setCondition(&ConfigurationCondition{
+			Type:    cond,
+			Status:  corev1.ConditionFalse,
+			Reason:  "RevisionFailed",
+			Message: fmt.Sprintf("revision %q failed with message: %s", name, message),
+		})
+	}
 }

pkg/apis/serving/v1alpha1/configuration_types_test.go

@@ -13,6 +13,7 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"strings"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
@@ -153,7 +154,7 @@ func TestConfigurationConditions(t *testing.T) {
 	}
 
 	// Add a new condition.
-	config.Status.SetCondition(foo)
+	config.Status.setCondition(foo)
 
 	if got, want := len(config.Status.Conditions), 1; got != want {
 		t.Fatalf("Unexpected Condition length; got %d, want %d", got, want)
@@ -167,7 +168,7 @@ func TestConfigurationConditions(t *testing.T) {
 	}
 
 	// Add a second condition.
-	config.Status.SetCondition(bar)
+	config.Status.setCondition(bar)
 
 	if got, want := len(config.Status.Conditions), 2; got != want {
 		t.Fatalf("Unexpected Condition length; got %d, want %d", got, want)
@@ -181,7 +182,7 @@ func TestConfigurationConditions(t *testing.T) {
 	}
 
 	// Add nil condition.
-	config.Status.SetCondition(nil)
+	config.Status.setCondition(nil)
 
 	if got, want := len(config.Status.Conditions), 1; got != want {
 		t.Fatalf("Unexpected Condition length; got %d, want %d", got, want)
@@ -255,3 +256,112 @@ func TestLatestReadyRevisionNameUpToDate(t *testing.T) {
 		}
 	}
 }
+
+func TestTypicalFlow(t *testing.T) {
+	r := &Configuration{}
+	r.Status.InitializeConditions()
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	r.Status.SetLatestCreatedRevisionName("foo")
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	r.Status.SetLatestReadyRevisionName("foo")
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	r.Status.SetLatestCreatedRevisionName("bar")
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	r.Status.SetLatestReadyRevisionName("bar")
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t)
+}
+
+func TestFailingFirstRevisionWithRecovery(t *testing.T) {
+	r := &Configuration{}
+	r.Status.InitializeConditions()
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	r.Status.SetLatestCreatedRevisionName("foo")
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	want := "the message"
+	r.Status.MarkLatestCreatedFailed("foo", want)
+	if c := checkConditionFailedConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t); !strings.Contains(c.Message, want) {
+		t.Errorf("MarkLatestCreatedFailed = %v, want substring %v", c.Message, want)
+	}
+	if c := checkConditionFailedConfiguration(r.Status, ConfigurationConditionReady, t); !strings.Contains(c.Message, want) {
+		t.Errorf("MarkLatestCreatedFailed = %v, want substring %v", c.Message, want)
+	}
+
+	// When a new revision comes along the LatestRevisionReady condition becomes Unknown,
+	// but Ready is still false (we've never seen a Ready Revision).
+	r.Status.SetLatestCreatedRevisionName("bar")
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionFailedConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	// When the new revision becomes ready, then Ready becomes true as well.
+	r.Status.SetLatestReadyRevisionName("bar")
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t)
+}
+
+func TestFailingSecondRevision(t *testing.T) {
+	r := &Configuration{}
+	r.Status.InitializeConditions()
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	r.Status.SetLatestCreatedRevisionName("foo")
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	r.Status.SetLatestReadyRevisionName("foo")
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	r.Status.SetLatestCreatedRevisionName("bar")
+	checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t)
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t)
+
+	// When the second revision fails, the Configuration remains "Ready" because there is a
+	// revision suitable for serving traffic.
+	want := "the message"
+	r.Status.MarkLatestCreatedFailed("bar", want)
+	if c := checkConditionFailedConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t); !strings.Contains(c.Message, want) {
+		t.Errorf("MarkLatestCreatedFailed = %v, want substring %v", c.Message, want)
+	}
+	checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t)
+}
+
+func checkConditionSucceededConfiguration(rs ConfigurationStatus, rct ConfigurationConditionType, t *testing.T) *ConfigurationCondition {
+	t.Helper()
+	return checkConditionConfiguration(rs, rct, corev1.ConditionTrue, t)
+}
+
+func checkConditionFailedConfiguration(rs ConfigurationStatus, rct ConfigurationConditionType, t *testing.T) *ConfigurationCondition {
+	t.Helper()
+	return checkConditionConfiguration(rs, rct, corev1.ConditionFalse, t)
+}
+
+func checkConditionOngoingConfiguration(rs ConfigurationStatus, rct ConfigurationConditionType, t *testing.T) *ConfigurationCondition {
+	t.Helper()
+	return checkConditionConfiguration(rs, rct, corev1.ConditionUnknown, t)
+}
+
+func checkConditionConfiguration(rs ConfigurationStatus, rct ConfigurationConditionType, cs corev1.ConditionStatus, t *testing.T) *ConfigurationCondition {
+	t.Helper()
+	r := rs.GetCondition(rct)
+	if r == nil {
+		t.Fatalf("Get(%v) = nil, wanted %v=%v", rct, rct, cs)
+	}
+	if r.Status != cs {
+		t.Fatalf("Get(%v) = %v, wanted %v", rct, r.Status, cs)
+	}
+	return r
+}