From 059a4b02a7ab69a376603fea968a4ee1b0ad9500 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Tue, 27 Oct 2026 11:37:01 -0500 Subject: [PATCH] fix: Show why a route leaves an NLB unserved When no route served an address, the load balancer's Programmed and Ready conditions said NotServed with "No route serves this address", even when a route was attached and had been rejected for a reason it already reported, such as a backend socket another NLB owns. Users had to find and read the route to learn why. Key changes: - Carry the reason and message of a rejected route onto the address and the load balancer's Programmed and Ready conditions - Read only conditions the route sets itself, never ones that follow the load balancer's status - Reconcile a load balancer when one of its routes changes --- ...etworkloadbalancer_dataplane_controller.go | 91 ++++++++++++- ...kloadbalancer_dataplane_controller_test.go | 121 ++++++++++++++++-- 2 files changed, 198 insertions(+), 14 deletions(-) diff --git a/internal/controller/networkloadbalancer_dataplane_controller.go b/internal/controller/networkloadbalancer_dataplane_controller.go index ee4b3cd9..046165d7 100644 --- a/internal/controller/networkloadbalancer_dataplane_controller.go +++ b/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 == "" { diff --git a/internal/controller/networkloadbalancer_dataplane_controller_test.go b/internal/controller/networkloadbalancer_dataplane_controller_test.go index 36e86e6b..943cfef3 100644 --- a/internal/controller/networkloadbalancer_dataplane_controller_test.go +++ b/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) +}