Fix condition handling of gateways and routes (#287)

* Refactor condition handling of gateways and routes

Author: Doyoon Kim

controllers/conditions.go

@@ -0,0 +1,32 @@
+package controllers
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func updateCondition(conditions []metav1.Condition, newCond metav1.Condition) []metav1.Condition {
+	newConditions := make([]metav1.Condition, 0)
+
+	found := false
+	for _, cond := range conditions {
+		if cond.Type == newCond.Type {
+			// Update existing condition. Time is kept only if status is unchanged.
+			newCond.LastTransitionTime = cond.LastTransitionTime
+			if cond.Status != newCond.Status {
+				newCond.LastTransitionTime = metav1.Now()
+			}
+			newConditions = append(newConditions, newCond)
+			found = true
+		} else {
+			newConditions = append(newConditions, cond)
+		}
+	}
+
+	if !found {
+		// Add new condition instead.
+		newCond.LastTransitionTime = metav1.Now()
+		newConditions = append(newConditions, newCond)
+	}
+
+	return newConditions
+}

controllers/gateway_controller.go

@@ -19,8 +19,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"time"
-
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
 
@@ -258,21 +256,15 @@ func (r *GatewayReconciler) cleanupGatewayResources(ctx context.Context, gw *gat
 
 func (r *GatewayReconciler) updateGatewayStatus(ctx context.Context, serviceNetworkStatus *latticestore.ServiceNetwork, gw *gateway_api.Gateway) error {
 
-	glog.V(6).Infof("updateGatewayStatus: updating last transition time \n")
 	gwOld := gw.DeepCopy()
 
-	if len(gw.Status.Conditions) <= 1 {
-		condition := metav1.Condition{}
-		gw.Status.Conditions = append(gw.Status.Conditions, condition)
-	}
-
-	gw.Status.Conditions[1].LastTransitionTime = metav1.NewTime(time.Now())
-	gw.Status.Conditions[1].Status = "True"
-	gw.Status.Conditions[1].Message = fmt.Sprintf("aws-gateway-arn: %s", serviceNetworkStatus.ARN)
-	gw.Status.Conditions[1].Reason = "Reconciled"
-	gw.Status.Conditions[1].ObservedGeneration = gw.Generation
-	gw.Status.Conditions[0].ObservedGeneration = gw.Generation // update the accept
-	gw.Status.Conditions[1].Type = string(gateway_api.GatewayConditionProgrammed)
+	gw.Status.Conditions = updateCondition(gw.Status.Conditions, metav1.Condition{
+		Type:               string(gateway_api.GatewayConditionProgrammed),
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: gw.Generation,
+		Reason:             string(gateway_api.GatewayReasonProgrammed),
+		Message:            fmt.Sprintf("aws-gateway-arn: %s", serviceNetworkStatus.ARN),
+	})
 
 	// TODO following is causing crash on some platform, see https://t.corp.amazon.com/b7c9ea6c-5168-4616-b718-c1bdf78dbdf1/communication
 	//gw.Annotations["gateway.networking.k8s.io/aws-gateway-id"] = serviceNetworkStatus.ID
@@ -289,20 +281,25 @@ func (r *GatewayReconciler) updateGatewayAcceptStatus(ctx context.Context, gw *g
 
 	gwOld := gw.DeepCopy()
 
-	glog.V(6).Infof("updateGatewayStatus: updating last transition time \n")
-	if gw.Status.Conditions[0].LastTransitionTime == eventhandlers.ZeroTransitionTime {
-		gw.Status.Conditions[0].LastTransitionTime = metav1.NewTime(time.Now())
-	}
+	var cond metav1.Condition
 	if accepted {
-		gw.Status.Conditions[0].Status = "True"
-		gw.Status.Conditions[0].Reason = "Accepted"
+		cond = metav1.Condition{
+			Type:               string(gateway_api.GatewayConditionAccepted),
+			ObservedGeneration: gw.Generation,
+			Message:            config.LatticeGatewayControllerName,
+			Status:             metav1.ConditionTrue,
+			Reason:             string(gateway_api.GatewayReasonAccepted),
+		}
 	} else {
-		gw.Status.Conditions[0].Status = "False"
-		gw.Status.Conditions[0].Reason = "Invalid"
+		cond = metav1.Condition{
+			Type:               string(gateway_api.GatewayConditionAccepted),
+			ObservedGeneration: gw.Generation,
+			Message:            config.LatticeGatewayControllerName,
+			Status:             metav1.ConditionFalse,
+			Reason:             string(gateway_api.GatewayReasonInvalid),
+		}
 	}
-	gw.Status.Conditions[0].Message = config.LatticeGatewayControllerName
-	gw.Status.Conditions[0].ObservedGeneration = gw.Generation
-	gw.Status.Conditions[0].Type = string(gateway_api.GatewayConditionAccepted)
+	gw.Status.Conditions = updateCondition(gw.Status.Conditions, cond)
 
 	if err := r.Client.Status().Patch(ctx, gw, client.MergeFrom(gwOld)); err != nil {
 		glog.V(2).Infof("Failed to Patch acceptance status, err %v gw %v", err, gw)
@@ -312,27 +309,6 @@ func (r *GatewayReconciler) updateGatewayAcceptStatus(ctx context.Context, gw *g
 	return nil
 }
 
-func (r *GatewayReconciler) updateBadStatus(ctx context.Context, message string, gw *gateway_api.Gateway) error {
-
-	gwOld := gw.DeepCopy()
-
-	glog.V(6).Infof("updateGatewayStatus: updating last transition time \n")
-	if gw.Status.Conditions[0].LastTransitionTime == eventhandlers.ZeroTransitionTime {
-		gw.Status.Conditions[0].LastTransitionTime = metav1.NewTime(time.Now())
-	}
-
-	gw.Status.Conditions[0].Status = "False"
-	gw.Status.Conditions[0].Message = message
-	gw.Status.Conditions[0].Reason = "NoReconcile"
-	gw.Status.Conditions[0].Type = "NotAccepted"
-
-	if err := r.Client.Status().Patch(ctx, gw, client.MergeFrom(gwOld)); err != nil {
-		return errors.Wrapf(err, "failed to update gateway status")
-	}
-
-	return nil
-}
-
 func UpdateHTTPRouteListenerStatus(ctx context.Context, k8sclient client.Client, httproute *gateway_api.HTTPRoute) error {
 	gw := &gateway_api.Gateway{}
 
@@ -391,8 +367,6 @@ func UpdateGWListenerStatus(ctx context.Context, k8sclient client.Client, gw *ga
 
 	glog.V(6).Infof("Before update, the snapshot of listeners  %v \n", gw.Status.Listeners)
 
-	gw.Status.Listeners = make([]gateway_api.ListenerStatus, 0)
-
 	httpRouteList := &gateway_api.HTTPRouteList{}
 
 	k8sclient.List(context.TODO(), httpRouteList)
@@ -441,7 +415,7 @@ func UpdateGWListenerStatus(ctx context.Context, k8sclient client.Client, gw *ga
 				Status:             metav1.ConditionFalse,
 				Reason:             string(gateway_api.ListenerReasonInvalidRouteKinds),
 				ObservedGeneration: gw.Generation,
-				LastTransitionTime: metav1.NewTime(time.Now()),
+				LastTransitionTime: metav1.Now(),
 			}
 			listenerStatus.SupportedKinds = supportedkind
 			listenerStatus.Conditions = append(listenerStatus.Conditions, condition)
@@ -451,12 +425,11 @@ func UpdateGWListenerStatus(ctx context.Context, k8sclient client.Client, gw *ga
 			hasValidListener = true
 
 			condition := metav1.Condition{
-				Type:               "Accepted",
-				Status:             "True",
-				Reason:             "Accepted",
+				Type:               string(gateway_api.ListenerConditionAccepted),
+				Status:             metav1.ConditionTrue,
+				Reason:             string(gateway_api.ListenerReasonAccepted),
 				ObservedGeneration: gw.Generation,
-				// TODO need to use previous one
-				LastTransitionTime: metav1.NewTime(time.Now()),
+				LastTransitionTime: metav1.Now(),
 			}
 
 			for _, httpRoute := range httpRouteList.Items {
@@ -499,8 +472,20 @@ func UpdateGWListenerStatus(ctx context.Context, k8sclient client.Client, gw *ga
 			listenerStatus.Conditions = append(listenerStatus.Conditions, condition)
 
 		}
-		gw.Status.Listeners = append(gw.Status.Listeners, listenerStatus)
 
+		found := false
+		for i, oldStatus := range gw.Status.Listeners {
+			if oldStatus.Name == listenerStatus.Name {
+				gw.Status.Listeners[i].AttachedRoutes = listenerStatus.AttachedRoutes
+				gw.Status.Listeners[i].SupportedKinds = listenerStatus.SupportedKinds
+				// Only have one condition in the logic
+				gw.Status.Listeners[i].Conditions = updateCondition(gw.Status.Listeners[i].Conditions, listenerStatus.Conditions[0])
+				found = true
+			}
+		}
+		if !found {
+			gw.Status.Listeners = append(gw.Status.Listeners, listenerStatus)
+		}
 	}
 
 	glog.V(6).Infof("After update, the snapshot of listener status %v", gw.Status.Listeners)

controllers/httproute_controller.go

@@ -19,8 +19,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"time"
-
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
 
@@ -276,11 +274,9 @@ func (r *HTTPRouteReconciler) reconcileHTTPRouteResource(ctx context.Context, ht
 func (r *HTTPRouteReconciler) updateHTTPRouteStatus(ctx context.Context, dns string, httproute *gateway_api.HTTPRoute) error {
 	glog.V(6).Infof("updateHTTPRouteStatus: httproute %v, dns %v\n", httproute, dns)
 	httprouteOld := httproute.DeepCopy()
-	compare := true
 
 	if len(httproute.ObjectMeta.Annotations) == 0 {
 		httproute.ObjectMeta.Annotations = make(map[string]string)
-		compare = false
 	}
 
 	httproute.ObjectMeta.Annotations[LatticeAssignedDomainName] = dns
@@ -292,46 +288,34 @@ func (r *HTTPRouteReconciler) updateHTTPRouteStatus(ctx context.Context, dns str
 
 	if len(httproute.Status.RouteStatus.Parents) == 0 {
 		httproute.Status.RouteStatus.Parents = make([]gateway_api.RouteParentStatus, 1)
-		compare = false
 	}
-
 	httproute.Status.RouteStatus.Parents[0].ParentRef = httproute.Spec.ParentRefs[0]
 	httproute.Status.RouteStatus.Parents[0].ControllerName = config.LatticeGatewayControllerName
 
-	timeNow := metav1.NewTime(time.Now())
-	if httprouteOld.Status.RouteStatus.Parents != nil {
-		if len(httprouteOld.Status.RouteStatus.Parents[0].Conditions) > 0 {
-			timeNow = httprouteOld.Status.Parents[0].Conditions[0].LastTransitionTime
-		}
-	}
-
-	accepted := metav1.Condition{
-		Type:               string(gateway_api.RouteConditionAccepted),
-		Status:             metav1.ConditionTrue,
-		ObservedGeneration: httproute.Generation,
-		LastTransitionTime: timeNow,
-		Reason:             string(gateway_api.RouteReasonAccepted),
-		Message:            fmt.Sprintf("DNS Name: %s", dns),
-	}
-	resolvedRefs := metav1.Condition{
-		Type:               string(gateway_api.RouteConditionResolvedRefs),
-		Status:             metav1.ConditionTrue,
-		ObservedGeneration: httproute.Generation,
-		LastTransitionTime: timeNow,
-		Reason:             string(gateway_api.RouteReasonResolvedRefs),
-		Message:            fmt.Sprintf("DNS Name: %s", dns),
-	}
-	httproute.Status.RouteStatus.Parents[0].Conditions = []metav1.Condition{
-		accepted,
-		resolvedRefs,
-	}
-
 	// Update listener Status
-	UpdateHTTPRouteListenerStatus(ctx, r.Client, httproute)
-
-	if compare && r.compareHttproutes(httproute, httprouteOld) {
-		glog.V(6).Infof("updateHTTPRouteStatus: httproute is already up-to-date %v, dns %v\n", httproute, dns)
-		return nil
+	if err := UpdateHTTPRouteListenerStatus(ctx, r.Client, httproute); err != nil {
+		updateRouteCondition(httproute, metav1.Condition{
+			Type:               string(gateway_api.RouteConditionAccepted),
+			Status:             metav1.ConditionFalse,
+			ObservedGeneration: httproute.Generation,
+			Reason:             string(gateway_api.RouteReasonNoMatchingParent),
+			Message:            fmt.Sprintf("Could not match gateway %s: %s", httproute.Spec.ParentRefs[0].Name, err.Error()),
+		})
+	} else {
+		updateRouteCondition(httproute, metav1.Condition{
+			Type:               string(gateway_api.RouteConditionAccepted),
+			Status:             metav1.ConditionTrue,
+			ObservedGeneration: httproute.Generation,
+			Reason:             string(gateway_api.RouteReasonAccepted),
+			Message:            fmt.Sprintf("DNS Name: %s", dns),
+		})
+		updateRouteCondition(httproute, metav1.Condition{
+			Type:               string(gateway_api.RouteConditionResolvedRefs),
+			Status:             metav1.ConditionTrue,
+			ObservedGeneration: httproute.Generation,
+			Reason:             string(gateway_api.RouteReasonResolvedRefs),
+			Message:            fmt.Sprintf("DNS Name: %s", dns),
+		})
 	}
 
 	if err := r.Client.Status().Patch(ctx, httproute, client.MergeFrom(httprouteOld)); err != nil {
@@ -343,31 +327,8 @@ func (r *HTTPRouteReconciler) updateHTTPRouteStatus(ctx context.Context, dns str
 	return nil
 }
 
-func (r *HTTPRouteReconciler) compareHttproutes(httproute1 *gateway_api.HTTPRoute, httproute2 *gateway_api.HTTPRoute) bool {
-
-	result := false
-
-	if httproute1 == httproute2 {
-		return true
-	}
-
-	if (httproute1.Name == httproute2.Name) && (httproute1.Namespace == httproute2.Namespace) &&
-		(len(httproute1.Status.RouteStatus.Parents[0].Conditions) == len(httproute2.Status.RouteStatus.Parents[0].Conditions)) &&
-		(httproute1.Status.RouteStatus.Parents[0].ParentRef.Name == httproute2.Status.RouteStatus.Parents[0].ParentRef.Name) &&
-		(httproute1.Status.RouteStatus.Parents[0].ControllerName == httproute2.Status.RouteStatus.Parents[0].ControllerName) &&
-		(httproute1.ObjectMeta.Annotations[LatticeAssignedDomainName] == httproute2.ObjectMeta.Annotations[LatticeAssignedDomainName]) {
-		i := 0
-		for _, condition := range httproute1.Status.RouteStatus.Parents[0].Conditions {
-			if condition != httproute2.Status.RouteStatus.Parents[0].Conditions[i] {
-				result = false
-				break
-			}
-			i++
-		}
-		result = true
-	}
-
-	return result
+func updateRouteCondition(httproute *gateway_api.HTTPRoute, updated metav1.Condition) {
+	httproute.Status.RouteStatus.Parents[0].Conditions = updateCondition(httproute.Status.RouteStatus.Parents[0].Conditions, updated)
 }
 
 // SetupWithManager sets up the controller with the Manager.