Skip to content

fix: merge xRoute/xPolicy statuses across controllers in provider runner #7558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: merge xRoute/xPolicy statuses across controllers in provider runner #7558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3fbb337
fix: merge xRoute/xPolicy statuses across controllers in provider runner
y-rabie Nov 19, 2025
9769a3c
Add e2e tests for xRoute/xPolicy status cleanup
y-rabie Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 64 additions & 23 deletions internal/provider/kubernetes/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package kubernetes
import (
"context"
"fmt"
"reflect"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -111,7 +112,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
Spec: h.Spec,
Status: gwapiv1.HTTPRouteStatus{
RouteStatus: gwapiv1.RouteStatus{
Parents: mergeRouteParentStatus(h.Namespace, h.Status.Parents, valCopy.Parents),
Parents: mergeStatus(h.Namespace, r.envoyGateway.Gateway.ControllerName, h.Status.Parents, valCopy.Parents),
},
},
}
Expand Down Expand Up @@ -151,7 +152,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
Spec: g.Spec,
Status: gwapiv1.GRPCRouteStatus{
RouteStatus: gwapiv1.RouteStatus{
Parents: mergeRouteParentStatus(g.Namespace, g.Status.Parents, valCopy.Parents),
Parents: mergeStatus(g.Namespace, r.envoyGateway.Gateway.ControllerName, g.Status.Parents, valCopy.Parents),
},
},
}
Expand Down Expand Up @@ -193,7 +194,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
Spec: t.Spec,
Status: gwapiv1a2.TLSRouteStatus{
RouteStatus: gwapiv1.RouteStatus{
Parents: mergeRouteParentStatus(t.Namespace, t.Status.Parents, valCopy.Parents),
Parents: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Parents, valCopy.Parents),
},
},
}
Expand Down Expand Up @@ -235,7 +236,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
Spec: t.Spec,
Status: gwapiv1a2.TCPRouteStatus{
RouteStatus: gwapiv1.RouteStatus{
Parents: mergeRouteParentStatus(t.Namespace, t.Status.Parents, valCopy.Parents),
Parents: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Parents, valCopy.Parents),
},
},
}
Expand Down Expand Up @@ -277,7 +278,7 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
Spec: u.Spec,
Status: gwapiv1a2.UDPRouteStatus{
RouteStatus: gwapiv1.RouteStatus{
Parents: mergeRouteParentStatus(u.Namespace, u.Status.Parents, valCopy.Parents),
Parents: mergeStatus(u.Namespace, r.envoyGateway.Gateway.ControllerName, u.Status.Parents, valCopy.Parents),
},
},
}
Expand Down Expand Up @@ -317,7 +318,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
TypeMeta: t.TypeMeta,
ObjectMeta: t.ObjectMeta,
Spec: t.Spec,
Status: *valCopy,
Status: gwapiv1.PolicyStatus{
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
},
}
return tCopy
}),
Expand Down Expand Up @@ -355,7 +358,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
TypeMeta: t.TypeMeta,
ObjectMeta: t.ObjectMeta,
Spec: t.Spec,
Status: *valCopy,
Status: gwapiv1.PolicyStatus{
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
},
}
return tCopy
}),
Expand Down Expand Up @@ -393,7 +398,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
TypeMeta: t.TypeMeta,
ObjectMeta: t.ObjectMeta,
Spec: t.Spec,
Status: *valCopy,
Status: gwapiv1.PolicyStatus{
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
},
}
return tCopy
}),
Expand Down Expand Up @@ -431,7 +438,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
TypeMeta: t.TypeMeta,
ObjectMeta: t.ObjectMeta,
Spec: t.Spec,
Status: *valCopy,
Status: gwapiv1.PolicyStatus{
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
},
}
return tCopy
}),
Expand Down Expand Up @@ -467,7 +476,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
TypeMeta: t.TypeMeta,
ObjectMeta: t.ObjectMeta,
Spec: t.Spec,
Status: *valCopy,
Status: gwapiv1.PolicyStatus{
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
},
}
return tCopy
}),
Expand Down Expand Up @@ -505,7 +516,9 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
TypeMeta: t.TypeMeta,
ObjectMeta: t.ObjectMeta,
Spec: t.Spec,
Status: *valCopy,
Status: gwapiv1.PolicyStatus{
Ancestors: mergeStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Ancestors, valCopy.Ancestors),
},
}
return tCopy
}),
Expand Down Expand Up @@ -582,6 +595,15 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
tCopy := t.DeepCopy()
valCopy := val.DeepCopy()
setLastTransitionTimeInConditionsForPolicyStatus(valCopy, metav1.Now())
var oldAncestors []gwapiv1.PolicyAncestorStatus
o, found, err := unstructured.NestedFieldCopy(tCopy.Object, "status", "ancestors")
if found && err == nil {
oldAncestors, ok = o.([]gwapiv1.PolicyAncestorStatus)
if ok {
tCopy.Object["status"] = mergeStatus(t.GetNamespace(), r.envoyGateway.Gateway.ControllerName, oldAncestors, valCopy.Ancestors)
return tCopy
}
}
tCopy.Object["status"] = *valCopy
return tCopy
}),
Expand All @@ -593,31 +615,28 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context
}
}

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

// Range over old status parentRefs in order:
// 1. The parentRef exists in the new status: append the new one to the final status.
// 2. The parentRef doesn't exist in the new status and it's not our controller: append it to the final status.
// 3. The parentRef doesn't exist in the new status, and it is our controller: keep it in the final status.
// This is important for routes with multiple parent references - not all parents are updated in each reconciliation.
// 3. The parentRef doesn't exist in the new status, and it is our controller: don't append it to the final status.
for _, oldP := range old {
found := -1
for newI, newP := range new {
if gatewayapi.IsParentRefEqual(oldP.ParentRef, newP.ParentRef, ns) {
if isParentOrAncestorRefEqual(oldP, newP, ns) {
found = newI
break
}
}
if found >= 0 {
merged = append(merged, new[found])
} else {
// Keep all old parent statuses, regardless of controller.
// For routes with multiple parents managed by the same controller,
// not all parents are necessarily updated in each reconciliation.
} else if parentOrAncestorControllerName(oldP) != gwapiv1.GatewayController(controllerName) {
merged = append(merged, oldP)
}
}
Expand All @@ -626,7 +645,7 @@ func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []g
for _, newP := range new {
found := false
for _, mergedP := range merged {
if gatewayapi.IsParentRefEqual(newP.ParentRef, mergedP.ParentRef, ns) {
if isParentOrAncestorRefEqual(newP, mergedP, ns) {
found = true
break
}
Expand All @@ -639,6 +658,28 @@ func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []g
return merged
}

func isParentOrAncestorRefEqual[K any](firstRef, secondRef K, ns string) bool {
switch reflect.TypeOf(firstRef) {
case reflect.TypeOf(gwapiv1.RouteParentStatus{}):
return gatewayapi.IsParentRefEqual(any(firstRef).(gwapiv1.RouteParentStatus).ParentRef, any(secondRef).(gwapiv1.RouteParentStatus).ParentRef, ns)
case reflect.TypeOf(gwapiv1.PolicyAncestorStatus{}):
return gatewayapi.IsParentRefEqual(any(firstRef).(gwapiv1.PolicyAncestorStatus).AncestorRef, any(secondRef).(gwapiv1.PolicyAncestorStatus).AncestorRef, ns)
default:
return false
}
}

func parentOrAncestorControllerName[K any](ref K) gwapiv1.GatewayController {
switch reflect.TypeOf(ref) {
case reflect.TypeOf(gwapiv1.RouteParentStatus{}):
return any(ref).(gwapiv1.RouteParentStatus).ControllerName
case reflect.TypeOf(gwapiv1.PolicyAncestorStatus{}):
return any(ref).(gwapiv1.PolicyAncestorStatus).ControllerName
default:
return gwapiv1.GatewayController("")
}
}

func (r *gatewayAPIReconciler) updateStatusForGateway(ctx context.Context, gtw *gwapiv1.Gateway) {
// nil check for unit tests.
if r.statusUpdater == nil {
Expand Down
Loading
Loading