convert subset filter from a plugin to logic in director #1088

nirrozenbaum · 2025-06-29T10:19:04Z

This PR converts the existing SubsetFilter into a helper function in director.go. it also reverts unnecessary changes and updates the unit-tests accordingly.
After this change - when a subset filter is specified in the request metadata, the candidate pods are filtered BEFORE schedule is called, and filtering is done at the datastore level (instead of GetAll ,we can ListPods(predicate) ).

additional minor change is the replacement of interface{} with any in all places, to keep it consistent across our codebase (inconsistencies were found as part of this change)

cc: @ahg-g @kfswain

Signed-off-by: Nir Rozenbaum <[email protected]>

netlify · 2025-06-29T10:19:09Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`45fd751`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/68616cfcaae3650008649bec
😎 Deploy Preview	https://deploy-preview-1088--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2025-06-29T10:19:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nirrozenbaum

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [nirrozenbaum]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

nirrozenbaum · 2025-06-29T10:20:34Z

pkg/epp/requestcontrol/director.go

-	// Snapshot pod metrics from the datastore to:
-	// 1. Reduce concurrent access to the datastore.
-	// 2. Ensure consistent data during the scheduling operation of a request between all scheduling cycles.


this comment was moved to the helper function godoc

Signed-off-by: Nir Rozenbaum <[email protected]>

ahg-g · 2025-06-29T14:37:45Z

additional minor change is the replacement of interface{} with any in all places, to keep it consistent across our codebase (inconsistencies were found as part of this change)

I recommend against pass-by changes in the future.

ahg-g · 2025-06-29T14:59:34Z

pkg/epp/requestcontrol/director.go

+			return true
+		}
+		return false
+	})


Can we pls a trace log line indicating if the subset key is set, and the number of endpoints it included vs what the datastore has?

ahg-g · 2025-06-29T15:08:47Z

pkg/epp/requestcontrol/director.go

+	// --- 3. Call Scheduler (with the relevant candidate pods) ---
+	candidatePods, err := d.getCandidatePodsForScheduling(reqCtx.Request.Metadata)
+	if err != nil {
+		return reqCtx, errutil.Error{Code: errutil.BadRequest, Msg: fmt.Errorf("failed to find candidate pods: %w", err).Error()}


If the list was empty, it may well mean the proxy can't see any ready pods, so it is not a BadRequest error in this case. I recommend getCandidatePodsForScheduling to not return an error when the metadata exists but empty; and here we check the length of candidate pods, if zero we return a 503 (service unavailable).

Signed-off-by: Nir Rozenbaum <[email protected]>

ahg-g · 2025-06-29T16:56:33Z

/lgtm

…sigs#1088) * convert subset filter from a plugin to logic in director Signed-off-by: Nir Rozenbaum <[email protected]> * replace interface{} with any Signed-off-by: Nir Rozenbaum <[email protected]> * make linter happy Signed-off-by: Nir Rozenbaum <[email protected]> * address code review comments Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]>

Add APIs for the instantiated plugins to the EPP Handle (kubernetes-sigs#1039) * Added plugin instance APIs to plugins.Handle Signed-off-by: Shmuel Kallner <[email protected]> * An implementation of the new plugins.Handle APIs Signed-off-by: Shmuel Kallner <[email protected]> * Moved all configuration loading code to new package Signed-off-by: Shmuel Kallner <[email protected]> * Updates due to new and moved APIs Signed-off-by: Shmuel Kallner <[email protected]> * Cleanup of old configuration loading code Signed-off-by: Shmuel Kallner <[email protected]> --------- Signed-off-by: Shmuel Kallner <[email protected]> chore(deps): bump the kubernetes group with 6 updates (kubernetes-sigs#1050) Bumps the kubernetes group with 6 updates: | Package | From | To | | --- | --- | --- | | [k8s.io/api](https://github.com/kubernetes/api) | `0.33.1` | `0.33.2` | | [k8s.io/apiextensions-apiserver](https://github.com/kubernetes/apiextensions-apiserver) | `0.33.1` | `0.33.2` | | [k8s.io/apimachinery](https://github.com/kubernetes/apimachinery) | `0.33.1` | `0.33.2` | | [k8s.io/client-go](https://github.com/kubernetes/client-go) | `0.33.1` | `0.33.2` | | [k8s.io/code-generator](https://github.com/kubernetes/code-generator) | `0.33.1` | `0.33.2` | | [k8s.io/component-base](https://github.com/kubernetes/component-base) | `0.33.1` | `0.33.2` | Updates `k8s.io/api` from 0.33.1 to 0.33.2 - [Commits](kubernetes/api@v0.33.1...v0.33.2) Updates `k8s.io/apiextensions-apiserver` from 0.33.1 to 0.33.2 - [Release notes](https://github.com/kubernetes/apiextensions-apiserver/releases) - [Commits](kubernetes/apiextensions-apiserver@v0.33.1...v0.33.2) Updates `k8s.io/apimachinery` from 0.33.1 to 0.33.2 - [Commits](kubernetes/apimachinery@v0.33.1...v0.33.2) Updates `k8s.io/client-go` from 0.33.1 to 0.33.2 - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.33.1...v0.33.2) Updates `k8s.io/code-generator` from 0.33.1 to 0.33.2 - [Commits](kubernetes/code-generator@v0.33.1...v0.33.2) Updates `k8s.io/component-base` from 0.33.1 to 0.33.2 - [Commits](kubernetes/component-base@v0.33.1...v0.33.2) --- updated-dependencies: - dependency-name: k8s.io/api dependency-version: 0.33.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: kubernetes - dependency-name: k8s.io/apiextensions-apiserver dependency-version: 0.33.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: kubernetes - dependency-name: k8s.io/apimachinery dependency-version: 0.33.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: kubernetes - dependency-name: k8s.io/client-go dependency-version: 0.33.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: kubernetes - dependency-name: k8s.io/code-generator dependency-version: 0.33.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: kubernetes - dependency-name: k8s.io/component-base dependency-version: 0.33.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: kubernetes ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> remove datastore dependency from the scheduler (kubernetes-sigs#1049) * remove datastore dependency from the scheduler Signed-off-by: Nir Rozenbaum <[email protected]> * added back comments on snapshotting pods from datastore before calling schedule Signed-off-by: Nir Rozenbaum <[email protected]> * removed fake datastore from conformance scheduler test Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]> Add subsetting logic for epp (kubernetes-sigs#981) feat: Added a factory function for the DecisionTree filter (kubernetes-sigs#1053) * Added a factory function for the DecisionTreeFilter Signed-off-by: Shmuel Kallner <[email protected]> * Added tests of the factory function of the DecisionTreeFilter Signed-off-by: Shmuel Kallner <[email protected]> * Registered the factory function of the DecisionTreeFilter Signed-off-by: Shmuel Kallner <[email protected]> * Refactored the configuration loading Signed-off-by: Shmuel Kallner <[email protected]> --------- Signed-off-by: Shmuel Kallner <[email protected]> Adding pprof endpoints to metrics port (kubernetes-sigs#1069) feat: Add a context.Context to the plugins.HAndle interface (kubernetes-sigs#1076) * Added a context.Context to the plugins.Handle interface Signed-off-by: Shmuel Kallner <[email protected]> * Changes due to changes in internal APIs Signed-off-by: Shmuel Kallner <[email protected]> * Changes to tests due to changes in internal APIs Signed-off-by: Shmuel Kallner <[email protected]> --------- Signed-off-by: Shmuel Kallner <[email protected]> convert subset filter from a plugin to logic in director (kubernetes-sigs#1088) * convert subset filter from a plugin to logic in director Signed-off-by: Nir Rozenbaum <[email protected]> * replace interface{} with any Signed-off-by: Nir Rozenbaum <[email protected]> * make linter happy Signed-off-by: Nir Rozenbaum <[email protected]> * address code review comments Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]> chore(deps): bump golang.org/x/sync from 0.14.0 to 0.15.0 (kubernetes-sigs#1096) Bumps [golang.org/x/sync](https://github.com/golang/sync) from 0.14.0 to 0.15.0. - [Commits](golang/sync@v0.14.0...v0.15.0) --- updated-dependencies: - dependency-name: golang.org/x/sync dependency-version: 0.15.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Introduce plugins.TypedName to be used for Plugin base implementation (kubernetes-sigs#1086) * introduce TypedName to reduce boilerplate, modify plugins Signed-off-by: Etai Lev Ran <[email protected]> * implement GetTypedName() Signed-off-by: Etai Lev Ran <[email protected]> * Remove Type() and Name() from Plugin interface Signed-off-by: Etai Lev Ran <[email protected]> * use TypedName as private field, not embedded Signed-off-by: Etai Lev Ran <[email protected]> --------- Signed-off-by: Etai Lev Ran <[email protected]> move the conversion from pod metrics to scheduler pod representation one level up (kubernetes-sigs#1104) * move the converstion from pod metrics to scheduler pod representation one level up Signed-off-by: Nir Rozenbaum <[email protected]> * minor change in helper func Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]> handle picking multiple destinations in scheduling layer (kubernetes-sigs#1059) * implement multiple destination as the output of the scheduler Signed-off-by: Nir Rozenbaum <[email protected]> * updated max score picker unit tests to cover multiple pods Signed-off-by: Nir Rozenbaum <[email protected]> * imports Signed-off-by: Nir Rozenbaum <[email protected]> * unit-test fix Signed-off-by: Nir Rozenbaum <[email protected]> --------- Signed-off-by: Nir Rozenbaum <[email protected]> refactor: 🔨 use the more explicit singular form (kubernetes-sigs#1129)

convert subset filter from a plugin to logic in director

7b4c284

Signed-off-by: Nir Rozenbaum <[email protected]>

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 29, 2025

k8s-ci-robot requested review from ahg-g and robscott June 29, 2025 10:19

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 29, 2025

nirrozenbaum requested a review from kfswain June 29, 2025 10:19

nirrozenbaum commented Jun 29, 2025

View reviewed changes

replace interface{} with any

e8b861c

Signed-off-by: Nir Rozenbaum <[email protected]>

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 29, 2025

make linter happy

4475ac6

Signed-off-by: Nir Rozenbaum <[email protected]>

ahg-g reviewed Jun 29, 2025

View reviewed changes

address code review comments

45fd751

Signed-off-by: Nir Rozenbaum <[email protected]>

k8s-ci-robot assigned ahg-g Jun 29, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 29, 2025

k8s-ci-robot merged commit 3322cf8 into kubernetes-sigs:main Jun 29, 2025
9 checks passed

nirrozenbaum deleted the subset branch June 29, 2025 17:10

nirrozenbaum mentioned this pull request Jul 15, 2025

Support endpoint subsetting #415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

convert subset filter from a plugin to logic in director #1088

convert subset filter from a plugin to logic in director #1088

Uh oh!

nirrozenbaum commented Jun 29, 2025 •

edited

Loading

Uh oh!

netlify bot commented Jun 29, 2025 •

edited

Loading

Uh oh!

k8s-ci-robot commented Jun 29, 2025

Uh oh!

nirrozenbaum Jun 29, 2025

Uh oh!

ahg-g commented Jun 29, 2025

Uh oh!

ahg-g Jun 29, 2025

Uh oh!

nirrozenbaum Jun 29, 2025

Uh oh!

ahg-g Jun 29, 2025

Uh oh!

nirrozenbaum Jun 29, 2025

Uh oh!

ahg-g commented Jun 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

convert subset filter from a plugin to logic in director #1088

convert subset filter from a plugin to logic in director #1088

Uh oh!

Conversation

nirrozenbaum commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for gateway-api-inference-extension ready!

Uh oh!

k8s-ci-robot commented Jun 29, 2025

Uh oh!

nirrozenbaum Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g commented Jun 29, 2025

Uh oh!

ahg-g Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

nirrozenbaum Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

nirrozenbaum Jun 29, 2025

Choose a reason for hiding this comment

Uh oh!

ahg-g commented Jun 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nirrozenbaum commented Jun 29, 2025 •

edited

Loading

netlify bot commented Jun 29, 2025 •

edited

Loading