fix: apply QuotaPolicy changes to Envoy config without restart - #2766
AyushSawant18588 wants to merge 5 commits into
Conversation
Signed-off-by: Ayush Sawant <ayush.sawant@nutanix.com>
✅ Deploy Preview for theagentrouter canceled.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Signed-off-by: Ayush Sawant <ayush.sawant@nutanix.com>
Signed-off-by: Ayush Sawant <ayush.sawant@nutanix.com>
Signed-off-by: Ayush Sawant <ayush.sawant@nutanix.com>
|
@AyushSawant18588 could you fix the tests? |
|
One gap in the deletion change, which I ran into while working on the That can happen two ways:
I checked the first case with a controller test that creates and reconciles a Keeping |
| // counted it, the recomputed hash would be unchanged, the HTTPRoute update would be a no-op, | ||
| // and Envoy Gateway would not re-translate. Treating a terminating policy as already-absent | ||
| // makes the hash change deterministically, regardless of whether the cache has dropped it yet. | ||
| if !p.DeletionTimestamp.IsZero() { |
There was a problem hiding this comment.
could we add this check to the extension policy block in maybeInjectQuotaRateLimiting? Prevents the extension server from adding a soon-to-be-deleted descriptors
| return ctrl.Result{}, err | ||
| } | ||
| c.updateQuotaPolicyStatus(ctx, "aPolicy, aigv1a1.ConditionTypeAccepted, "QuotaPolicy reconciled successfully") | ||
| c.notifyAIGatewayRoutes(ctx, "aPolicy) |
There was a problem hiding this comment.
one minor thing to note is that changing a backend in spec.ref may remove an aigatewayroute leaving the annotation defined in ai_gateway_route.go orphaned. I think it should be fine because the extension server will still be triggered if they are the same gateway but something to consider.
| // The QuotaPolicy targetRefs index key is "<targetRef.Name>.<quotaPolicy.Namespace>", and a | ||
| // QuotaPolicy (LocalPolicyTargetReference) can only target a backend in its own namespace, so | ||
| // the backend's namespace is also the QuotaPolicy's namespace. | ||
| key := fmt.Sprintf("%s.%s", br.Name, backendNamespace) |
There was a problem hiding this comment.
do you mind making a helper function that does this so we don't duplicate this logic all over ie in syncAIServiceBackend?
ie
func namespacedNameIndexKey(name, namespace string) string {
return fmt.Sprintf("%s.%s", name, namespace)
}
Description
When a QuotaPolicy CR was updated, the new configuration was not applied to the data plane until the AI Gateway controller and the Envoy proxy pod were restarted.
The root cause is that a QuotaPolicy feeds two config planes: the rate limit service config (the numeric limits, pushed over xDS) and the Envoy data-plane config (the rate limit filter, cluster, and per-route descriptors, injected by the extension server's PostTranslateModify). The extension server only runs when Envoy Gateway re-translates, which happens when a resource it watches changes. QuotaPolicy is not such a resource. The controller tried to force re-translation by re-reconciling the HTTPRoute, but the regenerated HTTPRoute was identical (its content does not depend on the QuotaPolicy), so the update was a no-op and Envoy Gateway never re-translated.
This change stamps a hash of the applicable QuotaPolicy specs onto the generated HTTPRoute as the aigateway.envoyproxy.io/quota-policy-hash annotation (mirroring the existing stampGatewayConfigHash approach for GatewayConfig). A QuotaPolicy create/update/delete now changes the annotation, making the HTTPRoute genuinely change, which forces Envoy Gateway to re-translate and re-run PostTranslateModify with the latest policy. The hash covers each policy's full spec, so both value-only changes (e.g. a limit bump) and structural changes (e.g. a new model or bucket rule) are picked up live.
This change also fixes QuotaPolicy deletion not propagating to routes in different namespace. Deleting a QuotaPolicy now notifies the referencing AIGatewayRoutes (via the finalizer callback, while TargetRefs are still available) so their generated HTTPRoutes are re-stamped and Envoy Gateway re-translates without the deleted policy, no controller/Envoy restart required. The quota-policy-hash computation also skips terminating policies (non-zero DeletionTimestamp), closing the issue where a cached, soon-to-be-deleted policy could otherwise leave the hash unchanged.
Unit tests cover the hash computation and annotation behavior, and an e2e test verifies the annotation changes when a QuotaPolicy is updated live (no restart).