From ac0f106794a34bc2578450ff327656c592f1ad79 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 12 Mar 2024 19:57:15 -0700 Subject: [PATCH] skip publishing empty status for policies (#2902) * skip publishing empty status for policies * https://github.com/envoyproxy/gateway/pull/2802 skips computing status if a target resource cannot be found, mainly because that target maybe irrelevant to this specific translation, its hard to proactively find that out in the provider layer * This fix ensures that any empty status is not published and resets any existing status for a policy Signed-off-by: Arko Dasgupta * also fix for envoypatchpolicy Signed-off-by: Arko Dasgupta * also discard status for backendtlspolicy Signed-off-by: Arko Dasgupta --------- Signed-off-by: Arko Dasgupta --- internal/gatewayapi/runner/runner.go | 22 ++++++++++++++++++---- internal/provider/kubernetes/controller.go | 10 ++++++---- internal/xds/translator/runner/runner.go | 8 +++++++- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/internal/gatewayapi/runner/runner.go b/internal/gatewayapi/runner/runner.go index 13f2c6b9d08..4cb3d69d15f 100644 --- a/internal/gatewayapi/runner/runner.go +++ b/internal/gatewayapi/runner/runner.go @@ -7,6 +7,7 @@ package runner import ( "context" + "reflect" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -156,29 +157,42 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { r.ProviderResources.UDPRouteStatuses.Store(key, &udpRoute.Status) delete(statusesToDelete.UDPRouteStatusKeys, key) } + + // Skip updating status for policies with empty status + // They may have been skipped in this translation because + // their target is not found (not relevant) + for _, backendTLSPolicy := range result.BackendTLSPolicies { backendTLSPolicy := backendTLSPolicy key := utils.NamespacedName(backendTLSPolicy) - r.ProviderResources.BackendTLSPolicyStatuses.Store(key, &backendTLSPolicy.Status) + if !(reflect.ValueOf(backendTLSPolicy.Status).IsZero()) { + r.ProviderResources.BackendTLSPolicyStatuses.Store(key, &backendTLSPolicy.Status) + } delete(statusesToDelete.BackendTLSPolicyStatusKeys, key) } for _, clientTrafficPolicy := range result.ClientTrafficPolicies { clientTrafficPolicy := clientTrafficPolicy key := utils.NamespacedName(clientTrafficPolicy) - r.ProviderResources.ClientTrafficPolicyStatuses.Store(key, &clientTrafficPolicy.Status) + if !(reflect.ValueOf(clientTrafficPolicy.Status).IsZero()) { + r.ProviderResources.ClientTrafficPolicyStatuses.Store(key, &clientTrafficPolicy.Status) + } delete(statusesToDelete.ClientTrafficPolicyStatusKeys, key) } for _, backendTrafficPolicy := range result.BackendTrafficPolicies { backendTrafficPolicy := backendTrafficPolicy key := utils.NamespacedName(backendTrafficPolicy) - r.ProviderResources.BackendTrafficPolicyStatuses.Store(key, &backendTrafficPolicy.Status) + if !(reflect.ValueOf(backendTrafficPolicy.Status).IsZero()) { + r.ProviderResources.BackendTrafficPolicyStatuses.Store(key, &backendTrafficPolicy.Status) + } delete(statusesToDelete.BackendTrafficPolicyStatusKeys, key) } for _, securityPolicy := range result.SecurityPolicies { securityPolicy := securityPolicy key := utils.NamespacedName(securityPolicy) - r.ProviderResources.SecurityPolicyStatuses.Store(key, &securityPolicy.Status) + if !(reflect.ValueOf(securityPolicy.Status).IsZero()) { + r.ProviderResources.SecurityPolicyStatuses.Store(key, &securityPolicy.Status) + } delete(statusesToDelete.SecurityPolicyStatusKeys, key) } } diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index de40a862f18..228dce7436e 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -822,8 +822,9 @@ func (r *gatewayAPIReconciler) processBackendTrafficPolicies(ctx context.Context for _, policy := range backendTrafficPolicies.Items { policy := policy - // The status of BackendTrafficPolicies can't be discarded because the status - // can be modified by multiple controllers. + // Discard Status to reduce memory consumption in watchable + // It will be recomputed by the gateway-api layer + policy.Status = gwapiv1a2.PolicyStatus{} resourceTree.BackendTrafficPolicies = append(resourceTree.BackendTrafficPolicies, &policy) } return nil @@ -863,8 +864,9 @@ func (r *gatewayAPIReconciler) processBackendTLSPolicies( for _, policy := range backendTLSPolicies.Items { policy := policy - // The status of BackendTLSPolicies can't be discarded because the status - // can be modified by multiple controllers. + // Discard Status to reduce memory consumption in watchable + // It will be recomputed by the gateway-api layer + policy.Status = gwapiv1a2.PolicyStatus{} resourceTree.BackendTLSPolicies = append(resourceTree.BackendTLSPolicies, &policy) } diff --git a/internal/xds/translator/runner/runner.go b/internal/xds/translator/runner/runner.go index 114b00b4550..573afc38228 100644 --- a/internal/xds/translator/runner/runner.go +++ b/internal/xds/translator/runner/runner.go @@ -7,6 +7,7 @@ package runner import ( "context" + "reflect" ktypes "k8s.io/apimachinery/pkg/types" @@ -104,7 +105,12 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) { Name: e.Name, Namespace: e.Namespace, } - r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, e.Status) + // Skip updating status for policies with empty status + // They may have been skipped in this translation because + // their target is not found (not relevant) + if !(reflect.ValueOf(e.Status).IsZero()) { + r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, e.Status) + } delete(statusesToDelete, key) } // Discard the EnvoyPatchPolicyStatuses to reduce memory footprint