Skip to content

Commit 4da59ea

Browse files
y-rabiey-rabie
committed
fix: merge xRoute/xPolicy statuses across controllers in provider runner
Signed-off-by: y-rabie <[email protected]>
1 parent f8dd034 commit 4da59ea

File tree

4 files changed

+113
-133
lines changed

4 files changed

+113
-133
lines changed

‎internal/provider/kubernetes/status.go‎

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package kubernetes
88
import (
99
"context"
1010
"fmt"
11+
"reflect"
1112

1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1314
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -111,7 +112,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
111112
Spec: h.Spec,
112113
Status: gwapiv1.HTTPRouteStatus{
113114
RouteStatus: gwapiv1.RouteStatus{
114-
Parents: mergeRouteParentStatus(h.Namespace, h.Status.Parents, valCopy.Parents),
115+
Parents: mergeStatus(h.Namespace, r.envoyGateway.Gateway.ControllerName, h.Status.Parents, valCopy.Parents),
115116
},
116117
},
117118
}
@@ -151,7 +152,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
151152
Spec: g.Spec,
152153
Status: gwapiv1.GRPCRouteStatus{
153154
RouteStatus: gwapiv1.RouteStatus{
154-
Parents: mergeRouteParentStatus(g.Namespace, g.Status.Parents, valCopy.Parents),
155+
Parents: mergeStatus(g.Namespace, r.envoyGateway.Gateway.ControllerName, g.Status.Parents, valCopy.Parents),
155156
},
156157
},
157158
}
@@ -193,7 +194,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
193194
Spec: t.Spec,
194195
Status: gwapiv1a2.TLSRouteStatus{
195196
RouteStatus: gwapiv1.RouteStatus{
196-
Parents: mergeRouteParentStatus(t.Namespace, t.Status.Parents, valCopy.Parents),
197+
Parents: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Parents, valCopy.Parents),
197198
},
198199
},
199200
}
@@ -235,7 +236,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
235236
Spec: t.Spec,
236237
Status: gwapiv1a2.TCPRouteStatus{
237238
RouteStatus: gwapiv1.RouteStatus{
238-
Parents: mergeRouteParentStatus(t.Namespace, t.Status.Parents, valCopy.Parents),
239+
Parents: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Parents, valCopy.Parents),
239240
},
240241
},
241242
}
@@ -277,7 +278,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
277278
Spec: u.Spec,
278279
Status: gwapiv1a2.UDPRouteStatus{
279280
RouteStatus: gwapiv1.RouteStatus{
280-
Parents: mergeRouteParentStatus(u.Namespace, u.Status.Parents, valCopy.Parents),
281+
Parents: mergeStatus(u.Namespace, r.envoyGateway.Gateway.ControllerName, u.Status.Parents, valCopy.Parents),
281282
},
282283
},
283284
}
@@ -317,7 +318,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
317318
TypeMeta: t.TypeMeta,
318319
ObjectMeta: t.ObjectMeta,
319320
Spec: t.Spec,
320-
Status: *valCopy,
321+
Status: gwapiv1.PolicyStatus{
322+
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
323+
},
321324
}
322325
return tCopy
323326
}),
@@ -355,7 +358,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
355358
TypeMeta: t.TypeMeta,
356359
ObjectMeta: t.ObjectMeta,
357360
Spec: t.Spec,
358-
Status: *valCopy,
361+
Status: gwapiv1.PolicyStatus{
362+
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
363+
},
359364
}
360365
return tCopy
361366
}),
@@ -393,7 +398,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
393398
TypeMeta: t.TypeMeta,
394399
ObjectMeta: t.ObjectMeta,
395400
Spec: t.Spec,
396-
Status: *valCopy,
401+
Status: gwapiv1.PolicyStatus{
402+
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
403+
},
397404
}
398405
return tCopy
399406
}),
@@ -431,7 +438,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
431438
TypeMeta: t.TypeMeta,
432439
ObjectMeta: t.ObjectMeta,
433440
Spec: t.Spec,
434-
Status: *valCopy,
441+
Status: gwapiv1.PolicyStatus{
442+
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
443+
},
435444
}
436445
return tCopy
437446
}),
@@ -467,7 +476,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
467476
TypeMeta: t.TypeMeta,
468477
ObjectMeta: t.ObjectMeta,
469478
Spec: t.Spec,
470-
Status: *valCopy,
479+
Status: gwapiv1.PolicyStatus{
480+
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
481+
},
471482
}
472483
return tCopy
473484
}),
@@ -505,7 +516,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
505516
TypeMeta: t.TypeMeta,
506517
ObjectMeta: t.ObjectMeta,
507518
Spec: t.Spec,
508-
Status: *valCopy,
519+
Status: gwapiv1.PolicyStatus{
520+
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
521+
},
509522
}
510523
return tCopy
511524
}),
@@ -579,10 +592,11 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
579592
errChan <- err
580593
panic(err)
581594
}
582-
tCopy := t.DeepCopy()
583595
valCopy := val.DeepCopy()
584596
setLastTransitionTimeInConditionsForPolicyStatus(valCopy, metav1.Now())
585-
tCopy.Object["status"] = *valCopy
597+
tCopy := t.DeepCopy()
598+
oldStatus := gatewayapi.ExtServerPolicyStatusAsPolicyStatus(tCopy)
599+
tCopy.Object["status"] = mergeStatus(t.GetNamespace(), r.envoyGateway.Gateway.ControllerName, oldStatus.Ancestors, valCopy.Ancestors)
586600
return tCopy
587601
}),
588602
})
@@ -593,31 +607,28 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
593607
}
594608
}
595609

596-
// mergeRouteParentStatus merges the old and new RouteParentStatus.
597-
// This is needed because the RouteParentStatus doesn't support strategic merge patch yet.
598-
func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []gwapiv1.RouteParentStatus {
610+
// mergeStatus merges the old and new `RouteParentStatus`/`PolicyAncestorStatus`.
611+
// This is needed because the `RouteParentStatus`/`PolicyAncestorStatus` doesn't support strategic merge patch yet.
612+
// This depends on the fact that we get the full updated status of the route/policy (all parents/ancestors), and will break otherwise.
613+
func mergeStatus[K interface{}](ns, controllerName string, old, new []K) []K {
599614
// Allocating with worst-case capacity to avoid reallocation.
600-
merged := make([]gwapiv1.RouteParentStatus, 0, len(old)+len(new))
615+
merged := make([]K, 0, len(old)+len(new))
601616

602617
// Range over old status parentRefs in order:
603618
// 1. The parentRef exists in the new status: append the new one to the final status.
604619
// 2. The parentRef doesn't exist in the new status and it's not our controller: append it to the final status.
605-
// 3. The parentRef doesn't exist in the new status, and it is our controller: keep it in the final status.
606-
// This is important for routes with multiple parent references - not all parents are updated in each reconciliation.
620+
// 3. The parentRef doesn't exist in the new status, and it is our controller: don't append it to the final status.
607621
for _, oldP := range old {
608622
found := -1
609623
for newI, newP := range new {
610-
if gatewayapi.IsParentRefEqual(oldP.ParentRef, newP.ParentRef, ns) {
624+
if isParentOrAncestorRefEqual(oldP, newP, ns) {
611625
found = newI
612626
break
613627
}
614628
}
615629
if found >= 0 {
616630
merged = append(merged, new[found])
617-
} else {
618-
// Keep all old parent statuses, regardless of controller.
619-
// For routes with multiple parents managed by the same controller,
620-
// not all parents are necessarily updated in each reconciliation.
631+
} else if parentOrAncestorControllerName(oldP) != gwapiv1.GatewayController(controllerName) {
621632
merged = append(merged, oldP)
622633
}
623634
}
@@ -626,7 +637,7 @@ func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []g
626637
for _, newP := range new {
627638
found := false
628639
for _, mergedP := range merged {
629-
if gatewayapi.IsParentRefEqual(newP.ParentRef, mergedP.ParentRef, ns) {
640+
if isParentOrAncestorRefEqual(newP, mergedP, ns) {
630641
found = true
631642
break
632643
}
@@ -639,6 +650,28 @@ func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []g
639650
return merged
640651
}
641652

653+
func isParentOrAncestorRefEqual[K any](firstRef, secondRef K, ns string) bool {
654+
switch reflect.TypeOf(firstRef) {
655+
case reflect.TypeOf(gwapiv1.RouteParentStatus{}):
656+
return gatewayapi.IsParentRefEqual(any(firstRef).(gwapiv1.RouteParentStatus).ParentRef, any(secondRef).(gwapiv1.RouteParentStatus).ParentRef, ns)
657+
case reflect.TypeOf(gwapiv1.PolicyAncestorStatus{}):
658+
return gatewayapi.IsParentRefEqual(any(firstRef).(gwapiv1.PolicyAncestorStatus).AncestorRef, any(secondRef).(gwapiv1.PolicyAncestorStatus).AncestorRef, ns)
659+
default:
660+
return false
661+
}
662+
}
663+
664+
func parentOrAncestorControllerName[K any](ref K) gwapiv1.GatewayController {
665+
switch reflect.TypeOf(ref) {
666+
case reflect.TypeOf(gwapiv1.RouteParentStatus{}):
667+
return any(ref).(gwapiv1.RouteParentStatus).ControllerName
668+
case reflect.TypeOf(gwapiv1.PolicyAncestorStatus{}):
669+
return any(ref).(gwapiv1.PolicyAncestorStatus).ControllerName
670+
default:
671+
return gwapiv1.GatewayController("")
672+
}
673+
}
674+
642675
func (r *gatewayAPIReconciler) updateStatusForGateway(ctx context.Context, gtw *gwapiv1.Gateway) {
643676
// nil check for unit tests.
644677
if r.statusUpdater == nil {

0 commit comments

Comments
 (0)