From 659b57f1d698fe2fb28c1ded734fd2cdbecd0b28 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 27 Jul 2023 16:37:51 -0700 Subject: [PATCH] DeepCopy resources that require status updates * Was seeing constant churn between provider runner publishing resources and gateway-api runner receiving them. * Tried to debug it by printing the o/p of `cmp.Diff` between current and previous values ``` diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index 050394ba..50d09f6f 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -8,6 +8,7 @@ package runner import ( "context" + "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/gateway-api/apis/v1beta1" "sigs.k8s.io/yaml" @@ -49,6 +50,7 @@ func (r *Runner) Start(ctx context.Context) error { } func (r *Runner) subscribeAndTranslate(ctx context.Context) { + prev := &gatewayapi.Resources{} message.HandleSubscription(r.ProviderResources.GatewayAPIResources.Subscribe(ctx), func(update message.Update[string, *gatewayapi.Resources]) { val := update.Value @@ -56,6 +58,9 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { if update.Delete || val == nil { return } + diff := cmp.Diff(prev, val) + r.Logger.WithValues("output", "diff").Info(diff) + prev = val.DeepCopy() // Translate and publish IRs. t := &gatewayapi.Translator{ ``` Here's the o/p and its empty ``` 2023-07-27T23:55:29.795Z INFO gateway-api runner/runner.go:62 {"runner": "gateway-api", "output": "diff"} ``` * Using a DeepCopy for resources that were updating the `Status` subresource seems to have solved the issue, which implies that watchable doesnt like clients to mutate the value, even though they are meant to be a `DeepCopy` Fixes: https://github.com/envoyproxy/gateway/issues/1715 Signed-off-by: Arko Dasgupta --- internal/gatewayapi/envoypatchpolicy.go | 2 +- internal/gatewayapi/route.go | 10 +++++----- internal/gatewayapi/runner/runner.go | 2 ++ internal/infrastructure/runner/runner.go | 1 + internal/xds/server/runner/runner.go | 1 + 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/gatewayapi/envoypatchpolicy.go b/internal/gatewayapi/envoypatchpolicy.go index 4a9ff8cc6e..e5e9a2f33e 100644 --- a/internal/gatewayapi/envoypatchpolicy.go +++ b/internal/gatewayapi/envoypatchpolicy.go @@ -25,7 +25,7 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo }) for _, policy := range envoyPatchPolicies { - + policy := policy.DeepCopy() targetNs := policy.Spec.TargetRef.Namespace if targetNs == nil { // This status condition will not get updated in the resource because diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index dee403e2ef..8fc96264b9 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -39,7 +39,7 @@ func (t *Translator) ProcessHTTPRoutes(httpRoutes []*v1beta1.HTTPRoute, gateways } httpRoute := &HTTPRouteContext{ GatewayControllerName: t.GatewayControllerName, - HTTPRoute: h, + HTTPRoute: h.DeepCopy(), } // Find out if this route attaches to one of our Gateway's listeners, @@ -67,7 +67,7 @@ func (t *Translator) ProcessGRPCRoutes(grpcRoutes []*v1alpha2.GRPCRoute, gateway } grpcRoute := &GRPCRouteContext{ GatewayControllerName: t.GatewayControllerName, - GRPCRoute: g, + GRPCRoute: g.DeepCopy(), } // Find out if this route attaches to one of our Gateway's listeners, @@ -563,7 +563,7 @@ func (t *Translator) ProcessTLSRoutes(tlsRoutes []*v1alpha2.TLSRoute, gateways [ } tlsRoute := &TLSRouteContext{ GatewayControllerName: t.GatewayControllerName, - TLSRoute: tls, + TLSRoute: tls.DeepCopy(), } // Find out if this route attaches to one of our Gateway's listeners, @@ -685,7 +685,7 @@ func (t *Translator) ProcessUDPRoutes(udpRoutes []*v1alpha2.UDPRoute, gateways [ } udpRoute := &UDPRouteContext{ GatewayControllerName: t.GatewayControllerName, - UDPRoute: u, + UDPRoute: u.DeepCopy(), } // Find out if this route attaches to one of our Gateway's listeners, @@ -817,7 +817,7 @@ func (t *Translator) ProcessTCPRoutes(tcpRoutes []*v1alpha2.TCPRoute, gateways [ } tcpRoute := &TCPRouteContext{ GatewayControllerName: t.GatewayControllerName, - TCPRoute: tcp, + TCPRoute: tcp.DeepCopy(), } // Find out if this route attaches to one of our Gateway's listeners, diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index 050394ba30..7db9c05b96 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -51,6 +51,8 @@ func (r *Runner) Start(ctx context.Context) error { func (r *Runner) subscribeAndTranslate(ctx context.Context) { message.HandleSubscription(r.ProviderResources.GatewayAPIResources.Subscribe(ctx), func(update message.Update[string, *gatewayapi.Resources]) { + r.Logger.Info("received an update") + val := update.Value if update.Delete || val == nil { diff --git a/internal/infrastructure/runner/runner.go b/internal/infrastructure/runner/runner.go index 03bead88ef..2f7bd146df 100644 --- a/internal/infrastructure/runner/runner.go +++ b/internal/infrastructure/runner/runner.go @@ -57,6 +57,7 @@ func (r *Runner) subscribeToProxyInfraIR(ctx context.Context) { // Subscribe to resources message.HandleSubscription(r.InfraIR.Subscribe(ctx), func(update message.Update[string, *ir.Infra]) { + r.Logger.Info("received an update") val := update.Value if update.Delete { diff --git a/internal/xds/server/runner/runner.go b/internal/xds/server/runner/runner.go index c3482355a5..e76224a739 100644 --- a/internal/xds/server/runner/runner.go +++ b/internal/xds/server/runner/runner.go @@ -130,6 +130,7 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { key := update.Key val := update.Value + r.Logger.Info("received an update") var err error if update.Delete { err = r.cache.GenerateNewSnapshot(key, nil)