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

Name	Link
🔨 Latest commit	b6dcd34
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/689ba99d2018d300087ac8f4
😎 Deploy Preview	https://deploy-preview-1302--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.

The committers listed above are authorized under a signed CLA.

• ✅ login: rahulgurnani / name: Rahul Gurnani (d79f477, 68a2272, b6dcd34, 5c00a5f, 2e57ea2)

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rahulgurnani
Once this PR has been reviewed and has the lgtm label, please assign kfswain for approval. For more information see the Code Review Process.

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

Welcome @rahulgurnani!

It looks like this is your first PR to kubernetes-sigs/gateway-api-inference-extension 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/gateway-api-inference-extension has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

Hi @rahulgurnani. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

/ok-to-test

@rahulgurnani can you please update the configuration options in the chart readme?
https://github.com/kubernetes-sigs/gateway-api-inference-extension/tree/main/config/charts/inferencepool#configuration

Can we make the flags generic similar to the env vars? This way we don't have to keep updating the chart if the EPP flags change.

PR needs rebase.

Can we make the flags generic similar to the env vars? This way we don't have to keep updating the chart if the EPP flags change.

Yes.. This would mean, adding the flags to be part of the env flag in values.yaml. Most of the flags are already read in runner.go today:
https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/cmd/epp/runner/runner.go#L188

I think we can make this change and it resonates with me. What do others think about it? @kfswain @ahg-g @nirrozenbaum ?

Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages.

The list of commits with invalid commit messages:

• d79f477 Apply suggestion from @kfswain

@rahulgurnani: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Can we make the flags generic similar to the env vars? This way we don't have to keep updating the chart if the EPP flags change.

Yes.. This would mean, adding the flags to be part of the env flag in values.yaml. Most of the flags are already read in runner.go today: https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/cmd/epp/runner/runner.go#L188

I think we can make this change and it resonates with me. What do others think about it? @kfswain @ahg-g @nirrozenbaum ?

Just to clarify, I was just asking if it's possible to make the flags also generic in helm like the env vars(I never tried but I hope so). I didn't mean to convert all flags to env vars (I don't think we should...) in order to do that.

I agree with @liu-cong; the root issue(#1207) describes allowing arbitrary flags to be passed into the helm chart. I do think it can be a separate PR, as it's more for supporting downstream usage, and I don't think there is a strict need for it. But the PR should be a fast-follow, because if llm-d, or some other dependent creates a flag in their binary, they are hard blocked from using our helm chart.

If there is a way to set passthrough epp flags, then lets go for it.