Skip to content

Conversation

ahg-g
Copy link
Contributor

@ahg-g ahg-g commented Feb 2, 2025

Fixes #256
Fixes #158

The implementation is efficient in that it sets up an informer with a server-side selector on the pods that the pool selects. It also addresses a number of data staleness problems (e.g., selector being updated).

This can be expanded to support multi-tenant pools, but will be done as a followup if we decide to do it since we first need to define a way other than a flag to assign pools to epps (see #252)

@k8s-ci-robot k8s-ci-robot requested a review from danehans February 2, 2025 23:13
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g

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:

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

@k8s-ci-robot k8s-ci-robot requested a review from kfswain February 2, 2025 23:13
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 2, 2025
Copy link

netlify bot commented Feb 2, 2025

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

Name Link
🔨 Latest commit cdf7d0f
🔍 Latest deploy log https://app.netlify.com/sites/gateway-api-inference-extension/deploys/67a1916f1149c10009c3c571
😎 Deploy Preview https://deploy-preview-271--gateway-api-inference-extension.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@ahg-g ahg-g force-pushed the informer branch 6 times, most recently from 876a472 to 63ca5c8 Compare February 3, 2025 05:00
- dupword
- durationcheck
- fatcontext
- gci
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are not enforcing a specific import format, and so was causing issues.

@ahg-g ahg-g force-pushed the informer branch 2 times, most recently from c5fc0bf to 30e6024 Compare February 3, 2025 19:59
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2025
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2025
@ahg-g
Copy link
Contributor Author

ahg-g commented Feb 3, 2025

This supersedes #268

Copy link
Contributor

@liu-cong liu-cong left a comment

Choose a reason for hiding this comment

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

/lgtm A few nits left, feel free to address of leave as followups.

/hold for you to address the nits

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 4, 2025
@kfswain
Copy link
Collaborator

kfswain commented Feb 4, 2025

/hold

I would like to review this before we merge.

@kfswain
Copy link
Collaborator

kfswain commented Feb 4, 2025

I don't think this code can stay in this state.

But it's more important that we get a release out and this seems to work for now.

/lgtm
/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 4, 2025
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2025
@k8s-ci-robot k8s-ci-robot merged commit 9298849 into kubernetes-sigs:main Feb 4, 2025
8 checks passed
kfswain added a commit to kfswain/llm-instance-gateway that referenced this pull request Feb 6, 2025
k8s-ci-robot pushed a commit that referenced this pull request Feb 6, 2025
kfswain pushed a commit to kfswain/llm-instance-gateway that referenced this pull request Apr 29, 2025
…bernetes-sigs#271)

* Replace EndpointSlice reconciler with pod list backed by informer

* Addressed comments
kfswain added a commit to kfswain/llm-instance-gateway that referenced this pull request Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove EndpointSlice dependency Remove serviceName CLI Flag
4 participants