Skip to content

Commit cf21cfd

Browse files
vishalsolanki-fscursoragent
vishalsolanki-fs
and
cursoragent
committed
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 (#8926). - targetSelectors that match no objects never added the policy to the result set, so its status was never reconciled (#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 #8926 Fixes #8927 Signed-off-by: Vishal <vishal.solanki@fullscript.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 98c1be7 commit cf21cfd

10 files changed

Lines changed: 979 additions & 18 deletions

‎internal/gatewayapi/backendtrafficpolicy.go‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,14 @@ func (t *Translator) ProcessBackendTrafficPolicies(
263263

264264
handledPolicies := make(map[types.NamespacedName]*egv1a1.BackendTrafficPolicy, policyMapSize)
265265

266+
// Tracks policies that were successfully attached to at least one real target
267+
// during this pass. Any policy that ends up in res without attaching to a
268+
// target has carried-over (deep-copied) status from a previous generation that
269+
// is now stale, so we drop that status below. Cleaning up the resulting
270+
// detached status on the actual object is handled by the provider status
271+
// updater when it observes the status delete (see internal/provider/kubernetes/status.go).
272+
attachedPolicies := make(map[types.NamespacedName]bool, policyMapSize)
273+
266274
// Translate
267275
// 1. First translate Policies targeting RouteRules
268276
// 2. Next translate Policies targeting xRoutes
@@ -286,8 +294,10 @@ func (t *Translator) ProcessBackendTrafficPolicies(
286294
res = append(res, policy)
287295
}
288296

289-
t.processBackendTrafficPolicyForRoute(xdsIR,
290-
routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget)
297+
if t.processBackendTrafficPolicyForRoute(xdsIR,
298+
routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget) {
299+
attachedPolicies[policyName] = true
300+
}
291301
}
292302
}
293303
}
@@ -312,8 +322,10 @@ func (t *Translator) ProcessBackendTrafficPolicies(
312322
res = append(res, policy)
313323
}
314324

315-
t.processBackendTrafficPolicyForRoute(xdsIR,
316-
routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget)
325+
if t.processBackendTrafficPolicyForRoute(xdsIR,
326+
routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget) {
327+
attachedPolicies[policyName] = true
328+
}
317329
}
318330
}
319331
}
@@ -331,8 +343,10 @@ func (t *Translator) ProcessBackendTrafficPolicies(
331343
handledPolicies[policyName] = policy
332344
res = append(res, policy)
333345
}
334-
t.processBackendTrafficPolicyForGateway(xdsIR,
335-
gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget)
346+
if t.processBackendTrafficPolicyForGateway(xdsIR,
347+
gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget) {
348+
attachedPolicies[policyName] = true
349+
}
336350
}
337351
}
338352
}
@@ -356,12 +370,30 @@ func (t *Translator) ProcessBackendTrafficPolicies(
356370
handledPolicies[policyName] = policy
357371
res = append(res, policy)
358372
}
359-
t.processBackendTrafficPolicyForGateway(xdsIR,
360-
gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget)
373+
if t.processBackendTrafficPolicyForGateway(xdsIR,
374+
gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget) {
375+
attachedPolicies[policyName] = true
376+
}
361377
}
362378
}
363379
}
364380

381+
// Drop stale carried-over status for policies that were added to res (their
382+
// targetRef/targetRefs resolved to an object reference) but did not actually
383+
// attach to any target this pass because the referenced object does not exist
384+
// (#8926). Without this, the deep-copied Accepted=True condition with an
385+
// out-of-date observedGeneration would be republished as-is. Emptying the
386+
// ancestor list makes the gatewayapi runner treat the policy as having no
387+
// status, which deletes it from the status store and lets the provider status
388+
// updater clean up the stale status on the object. Policies whose selectors
389+
// matched nothing (#8927) are never added to res, so they are already handled
390+
// by that same delete path.
391+
for _, policy := range res {
392+
if !attachedPolicies[utils.NamespacedName(policy)] {
393+
policy.Status.Ancestors = nil
394+
}
395+
}
396+
365397
for _, policy := range res {
366398
// Truncate Ancestor list of longer than 16
367399
if len(policy.Status.Ancestors) > 16 {
@@ -431,7 +463,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
431463
gatewayPolicyMap map[NamespacedNameWithSection]*egv1a1.BackendTrafficPolicy,
432464
policy *egv1a1.BackendTrafficPolicy,
433465
currTarget policyTargetReferenceWithSectionName,
434-
) {
466+
) bool {
435467
var (
436468
targetedRoute RouteContext
437469
resolveErr *status.PolicyResolveError
@@ -443,7 +475,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
443475
// reconciled by multiple controllers. And the other controller may
444476
// have the target route.
445477
if targetedRoute == nil {
446-
return
478+
return false
447479
}
448480

449481
// Find the Gateway that the route belongs to and add it to the
@@ -493,7 +525,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
493525
policy.Generation,
494526
resolveErr,
495527
)
496-
return
528+
return true
497529
}
498530

499531
if policy.Spec.MergeType == nil {
@@ -595,7 +627,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
595627
// Check if this policy is overridden by other policies targeting at route rule levels
596628
// If policy target is route rule, we can skip the check
597629
if currTarget.SectionName != nil {
598-
return
630+
return true
599631
}
600632

601633
key := policyTargetRouteKey{
@@ -615,6 +647,8 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
615647
policy.Generation,
616648
)
617649
}
650+
651+
return true
618652
}
619653

620654
func (t *Translator) processBackendTrafficPolicyForGateway(
@@ -624,7 +658,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
624658
gatewayPolicyMergedMap *GatewayPolicyRouteMap,
625659
policy *egv1a1.BackendTrafficPolicy,
626660
currTarget policyTargetReferenceWithSectionName,
627-
) {
661+
) bool {
628662
var (
629663
targetedGateway *GatewayContext
630664
resolveErr *status.PolicyResolveError
@@ -633,7 +667,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
633667
// Negative statuses have already been assigned so it's safe to skip
634668
targetedGateway, resolveErr = resolveBackendTrafficPolicyGatewayTargetRef(currTarget, gatewayMap)
635669
if targetedGateway == nil {
636-
return
670+
return false
637671
}
638672

639673
// Find its ancestor reference by resolved gateway, even with resolve error
@@ -648,7 +682,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
648682
policy.Generation,
649683
resolveErr,
650684
)
651-
return
685+
return true
652686
}
653687

654688
// Set conditions for translation error if it got any
@@ -694,6 +728,8 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
694728
policy.Generation,
695729
)
696730
}
731+
732+
return true
697733
}
698734

699735
func resolveBackendTrafficPolicyGatewayTargetRef(

‎internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.in.yaml‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
gateways:
2+
- apiVersion: gateway.networking.k8s.io/v1
3+
kind: Gateway
4+
metadata:
5+
namespace: envoy-gateway
6+
name: gateway-1
7+
spec:
8+
gatewayClassName: envoy-gateway-class
9+
listeners:
10+
- name: listener-1
11+
protocol: HTTP
12+
port: 8081
13+
allowedRoutes:
14+
namespaces:
15+
from: All
16+
httpRoutes:
17+
- apiVersion: gateway.networking.k8s.io/v1
18+
kind: HTTPRoute
19+
metadata:
20+
namespace: default
21+
name: httproute-1
22+
labels:
23+
app: real-label
24+
spec:
25+
parentRefs:
26+
- namespace: envoy-gateway
27+
name: gateway-1
28+
sectionName: listener-1
29+
rules:
30+
- matches:
31+
- path:
32+
value: "/foo"
33+
backendRefs:
34+
- name: service-1
35+
port: 8080
36+
backendTrafficPolicies:
37+
- apiVersion: gateway.envoyproxy.io/v1alpha1
38+
kind: BackendTrafficPolicy
39+
metadata:
40+
namespace: default
41+
name: policy-with-unmatched-selector
42+
spec:
43+
targetSelectors:
44+
- group: gateway.networking.k8s.io
45+
kind: HTTPRoute
46+
matchLabels:
47+
app: no-such-label
48+
timeout:
49+
tcp:
50+
connectTimeout: 15s

‎internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.out.yaml‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
gateways:
2+
- apiVersion: gateway.networking.k8s.io/v1
3+
kind: Gateway
4+
metadata:
5+
name: gateway-1
6+
namespace: envoy-gateway
7+
spec:
8+
gatewayClassName: envoy-gateway-class
9+
listeners:
10+
- allowedRoutes:
11+
namespaces:
12+
from: All
13+
name: listener-1
14+
port: 8081
15+
protocol: HTTP
16+
status:
17+
listeners:
18+
- attachedRoutes: 1
19+
conditions:
20+
- lastTransitionTime: null
21+
message: Sending translated listener configuration to the data plane
22+
reason: Programmed
23+
status: "True"
24+
type: Programmed
25+
- lastTransitionTime: null
26+
message: Listener has been successfully translated
27+
reason: Accepted
28+
status: "True"
29+
type: Accepted
30+
- lastTransitionTime: null
31+
message: Listener references have been resolved
32+
reason: ResolvedRefs
33+
status: "True"
34+
type: ResolvedRefs
35+
name: listener-1
36+
supportedKinds:
37+
- group: gateway.networking.k8s.io
38+
kind: HTTPRoute
39+
- group: gateway.networking.k8s.io
40+
kind: GRPCRoute
41+
httpRoutes:
42+
- apiVersion: gateway.networking.k8s.io/v1
43+
kind: HTTPRoute
44+
metadata:
45+
labels:
46+
app: real-label
47+
name: httproute-1
48+
namespace: default
49+
spec:
50+
parentRefs:
51+
- name: gateway-1
52+
namespace: envoy-gateway
53+
sectionName: listener-1
54+
rules:
55+
- backendRefs:
56+
- name: service-1
57+
port: 8080
58+
matches:
59+
- path:
60+
value: /foo
61+
status:
62+
parents:
63+
- conditions:
64+
- lastTransitionTime: null
65+
message: Route is accepted
66+
reason: Accepted
67+
status: "True"
68+
type: Accepted
69+
- lastTransitionTime: null
70+
message: Resolved all the Object references for the Route
71+
reason: ResolvedRefs
72+
status: "True"
73+
type: ResolvedRefs
74+
controllerName: gateway.envoyproxy.io/gatewayclass-controller
75+
parentRef:
76+
name: gateway-1
77+
namespace: envoy-gateway
78+
sectionName: listener-1
79+
infraIR:
80+
envoy-gateway/gateway-1:
81+
proxy:
82+
listeners:
83+
- name: envoy-gateway/gateway-1/listener-1
84+
ports:
85+
- containerPort: 8081
86+
name: http-8081
87+
protocol: HTTP
88+
servicePort: 8081
89+
metadata:
90+
labels:
91+
gateway.envoyproxy.io/owning-gateway-name: gateway-1
92+
gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway
93+
ownerReference:
94+
kind: GatewayClass
95+
name: envoy-gateway-class
96+
name: envoy-gateway/gateway-1
97+
namespace: envoy-gateway-system
98+
xdsIR:
99+
envoy-gateway/gateway-1:
100+
accessLog:
101+
json:
102+
- path: /dev/stdout
103+
globalResources:
104+
proxyServiceCluster:
105+
metadata:
106+
kind: Service
107+
name: envoy-envoy-gateway-gateway-1-196ae069
108+
namespace: envoy-gateway-system
109+
sectionName: "8080"
110+
name: envoy-gateway/gateway-1
111+
settings:
112+
- addressType: IP
113+
endpoints:
114+
- host: 7.6.5.4
115+
port: 8080
116+
zone: zone1
117+
metadata:
118+
kind: Service
119+
name: envoy-envoy-gateway-gateway-1-196ae069
120+
namespace: envoy-gateway-system
121+
sectionName: "8080"
122+
name: envoy-gateway/gateway-1
123+
protocol: TCP
124+
http:
125+
- address: 0.0.0.0
126+
externalPort: 8081
127+
hostnames:
128+
- '*'
129+
metadata:
130+
kind: Gateway
131+
name: gateway-1
132+
namespace: envoy-gateway
133+
sectionName: listener-1
134+
name: envoy-gateway/gateway-1/listener-1
135+
path:
136+
escapedSlashesAction: UnescapeAndRedirect
137+
mergeSlashes: true
138+
port: 8081
139+
routes:
140+
- destination:
141+
metadata:
142+
kind: HTTPRoute
143+
name: httproute-1
144+
namespace: default
145+
name: httproute/default/httproute-1/rule/0
146+
settings:
147+
- addressType: IP
148+
endpoints:
149+
- host: 7.7.7.7
150+
port: 8080
151+
metadata:
152+
kind: Service
153+
name: service-1
154+
namespace: default
155+
sectionName: "8080"
156+
name: httproute/default/httproute-1/rule/0/backend/0
157+
protocol: HTTP
158+
weight: 1
159+
hostname: '*'
160+
isHTTP2: false
161+
metadata:
162+
kind: HTTPRoute
163+
name: httproute-1
164+
namespace: default
165+
name: httproute/default/httproute-1/rule/0/match/0/*
166+
pathMatch:
167+
distinct: false
168+
name: ""
169+
prefix: /foo
170+
readyListener:
171+
address: 0.0.0.0
172+
ipFamily: IPv4
173+
path: /ready
174+
port: 19003

0 commit comments

Comments
 (0)