Add a Ready status condition for EventListener

The Ready Status is true when the other 4 status conditions - (Service, Deployment, Progessing, and Available) are all true. The idea is that in the future, Ready would be the one Condition users could use to see if an EventListener is ready to serve traffic or not. Fixes tektoncd#932 Signed-off-by: Dibyo Mukherjee <[email protected]>
pratap0007 · May 12, 2021 · c82dabc · c82dabc
1 parent 689c7e7
commit c82dabc
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 5 deletions.
diff --git a/pkg/apis/triggers/v1alpha1/event_listener_types.go b/pkg/apis/triggers/v1alpha1/event_listener_types.go
@@ -177,7 +177,10 @@ const (
 	ClusterTriggerBindingKind TriggerBindingKind = "ClusterTriggerBinding"
 )
 
-var eventListenerCondSet = apis.NewLivingConditionSet(ServiceExists, DeploymentExists)
+var eventListenerCondSet = apis.NewLivingConditionSet(
+	ServiceExists,
+	DeploymentExists,
+)
 
 // GetCondition returns the Condition matching the given type.
 func (els *EventListenerStatus) GetCondition(t apis.ConditionType) *apis.Condition {
@@ -189,10 +192,34 @@ func (els *EventListenerStatus) GetCondition(t apis.ConditionType) *apis.Conditi
 // K8s API elsewhere.
 func (els *EventListenerStatus) SetCondition(newCond *apis.Condition) {
 	if newCond != nil {
+		// TODO: Should the ConditionManager be set somewhere?
 		eventListenerCondSet.Manage(els).SetCondition(*newCond)
 	}
 }
 
+func (els *EventListenerStatus) SetReadyCondition() {
+	for _, ct := range []apis.ConditionType{
+		ServiceExists,
+		DeploymentExists,
+		apis.ConditionType(appsv1.DeploymentProgressing),
+		apis.ConditionType(appsv1.DeploymentAvailable)} {
+		sc := els.GetCondition(ct)
+		if sc.Status != corev1.ConditionTrue {
+			els.SetCondition(&apis.Condition{
+				Type:    apis.ConditionReady,
+				Status:  corev1.ConditionFalse,
+				Message: fmt.Sprintf("Condition %s has status: %s with message: %s", sc.Type, sc.Status, sc.Message),
+			})
+			return
+		}
+	}
+	els.SetCondition(&apis.Condition{
+		Type:    apis.ConditionReady,
+		Status:  corev1.ConditionTrue,
+		Message: "EventListener is ready",
+	})
+}
+
 // SetDeploymentConditions sets the Deployment conditions on the EventListener,
 // which is a reflection of the actual Deployment status.
 func (els *EventListenerStatus) SetDeploymentConditions(deploymentConditions []appsv1.DeploymentCondition) {
@@ -255,6 +282,7 @@ func (els *EventListenerStatus) InitializeConditions() {
 	for _, condition := range []apis.ConditionType{
 		ServiceExists,
 		DeploymentExists,
+		apis.ConditionReady,
 	} {
 		els.SetCondition(&apis.Condition{
 			Type:   condition,

diff --git a/pkg/apis/triggers/v1alpha1/event_listener_types_test.go b/pkg/apis/triggers/v1alpha1/event_listener_types_test.go
@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -101,6 +102,7 @@ func TestInitializeConditions(t *testing.T) {
 	var conditionTypes = []apis.ConditionType{
 		ServiceExists,
 		DeploymentExists,
+		apis.ConditionReady,
 	}
 	els := &EventListenerStatus{}
 	els.InitializeConditions()
@@ -283,3 +285,117 @@ func TestSetConditionsForDynamicObjects(t *testing.T) {
 		Message: "Message",
 	}})
 }
+
+func TestEventListenerStatus_SetReadyCondition(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		initial *EventListenerStatus
+		want    *EventListenerStatus
+	}{{
+		name: "set Ready to true when all dependent conditions are true",
+		initial: &EventListenerStatus{
+			Status: duckv1.Status{
+				Conditions: []apis.Condition{{
+					Type:    DeploymentExists,
+					Status:  corev1.ConditionTrue,
+					Message: "Deployment Exists",
+				}, {
+					Type:    ServiceExists,
+					Status:  corev1.ConditionTrue,
+					Message: "Service Exists",
+				}, {
+					Type:    apis.ConditionType(appsv1.DeploymentProgressing),
+					Status:  corev1.ConditionTrue,
+					Message: `ReplicaSet "el-example-655bf689f" has successfully progressed.`,
+				}, {
+					Type:    apis.ConditionType(appsv1.DeploymentAvailable),
+					Status:  corev1.ConditionTrue,
+					Message: "Deployment has minimum availability.",
+				}},
+			},
+		},
+		want: &EventListenerStatus{
+			Status: duckv1.Status{
+				Conditions: []apis.Condition{{
+					Type:    DeploymentExists,
+					Status:  corev1.ConditionTrue,
+					Message: "Deployment Exists",
+				}, {
+					Type:    ServiceExists,
+					Status:  corev1.ConditionTrue,
+					Message: "Service Exists",
+				}, {
+					Type:    apis.ConditionType(appsv1.DeploymentProgressing),
+					Status:  corev1.ConditionTrue,
+					Message: `ReplicaSet "el-example-655bf689f" has successfully progressed.`,
+				}, {
+					Type:    apis.ConditionType(appsv1.DeploymentAvailable),
+					Status:  corev1.ConditionTrue,
+					Message: "Deployment has minimum availability.",
+				}, {
+					Type:    apis.ConditionReady,
+					Status:  corev1.ConditionTrue,
+					Message: "EventListener is ready",
+				}},
+			},
+		},
+	}, {
+		name: "set Ready to false when at least one dependent condition is false",
+		initial: &EventListenerStatus{
+			Status: duckv1.Status{
+				Conditions: []apis.Condition{{
+					Type:    DeploymentExists,
+					Status:  corev1.ConditionTrue,
+					Message: "Deployment Exists",
+				}, {
+					Type:    ServiceExists,
+					Status:  corev1.ConditionFalse,
+					Message: "Service does not exist",
+				}, {
+					Type:    apis.ConditionType(appsv1.DeploymentProgressing),
+					Status:  corev1.ConditionTrue,
+					Message: `ReplicaSet "el-example-655bf689f" has successfully progressed.`,
+				}, {
+					Type:    apis.ConditionType(appsv1.DeploymentAvailable),
+					Status:  corev1.ConditionTrue,
+					Message: "Deployment has minimum availability.",
+				}},
+			},
+		},
+		want: &EventListenerStatus{
+			Status: duckv1.Status{
+				Conditions: []apis.Condition{{
+					Type:    DeploymentExists,
+					Status:  corev1.ConditionTrue,
+					Message: "Deployment Exists",
+				}, {
+					Type:    ServiceExists,
+					Status:  corev1.ConditionFalse,
+					Message: "Service does not exist",
+				}, {
+					Type:    apis.ConditionType(appsv1.DeploymentProgressing),
+					Status:  corev1.ConditionTrue,
+					Message: `ReplicaSet "el-example-655bf689f" has successfully progressed.`,
+				}, {
+					Type:    apis.ConditionType(appsv1.DeploymentAvailable),
+					Status:  corev1.ConditionTrue,
+					Message: "Deployment has minimum availability.",
+				}, {
+					Type:    apis.ConditionReady,
+					Status:  corev1.ConditionFalse,
+					Message: "Condition Service has status: False with message: Service does not exist",
+				}},
+			},
+		},
+	}} {
+		t.Run(tc.name, func(t *testing.T) {
+			tc.initial.SetReadyCondition()
+			if diff := cmp.Diff(tc.want, tc.initial, cmpopts.IgnoreTypes(
+				apis.Condition{}.LastTransitionTime.Inner.Time), cmpopts.SortSlices(func(x, y apis.Condition) bool {
+				return x.Type < y.Type
+			})); diff != "" {
+				t.Fatalf("SetReadyCondition() mismatch. -want/+got: %s", diff)
+			}
+		})
+	}
+}
diff --git a/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go b/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go
@@ -124,7 +124,9 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, el *v1alpha1.EventListen
 	}
 	deploymentReconcileError := r.reconcileDeployment(ctx, logger, el)
 	serviceReconcileError := r.reconcileService(ctx, logger, el)
-
+	if el.Spec.Resources.CustomResource == nil {
+		el.Status.SetReadyCondition()
+	}
 	return wrapError(serviceReconcileError, deploymentReconcileError)
 }
 

diff --git a/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go b/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go
@@ -99,6 +99,11 @@ var (
 	replicas int32 = 1
 )
 
+// compareCondition compares two conditions based on their Type field.
+func compareCondition(x, y apis.Condition) bool {
+	return x.Type < y.Type
+}
+
 // getEventListenerTestAssets returns TestAssets that have been seeded with the
 // given test.Resources r where r represents the state of the system
 func getEventListenerTestAssets(t *testing.T, r test.Resources, c *Config) (test.Assets, context.CancelFunc) {
@@ -532,6 +537,9 @@ func withKnativeStatus(el *v1alpha1.EventListener) {
 		}, {
 			Type:   v1alpha1.DeploymentExists,
 			Status: corev1.ConditionFalse,
+		}, {
+			Type:   apis.ConditionReady,
+			Status: corev1.ConditionFalse,
 		}},
 	}
 }
@@ -553,6 +561,10 @@ func withStatus(el *v1alpha1.EventListener) {
 				Status:  corev1.ConditionTrue,
 				Message: fmt.Sprintf("ReplicaSet \"%s\" has successfully progressed.", eventListenerName),
 				Reason:  "NewReplicaSetAvailable",
+			}, {
+				Type:    apis.ConditionReady,
+				Status:  corev1.ConditionTrue,
+				Message: "EventListener is ready",
 			}, {
 				Type:    v1alpha1.ServiceExists,
 				Status:  corev1.ConditionTrue,
@@ -1368,7 +1380,7 @@ func TestReconcile(t *testing.T) {
 			ConfigMaps:     []*corev1.ConfigMap{loggingConfigMap, observabilityConfigMap},
 		},
 	}, {
-		name: "eventlistener with added env for custome resource",
+		name: "eventlistener with added env for custom resource",
 		key:  reconcileKey,
 		startResources: test.Resources{
 			Namespaces:     []*corev1.Namespace{namespaceResource},
@@ -1476,7 +1488,7 @@ func TestReconcile(t *testing.T) {
 			if diff := cmp.Diff(tt.endResources, *actualEndResources, cmpopts.IgnoreTypes(
 				apis.Condition{}.LastTransitionTime.Inner.Time,
 				metav1.ObjectMeta{}.Finalizers,
-			)); diff != "" {
+			), cmpopts.SortSlices(compareCondition)); diff != "" {
 				t.Errorf("eventlistener.Reconcile() equality mismatch. Diff request body: -want +got: %s", diff)
 			}
 		})
@@ -1642,7 +1654,7 @@ func TestReconcile_InvalidForCustomResource(t *testing.T) {
 			if diff := cmp.Diff(tt.endResources, *actualEndResources, cmpopts.IgnoreTypes(
 				apis.Condition{}.LastTransitionTime.Inner.Time,
 				metav1.ObjectMeta{}.Finalizers,
-			)); diff == "" {
+			), cmpopts.SortSlices(compareCondition)); diff == "" {
 				t.Errorf("eventlistener.Reconcile() equality mismatch. Diff request body: -want +got: %s", diff)
 			}
 		})