Skip to content

fix: clean up stale BackendTrafficPolicy status when it attaches to no target #9420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vishalsolanki-fs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: clean up stale BackendTrafficPolicy status when it attaches to no target #9420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 51 additions & 15 deletions internal/gatewayapi/backendtrafficpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
}
}
}
Expand All @@ -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
}
}
}
}
Expand All @@ -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
}
}
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -491,7 +523,7 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
policy.Generation,
resolveErr,
)
return
return true
}

if policy.Spec.MergeType == nil {
Expand Down Expand Up @@ -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{
Expand All @@ -613,6 +645,8 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
policy.Generation,
)
}

return true
}

func (t *Translator) processBackendTrafficPolicyForGateway(
Expand All @@ -622,7 +656,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
gatewayPolicyMergedMap *GatewayPolicyRouteMap,
policy *egv1a1.BackendTrafficPolicy,
currTarget policyTargetReferenceWithSectionName,
) {
) bool {
var (
targetedGateway *GatewayContext
resolveErr *status.PolicyResolveError
Expand All @@ -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
Expand All @@ -646,7 +680,7 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
policy.Generation,
resolveErr,
)
return
return true
}

// Set conditions for translation error if it got any
Expand Down Expand Up @@ -692,6 +726,8 @@ func (t *Translator) processBackendTrafficPolicyForGateway(
policy.Generation,
)
}

return true
}

func resolveBackendTrafficPolicyGatewayTargetRef(
Expand Down
50 changes: 50 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.in.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
174 changes: 174 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-selector-no-match.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Loading