fix: Show why a route leaves a network load balancer unserved

internal/controller/networkloadbalancer_dataplane_controller.go

@@ -5,6 +5,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"slices"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -16,7 +17,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/cluster"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder"
+	mchandler "sigs.k8s.io/multicluster-runtime/pkg/handler"
 	mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager"
 	"sigs.k8s.io/multicluster-runtime/pkg/multicluster"
 	mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile"
@@ -26,7 +29,10 @@ import (
 	"go.datum.net/network-services-operator/internal/downstreamclient"
 )
 
-const nlbReasonPendingDataPlane = "PendingDataPlane"
+const (
+	nlbReasonPendingDataPlane = "PendingDataPlane"
+	nlbReasonNotServed        = "NotServed"
+)
 
 // NetworkLoadBalancerDataPlaneReconciler reports what serving locations say
 // about a load balancer's addresses. Locations write status on their copy of
@@ -39,6 +45,7 @@ type NetworkLoadBalancerDataPlaneReconciler struct {
 // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkloadbalancers,verbs=get;list;watch
 // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkloadbalancers/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkloadbalancercellintents,verbs=get;list;watch
+// +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkloadbalancerroutes,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
 
 func (r *NetworkLoadBalancerDataPlaneReconciler) Reconcile(ctx context.Context, req mcreconcile.Request) (ctrl.Result, error) {
@@ -80,8 +87,13 @@ func (r *NetworkLoadBalancerDataPlaneReconciler) reconcileDataPlane(
 		}
 	}
 
+	var routes networkingv1alpha.NetworkLoadBalancerRouteList
+	if err := projectClient.List(ctx, &routes, client.InNamespace(lb.Namespace)); err != nil {
+		return fmt.Errorf("list load balancer routes: %w", err)
+	}
+
 	before := lb.DeepCopy()
-	applyNetworkLoadBalancerDataPlane(&lb, intents)
+	applyNetworkLoadBalancerDataPlane(&lb, intents, routes.Items)
 	if equality.Semantic.DeepEqual(before.Status, lb.Status) {
 		return nil
 	}
@@ -91,7 +103,11 @@ func (r *NetworkLoadBalancerDataPlaneReconciler) reconcileDataPlane(
 	return nil
 }
 
-func applyNetworkLoadBalancerDataPlane(lb *networkingv1alpha.NetworkLoadBalancer, intents []networkingv1alpha.NetworkLoadBalancerCellIntent) {
+func applyNetworkLoadBalancerDataPlane(
+	lb *networkingv1alpha.NetworkLoadBalancer,
+	intents []networkingv1alpha.NetworkLoadBalancerCellIntent,
+	routes []networkingv1alpha.NetworkLoadBalancerRoute,
+) {
 	var addresses []*networkingv1alpha.NetworkLoadBalancerAddressStatus
 	for i := range lb.Status.Endpoints {
 		endpoint := &lb.Status.Endpoints[i]
@@ -105,6 +121,11 @@ func applyNetworkLoadBalancerDataPlane(lb *networkingv1alpha.NetworkLoadBalancer
 			if !known {
 				continue
 			}
+			if programmed.Reason == nlbReasonNotServed {
+				if rejection, ok := routeRejection(lb, routes, endpoint.RequestName); ok {
+					programmed, announced = rejection, rejection
+				}
+			}
 			setLoadBalancerCondition(&address.Conditions, lb.Generation, networkingv1alpha.NetworkLoadBalancerProgrammed,
 				programmed.Status, programmed.Reason, programmed.Message)
 			setLoadBalancerCondition(&address.Conditions, lb.Generation, networkingv1alpha.NetworkLoadBalancerAnnounced,
@@ -159,10 +180,53 @@ func addressDataPlane(
 			return metav1.Condition{}, metav1.Condition{}, false
 		}
 	}
-	unserved := metav1.Condition{Status: metav1.ConditionFalse, Reason: "NotServed", Message: "No route serves this address"}
+	unserved := metav1.Condition{Status: metav1.ConditionFalse, Reason: nlbReasonNotServed, Message: "No route serves this address"}
 	return unserved, unserved, true
 }
 
+func routeRejection(
+	lb *networkingv1alpha.NetworkLoadBalancer, routes []networkingv1alpha.NetworkLoadBalancerRoute, requestName string,
+) (metav1.Condition, bool) {
+	listeners := map[string]bool{}
+	for _, listener := range lb.Spec.Listeners {
+		if slices.Contains(listener.AddressRefs, requestName) {
+			listeners[listener.Name] = true
+		}
+	}
+	sorted := slices.Clone(routes)
+	slices.SortFunc(sorted, func(a, b networkingv1alpha.NetworkLoadBalancerRoute) int { return strings.Compare(a.Name, b.Name) })
+	for i := range sorted {
+		route := &sorted[i]
+		for _, parent := range route.Status.Parents {
+			if parent.Name != lb.Name || !listeners[parent.Listener] {
+				continue
+			}
+			for _, conditionType := range []string{
+				networkingv1alpha.NetworkLoadBalancerRouteAccepted,
+				networkingv1alpha.NetworkLoadBalancerRouteResolvedRefs,
+				networkingv1alpha.NetworkLoadBalancerRouteProgrammed,
+			} {
+				condition := apimeta.FindStatusCondition(parent.Conditions, conditionType)
+				if condition == nil || condition.Status != metav1.ConditionFalse || condition.ObservedGeneration != route.Generation {
+					continue
+				}
+				if conditionType != networkingv1alpha.NetworkLoadBalancerRouteAccepted && !parent.Effective {
+					continue
+				}
+				if conditionType == networkingv1alpha.NetworkLoadBalancerRouteProgrammed && condition.Reason != nlbRouteReasonNoEligibleEndpoints {
+					continue
+				}
+				return metav1.Condition{
+					Status:  metav1.ConditionFalse,
+					Reason:  condition.Reason,
+					Message: fmt.Sprintf("Route %q: %s", route.Name, condition.Message),
+				}, true
+			}
+		}
+	}
+	return metav1.Condition{}, false
+}
+
 func reportedIntentCondition(intent *networkingv1alpha.NetworkLoadBalancerCellIntent, conditionType string) metav1.Condition {
 	pending := metav1.Condition{Status: metav1.ConditionUnknown, Reason: nlbReasonPendingDataPlane,
 		Message: "Waiting for the serving location to report"}
@@ -235,11 +299,30 @@ func (r *NetworkLoadBalancerDataPlaneReconciler) SetupWithManager(mgr mcmanager.
 	}
 	return mcbuilder.ControllerManagedBy(mgr).
 		For(&networkingv1alpha.NetworkLoadBalancer{}, mcbuilder.WithEngageWithLocalCluster(false)).
+		Watches(&networkingv1alpha.NetworkLoadBalancerRoute{},
+			func(clusterName multicluster.ClusterName, _ cluster.Cluster) mchandler.EventHandler {
+				return mchandler.ForCluster(handler.EnqueueRequestsFromMapFunc(loadBalancersForRoute), clusterName)
+			}).
 		WatchesRawSource(intentSource).
 		Named("networkloadbalancerdataplane").
 		Complete(r)
 }
 
+func loadBalancersForRoute(_ context.Context, obj client.Object) []reconcile.Request {
+	route, ok := obj.(*networkingv1alpha.NetworkLoadBalancerRoute)
+	if !ok {
+		return nil
+	}
+	var requests []reconcile.Request
+	for _, ref := range route.Spec.ParentRefs {
+		request := reconcile.Request{NamespacedName: types.NamespacedName{Namespace: route.Namespace, Name: ref.Name}}
+		if !slices.Contains(requests, request) {
+			requests = append(requests, request)
+		}
+	}
+	return requests
+}
+
 func loadBalancerForCellIntent(intent *networkingv1alpha.NetworkLoadBalancerCellIntent) []mcreconcile.Request {
 	source := intent.Spec.Source
 	if source.ProjectCluster == "" || source.Namespace == "" || source.ParentName == "" {

internal/controller/networkloadbalancer_dataplane_controller_test.go

@@ -61,7 +61,7 @@ func reportIntent(intent *networkingv1alpha.NetworkLoadBalancerCellIntent, progr
 func TestNLBDataPlaneReportsServedAddress(t *testing.T) {
 	lb, intent := dataPlaneFixture()
 	reportIntent(&intent, metav1.ConditionTrue, metav1.ConditionTrue)
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 
 	address := lb.Status.Endpoints[0].Addresses[0]
 	require.True(t, apimeta.IsStatusConditionTrue(address.Conditions, networkingv1alpha.NetworkLoadBalancerProgrammed))
@@ -75,7 +75,7 @@ func TestNLBDataPlaneWaitsForCurrentReport(t *testing.T) {
 	lb, intent := dataPlaneFixture()
 	reportIntent(&intent, metav1.ConditionTrue, metav1.ConditionTrue)
 	intent.Generation = 3
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 
 	programmed := apimeta.FindStatusCondition(lb.Status.Conditions, networkingv1alpha.NetworkLoadBalancerProgrammed)
 	require.NotNil(t, programmed)
@@ -87,7 +87,7 @@ func TestNLBDataPlaneWaitsForCurrentReport(t *testing.T) {
 func TestNLBDataPlaneNotReadyUntilAnnounced(t *testing.T) {
 	lb, intent := dataPlaneFixture()
 	reportIntent(&intent, metav1.ConditionTrue, metav1.ConditionFalse)
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 
 	require.True(t, apimeta.IsStatusConditionTrue(lb.Status.Conditions, networkingv1alpha.NetworkLoadBalancerProgrammed))
 	ready := apimeta.FindStatusCondition(lb.Status.Conditions, networkingv1alpha.NetworkLoadBalancerReady)
@@ -98,15 +98,15 @@ func TestNLBDataPlaneNotReadyUntilAnnounced(t *testing.T) {
 func TestNLBDataPlaneKeepsAddressUntilWithdrawalConfirmed(t *testing.T) {
 	lb, intent := dataPlaneFixture()
 	reportIntent(&intent, metav1.ConditionTrue, metav1.ConditionTrue)
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 
 	intent.Spec.State = networkingv1alpha.NetworkLoadBalancerCellIntentWithdrawn
 	intent.Generation = 3
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 	require.True(t, apimeta.IsStatusConditionTrue(lb.Status.Endpoints[0].Addresses[0].Conditions, networkingv1alpha.NetworkLoadBalancerProgrammed))
 
 	reportIntent(&intent, metav1.ConditionFalse, metav1.ConditionFalse)
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 	address := lb.Status.Endpoints[0].Addresses[0]
 	programmed := apimeta.FindStatusCondition(address.Conditions, networkingv1alpha.NetworkLoadBalancerProgrammed)
 	require.NotNil(t, programmed)
@@ -119,7 +119,7 @@ func TestNLBDataPlaneIgnoresOtherAddresses(t *testing.T) {
 	lb, intent := dataPlaneFixture()
 	reportIntent(&intent, metav1.ConditionTrue, metav1.ConditionTrue)
 	intent.Spec.VIP = "2607:ed40:63::3"
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 
 	programmed := apimeta.FindStatusCondition(lb.Status.Endpoints[0].Addresses[0].Conditions, networkingv1alpha.NetworkLoadBalancerProgrammed)
 	require.NotNil(t, programmed)
@@ -135,13 +135,13 @@ func TestNLBRouteProgrammedFollowsListenerAddresses(t *testing.T) {
 	require.False(t, ok)
 
 	reportIntent(&intent, metav1.ConditionTrue, metav1.ConditionTrue)
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 	programmed, ok := listenerAddressesProgrammed(lb, listener)
 	require.True(t, ok)
 	require.Equal(t, metav1.ConditionTrue, programmed.Status)
 
 	intent.Generation = 3
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 	programmed, ok = listenerAddressesProgrammed(lb, listener)
 	require.True(t, ok)
 	require.Equal(t, metav1.ConditionFalse, programmed.Status)
@@ -151,7 +151,7 @@ func TestNLBRouteProgrammedFollowsListenerAddresses(t *testing.T) {
 func TestNLBDataPlaneClearsTerminatingLoadBalancerStatus(t *testing.T) {
 	lb, intent := dataPlaneFixture()
 	reportIntent(&intent, metav1.ConditionTrue, metav1.ConditionTrue)
-	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent})
+	applyNetworkLoadBalancerDataPlane(lb, []networkingv1alpha.NetworkLoadBalancerCellIntent{intent}, nil)
 	lb.Finalizers = []string{"networking.test/hold"}
 	now := metav1.Now()
 	lb.DeletionTimestamp = &now
@@ -163,3 +163,104 @@ func TestNLBDataPlaneClearsTerminatingLoadBalancerStatus(t *testing.T) {
 	require.False(t, lb.Status.Endpoints[0].Addresses[0].Announced)
 	require.True(t, apimeta.IsStatusConditionFalse(lb.Status.Conditions, networkingv1alpha.NetworkLoadBalancerReady))
 }
+
+func rejectedRoute(name, lbName, listener string, effective bool, conditions ...metav1.Condition) networkingv1alpha.NetworkLoadBalancerRoute {
+	route := networkingv1alpha.NetworkLoadBalancerRoute{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default", Generation: 4}}
+	for i := range conditions {
+		conditions[i].ObservedGeneration = route.Generation
+		conditions[i].LastTransitionTime = metav1.Now()
+	}
+	route.Status.Parents = []networkingv1alpha.NetworkLoadBalancerRouteParentStatus{{
+		Name: lbName, Listener: listener, Effective: effective, Conditions: conditions,
+	}}
+	return route
+}
+
+func withListener(lb *networkingv1alpha.NetworkLoadBalancer) *networkingv1alpha.NetworkLoadBalancer {
+	lb.Spec.Listeners = []networkingv1alpha.NetworkLoadBalancerListener{{Name: "tcp-80", AddressRefs: []string{"public"}}}
+	return lb
+}
+
+func TestNLBDataPlaneSurfacesRouteRejection(t *testing.T) {
+	conflict := metav1.Condition{Type: networkingv1alpha.NetworkLoadBalancerRouteResolvedRefs, Status: metav1.ConditionFalse,
+		Reason: nlbRouteReasonBackendClaimConflict, Message: "Backend socket is already selected by older route \"old\""}
+	lb, _ := dataPlaneFixture()
+	applyNetworkLoadBalancerDataPlane(withListener(lb), nil, []networkingv1alpha.NetworkLoadBalancerRoute{
+		rejectedRoute("web", "web", "tcp-80", true, conflict),
+	})
+
+	for _, conditionType := range []string{networkingv1alpha.NetworkLoadBalancerProgrammed, networkingv1alpha.NetworkLoadBalancerReady} {
+		condition := apimeta.FindStatusCondition(lb.Status.Conditions, conditionType)
+		require.NotNil(t, condition)
+		require.Equal(t, metav1.ConditionFalse, condition.Status)
+		require.Equal(t, nlbRouteReasonBackendClaimConflict, condition.Reason)
+		require.Equal(t, `Route "web": `+conflict.Message, condition.Message)
+	}
+	announced := apimeta.FindStatusCondition(lb.Status.Endpoints[0].Addresses[0].Conditions, networkingv1alpha.NetworkLoadBalancerAnnounced)
+	require.NotNil(t, announced)
+	require.Equal(t, nlbRouteReasonBackendClaimConflict, announced.Reason)
+}
+
+func TestNLBDataPlaneSurfacesRouteWithNoEligibleEndpoints(t *testing.T) {
+	lb, _ := dataPlaneFixture()
+	applyNetworkLoadBalancerDataPlane(withListener(lb), nil, []networkingv1alpha.NetworkLoadBalancerRoute{
+		rejectedRoute("web", "web", "tcp-80", true,
+			metav1.Condition{Type: networkingv1alpha.NetworkLoadBalancerRouteResolvedRefs, Status: metav1.ConditionTrue, Reason: "Resolved"},
+			metav1.Condition{Type: networkingv1alpha.NetworkLoadBalancerRouteProgrammed, Status: metav1.ConditionFalse,
+				Reason: nlbRouteReasonNoEligibleEndpoints, Message: "No enabled backend"}),
+	})
+
+	ready := apimeta.FindStatusCondition(lb.Status.Conditions, networkingv1alpha.NetworkLoadBalancerReady)
+	require.NotNil(t, ready)
+	require.Equal(t, nlbRouteReasonNoEligibleEndpoints, ready.Reason)
+}
+
+func TestNLBDataPlaneIgnoresRouteStatusItCannotExplain(t *testing.T) {
+	awaiting := metav1.Condition{Type: networkingv1alpha.NetworkLoadBalancerRouteProgrammed, Status: metav1.ConditionFalse,
+		Reason: nlbRouteReasonAwaitingDataPlane, Message: "waiting"}
+	conflict := metav1.Condition{Type: networkingv1alpha.NetworkLoadBalancerRouteResolvedRefs, Status: metav1.ConditionFalse,
+		Reason: nlbRouteReasonBackendClaimConflict, Message: "conflict"}
+	staleRoute := rejectedRoute("stale", "web", "tcp-80", true, conflict)
+	staleRoute.Generation = 5
+
+	for name, route := range map[string]networkingv1alpha.NetworkLoadBalancerRoute{
+		"follows load balancer status": rejectedRoute("web", "web", "tcp-80", true, awaiting),
+		"other load balancer":          rejectedRoute("web", "api", "tcp-80", true, conflict),
+		"other listener":               rejectedRoute("web", "web", "udp-53", true, conflict),
+		"superseded route":             rejectedRoute("web", "web", "tcp-80", false, conflict),
+		"stale report":                 staleRoute,
+	} {
+		t.Run(name, func(t *testing.T) {
+			lb, _ := dataPlaneFixture()
+			applyNetworkLoadBalancerDataPlane(withListener(lb), nil, []networkingv1alpha.NetworkLoadBalancerRoute{route})
+			ready := apimeta.FindStatusCondition(lb.Status.Conditions, networkingv1alpha.NetworkLoadBalancerReady)
+			require.NotNil(t, ready)
+			require.Equal(t, nlbReasonNotServed, ready.Reason)
+		})
+	}
+}
+
+func TestNLBDataPlaneKeepsServedAddressDespiteRejectedRoute(t *testing.T) {
+	lb, intent := dataPlaneFixture()
+	reportIntent(&intent, metav1.ConditionTrue, metav1.ConditionTrue)
+	applyNetworkLoadBalancerDataPlane(withListener(lb), []networkingv1alpha.NetworkLoadBalancerCellIntent{intent},
+		[]networkingv1alpha.NetworkLoadBalancerRoute{rejectedRoute("web", "web", "tcp-80", true,
+			metav1.Condition{Type: networkingv1alpha.NetworkLoadBalancerRouteResolvedRefs, Status: metav1.ConditionFalse,
+				Reason: nlbRouteReasonBackendClaimConflict, Message: "conflict"})})
+
+	require.True(t, apimeta.IsStatusConditionTrue(lb.Status.Conditions, networkingv1alpha.NetworkLoadBalancerReady))
+}
+
+func TestNLBDataPlaneEnqueuesEachRouteParentOnce(t *testing.T) {
+	route := &networkingv1alpha.NetworkLoadBalancerRoute{
+		ObjectMeta: metav1.ObjectMeta{Name: "web", Namespace: "default"},
+		Spec: networkingv1alpha.NetworkLoadBalancerRouteSpec{ParentRefs: []networkingv1alpha.NetworkLoadBalancerRouteParentRef{
+			{Name: "web", Listener: "tcp-80"}, {Name: "web", Listener: "udp-53"}, {Name: "api"},
+		}},
+	}
+	requests := loadBalancersForRoute(context.Background(), route)
+	require.Len(t, requests, 2)
+	require.Equal(t, "web", requests[0].Name)
+	require.Equal(t, "api", requests[1].Name)
+	require.Equal(t, "default", requests[0].Namespace)
+}