Skip to content

Commit 72cf7cc

Browse files
vishalsolanki-fscursoragent
authored and
zirain
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 85a5c60 commit 72cf7cc

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
@@ -261,6 +261,14 @@ func (t *Translator) ProcessBackendTrafficPolicies(
261261

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

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

287-
t.processBackendTrafficPolicyForRoute(xdsIR,
288-
routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget)
295+
if t.processBackendTrafficPolicyForRoute(xdsIR,
296+
routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget) {
297+
attachedPolicies[policyName] = true
298+
}
289299
}
290300
}
291301
}
@@ -310,8 +320,10 @@ func (t *Translator) ProcessBackendTrafficPolicies(
310320
res = append(res, policy)
311321
}
312322

313-
t.processBackendTrafficPolicyForRoute(xdsIR,
314-
routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget)
323+
if t.processBackendTrafficPolicyForRoute(xdsIR,
324+
routeMap, gatewayRouteMap, gatewayPolicyMerged, gatewayPolicyMap, policy, currTarget) {
325+
attachedPolicies[policyName] = true
326+
}
315327
}
316328
}
317329
}
@@ -329,8 +341,10 @@ func (t *Translator) ProcessBackendTrafficPolicies(
329341
handledPolicies[policyName] = policy
330342
res = append(res, policy)
331343
}
332-
t.processBackendTrafficPolicyForGateway(xdsIR,
333-
gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget)
344+
if t.processBackendTrafficPolicyForGateway(xdsIR,
345+
gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget) {
346+
attachedPolicies[policyName] = true
347+
}
334348
}
335349
}
336350
}
@@ -354,12 +368,30 @@ func (t *Translator) ProcessBackendTrafficPolicies(
354368
handledPolicies[policyName] = policy
355369
res = append(res, policy)
356370
}
357-
t.processBackendTrafficPolicyForGateway(xdsIR,
358-
gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget)
371+
if t.processBackendTrafficPolicyForGateway(xdsIR,
372+
gatewayMap, gatewayRouteMap, gatewayPolicyMerged, policy, currTarget) {
373+
attachedPolicies[policyName] = true
374+
}
359375
}
360376
}
361377
}
362378

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

447479
// Find the Gateway that the route belongs to and add it to the
@@ -491,7 +523,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
491523
policy.Generation,
492524
resolveErr,
493525
)
494-
return
526+
return true
495527
}
496528

497529
if policy.Spec.MergeType == nil {
@@ -593,7 +625,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
593625
// Check if this policy is overridden by other policies targeting at route rule levels
594626
// If policy target is route rule, we can skip the check
595627
if currTarget.SectionName != nil {
596-
return
628+
return true
597629
}
598630

599631
key := policyTargetRouteKey{
@@ -613,6 +645,8 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
613645
policy.Generation,
614646
)
615647
}
648+
649+
return true
616650
}
617651

618652
func (t *Translator) processBackendTrafficPolicyForGateway(
@@ -622,7 +656,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
622656
gatewayPolicyMergedMap *GatewayPolicyRouteMap,
623657
policy *egv1a1.BackendTrafficPolicy,
624658
currTarget policyTargetReferenceWithSectionName,
625-
) {
659+
) bool {
626660
var (
627661
targetedGateway *GatewayContext
628662
resolveErr *status.PolicyResolveError
@@ -631,7 +665,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
631665
// Negative statuses have already been assigned so it's safe to skip
632666
targetedGateway, resolveErr = resolveBackendTrafficPolicyGatewayTargetRef(currTarget, gatewayMap)
633667
if targetedGateway == nil {
634-
return
668+
return false
635669
}
636670

637671
// Find its ancestor reference by resolved gateway, even with resolve error
@@ -646,7 +680,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
646680
policy.Generation,
647681
resolveErr,
648682
)
649-
return
683+
return true
650684
}
651685

652686
// Set conditions for translation error if it got any
@@ -692,6 +726,8 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
692726
policy.Generation,
693727
)
694728
}
729+
730+
return true
695731
}
696732

697733
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)