From 72cf7ccf655f9e2850fce2feef1a4bc8b29371fc Mon Sep 17 00:00:00 2001 From: Vishal Date: Sun, 5 Jul 2026 13:39:54 -0400 Subject: [PATCH] fix: clean up stale BackendTrafficPolicy status when it attaches to no target A BackendTrafficPolicy that resolves to zero targets left its status stale: - targetRef/targetRefs pointing at a nonexistent object kept the policy in the result set carrying its deep-copied prior status, so Accepted=True persisted with an out-of-date observedGeneration (envoyproxy#8926). - targetSelectors that match no objects never added the policy to the result set, so its status was never reconciled (envoyproxy#8927). Following review feedback, the stale status is cleaned up in the provider status updater rather than by synthesizing a condition in the translator: - The translator stops propagating carried-over stale status for a policy that attached to zero targets this pass (subtractive; no synthesized condition or ancestorRef). Both cases then produce no status for the policy. - The BackendTrafficPolicy status updater handles the resulting status delete by stripping the ancestor conditions owned by this controller from the live object, while preserving ancestors owned by other controllers. If the object itself was deleted, the updater's Get returns NotFound and it is a no-op. Intentionally scoped to BackendTrafficPolicy; the same pattern can be generalized to the sibling policy types in a follow-up. Fixes envoyproxy#8926 Fixes envoyproxy#8927 Signed-off-by: Vishal Co-authored-by: Cursor --- internal/gatewayapi/backendtrafficpolicy.go | 66 ++++-- ...endtrafficpolicy-selector-no-match.in.yaml | 50 +++++ ...ndtrafficpolicy-selector-no-match.out.yaml | 174 ++++++++++++++++ ...y-targetref-not-found-stale-status.in.yaml | 63 ++++++ ...-targetref-not-found-stale-status.out.yaml | 189 ++++++++++++++++++ ...dtrafficpolicy-targetref-not-found.in.yaml | 47 +++++ ...trafficpolicy-targetref-not-found.out.yaml | 188 +++++++++++++++++ internal/provider/kubernetes/status.go | 54 ++++- internal/provider/kubernetes/status_test.go | 165 +++++++++++++++ .../8926-btp-status-target-not-found.md | 1 + 10 files changed, 979 insertions(+), 18 deletions(-) create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.in.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.out.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found-stale-status.in.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found-stale-status.out.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found.in.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found.out.yaml create mode 100644 release-notes/current/bug_fixes/8926-btp-status-target-not-found.md diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index 933ba491fb..aef76ed05d 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -261,6 +261,14 @@ func (t *Translator) ProcessBackendTrafficPolicies( handledPolicies := make(map[types.NamespacedName]*egv1a1.BackendTrafficPolicy, policyMapSize) + // Tracks policies that were successfully attached to at least one real target + // during this pass. Any policy that ends up in res without attaching to a + // target has carried-over (deep-copied) status from a previous generation that + // is now stale, so we drop that status below. Cleaning up the resulting + // detached status on the actual object is handled by the provider status + // updater when it observes the status delete (see internal/provider/kubernetes/status.go). + attachedPolicies := make(map[types.NamespacedName]bool, policyMapSize) + // Translate // 1. First translate Policies targeting RouteRules // 2. Next translate Policies targeting xRoutes @@ -284,8 +292,10 @@ func (t *Translator) ProcessBackendTrafficPolicies( res = append(res, policy) } - t.processBackendTrafficPolicyForRoute(xdsIR, - routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget) + if t.processBackendTrafficPolicyForRoute(xdsIR, + routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget) { + attachedPolicies[policyName] = true + } } } } @@ -310,8 +320,10 @@ func (t *Translator) ProcessBackendTrafficPolicies( res = append(res, policy) } - t.processBackendTrafficPolicyForRoute(xdsIR, - routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget) + if t.processBackendTrafficPolicyForRoute(xdsIR, + routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget) { + attachedPolicies[policyName] = true + } } } } @@ -329,8 +341,10 @@ func (t *Translator) ProcessBackendTrafficPolicies( handledPolicies[policyName] = policy res = append(res, policy) } - t.processBackendTrafficPolicyForGateway(xdsIR, - gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget) + if t.processBackendTrafficPolicyForGateway(xdsIR, + gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget) { + attachedPolicies[policyName] = true + } } } } @@ -354,12 +368,30 @@ func (t *Translator) ProcessBackendTrafficPolicies( handledPolicies[policyName] = policy res = append(res, policy) } - t.processBackendTrafficPolicyForGateway(xdsIR, - gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget) + if t.processBackendTrafficPolicyForGateway(xdsIR, + gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget) { + attachedPolicies[policyName] = true + } } } } + // Drop stale carried-over status for policies that were added to res (their + // targetRef/targetRefs resolved to an object reference) but did not actually + // attach to any target this pass because the referenced object does not exist + // (#8926). Without this, the deep-copied Accepted=True condition with an + // out-of-date observedGeneration would be republished as-is. Emptying the + // ancestor list makes the gatewayapi runner treat the policy as having no + // status, which deletes it from the status store and lets the provider status + // updater clean up the stale status on the object. Policies whose selectors + // matched nothing (#8927) are never added to res, so they are already handled + // by that same delete path. + for _, policy := range res { + if !attachedPolicies[utils.NamespacedName(policy)] { + policy.Status.Ancestors = nil + } + } + for _, policy := range res { // Truncate Ancestor list of longer than 16 if len(policy.Status.Ancestors) > 16 { @@ -429,7 +461,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute( gatewayPolicyMap map[NamespacedNameWithSection]*egv1a1.BackendTrafficPolicy, policy *egv1a1.BackendTrafficPolicy, currTarget policyTargetReferenceWithSectionName, -) { +) bool { var ( targetedRoute RouteContext resolveErr *status.PolicyResolveError @@ -441,7 +473,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute( // reconciled by multiple controllers. And the other controller may // have the target route. if targetedRoute == nil { - return + return false } // Find the Gateway that the route belongs to and add it to the @@ -491,7 +523,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute( policy.Generation, resolveErr, ) - return + return true } if policy.Spec.MergeType == nil { @@ -593,7 +625,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute( // Check if this policy is overridden by other policies targeting at route rule levels // If policy target is route rule, we can skip the check if currTarget.SectionName != nil { - return + return true } key := policyTargetRouteKey{ @@ -613,6 +645,8 @@ func (t *Translator) processBackendTrafficPolicyForRoute( policy.Generation, ) } + + return true } func (t *Translator) processBackendTrafficPolicyForGateway( @@ -622,7 +656,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway( gatewayPolicyMergedMap *GatewayPolicyRouteMap, policy *egv1a1.BackendTrafficPolicy, currTarget policyTargetReferenceWithSectionName, -) { +) bool { var ( targetedGateway *GatewayContext resolveErr *status.PolicyResolveError @@ -631,7 +665,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway( // Negative statuses have already been assigned so it's safe to skip targetedGateway, resolveErr = resolveBackendTrafficPolicyGatewayTargetRef(currTarget, gatewayMap) if targetedGateway == nil { - return + return false } // Find its ancestor reference by resolved gateway, even with resolve error @@ -646,7 +680,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway( policy.Generation, resolveErr, ) - return + return true } // Set conditions for translation error if it got any @@ -692,6 +726,8 @@ func (t *Translator) processBackendTrafficPolicyForGateway( policy.Generation, ) } + + return true } func resolveBackendTrafficPolicyGatewayTargetRef( diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.in.yaml new file mode 100644 index 0000000000..60d4ca7fa7 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.in.yaml @@ -0,0 +1,50 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: listener-1 + protocol: HTTP + port: 8081 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + labels: + app: real-label + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: listener-1 + rules: + - matches: + - path: + value: "/foo" + backendRefs: + - name: service-1 + port: 8080 +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + namespace: default + name: policy-with-unmatched-selector + spec: + targetSelectors: + - group: gateway.networking.k8s.io + kind: HTTPRoute + matchLabels: + app: no-such-label + timeout: + tcp: + connectTimeout: 15s diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.out.yaml new file mode 100644 index 0000000000..1d49f4ee6f --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.out.yaml @@ -0,0 +1,174 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: listener-1 + port: 8081 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: listener-1 + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + labels: + app: real-label + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - name: envoy-gateway/gateway-1/listener-1 + ports: + - containerPort: 8081 + name: http-8081 + protocol: HTTP + servicePort: 8081 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 8081 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 + name: envoy-gateway/gateway-1/listener-1 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 8081 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /foo + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found-stale-status.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found-stale-status.in.yaml new file mode 100644 index 0000000000..8e9424ae7c --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found-stale-status.in.yaml @@ -0,0 +1,63 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: listener-1 + protocol: HTTP + port: 8081 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: listener-1 + rules: + - matches: + - path: + value: "/foo" + backendRefs: + - name: service-1 + port: 8080 +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + namespace: default + name: policy-was-attached-now-missing + generation: 2 + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: does-not-exist + timeout: + tcp: + connectTimeout: 15s + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + observedGeneration: 1 + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found-stale-status.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found-stale-status.out.yaml new file mode 100644 index 0000000000..b4be5b066d --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found-stale-status.out.yaml @@ -0,0 +1,189 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + generation: 2 + name: policy-was-attached-now-missing + namespace: default + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: does-not-exist + timeout: + tcp: + connectTimeout: 15s + status: + ancestors: null +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: listener-1 + port: 8081 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: listener-1 + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - name: envoy-gateway/gateway-1/listener-1 + ports: + - containerPort: 8081 + name: http-8081 + protocol: HTTP + servicePort: 8081 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 8081 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 + name: envoy-gateway/gateway-1/listener-1 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 8081 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /foo + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found.in.yaml new file mode 100644 index 0000000000..341ccd2e12 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found.in.yaml @@ -0,0 +1,47 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: listener-1 + protocol: HTTP + port: 8081 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: listener-1 + rules: + - matches: + - path: + value: "/foo" + backendRefs: + - name: service-1 + port: 8080 +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + namespace: default + name: policy-for-nonexistent-route + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: does-not-exist + timeout: + tcp: + connectTimeout: 15s diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found.out.yaml new file mode 100644 index 0000000000..1e30ca215a --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-targetref-not-found.out.yaml @@ -0,0 +1,188 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + name: policy-for-nonexistent-route + namespace: default + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: does-not-exist + timeout: + tcp: + connectTimeout: 15s + status: + ancestors: null +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: listener-1 + port: 8081 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: listener-1 + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - name: envoy-gateway/gateway-1/listener-1 + ports: + - containerPort: 8081 + name: http-8081 + protocol: HTTP + servicePort: 8081 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 8081 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: listener-1 + name: envoy-gateway/gateway-1/listener-1 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 8081 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /foo + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/provider/kubernetes/status.go b/internal/provider/kubernetes/status.go index 438c917be2..011a32a429 100644 --- a/internal/provider/kubernetes/status.go +++ b/internal/provider/kubernetes/status.go @@ -413,12 +413,18 @@ func (r *gatewayAPIReconciler) updateStatusFromSubscriptions(ctx context.Context message.Metadata{Runner: string(egv1a1.LogComponentProviderRunner), Message: message.BackendTrafficPolicyStatusMessageName}, r.subscriptions.backendTrafficPolicyStatuses, func(update message.Update[types.NamespacedName, *gwapiv1.PolicyStatus], errChan chan error) { - // skip delete updates. + key := update.Key + val := update.Value + + // A delete means the translator no longer produces status for this + // policy (e.g. its targetRef points at a nonexistent object, or its + // targetSelectors stopped matching anything). Clean up any stale + // status this controller previously wrote so it is not left behind. if update.Delete { + r.statusUpdater.Send(r.backendTrafficPolicyStatusCleanupUpdate(key, errChan)) return } - key := update.Key - val := update.Value + r.statusUpdater.Send(Update{ NamespacedName: key, Resource: new(egv1a1.BackendTrafficPolicy), @@ -732,6 +738,48 @@ func mergeRouteParentStatus(ns string, old, new []gwapiv1.RouteParentStatus) []g return merged } +// removePolicyStatusForController returns the ancestor statuses with all entries +// owned by the given controller removed, while preserving ancestors owned by other +// controllers. It is used to clean up stale status for a policy that this controller +// no longer attaches to any target. A nil slice is returned when no ancestors remain +// so the resulting status has an empty (rather than [] with residual) ancestor list. +func removePolicyStatusForController(ancestors []gwapiv1.PolicyAncestorStatus, controller gwapiv1.GatewayController) []gwapiv1.PolicyAncestorStatus { + var remaining []gwapiv1.PolicyAncestorStatus + for _, ancestor := range ancestors { + if ancestor.ControllerName != controller { + remaining = append(remaining, ancestor) + } + } + return remaining +} + +// backendTrafficPolicyStatusCleanupUpdate builds a status Update that strips the +// ancestor conditions owned by this controller from a BackendTrafficPolicy, while +// preserving ancestors owned by other controllers. It is used when the translator +// stops producing status for a policy (its targetRef points at a nonexistent object, +// or its targetSelectors stopped matching anything) so a stale Accepted=True condition +// with an out-of-date observedGeneration is not left behind. If the object itself was +// deleted, the status updater's Get returns NotFound and applying this is a no-op. +func (r *gatewayAPIReconciler) backendTrafficPolicyStatusCleanupUpdate(key types.NamespacedName, errChan chan error) Update { + return Update{ + NamespacedName: key, + Resource: new(egv1a1.BackendTrafficPolicy), + Mutator: MutatorFunc(func(obj client.Object) client.Object { + t, ok := obj.(*egv1a1.BackendTrafficPolicy) + if !ok { + err := fmt.Errorf("unsupported object type %T", obj) + if errChan != nil { + errChan <- err + } + panic(err) + } + tCopy := t.DeepCopy() + tCopy.Status.Ancestors = removePolicyStatusForController(tCopy.Status.Ancestors, r.classController) + return tCopy + }), + } +} + 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 index 8c619fbe8e..56477b168e 100644 --- a/internal/provider/kubernetes/status_test.go +++ b/internal/provider/kubernetes/status_test.go @@ -6,11 +6,20 @@ package kubernetes import ( + "context" + "os" "reflect" "testing" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/envoygateway" + "github.com/envoyproxy/gateway/internal/logging" ) func Test_mergeRouteParentStatus(t *testing.T) { @@ -887,3 +896,159 @@ func Test_mergeRouteParentStatus(t *testing.T) { }) } } + +func Test_removePolicyStatusForController(t *testing.T) { + const ours = gwapiv1.GatewayController("gateway.envoyproxy.io/gatewayclass-controller") + const theirs = gwapiv1.GatewayController("istio.io/gateway-controller") + + ancestor := func(controller gwapiv1.GatewayController, name string) gwapiv1.PolicyAncestorStatus { + return gwapiv1.PolicyAncestorStatus{ + ControllerName: controller, + AncestorRef: gwapiv1.ParentReference{Name: gwapiv1.ObjectName(name)}, + Conditions: []metav1.Condition{ + { + Type: string(gwapiv1.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gwapiv1.PolicyReasonAccepted), + ObservedGeneration: 1, + }, + }, + } + } + + tests := []struct { + name string + ancestors []gwapiv1.PolicyAncestorStatus + controller gwapiv1.GatewayController + want []gwapiv1.PolicyAncestorStatus + }{ + { + // #8926 / #8927: policy attached to nothing this pass, its only ancestor + // is ours and stale -> it gets cleared so the stale Accepted=True is gone. + name: "single stale ancestor of ours is cleared", + ancestors: []gwapiv1.PolicyAncestorStatus{ancestor(ours, "gateway1")}, + controller: ours, + want: nil, + }, + { + // Multi-controller: only our ancestor is removed, the other controller's + // status is preserved untouched. + name: "only our ancestor removed, other controller preserved", + ancestors: []gwapiv1.PolicyAncestorStatus{ancestor(theirs, "gateway1"), ancestor(ours, "gateway2")}, + controller: ours, + want: []gwapiv1.PolicyAncestorStatus{ancestor(theirs, "gateway1")}, + }, + { + name: "no ancestors of ours leaves status untouched", + ancestors: []gwapiv1.PolicyAncestorStatus{ancestor(theirs, "gateway1")}, + controller: ours, + want: []gwapiv1.PolicyAncestorStatus{ancestor(theirs, "gateway1")}, + }, + { + name: "empty input returns empty", + ancestors: nil, + controller: ours, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := removePolicyStatusForController(tt.ancestors, tt.controller); !reflect.DeepEqual(got, tt.want) { + t.Errorf("removePolicyStatusForController() = %v, want %v", got, tt.want) + } + }) + } +} + +// TestBackendTrafficPolicyStatusCleanupOnDelete exercises the delete-path cleanup +// end-to-end through the real UpdateHandler.apply against a fake client. It covers +// both filed issues (a BackendTrafficPolicy whose target went missing, #8926, and one +// whose selectors stopped matching, #8927) since both surface identically: the object +// carries a stale Accepted=True condition with an out-of-date observedGeneration that +// must be removed once the translator stops producing status for it. +func TestBackendTrafficPolicyStatusCleanupOnDelete(t *testing.T) { + const ours = gwapiv1.GatewayController("gateway.envoyproxy.io/gatewayclass-controller") + const theirs = gwapiv1.GatewayController("istio.io/gateway-controller") + + staleAncestor := func(controller gwapiv1.GatewayController, name string) gwapiv1.PolicyAncestorStatus { + return gwapiv1.PolicyAncestorStatus{ + ControllerName: controller, + AncestorRef: gwapiv1.ParentReference{Name: gwapiv1.ObjectName(name)}, + Conditions: []metav1.Condition{{ + Type: string(gwapiv1.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(gwapiv1.PolicyReasonAccepted), + ObservedGeneration: 1, // stale: object generation below is 2 + }}, + } + } + + tests := []struct { + name string + existing *egv1a1.BackendTrafficPolicy + key types.NamespacedName + wantAncestors []gwapiv1.PolicyAncestorStatus + }{ + { + name: "stale status owned solely by our controller is cleared", + existing: &egv1a1.BackendTrafficPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "btp-1", Generation: 2}, + Status: gwapiv1.PolicyStatus{Ancestors: []gwapiv1.PolicyAncestorStatus{staleAncestor(ours, "gateway-1")}}, + }, + key: types.NamespacedName{Namespace: "default", Name: "btp-1"}, + wantAncestors: nil, + }, + { + name: "only our ancestor is cleared, other controller's status preserved", + existing: &egv1a1.BackendTrafficPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "btp-2", Generation: 2}, + Status: gwapiv1.PolicyStatus{Ancestors: []gwapiv1.PolicyAncestorStatus{ + staleAncestor(theirs, "gateway-1"), + staleAncestor(ours, "gateway-2"), + }}, + }, + key: types.NamespacedName{Namespace: "default", Name: "btp-2"}, + wantAncestors: []gwapiv1.PolicyAncestorStatus{staleAncestor(theirs, "gateway-1")}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cli := fakeclient.NewClientBuilder(). + WithScheme(envoygateway.GetScheme()). + WithObjects(tt.existing). + WithStatusSubresource(tt.existing). + Build() + + logger := logging.DefaultLogger(os.Stdout, egv1a1.LogLevelInfo) + u := NewUpdateHandler(logger.Logger, cli) + r := &gatewayAPIReconciler{ + log: logger, + classController: ours, + } + + u.apply(r.backendTrafficPolicyStatusCleanupUpdate(tt.key, nil)) + + got := &egv1a1.BackendTrafficPolicy{} + require.NoError(t, cli.Get(context.Background(), tt.key, got)) + require.Equal(t, tt.wantAncestors, got.Status.Ancestors) + }) + } + + t.Run("delete for an already-removed object is a no-op", func(t *testing.T) { + cli := fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).Build() + logger := logging.DefaultLogger(os.Stdout, egv1a1.LogLevelInfo) + u := NewUpdateHandler(logger.Logger, cli) + r := &gatewayAPIReconciler{ + log: logger, + classController: ours, + } + + // Must not panic or error when the object no longer exists. + u.apply(r.backendTrafficPolicyStatusCleanupUpdate(types.NamespacedName{Namespace: "default", Name: "gone"}, nil)) + + got := &egv1a1.BackendTrafficPolicy{} + err := cli.Get(context.Background(), types.NamespacedName{Namespace: "default", Name: "gone"}, got) + require.Error(t, err) // still not found + }) +} diff --git a/release-notes/current/bug_fixes/8926-btp-status-target-not-found.md b/release-notes/current/bug_fixes/8926-btp-status-target-not-found.md new file mode 100644 index 0000000000..ac84273a7d --- /dev/null +++ b/release-notes/current/bug_fixes/8926-btp-status-target-not-found.md @@ -0,0 +1 @@ +Fixed BackendTrafficPolicy status going stale when the policy stops attaching to any target. A policy whose `targetRef`/`targetRefs` points at a nonexistent object or whose `targetSelectors` no longer match anything now has its stale ancestor status cleaned up instead of retaining an outdated `Accepted=True` condition with an out-of-date `observedGeneration`.