Merge pull request #4189 from zac-nixon/znixon/gateway-status

[feat gw api] Implement Gateway Status + Some bug fixes for Gateway

Author: k8s-ci-robot

controllers/gateway/config_resolver.go

@@ -29,10 +29,10 @@ func newGatewayConfigResolver() gatewayConfigResolver {
 func (resolver *gatewayConfigResolverImpl) getLoadBalancerConfigForGateway(ctx context.Context, k8sClient client.Client, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass) (elbv2gw.LoadBalancerConfiguration, error) {
 
 	// If the Gateway Class isn't accepted, we shouldn't try to reconcile this Gateway.
-	derivedStatus, _ := deriveGatewayClassAcceptedStatus(gwClass)
+	derivedStatusIndx, ok := deriveAcceptedConditionIndex(gwClass)
 
-	if derivedStatus != metav1.ConditionTrue {
-		return elbv2gw.LoadBalancerConfiguration{}, errors.Errorf("Unable to materialize gateway when gateway class [%s] is not accepted. GatewayClass status is %s", gwClass.Name, derivedStatus)
+	if !ok || gwClass.Status.Conditions[derivedStatusIndx].Status != metav1.ConditionTrue {
+		return elbv2gw.LoadBalancerConfiguration{}, errors.Errorf("Unable to materialize gateway when gateway class [%s] is not accepted", gwClass.Name)
 	}
 
 	gatewayClassLBConfig, err := resolver.configResolverFn(ctx, k8sClient, gwClass.Spec.ParametersRef)

controllers/gateway/gateway_controller.go

@@ -3,9 +3,11 @@ package gateway
 import (
 	"context"
 	"fmt"
+	elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
@@ -35,6 +37,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 	gwv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gwalpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
+	"time"
+)
+
+const (
+	requeueMessage          = "Monitoring provisioning state"
+	statusUpdateRequeueTime = 2 * time.Minute
 )
 
 var _ Reconciler = &gatewayReconciler{}
@@ -86,6 +94,7 @@ func newGatewayReconciler(controllerName string, lbType elbv2model.LoadBalancerT
 		reconcileTracker:        reconcileTracker,
 		cfgResolver:             cfgResolver,
 		routeReconciler:         routeReconciler,
+		gatewayConditionUpdater: prepareGatewayConditionUpdate,
 	}
 }
 
@@ -107,6 +116,7 @@ type gatewayReconciler struct {
 	logger                  logr.Logger
 	metricsCollector        lbcmetrics.MetricCollector
 	reconcileTracker        func(namespaceName types.NamespacedName)
+	gatewayConditionUpdater func(gw *gwv1.Gateway, targetConditionType string, newStatus metav1.ConditionStatus, reason string, message string) bool
 
 	cfgResolver     gatewayConfigResolver
 	routeReconciler routeutils.RouteReconciler
@@ -186,12 +196,23 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R
 	mergedLbConfig, err := r.cfgResolver.getLoadBalancerConfigForGateway(ctx, r.k8sClient, gw, gwClass)
 
 	if err != nil {
+		statusErr := r.updateGatewayStatusFailure(ctx, gw, gwv1.GatewayReasonInvalid, err)
+		if statusErr != nil {
+			r.logger.Error(statusErr, "Unable to update gateway status on failure to retrieve attached config")
+		}
 		return err
 	}
 
 	allRoutes, err := r.gatewayLoader.LoadRoutesForGateway(ctx, *gw, r.routeFilter, r.routeReconciler)
 
 	if err != nil {
+		var loaderErr routeutils.LoaderError
+		if errors.As(err, &loaderErr) {
+			statusErr := r.updateGatewayStatusFailure(ctx, gw, loaderErr.GetGatewayReason(), loaderErr)
+			if statusErr != nil {
+				r.logger.Error(statusErr, "Unable to update gateway status on failure to build routes")
+			}
+		}
 		return err
 	}
 
@@ -248,18 +269,14 @@ func (r *gatewayReconciler) reconcileUpdate(ctx context.Context, gw *gwv1.Gatewa
 	if err != nil {
 		return err
 	}
-	lbDNS, err := lb.DNSName().Resolve(ctx)
-	if err != nil {
-		return err
-	}
 
 	if !backendSGRequired {
 		if err := r.backendSGProvider.Release(ctx, networking.ResourceTypeGateway, []types.NamespacedName{k8s.NamespacedName(gw)}); err != nil {
 			return err
 		}
 	}
 
-	if err = r.updateGatewayStatus(ctx, lbDNS, gw); err != nil {
+	if err = r.updateGatewayStatusSuccess(ctx, lb.Status, gw); err != nil {
 		r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedUpdateStatus, fmt.Sprintf("Failed update status due to %v", err))
 		return err
 	}
@@ -295,28 +312,58 @@ func (r *gatewayReconciler) buildModel(ctx context.Context, gw *gwv1.Gateway, cf
 	return stack, lb, backendSGRequired, nil
 }
 
-func (r *gatewayReconciler) updateGatewayStatus(ctx context.Context, lbDNS string, gw *gwv1.Gateway) error {
-	// TODO Consider LB ARN.
+func (r *gatewayReconciler) updateGatewayStatusSuccess(ctx context.Context, lbStatus *elbv2model.LoadBalancerStatus, gw *gwv1.Gateway) error {
+	// LB Status should always be set, if it's not, we need to prevent NPE
+	if lbStatus == nil {
+		r.logger.Info("Unable to update Gateway Status due to null LB status")
+		return nil
+	}
+	gwOld := gw.DeepCopy()
+
+	var needPatch bool
+	var requeueNeeded bool
+	if isGatewayProgrammed(*lbStatus) {
+		needPatch = r.gatewayConditionUpdater(gw, string(gwv1.GatewayConditionProgrammed), metav1.ConditionTrue, string(gwv1.GatewayConditionProgrammed), lbStatus.LoadBalancerARN)
+	} else {
+		requeueNeeded = true
+	}
 
-	// Gateway Address Status
+	needPatch = r.gatewayConditionUpdater(gw, string(gwv1.GatewayConditionAccepted), metav1.ConditionTrue, string(gwv1.GatewayConditionAccepted), "") || needPatch
 	if len(gw.Status.Addresses) != 1 ||
-		gw.Status.Addresses[0].Value != "" ||
-		gw.Status.Addresses[0].Value != lbDNS {
-		gwOld := gw.DeepCopy()
+		gw.Status.Addresses[0].Value != lbStatus.DNSName {
 		ipAddressType := gwv1.HostnameAddressType
 		gw.Status.Addresses = []gwv1.GatewayStatusAddress{
 			{
 				Type:  &ipAddressType,
-				Value: lbDNS,
+				Value: lbStatus.DNSName,
 			},
 		}
+		needPatch = true
+	}
+
+	if needPatch {
 		if err := r.k8sClient.Status().Patch(ctx, gw, client.MergeFrom(gwOld)); err != nil {
 			return errors.Wrapf(err, "failed to update gw status: %v", k8s.NamespacedName(gw))
 		}
 	}
 
-	// TODO: Listener status ListenerStatus
-	// https://github.com/aws/aws-application-networking-k8s/blob/main/pkg/controllers/gateway_controller.go#L350
+	if requeueNeeded {
+		return runtime.NewRequeueNeededAfter(requeueMessage, statusUpdateRequeueTime)
+	}
+
+	return nil
+}
+
+func (r *gatewayReconciler) updateGatewayStatusFailure(ctx context.Context, gw *gwv1.Gateway, reason gwv1.GatewayConditionReason, err error) error {
+	gwOld := gw.DeepCopy()
+
+	needPatch := r.gatewayConditionUpdater(gw, string(gwv1.GatewayConditionAccepted), metav1.ConditionFalse, string(reason), err.Error())
+
+	if needPatch {
+		if err := r.k8sClient.Status().Patch(ctx, gw, client.MergeFrom(gwOld)); err != nil {
+			return errors.Wrapf(err, "failed to update gw status: %v", k8s.NamespacedName(gw))
+		}
+	}
 
 	return nil
 }
@@ -475,3 +522,12 @@ func (r *gatewayReconciler) setupNLBGatewayControllerWatches(ctrl controller.Con
 	return nil
 
 }
+
+func isGatewayProgrammed(lbStatus elbv2model.LoadBalancerStatus) bool {
+	if lbStatus.ProvisioningState == nil {
+		return false
+	}
+
+	return lbStatus.ProvisioningState.Code == elbv2types.LoadBalancerStateEnumActive || lbStatus.ProvisioningState.Code == elbv2types.LoadBalancerStateEnumActiveImpaired
+
+}

controllers/gateway/utils.go

@@ -14,16 +14,21 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode/utf8"
 )
 
 const (
 	gatewayClassAnnotationLastProcessedConfig          = "elbv2.k8s.aws/last-processed-config"
 	gatewayClassAnnotationLastProcessedConfigTimestamp = gatewayClassAnnotationLastProcessedConfig + "-timestamp"
+
+	// The max message that can be stored in a condition
+	maxMessageLength = 32700
 )
 
+// updateGatewayClassLastProcessedConfig updates the gateway class annotations with the last processed lb config resource version or "" if no lb config is attached to the gatewayclass
 func updateGatewayClassLastProcessedConfig(ctx context.Context, k8sClient client.Client, gwClass *gwv1.GatewayClass, lbConf *elbv2gw.LoadBalancerConfiguration) error {
 
-	calculatedVersion := gatewayClassAnnotationLastProcessedConfig
+	calculatedVersion := ""
 
 	if lbConf != nil {
 		calculatedVersion = lbConf.ResourceVersion
@@ -40,12 +45,16 @@ func updateGatewayClassLastProcessedConfig(ctx context.Context, k8sClient client
 	}
 
 	gwClassOld := gwClass.DeepCopy()
+	if gwClass.Annotations == nil {
+		gwClass.Annotations = make(map[string]string)
+	}
 	gwClass.Annotations[gatewayClassAnnotationLastProcessedConfig] = calculatedVersion
 	gwClass.Annotations[gatewayClassAnnotationLastProcessedConfigTimestamp] = strconv.FormatInt(time.Now().Unix(), 10)
 
 	return k8sClient.Patch(ctx, gwClass, client.MergeFrom(gwClassOld))
 }
 
+// getStoredProcessedConfig retrieves the resource version attached to the lb config referenced by the gateway class or nil if no such mapping exists.
 func getStoredProcessedConfig(gwClass *gwv1.GatewayClass) *string {
 	var storedVersion *string
 
@@ -58,10 +67,20 @@ func getStoredProcessedConfig(gwClass *gwv1.GatewayClass) *string {
 	return storedVersion
 }
 
+// updateGatewayClassAcceptedCondition updates the 'accepted' condition on the gateway class to the passed in parameters. if no 'Accepted' condition exists, do nothing.
 func updateGatewayClassAcceptedCondition(ctx context.Context, k8sClient client.Client, gwClass *gwv1.GatewayClass, newStatus metav1.ConditionStatus, reason string, message string) error {
-	derivedStatus, indxToUpdate := deriveGatewayClassAcceptedStatus(gwClass)
+	indxToUpdate, ok := deriveAcceptedConditionIndex(gwClass)
+
+	if ok {
+
+		storedStatus := gwClass.Status.Conditions[indxToUpdate].Status
+		storedMessage := gwClass.Status.Conditions[indxToUpdate].Message
+		storedReason := gwClass.Status.Conditions[indxToUpdate].Reason
+
+		if storedStatus == newStatus && storedMessage == message && storedReason == reason {
+			return nil
+		}
 
-	if indxToUpdate != -1 && derivedStatus != newStatus {
 		gwClassOld := gwClass.DeepCopy()
 		gwClass.Status.Conditions[indxToUpdate].LastTransitionTime = metav1.NewTime(time.Now())
 		gwClass.Status.Conditions[indxToUpdate].ObservedGeneration = gwClass.Generation
@@ -75,15 +94,56 @@ func updateGatewayClassAcceptedCondition(ctx context.Context, k8sClient client.C
 	return nil
 }
 
-func deriveGatewayClassAcceptedStatus(gwClass *gwv1.GatewayClass) (metav1.ConditionStatus, int) {
+// prepareGatewayConditionUpdate inserts the necessary data into the condition field of the gateway. The caller should patch the corresponding gateway. Returns false when no change was performed.
+func prepareGatewayConditionUpdate(gw *gwv1.Gateway, targetConditionType string, newStatus metav1.ConditionStatus, reason string, message string) bool {
+
+	indxToUpdate := -1
+	var derivedCondition metav1.Condition
+	for i, condition := range gw.Status.Conditions {
+		if condition.Type == targetConditionType {
+			indxToUpdate = i
+			derivedCondition = condition
+			break
+		}
+	}
+
+	// 32768 is the max message limit
+	truncatedMessage := truncateMessage(message)
+
+	if indxToUpdate != -1 {
+		if derivedCondition.Status != newStatus || derivedCondition.Message != truncatedMessage || derivedCondition.Reason != reason {
+			gw.Status.Conditions[indxToUpdate].LastTransitionTime = metav1.NewTime(time.Now())
+			gw.Status.Conditions[indxToUpdate].ObservedGeneration = gw.Generation
+			gw.Status.Conditions[indxToUpdate].Status = newStatus
+			gw.Status.Conditions[indxToUpdate].Message = truncatedMessage
+			gw.Status.Conditions[indxToUpdate].Reason = reason
+			return true
+		}
+	}
+	return false
+}
+
+func truncateMessage(s string) string {
+	if utf8.RuneCountInString(s) <= maxMessageLength {
+		return s
+	}
+
+	runes := []rune(s)
+	return string(runes[:maxMessageLength]) + "..."
+}
+
+// deriveAcceptedConditionIndex returns the index of the condition pertaining to the accepted condition.
+// -1 if the condition doesn't exist
+func deriveAcceptedConditionIndex(gwClass *gwv1.GatewayClass) (int, bool) {
 	for i, v := range gwClass.Status.Conditions {
 		if v.Type == string(gwv1.GatewayClassReasonAccepted) {
-			return v.Status, i
+			return i, true
 		}
 	}
-	return metav1.ConditionFalse, -1
+	return -1, false
 }
 
+// resolveLoadBalancerConfig returns the lb config referenced in the ParametersReference.
 func resolveLoadBalancerConfig(ctx context.Context, k8sClient client.Client, reference *gwv1.ParametersReference) (*elbv2gw.LoadBalancerConfiguration, error) {
 	var lbConf *elbv2gw.LoadBalancerConfiguration