diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go index 5735260e02..a0e4f0ef76 100644 --- a/internal/provider/kubernetes/status.go +++ b/internal/provider/kubernetes/status.go @@ -109,7 +109,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext Spec: h.Spec, Status: gwapiv1.HTTPRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(h.Namespace, r.envoyGateway.Gateway.ControllerName, h.Status.Parents, val.Parents), + Parents: mergeRouteParentStatus(h.Namespace, h.Status.Parents, val.Parents), }, }, } @@ -147,7 +147,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext Spec: g.Spec, Status: gwapiv1.GRPCRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(g.Namespace, r.envoyGateway.Gateway.ControllerName, g.Status.Parents, val.Parents), + Parents: mergeRouteParentStatus(g.Namespace, g.Status.Parents, val.Parents), }, }, } @@ -187,7 +187,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext Spec: t.Spec, Status: gwapiv1a2.TLSRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Parents, val.Parents), + Parents: mergeRouteParentStatus(t.Namespace, t.Status.Parents, val.Parents), }, }, } @@ -227,7 +227,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext Spec: t.Spec, Status: gwapiv1a2.TCPRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(t.Namespace, r.envoyGateway.Gateway.ControllerName, t.Status.Parents, val.Parents), + Parents: mergeRouteParentStatus(t.Namespace, t.Status.Parents, val.Parents), }, }, } @@ -267,7 +267,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext Spec: u.Spec, Status: gwapiv1a2.UDPRouteStatus{ RouteStatus: gwapiv1.RouteStatus{ - Parents: mergeRouteParentStatus(u.Namespace, r.envoyGateway.Gateway.ControllerName, u.Status.Parents, val.Parents), + Parents: mergeRouteParentStatus(u.Namespace, u.Status.Parents, val.Parents), }, }, } @@ -569,14 +569,15 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext // mergeRouteParentStatus merges the old and new RouteParentStatus. // This is needed because the RouteParentStatus doesn't support strategic merge patch yet. -func mergeRouteParentStatus(ns, controllerName string, old, new []gwapiv1.RouteParentStatus) []gwapiv1.RouteParentStatus { +func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []gwapiv1.RouteParentStatus { // Allocating with worst-case capacity to avoid reallocation. merged := make([]gwapiv1.RouteParentStatus, 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: don't 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. for _, oldP := range old { found := -1 for newI, newP := range new { @@ -587,7 +588,10 @@ func mergeRouteParentStatus(ns, controllerName string, old, new []gwapiv1.RouteP } if found >= 0 { merged = append(merged, new[found]) - } else if oldP.ControllerName != gwapiv1.GatewayController(controllerName) { + } 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. merged = append(merged, oldP) } } diff --git a/internal/provider/kubernetes/status_test.go b/internal/provider/kubernetes/status_test.go index 63b7fdea7f..1cb4c5730a 100644 --- a/internal/provider/kubernetes/status_test.go +++ b/internal/provider/kubernetes/status_test.go @@ -361,6 +361,24 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, { ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", ParentRef: gwapiv1.ParentReference{ @@ -456,6 +474,24 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, }, }, @@ -670,6 +706,24 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, want: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, { ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", ParentRef: gwapiv1.ParentReference{ @@ -690,9 +744,10 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, }, }, - // Similar note about practicality of occurrence. + // Test that parent refs managed by our controller are preserved even when not in new update. + // This is important for routes with multiple parent references. { - name: "old contains one parentRef of ours, and it gets dropped in new.", + name: "old contains one parentRef of ours, and it's not in new - should be preserved.", args: args{ old: []gwapiv1.RouteParentStatus{ { @@ -716,12 +771,118 @@ func Test_mergeRouteParentStatus(t *testing.T) { }, new: []gwapiv1.RouteParentStatus{}, }, - want: []gwapiv1.RouteParentStatus{}, + want: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + }, + }, + // Test multi-parent scenario where only one parent is updated at a time. + { + name: "multiple parents from same controller - update one, preserve others", + args: args{ + old: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + }, + }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + }, + }, + }, + new: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + }, + }, + want: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway1", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + Reason: "Accepted", + }, + }, + }, + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := mergeRouteParentStatus("default", "gateway.envoyproxy.io/gatewayclass-controller", tt.args.old, tt.args.new); !reflect.DeepEqual(got, tt.want) { + if got := mergeRouteParentStatus("default", tt.args.old, tt.args.new); !reflect.DeepEqual(got, tt.want) { t.Errorf("mergeRouteParentStatus() = %v, want %v", got, tt.want) } })