-
Notifications
You must be signed in to change notification settings - Fork 191
Description
What happened:
#1366 (comment) changed InferencePool.ExtensionRef.PortNumber to be non-pointer as // +kubebuilder:validation:Minimum=1
// PortNumber defines a network port.
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
type PortNumber int32
reading the API spec (When unspecified, implementations SHOULD infer a default value of 9002 when the Kind is Service), seems like pointer is needed here and the "0" usually means all ports instead of not setting. Wondering whether it is a little bit weird here to use "0" as a not set indicator
There are three options
-
don't use kube-api linter convention and use pointer type here
-
Keep it as what it is and the implementation should infer a default value of 9002 when the Kind is Service and the port number is 0 as 0 means unset here
-
Use
// +optional
PortNumber *PortNumber `json:"portNumber,omitempty"`
// PortNumber defines a network port.
//
// +kubebuilder:validation:Minimum=0
// +kubebuilder:validation:Maximum=65535
type PortNumber int32
Nil means unset and 0 means all ports, but do we need to support all ports here?
I feel option 1 is better, see more context here #1387
What you expected to happen:
How to reproduce it (as minimally and precisely as possible):
Anything else we need to know?:
Environment:
- Kubernetes version (use
kubectl version): - Inference extension version (use
git describe --tags --dirty --always): - Cloud provider or hardware configuration:
- Install tools:
- Others: