From 47a096f5e754d5871226960e44245634411ec631 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Thu, 31 Oct 2024 03:38:30 +0000 Subject: [PATCH] fix route status Signed-off-by: Huabing Zhao --- internal/provider/kubernetes/status.go | 44 ++- internal/provider/kubernetes/status_test.go | 286 ++++++++++++++++++ .../provider/kubernetes/status_updater.go | 1 - release-notes/current.yaml | 1 + 4 files changed, 320 insertions(+), 12 deletions(-) create mode 100644 internal/provider/kubernetes/status_test.go diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go index c3d5553b0bfd..6183363e360d 100644 --- a/internal/provider/kubernetes/status.go +++ b/internal/provider/kubernetes/status.go @@ -74,7 +74,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext panic(err) } hCopy := h.DeepCopy() - hCopy.Status.Parents = val.Parents + hCopy.Status.Parents = mergeRouteParentStatus(h.Status.Parents, val.Parents) return hCopy }), }) @@ -97,15 +97,15 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext NamespacedName: key, Resource: new(gwapiv1.GRPCRoute), Mutator: MutatorFunc(func(obj client.Object) client.Object { - h, ok := obj.(*gwapiv1.GRPCRoute) + g, ok := obj.(*gwapiv1.GRPCRoute) if !ok { err := fmt.Errorf("unsupported object type %T", obj) errChan <- err panic(err) } - hCopy := h.DeepCopy() - hCopy.Status.Parents = val.Parents - return hCopy + gCopy := g.DeepCopy() + gCopy.Status.Parents = mergeRouteParentStatus(g.Status.Parents, val.Parents) + return gCopy }), }) }, @@ -136,7 +136,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext panic(err) } tCopy := t.DeepCopy() - tCopy.Status.Parents = val.Parents + tCopy.Status.Parents = mergeRouteParentStatus(t.Status.Parents, val.Parents) return tCopy }), }) @@ -168,7 +168,7 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext panic(err) } tCopy := t.DeepCopy() - tCopy.Status.Parents = val.Parents + tCopy.Status.Parents = mergeRouteParentStatus(t.Status.Parents, val.Parents) return tCopy }), }) @@ -193,15 +193,15 @@ func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context, ext NamespacedName: key, Resource: new(gwapiv1a2.UDPRoute), Mutator: MutatorFunc(func(obj client.Object) client.Object { - t, ok := obj.(*gwapiv1a2.UDPRoute) + u, ok := obj.(*gwapiv1a2.UDPRoute) if !ok { err := fmt.Errorf("unsupported object type %T", obj) errChan <- err panic(err) } - tCopy := t.DeepCopy() - tCopy.Status.Parents = val.Parents - return tCopy + uCopy := u.DeepCopy() + uCopy.Status.Parents = mergeRouteParentStatus(u.Status.Parents, val.Parents) + return uCopy }), }) }, @@ -469,6 +469,28 @@ 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(old, new []gwapiv1.RouteParentStatus) []gwapiv1.RouteParentStatus { + merged := make([]gwapiv1.RouteParentStatus, len(old)) + _ = copy(merged, old) + for _, parent := range new { + found := -1 + for i, existing := range old { + if parent.ParentRef.Name == existing.ParentRef.Name { + found = i + break + } + } + if found >= 0 { + merged[found] = parent + } else { + merged = append(merged, parent) + } + } + return merged +} + func (r *gatewayAPIReconciler) updateStatusForGateway(ctx context.Context, gtw *gwapiv1.Gateway) { // nil check for unit tests. if r.statusUpdater == nil { diff --git a/internal/provider/kubernetes/status_test.go b/internal/provider/kubernetes/status_test.go new file mode 100644 index 000000000000..a9f1866b3b88 --- /dev/null +++ b/internal/provider/kubernetes/status_test.go @@ -0,0 +1,286 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package kubernetes + +import ( + "reflect" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func Test_mergeRouteParentStatus(t *testing.T) { + type args struct { + old []gwapiv1.RouteParentStatus + new []gwapiv1.RouteParentStatus + } + tests := []struct { + name string + args args + want []gwapiv1.RouteParentStatus + }{ + { + name: "merge old and new", + 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", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + }, + new: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + 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.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + + { + name: "override an existing parent", + 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", + }, + { + 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", + }, + { + Type: string(gwapiv1.RouteConditionResolvedRefs), + Status: metav1.ConditionTrue, + Reason: "ResolvedRefs", + }, + }, + }, + }, + new: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + 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.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + + { + name: "nothing changed", + 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", + }, + { + 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.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + new: []gwapiv1.RouteParentStatus{ + { + ControllerName: "gateway.envoyproxy.io/gatewayclass-controller", + ParentRef: gwapiv1.ParentReference{ + Name: "gateway2", + }, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.RouteConditionAccepted), + Status: metav1.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + 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.ConditionFalse, + Reason: "SomeReason", + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := mergeRouteParentStatus(tt.args.old, tt.args.new); !reflect.DeepEqual(got, tt.want) { + t.Errorf("mergeRouteParentStatus() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/provider/kubernetes/status_updater.go b/internal/provider/kubernetes/status_updater.go index 24adaedd563a..886bf6fefca8 100644 --- a/internal/provider/kubernetes/status_updater.go +++ b/internal/provider/kubernetes/status_updater.go @@ -106,7 +106,6 @@ func (u *UpdateHandler) apply(update Update) { } newObj.SetUID(obj.GetUID()) - return u.client.Status().Update(context.Background(), newObj) }); err != nil { u.log.Error(err, "unable to update status", "name", update.NamespacedName.Name, diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 1268ce35b0f2..af6c58431973 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -16,6 +16,7 @@ new features: | bug fixes: | Only log endpoint configuration in verbose logging mode (`-v 4` or higher) The xDS translation failed when wasm http code source configured without a sha + HTTPRoute status only shows one parent when targeting multiple Gateways from different GatewayClasses # Enhancements that improve performance. performance improvements: |