Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename TargetPortNumber to PortNumber in ExtensionRef #376

Closed
Tracked by #362
ahg-g opened this issue Feb 20, 2025 · 11 comments
Closed
Tracked by #362

Rename TargetPortNumber to PortNumber in ExtensionRef #376

ahg-g opened this issue Feb 20, 2025 · 11 comments
Assignees

Comments

@ahg-g
Copy link
Contributor

ahg-g commented Feb 20, 2025

In ExtensionReference in the InferencePool we name the port number TargetPortNumber, this is misleading since it should be the port number on the Service fronting the epp pods, not the port number on the epp pods themselves.

@hzxuzhonghu
Copy link
Member

There is a targetPortNumber in Spec https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/api/v1alpha1/inferencepool_types.go#L61

I may need to read more about the code to understand the diff

BTW what is EPP abbrev for, it was ext-proc

@ahg-g
Copy link
Contributor Author

ahg-g commented Feb 20, 2025

There is a targetPortNumber in Spec https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/api/v1alpha1/inferencepool_types.go#L61

This is the targetPortNumber on the model servers.

This issue is referring to the port number of the epp (ext-proc) pods that the proxy will use to communicate with via the ext-proc protocol.

BTW what is EPP abbrev for, it was ext-proc

EPP is an abbreviation of "endpoint picker".

@hzxuzhonghu
Copy link
Member

hzxuzhonghu commented Feb 20, 2025

got it, i am not sure what is ExtensionReference for? Seems we donot care about it at all. It is just set to envoy ext-proc filter(by patch) rather than used by epp

@ahg-g
Copy link
Contributor Author

ahg-g commented Feb 20, 2025

It is for the gateway controller that sets up the extension with the proxy. It needs to know where the extension is running to configure the proxy with the extension.

@hzxuzhonghu
Copy link
Member

/assign

@ahg-g
Copy link
Contributor Author

ahg-g commented Feb 21, 2025

Thanks @hzxuzhonghu for working on this, note that this requires changing the api to v1alpha2; we can do that by simply renaming v1alpha1 to v1alpha2 since k8s deprecation policy allows deprecating an alpha API in the same release: https://kubernetes.io/docs/reference/using-api/deprecation-policy/

@hzxuzhonghu
Copy link
Member

Are there anyother fields that need to be deprecated or mutated, i can change at the sametime

@ahg-g
Copy link
Contributor Author

ahg-g commented Feb 21, 2025

Yes: #379 and potentially #385

@ahg-g
Copy link
Contributor Author

ahg-g commented Feb 21, 2025

btw, I see @kfswain assigned #379 to himself

@kfswain
Copy link
Collaborator

kfswain commented Feb 21, 2025

If you'd like to take #379 , feel free!

@hzxuzhonghu
Copy link
Member

Ok, let me try to file a pr next monday

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

No branches or pull requests

3 participants