Skip to content

feat: add priority to backendRef#673

Merged
yuzisun merged 7 commits intoenvoyproxy:mainfrom
cmaddalozzo:add-backend-ref-priority
Jun 5, 2025
Merged

feat: add priority to backendRef#673
yuzisun merged 7 commits intoenvoyproxy:mainfrom
cmaddalozzo:add-backend-ref-priority

Conversation

@cmaddalozzo
Copy link
Contributor

@cmaddalozzo cmaddalozzo commented Jun 3, 2025

Commit Message

Add priority field to AIGatewayRouteRuleBackendRef. This will set priority on the underlying LocalityLB endpoint in Envoy.

Related Issues/PRs (if applicable)

N/A

Special notes for reviewers (if applicable)

In order to get the Envoy Gateway to reconcile the underlying HTTPRoute when the priorities in the AIGatewayRoute spec change we need to add an annotation. This annotation just store the priorities for each rule. This ensure we only trigger reconciliation when the priorities actually change.

@cmaddalozzo cmaddalozzo force-pushed the add-backend-ref-priority branch from eb2aa3b to 102c94b Compare June 3, 2025 19:29
@cmaddalozzo cmaddalozzo changed the title Add priority to backendRef. Set priority on cluster endpoints in feat: add priority to backendRef. Set priority on cluster endpoints in Jun 3, 2025
@cmaddalozzo cmaddalozzo changed the title feat: add priority to backendRef. Set priority on cluster endpoints in feat: add priority to backendRef Jun 3, 2025
@cmaddalozzo cmaddalozzo marked this pull request as ready for review June 3, 2025 19:59
@cmaddalozzo cmaddalozzo requested a review from a team as a code owner June 3, 2025 19:59
@cmaddalozzo cmaddalozzo force-pushed the add-backend-ref-priority branch from 76f2dbe to 79f2fcd Compare June 4, 2025 14:51
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you might want to update the provider_fallback e2e ?

@mathetake
Copy link
Member

i am glad that the core change is much smaller than i thought! I think the rest is documentation + e2e tests

@cmaddalozzo cmaddalozzo force-pushed the add-backend-ref-priority branch from 515f83b to 02c5208 Compare June 4, 2025 19:23
@cmaddalozzo
Copy link
Contributor Author

maybe you might want to update the provider_fallback e2e ?

I don't think we will be able to add this until the previous priority retry predicate PR makes it into Envoy Gateway.

envoyproxy/gateway#6204

@mathetake
Copy link
Member

hmm i was under the impression that this could be used instead of fallback: true option in the e2e test. not sure what i am missing

@cmaddalozzo
Copy link
Contributor Author

hmm i was under the impression that this could be used instead of fallback: true option in the e2e test. not sure what i am missing

ah, yes that will work. The part that can't be tested yet is retrying across priorities in a dynamic way.

@mathetake
Copy link
Member

cool, could you go ahead and make the change like the below;

diff --git a/examples/provider_fallback/base.yaml b/examples/provider_fallback/base.yaml
index b695ae4..ea3c5bc 100644
--- a/examples/provider_fallback/base.yaml
+++ b/examples/provider_fallback/base.yaml
@@ -41,8 +41,11 @@ spec:
               name: x-ai-eg-model
               value: us.meta.llama3-2-1b-instruct-v1:0
       backendRefs:
-        - name: provider-fallback-always-failing-upstream  # This is the primary backend and trying to speak TLS, which always fails.
+        - name: provider-fallback-always-failing-upstream
+          # This is the primary backend and trying to speak TLS, which always fails.
+          priority: 0
         - name: provider-fallback-aws
+          priority: 1
 ---
 apiVersion: aigateway.envoyproxy.io/v1alpha1
 kind: AIServiceBackend
@@ -80,8 +83,6 @@ metadata:
   name: provider-fallback-aws
   namespace: default
 spec:
-  # Indicate that this backend is a fallback backend, meaning that it will only be used if the primary backend fails.
-  fallback: true
   endpoints:
     - fqdn:
         hostname: bedrock-runtime.us-east-1.amazonaws.com

i believe this should work and then we are good to go (i think we can make this part of v0.2)

cmaddalozzo and others added 7 commits June 4, 2025 18:58
extension server.

Signed-off-by: Curtis Maddalozzo <cmaddalozzo@bloomberg.net>
Signed-off-by: Curtis Maddalozzo <cmaddalozzo@bloomberg.net>
Signed-off-by: Curtis Maddalozzo <cmaddalozzo@bloomberg.net>
Signed-off-by: Curtis Maddalozzo <cmaddalozzo@bloomberg.net>
Signed-off-by: Curtis Maddalozzo <cmaddalozzo@bloomberg.net>
Signed-off-by: Curtis Maddalozzo <cmaddalozzo@bloomberg.net>
Signed-off-by: Dan Sun <dsun20@bloomberg.net>
@yuzisun yuzisun force-pushed the add-backend-ref-priority branch from a2ed196 to 9e2d702 Compare June 4, 2025 23:00
@yuzisun
Copy link
Contributor

yuzisun commented Jun 4, 2025

cool, could you go ahead and make the change like the below;

diff --git a/examples/provider_fallback/base.yaml b/examples/provider_fallback/base.yaml
index b695ae4..ea3c5bc 100644
--- a/examples/provider_fallback/base.yaml
+++ b/examples/provider_fallback/base.yaml
@@ -41,8 +41,11 @@ spec:
               name: x-ai-eg-model
               value: us.meta.llama3-2-1b-instruct-v1:0
       backendRefs:
-        - name: provider-fallback-always-failing-upstream  # This is the primary backend and trying to speak TLS, which always fails.
+        - name: provider-fallback-always-failing-upstream
+          # This is the primary backend and trying to speak TLS, which always fails.
+          priority: 0
         - name: provider-fallback-aws
+          priority: 1
 ---
 apiVersion: aigateway.envoyproxy.io/v1alpha1
 kind: AIServiceBackend
@@ -80,8 +83,6 @@ metadata:
   name: provider-fallback-aws
   namespace: default
 spec:
-  # Indicate that this backend is a fallback backend, meaning that it will only be used if the primary backend fails.
-  fallback: true
   endpoints:
     - fqdn:
         hostname: bedrock-runtime.us-east-1.amazonaws.com

i believe this should work and then we are good to go (i think we can make this part of v0.2)

@mathetake I updated, going to update my doc with the same change.

Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome

- name: provider-fallback-always-failing-upstream # This is the primary backend and trying to speak TLS, which always fails.
priority: 0
- name: provider-fallback-aws
priority: 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also delete the fallback: true below as it's overridden by this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I deleted the fallback flag below

@yuzisun yuzisun merged commit 0a0ef4a into envoyproxy:main Jun 5, 2025
17 checks passed
cmaddalozzo added a commit to cmaddalozzo/ai-gateway that referenced this pull request Jun 11, 2025
**Commit Message**

Add priority field to AIGatewayRouteRuleBackendRef. This will set
priority on the underlying LocalityLB endpoint in Envoy.

**Related Issues/PRs (if applicable)**

N/A

**Special notes for reviewers (if applicable)**

In order to get the Envoy Gateway to reconcile the underlying
`HTTPRoute` when the priorities in the `AIGatewayRoute` spec change we
need to add an annotation. This annotation just store the priorities for
each rule. This ensure we only trigger reconciliation when the
priorities actually change.

---------

Signed-off-by: Curtis Maddalozzo <cmaddalozzo@bloomberg.net>
Signed-off-by: Dan Sun <dsun20@bloomberg.net>
Co-authored-by: Dan Sun <dsun20@bloomberg.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants