Reset status conditions in GatewayAPI translator (#516)

* Reset status conditions in GatewayAPI translator * reset the conditions field within the listeners field (Gateway status) and the the parents field (HTTPRoute Status) before further setting any newer conditions so older conditons that are no longer true can be reset. * Enhance the status updater to merge the listener and parents conditions so existing older conditions that are same as the newer ones (except the LastTransitionTime field) can be retained and writes to the API Server can be mitigated Fixes: #415 Signed-off-by: Arko Dasgupta <arko@tetrate.io> * test Signed-off-by: Arko Dasgupta <arko@tetrate.io> * use reflect.DeepEqual Signed-off-by: Arko Dasgupta <arko@tetrate.io> * rm merge logic and rely on cmp status Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: Arko Dasgupta <arko@tetrate.io>
envoyproxy · Oct 7, 2022 · c4fbdc0 · c4fbdc0
1 parent 3ec7db1
commit c4fbdc0
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 44 deletions.
diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go
@@ -20,37 +20,6 @@ type GatewayContext struct {
 	listeners map[v1beta1.SectionName]*ListenerContext
 }
 
-func (g *GatewayContext) SetCondition(conditionType v1beta1.GatewayConditionType, status metav1.ConditionStatus, reason v1beta1.GatewayConditionReason, message string) {
-	cond := metav1.Condition{
-		Type:               string(conditionType),
-		Status:             status,
-		Reason:             string(reason),
-		Message:            message,
-		ObservedGeneration: g.Generation,
-		LastTransitionTime: metav1.NewTime(time.Now()),
-	}
-
-	idx := -1
-	for i, existing := range g.Status.Conditions {
-		if existing.Type == cond.Type {
-			// return early if the condition is unchanged
-			if existing.Status == cond.Status &&
-				existing.Reason == cond.Reason &&
-				existing.Message == cond.Message {
-				return
-			}
-			idx = i
-			break
-		}
-	}
-
-	if idx > -1 {
-		g.Status.Conditions[idx] = cond
-	} else {
-		g.Status.Conditions = append(g.Status.Conditions, cond)
-	}
-}
-
 func (g *GatewayContext) GetListenerContext(listenerName v1beta1.SectionName) *ListenerContext {
 	if g.listeners == nil {
 		g.listeners = make(map[v1beta1.SectionName]*ListenerContext)
@@ -135,6 +104,10 @@ func (l *ListenerContext) SetCondition(conditionType v1beta1.ListenerConditionTy
 	}
 }
 
+func (l *ListenerContext) ResetConditions() {
+	l.gateway.Status.Listeners[l.listenerStatusIdx].Conditions = make([]metav1.Condition, 0)
+}
+
 func (l *ListenerContext) SetSupportedKinds(kinds ...v1beta1.RouteGroupKind) {
 	l.gateway.Status.Listeners[l.listenerStatusIdx].SupportedKinds = kinds
 }
@@ -213,7 +186,7 @@ func (h *HTTPRouteContext) GetRouteParentContext(forParentRef v1beta1.ParentRefe
 
 	var parentRef *v1beta1.ParentReference
 	for i, p := range h.Spec.ParentRefs {
-		if p == forParentRef {
+		if reflect.DeepEqual(p, forParentRef) {
 			parentRef = &h.Spec.ParentRefs[i]
 			break
 		}
@@ -295,6 +268,10 @@ func (r *RouteParentContext) SetCondition(conditionType v1beta1.RouteConditionTy
 	}
 }
 
+func (r *RouteParentContext) ResetConditions() {
+	r.route.Status.Parents[r.routeParentStatusIdx].Conditions = make([]metav1.Condition, 0)
+}
+
 func (r *RouteParentContext) IsAccepted() bool {
 	for _, cond := range r.route.Status.Parents[r.routeParentStatusIdx].Conditions {
 		if cond.Type == string(v1beta1.RouteConditionAccepted) && cond.Status == metav1.ConditionTrue {

diff --git a/internal/gatewayapi/contexts_test.go b/internal/gatewayapi/contexts_test.go
@@ -27,14 +27,6 @@ func TestContexts(t *testing.T) {
 		Gateway: gateway,
 	}
 
-	gctx.SetCondition(v1beta1.GatewayConditionReady, metav1.ConditionTrue, v1beta1.GatewayReasonReady, "Gateway is ready")
-
-	require.Len(t, gateway.Status.Conditions, 1)
-	require.EqualValues(t, gateway.Status.Conditions[0].Type, v1beta1.GatewayConditionReady)
-	require.EqualValues(t, gateway.Status.Conditions[0].Status, metav1.ConditionTrue)
-	require.EqualValues(t, gateway.Status.Conditions[0].Reason, v1beta1.GatewayReasonReady)
-	require.EqualValues(t, gateway.Status.Conditions[0].Message, "Gateway is ready")
-
 	lctx := gctx.GetListenerContext("http")
 	require.NotNil(t, lctx)
 
@@ -53,4 +45,7 @@ func TestContexts(t *testing.T) {
 	require.Len(t, gateway.Status.Listeners, 1)
 	require.Len(t, gateway.Status.Listeners[0].SupportedKinds, 1)
 	require.EqualValues(t, gateway.Status.Listeners[0].SupportedKinds[0].Kind, "HTTPRoute")
+
+	lctx.ResetConditions()
+	require.Len(t, gateway.Status.Listeners[0].Conditions, 0)
 }
diff --git a/internal/gatewayapi/translator.go b/internal/gatewayapi/translator.go
@@ -144,7 +144,9 @@ func (t *Translator) GetRelevantGateways(gateways []*v1beta1.Gateway) []*Gateway
 
 			for _, listener := range gateway.Spec.Listeners {
 				l := gc.GetListenerContext(listener.Name)
-				// Reset attached route count since it will be recomputed during translation.
+				// Reset conditions and attached route count
+				// since it will be recomputed during translation.
+				l.ResetConditions()
 				l.ResetAttachedRoutes()
 			}
 
@@ -637,6 +639,8 @@ func (t *Translator) ProcessHTTPRoutes(httpRoutes []*v1beta1.HTTPRoute, gateways
 			relevantRoute = true
 
 			parentRefCtx := httpRoute.GetRouteParentContext(parentRef)
+			// Reset conditions since they will be recomputed during translation
+			parentRefCtx.ResetConditions()
 
 			if !HasReadyListener(selectedListeners) {
 				parentRefCtx.SetCondition(v1beta1.RouteConditionAccepted, metav1.ConditionFalse, "NoReadyListeners", "There are no ready listeners for this parent ref")

diff --git a/internal/provider/kubernetes/httproute.go b/internal/provider/kubernetes/httproute.go
@@ -331,10 +331,13 @@ func (r *httpRouteReconciler) subscribeAndUpdateStatus(ctx context.Context) {
 				NamespacedName: key,
 				Resource:       new(gwapiv1b1.HTTPRoute),
 				Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
-					if _, ok := obj.(*gwapiv1b1.HTTPRoute); !ok {
+					h, ok := obj.(*gwapiv1b1.HTTPRoute)
+					if !ok {
 						panic(fmt.Sprintf("unsupported object type %T", obj))
 					}
-					return val
+					hCopy := h.DeepCopy()
+					hCopy.Status.Parents = val.Status.Parents
+					return hCopy
 				}),
 			})
 		}

diff --git a/internal/status/status.go b/internal/status/status.go
@@ -144,7 +144,7 @@ func (u *UpdateWriter) Send(update Update) {
 //  Gateway
 //  HTTPRoute
 func isStatusEqual(objA, objB interface{}) bool {
-	opts := cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")
+	opts := cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime", "ObservedGeneration")
 	switch a := objA.(type) {
 	case *gwapiv1b1.GatewayClass:
 		if b, ok := objB.(*gwapiv1b1.GatewayClass); ok {