Skip to content

Conversation

@rayne-Li
Copy link

Pull Request Description

Add external-filter in Header for advanced routing

Related Issues

Resolves: #1803

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @rayne-Li, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant enhancement to the gateway's routing capabilities by allowing advanced pod selection based on Kubernetes labels. A new external-filter header can now be included in requests, enabling users to specify Kubernetes label selectors. The gateway will then use these selectors to dynamically filter available pods, ensuring requests are routed only to pods matching the specified criteria, thus providing more granular control over traffic distribution.

Highlights

  • Advanced Routing with External Filter Header: Introduced a new external-filter header that allows for advanced routing by enabling dynamic filtering of Kubernetes pods based on their labels using standard Kubernetes label selector syntax.
  • Pod Filtering Logic: Implemented new logic within the selectTargetPod function to parse the external-filter expression and apply it to filter the list of available pods, ensuring only matching pods are considered for routing.
  • Header Integration: Integrated the new HeaderExternalFilter into the request processing flow, allowing the gateway to read and utilize the filter expression provided in incoming request headers.
  • Enhanced Test Coverage: Expanded unit tests for the selectTargetPod function to include scenarios specifically validating the functionality of the new external filter, ensuring its correct behavior.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an external-filter header for advanced routing by filtering pods based on Kubernetes label selectors. The core logic is sound, but I have a few suggestions for improvement. The pod filtering can be optimized for better memory efficiency. The logging around the new filter can be made less verbose by logging only when a filter is actually provided. Most importantly, the unit test for this new feature is currently not testing the filtering logic correctly; I've provided a corrected version to ensure the feature is properly validated. Addressing these points will improve the performance, observability, and correctness of the new feature.

Copy link
Collaborator

@googs1025 googs1025 left a comment

Choose a reason for hiding this comment

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

@rayne-Li Thank you for your proposal, but I'm not entirely sure if this breaks the separation of concerns principle between the gateway and workload. 🤔 Could we achieve this separation at the workload level instead? Or could this be accomplished within the existing model.aibrix.ai/name labeling strategy? Additionally, with this filtering approach, will the target pod we obtain still be optimal?

@rayne-Li
Copy link
Author

rayne-Li commented Nov 27, 2025

@rayne-Li Thank you for your proposal, but I'm not entirely sure if this breaks the separation of concerns principle between the gateway and workload. 🤔 Could we achieve this separation at the workload level instead? Or could this be accomplished within the existing model.aibrix.ai/name labeling strategy? Additionally, with this filtering approach, will the target pod we obtain still be optimal?

@googs1025 As shown in the code:

func (s *Server) selectTargetPod(ctx *types.RoutingContext, pods types.PodList, externalFilterExpr string) {
    // incoming var: pods are all pod with exact model.name
    // so this won't change whether the pods are the best choices
    // FilterRoutablePods filters ready pods, so I extend that by adding an external filtering function
    readyPods := utils.FilterRoutablePods(pods.All())
    // ......
    for _, p := range readyPods {
        // filtering pod by externalFilter
    }
}

The pod filtering happens after utils.FilterRoutablePods (which only filters for ready pods),
so it does not change the optimal pod selected by the routing strategy.

What I want is a common and pluggable filtering method that does not affect any existing routing-strategy logic and can be easily injected or bypassed.
If we rely on another pod label like model.aibrix.ai/name, it may interfere with ModelCache and s.cache.HasModel, making the pod label-select logic more complicated.

@rayne-Li rayne-Li force-pushed the feature/add-external-filter branch from a97b660 to 43ee93e Compare November 27, 2025 10:35
@googs1025
Copy link
Collaborator

@rayne-Li Thank you for your proposal, but I'm not entirely sure if this breaks the separation of concerns principle between the gateway and workload. 🤔 Could we achieve this separation at the workload level instead? Or could this be accomplished within the existing model.aibrix.ai/name labeling strategy? Additionally, with this filtering approach, will the target pod we obtain still be optimal?

@googs1025 As shown in the code:

func (s *Server) selectTargetPod(ctx *types.RoutingContext, pods types.PodList, externalFilterExpr string) {
    // incoming var: pods are all pod with exact model.name
    // so this won't change whether the pods are the best choices
    // FilterRoutablePods filters ready pods, so I extend that by adding an external filtering function
    readyPods := utils.FilterRoutablePods(pods.All())
    // ......
    for _, p := range readyPods {
        // filtering pod by externalFilter
    }
}

The pod filtering happens after utils.FilterRoutablePods (which only filters for ready pods), so it does not change the optimal pod selected by the routing strategy.

What I want is a common and pluggable filtering method that does not affect any existing routing-strategy logic and can be easily injected or bypassed. If we rely on another pod label like model.aibrix.ai/name, it may interfere with ModelCache and s.cache.HasModel, making the pod label-select logic more complicated.

thanks for the detailed explanation. For now, this simple filtering is sufficient for routing. 😄

@rayne-Li rayne-Li force-pushed the feature/add-external-filter branch from 154a745 to f812add Compare November 28, 2025 03:44
@rayne-Li rayne-Li force-pushed the feature/add-external-filter branch from f812add to 692eb0c Compare November 28, 2025 04:08
Signed-off-by: rayneLi <[email protected]>

Signed-off-by: rayneLi <[email protected]>
Copy link
Collaborator

@googs1025 googs1025 left a comment

Choose a reason for hiding this comment

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

LGTM. thanks for your implement 😄

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.

Add external-filter in Header for advanced routing

3 participants