From 91dca8da40302e621a9a150dfdcb098becb518f7 Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 09:55:42 -0700 Subject: [PATCH 01/11] make port definitions symmetric --- api/v1/inferencepool_types.go | 6 ++++-- api/v1/zz_generated.deepcopy.go | 6 +++--- apix/v1alpha2/inferencepool_conversion.go | 6 +++--- apix/v1alpha2/inferencepool_conversion_test.go | 6 +++--- .../api/v1/endpointpickerref.go | 10 +++++----- ...rence.networking.k8s.io_inferencepools.yaml | 18 +++++++++++++----- 6 files changed, 31 insertions(+), 21 deletions(-) diff --git a/api/v1/inferencepool_types.go b/api/v1/inferencepool_types.go index af945827e..a1ab77683 100644 --- a/api/v1/inferencepool_types.go +++ b/api/v1/inferencepool_types.go @@ -91,6 +91,9 @@ type Port struct { // The number must be in the range 1 to 65535. // // +required + // + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=65535 Number PortNumber `json:"number,omitempty"` } @@ -130,8 +133,7 @@ type EndpointPickerRef struct { // unspecified (defaults to "Service"). // // +optional - //nolint:kubeapilinter // ignore kubeapilinter here as we want to use pointer as zero means all ports in convention, we don't make to use 0 to indicate not set. - PortNumber *PortNumber `json:"portNumber,omitempty"` + Port *Port `json:"port,omitempty"` // FailureMode configures how the parent handles the case when the Endpoint Picker extension // is non-responsive. When unspecified, defaults to "FailClose". diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index b42573ce2..3b003632a 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -33,9 +33,9 @@ func (in *EndpointPickerRef) DeepCopyInto(out *EndpointPickerRef) { *out = new(Group) **out = **in } - if in.PortNumber != nil { - in, out := &in.PortNumber, &out.PortNumber - *out = new(PortNumber) + if in.Port != nil { + in, out := &in.Port, &out.Port + *out = new(Port) **out = **in } } diff --git a/apix/v1alpha2/inferencepool_conversion.go b/apix/v1alpha2/inferencepool_conversion.go index 31ea997bf..01520dc4b 100644 --- a/apix/v1alpha2/inferencepool_conversion.go +++ b/apix/v1alpha2/inferencepool_conversion.go @@ -254,7 +254,7 @@ func convertExtensionRefToV1(src *Extension) (v1.EndpointPickerRef, error) { } endpointPickerRef.Name = v1.ObjectName(src.Name) if src.PortNumber != nil { - endpointPickerRef.PortNumber = ptr.To(v1.PortNumber(*src.PortNumber)) + endpointPickerRef.Port = ptr.To(v1.Port{Number: v1.PortNumber(*src.PortNumber)}) } if src.FailureMode != nil { endpointPickerRef.FailureMode = v1.EndpointPickerFailureMode(*src.FailureMode) @@ -275,8 +275,8 @@ func convertEndpointPickerRefFromV1(src *v1.EndpointPickerRef) (Extension, error extension.Kind = ptr.To(Kind(src.Kind)) } extension.Name = ObjectName(src.Name) - if src.PortNumber != nil { - extension.PortNumber = ptr.To(PortNumber(*src.PortNumber)) + if src.Port != nil { + extension.PortNumber = ptr.To(PortNumber(src.Port.Number)) } if src.FailureMode != "" { extension.FailureMode = ptr.To(ExtensionFailureMode(src.FailureMode)) diff --git a/apix/v1alpha2/inferencepool_conversion_test.go b/apix/v1alpha2/inferencepool_conversion_test.go index a4147f2df..1c19b2e36 100644 --- a/apix/v1alpha2/inferencepool_conversion_test.go +++ b/apix/v1alpha2/inferencepool_conversion_test.go @@ -34,7 +34,7 @@ var ( v1Group = v1.Group("my-group") v1Kind = v1.Kind("MyKind") v1FailureMode = v1.EndpointPickerFailureMode("Deny") - v1PortNumber = v1.PortNumber(9000) + v1Port = v1.Port{Number: 9000} ) func TestInferencePoolConvertTo(t *testing.T) { @@ -110,7 +110,7 @@ func TestInferencePoolConvertTo(t *testing.T) { Group: &v1Group, Kind: v1Kind, Name: "my-epp-service", - PortNumber: &v1PortNumber, + Port: &v1Port, FailureMode: v1FailureMode, }, }, @@ -433,7 +433,7 @@ func TestInferencePoolConvertFrom(t *testing.T) { Group: &v1Group, Kind: v1Kind, Name: "my-epp-service", - PortNumber: &v1PortNumber, + Port: &v1Port, FailureMode: v1FailureMode, }, }, diff --git a/client-go/applyconfiguration/api/v1/endpointpickerref.go b/client-go/applyconfiguration/api/v1/endpointpickerref.go index 8a9991bc5..0d7886239 100644 --- a/client-go/applyconfiguration/api/v1/endpointpickerref.go +++ b/client-go/applyconfiguration/api/v1/endpointpickerref.go @@ -28,7 +28,7 @@ type EndpointPickerRefApplyConfiguration struct { Group *apiv1.Group `json:"group,omitempty"` Kind *apiv1.Kind `json:"kind,omitempty"` Name *apiv1.ObjectName `json:"name,omitempty"` - PortNumber *apiv1.PortNumber `json:"portNumber,omitempty"` + Port *PortApplyConfiguration `json:"port,omitempty"` FailureMode *apiv1.EndpointPickerFailureMode `json:"failureMode,omitempty"` } @@ -62,11 +62,11 @@ func (b *EndpointPickerRefApplyConfiguration) WithName(value apiv1.ObjectName) * return b } -// WithPortNumber sets the PortNumber field in the declarative configuration to the given value +// WithPort sets the Port field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the PortNumber field is set to the value of the last call. -func (b *EndpointPickerRefApplyConfiguration) WithPortNumber(value apiv1.PortNumber) *EndpointPickerRefApplyConfiguration { - b.PortNumber = &value +// If called multiple times, the Port field is set to the value of the last call. +func (b *EndpointPickerRefApplyConfiguration) WithPort(value *PortApplyConfiguration) *EndpointPickerRefApplyConfiguration { + b.Port = value return b } diff --git a/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml b/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml index 66cbcec6f..ef5f44067 100644 --- a/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml +++ b/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml @@ -89,15 +89,23 @@ spec: maxLength: 253 minLength: 1 type: string - portNumber: + port: description: |- PortNumber is the port number of the Endpoint Picker extension service. When unspecified, implementations SHOULD infer a default value of 9002 when the kind field is "Service" or unspecified (defaults to "Service"). - format: int32 - maximum: 65535 - minimum: 1 - type: integer + properties: + number: + description: |- + Number defines the port number to access the selected model server Pods. + The number must be in the range 1 to 65535. + format: int32 + maximum: 65535 + minimum: 1 + type: integer + required: + - number + type: object required: - name type: object From b2c12ff323de4ccc3101e6c6874ba3a083dc4120 Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 10:03:18 -0700 Subject: [PATCH 02/11] fixed linter Signed-off-by: Xiyue Yu --- api/v1/inferencepool_types.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/v1/inferencepool_types.go b/api/v1/inferencepool_types.go index a1ab77683..7da451f29 100644 --- a/api/v1/inferencepool_types.go +++ b/api/v1/inferencepool_types.go @@ -92,8 +92,6 @@ type Port struct { // // +required // - // +kubebuilder:validation:Minimum=1 - // +kubebuilder:validation:Maximum=65535 Number PortNumber `json:"number,omitempty"` } From 1716d2fc87abe9119ee4cc3a24c070b18b088104 Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 10:03:41 -0700 Subject: [PATCH 03/11] change spec --- site-src/reference/spec.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site-src/reference/spec.md b/site-src/reference/spec.md index 12ce3d7ab..ef6fc0853 100644 --- a/site-src/reference/spec.md +++ b/site-src/reference/spec.md @@ -51,7 +51,7 @@ _Appears in:_ | `group` _[Group](#group)_ | Group is the group of the referent API object. When unspecified, the default value
is "", representing the Core API group. | | MaxLength: 253
MinLength: 0
Pattern: `^$\|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
| | `kind` _[Kind](#kind)_ | Kind is the Kubernetes resource kind of the referent.
Required if the referent is ambiguous, e.g. service with multiple ports.
Defaults to "Service" when not specified.
ExternalName services can refer to CNAME DNS records that may live
outside of the cluster and as such are difficult to reason about in
terms of conformance. They also may not be safe to forward to (see
CVE-2021-25740 for more information). Implementations MUST NOT
support ExternalName Services. | Service | MaxLength: 63
MinLength: 1
Pattern: `^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$`
| | `name` _[ObjectName](#objectname)_ | Name is the name of the referent API object. | | MaxLength: 253
MinLength: 1
| -| `portNumber` _[PortNumber](#portnumber)_ | PortNumber is the port number of the Endpoint Picker extension service. When unspecified,
implementations SHOULD infer a default value of 9002 when the kind field is "Service" or
unspecified (defaults to "Service"). | | Maximum: 65535
Minimum: 1
| +| `port` _[Port](#port)_ | PortNumber is the port number of the Endpoint Picker extension service. When unspecified,
implementations SHOULD infer a default value of 9002 when the kind field is "Service" or
unspecified (defaults to "Service"). | | | | `failureMode` _[EndpointPickerFailureMode](#endpointpickerfailuremode)_ | FailureMode configures how the parent handles the case when the Endpoint Picker extension
is non-responsive. When unspecified, defaults to "FailClose". | FailClose | Enum: [FailOpen FailClose]
| @@ -340,6 +340,7 @@ Port defines the network port that will be exposed by this InferencePool. _Appears in:_ +- [EndpointPickerRef](#endpointpickerref) - [InferencePoolSpec](#inferencepoolspec) | Field | Description | Default | Validation | @@ -358,7 +359,6 @@ _Validation:_ - Minimum: 1 _Appears in:_ -- [EndpointPickerRef](#endpointpickerref) - [Port](#port) From ca703a412cc36214a8698a5a56a7175296a38d2b Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 10:15:55 -0700 Subject: [PATCH 04/11] clean comment --- api/v1/inferencepool_types.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/v1/inferencepool_types.go b/api/v1/inferencepool_types.go index 7da451f29..1617237df 100644 --- a/api/v1/inferencepool_types.go +++ b/api/v1/inferencepool_types.go @@ -91,7 +91,6 @@ type Port struct { // The number must be in the range 1 to 65535. // // +required - // Number PortNumber `json:"number,omitempty"` } From ca3d9dd43a88f33bafea0d873ee368621d21ff8d Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 12:55:14 -0700 Subject: [PATCH 05/11] fixed port --- api/v1/inferencepool_types.go | 10 +++++++--- .../inference.networking.k8s.io_inferencepools.yaml | 12 +++++++++--- site-src/reference/spec.md | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/api/v1/inferencepool_types.go b/api/v1/inferencepool_types.go index 1617237df..6c9f02571 100644 --- a/api/v1/inferencepool_types.go +++ b/api/v1/inferencepool_types.go @@ -96,6 +96,7 @@ type Port struct { // EndpointPickerRef specifies a reference to an Endpoint Picker extension and its // associated configuration. +// +kubebuilder:validation:XValidation:rule="self.kind != 'Service' || has(self.port)",message="port is required when kind is 'Service'" type EndpointPickerRef struct { // Group is the group of the referent API object. When unspecified, the default value // is "", representing the Core API group. @@ -125,9 +126,12 @@ type EndpointPickerRef struct { // +required Name ObjectName `json:"name,omitempty"` - // PortNumber is the port number of the Endpoint Picker extension service. When unspecified, - // implementations SHOULD infer a default value of 9002 when the kind field is "Service" or - // unspecified (defaults to "Service"). + // Port is the port of the Endpoint Picker extension service. + // + // Port is required when the referent is a Kubernetes Service. In this + // case, the port number is the service port number, not the target port. + // For other resources, destination port might be derived from the referent + // resource or this field. // // +optional Port *Port `json:"port,omitempty"` diff --git a/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml b/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml index ef5f44067..4a1a2a577 100644 --- a/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml +++ b/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml @@ -91,9 +91,12 @@ spec: type: string port: description: |- - PortNumber is the port number of the Endpoint Picker extension service. When unspecified, - implementations SHOULD infer a default value of 9002 when the kind field is "Service" or - unspecified (defaults to "Service"). + Port is the port of the Endpoint Picker extension service. + + Port is required when the referent is a Kubernetes Service. In this + case, the port number is the service port number, not the target port. + For other resources, destination port might be derived from the referent + resource or this field. properties: number: description: |- @@ -109,6 +112,9 @@ spec: required: - name type: object + x-kubernetes-validations: + - message: port is required when kind is 'Service' + rule: self.kind != 'Service' || has(self.port) selector: description: |- Selector determines which Pods are members of this inference pool. diff --git a/site-src/reference/spec.md b/site-src/reference/spec.md index ef6fc0853..11080fe24 100644 --- a/site-src/reference/spec.md +++ b/site-src/reference/spec.md @@ -51,7 +51,7 @@ _Appears in:_ | `group` _[Group](#group)_ | Group is the group of the referent API object. When unspecified, the default value
is "", representing the Core API group. | | MaxLength: 253
MinLength: 0
Pattern: `^$\|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
| | `kind` _[Kind](#kind)_ | Kind is the Kubernetes resource kind of the referent.
Required if the referent is ambiguous, e.g. service with multiple ports.
Defaults to "Service" when not specified.
ExternalName services can refer to CNAME DNS records that may live
outside of the cluster and as such are difficult to reason about in
terms of conformance. They also may not be safe to forward to (see
CVE-2021-25740 for more information). Implementations MUST NOT
support ExternalName Services. | Service | MaxLength: 63
MinLength: 1
Pattern: `^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$`
| | `name` _[ObjectName](#objectname)_ | Name is the name of the referent API object. | | MaxLength: 253
MinLength: 1
| -| `port` _[Port](#port)_ | PortNumber is the port number of the Endpoint Picker extension service. When unspecified,
implementations SHOULD infer a default value of 9002 when the kind field is "Service" or
unspecified (defaults to "Service"). | | | +| `port` _[Port](#port)_ | Port is the port of the Endpoint Picker extension service.
Port is required when the referent is a Kubernetes Service. In this
case, the port number is the service port number, not the target port.
For other resources, destination port might be derived from the referent
resource or this field. | | | | `failureMode` _[EndpointPickerFailureMode](#endpointpickerfailuremode)_ | FailureMode configures how the parent handles the case when the Endpoint Picker extension
is non-responsive. When unspecified, defaults to "FailClose". | FailClose | Enum: [FailOpen FailClose]
| From 61952117bf44ea5be05e8297ec9ca9ee67c9e5bf Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 13:03:34 -0700 Subject: [PATCH 06/11] change port default --- api/v1/inferencepool_types.go | 2 +- config/manifests/inferencepool-resources.yaml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/api/v1/inferencepool_types.go b/api/v1/inferencepool_types.go index 6c9f02571..8448adc76 100644 --- a/api/v1/inferencepool_types.go +++ b/api/v1/inferencepool_types.go @@ -127,7 +127,7 @@ type EndpointPickerRef struct { Name ObjectName `json:"name,omitempty"` // Port is the port of the Endpoint Picker extension service. - // + // // Port is required when the referent is a Kubernetes Service. In this // case, the port number is the service port number, not the target port. // For other resources, destination port might be derived from the referent diff --git a/config/manifests/inferencepool-resources.yaml b/config/manifests/inferencepool-resources.yaml index 7c31da97e..ffe19654b 100644 --- a/config/manifests/inferencepool-resources.yaml +++ b/config/manifests/inferencepool-resources.yaml @@ -15,6 +15,9 @@ spec: app: vllm-llama3-8b-instruct endpointPickerRef: name: vllm-llama3-8b-instruct-epp + kind: Service + port: + number: 9002 --- apiVersion: v1 kind: Service From d9cbca543a7adf72153f9c826bb8be6d012c151d Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 13:24:09 -0700 Subject: [PATCH 07/11] fixed test --- conformance/tests/inferencepool_invalid_epp_service.yaml | 3 +++ test/testdata/inferencepool-e2e.yaml | 3 +++ test/testdata/inferencepool-leader-election-e2e.yaml | 3 +++ test/testdata/inferencepool-with-model-hermetic.yaml | 3 +++ 4 files changed, 12 insertions(+) diff --git a/conformance/tests/inferencepool_invalid_epp_service.yaml b/conformance/tests/inferencepool_invalid_epp_service.yaml index cdc2048f7..b3dc70e19 100644 --- a/conformance/tests/inferencepool_invalid_epp_service.yaml +++ b/conformance/tests/inferencepool_invalid_epp_service.yaml @@ -11,6 +11,9 @@ spec: - number: 3000 endpointPickerRef: name: non-existent-epp-svc + kind: Service + port: + number: 9002 --- apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute diff --git a/test/testdata/inferencepool-e2e.yaml b/test/testdata/inferencepool-e2e.yaml index 9fa50593a..b8d7fb697 100644 --- a/test/testdata/inferencepool-e2e.yaml +++ b/test/testdata/inferencepool-e2e.yaml @@ -12,6 +12,9 @@ spec: endpointPickerRef: name: vllm-llama3-8b-instruct-epp namespace: $E2E_NS + kind: Service + port: + number: 9002 --- apiVersion: v1 kind: Service diff --git a/test/testdata/inferencepool-leader-election-e2e.yaml b/test/testdata/inferencepool-leader-election-e2e.yaml index 8c243470f..9ba5dcb4a 100644 --- a/test/testdata/inferencepool-leader-election-e2e.yaml +++ b/test/testdata/inferencepool-leader-election-e2e.yaml @@ -10,6 +10,9 @@ spec: endpointPickerRef: name: vllm-llama3-8b-instruct-epp namespace: $E2E_NS + kind: Service + port: + number: 9002 --- apiVersion: v1 kind: Service diff --git a/test/testdata/inferencepool-with-model-hermetic.yaml b/test/testdata/inferencepool-with-model-hermetic.yaml index 794b7fcd4..df6ae30db 100644 --- a/test/testdata/inferencepool-with-model-hermetic.yaml +++ b/test/testdata/inferencepool-with-model-hermetic.yaml @@ -11,6 +11,9 @@ spec: app: vllm-llama3-8b-instruct-pool endpointPickerRef: name: epp + kind: Service + port: + number: 9002 --- apiVersion: inference.networking.x-k8s.io/v1alpha2 kind: InferenceObjective From 6c5bf6a5410d54ee098455a51d0e1b3469b8e012 Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 14:27:37 -0700 Subject: [PATCH 08/11] draft cel test --- test/cel/inferencepool_test.go | 132 +++++++++++++++++++++++++++++++++ test/cel/main_test.go | 127 +++++++++++++++++++++++++++++++ 2 files changed, 259 insertions(+) create mode 100644 test/cel/inferencepool_test.go create mode 100644 test/cel/main_test.go diff --git a/test/cel/inferencepool_test.go b/test/cel/inferencepool_test.go new file mode 100644 index 000000000..3179df4df --- /dev/null +++ b/test/cel/inferencepool_test.go @@ -0,0 +1,132 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + "fmt" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/gateway-api-inference-extension/api/v1" +) + +func TestValidateInferencePool(t *testing.T) { + ctx := context.Background() + + // baseInferencePool is a valid, minimal InferencePool resource. + // We use a non-Service kind for the picker to ensure the base object is valid + // without needing a port, making it a neutral starting point for mutations. + baseInferencePool := v1.InferencePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "base-pool", + Namespace: metav1.NamespaceDefault, + }, + Spec: v1.InferencePoolSpec{ + TargetPorts: []v1.Port{ + {Number: 8000}, + }, + Selector: v1.LabelSelector{ + MatchLabels: map[v1.LabelKey]v1.LabelValue{ + "app": "my-model-server", + }, + }, + EndpointPickerRef: v1.EndpointPickerRef{ + Name: "epp", + Kind: "Service", + Port: ptrTo(v1.Port{Number: 9000}), + }, + }, + } + + testCases := []struct { + desc string + mutate func(ip *v1.InferencePool) + wantErrors []string + }{ + { + desc: "fails validation when kind is unset (defaults to Service) and port is missing", + mutate: func(ip *v1.InferencePool) { + // By setting Kind to an empty string, we rely on the API server's default value of "Service". + ip.Spec.EndpointPickerRef.Kind = "" + ip.Spec.EndpointPickerRef.Name = "vllm-llama3-8b-instruct-epp" + ip.Spec.EndpointPickerRef.Port = nil + }, + wantErrors: []string{"port is required when kind is 'Service'"}, + }, + { + desc: "fails validation when kind is explicitly 'Service' and port is missing", + mutate: func(ip *v1.InferencePool) { + ip.Spec.EndpointPickerRef.Kind = "Service" + ip.Spec.EndpointPickerRef.Name = "vllm-llama3-8b-instruct-epp" + ip.Spec.EndpointPickerRef.Port = nil + }, + wantErrors: []string{"port is required when kind is 'Service'"}, + }, + { + desc: "passes validation when kind is 'Service' and port is present", + mutate: func(ip *v1.InferencePool) { + ip.Spec.EndpointPickerRef.Kind = "Service" + ip.Spec.EndpointPickerRef.Name = "vllm-llama3-8b-instruct-epp" + ip.Spec.EndpointPickerRef.Port = &v1.Port{ + Number: 9002, + } + }, + // No errors expected, so wantErrors is nil or empty. + wantErrors: nil, + }, + { + desc: "passes validation with a valid, minimal configuration", + mutate: func(ip *v1.InferencePool) { + // This mutation just uses the base configuration, which should be valid. + // It's a good sanity check. The base uses a non-Service Kind. + }, + wantErrors: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + ip := baseInferencePool.DeepCopy() + // Use a unique name for each test case to avoid conflicts. + ip.Name = fmt.Sprintf("test-pool-%v", time.Now().UnixNano()) + + if tc.mutate != nil { + tc.mutate(ip) + } + err := k8sClient.Create(ctx, ip) + + // This is a boolean XOR. It's true if one is true, but not both. + // It ensures that an error is returned if and only if we expect one. + if (len(tc.wantErrors) != 0) != (err != nil) { + t.Fatalf("Unexpected response while creating InferencePool; got err=\n%v\n; want error=%v", err, tc.wantErrors != nil) + } + + // If we got an error, check that it contains the expected substrings. + var missingErrorStrings []string + for _, wantError := range tc.wantErrors { + if !celErrorStringMatches(err.Error(), wantError) { + missingErrorStrings = append(missingErrorStrings, wantError) + } + } + if len(missingErrorStrings) != 0 { + t.Errorf("Unexpected response while creating InferencePool; got err=\n%v\n; missing strings within error=%q", err, missingErrorStrings) + } + }) + } +} diff --git a/test/cel/main_test.go b/test/cel/main_test.go new file mode 100644 index 000000000..8da3344f9 --- /dev/null +++ b/test/cel/main_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + inferencev1 "sigs.k8s.io/gateway-api-inference-extension/api/v1" +) + +var k8sClient client.Client + +func TestMain(m *testing.M) { + scheme := runtime.NewScheme() + var restConfig *rest.Config + var testEnv *envtest.Environment + var err error + + inferencev1.AddToScheme(scheme) + + // Add core APIs in case we refer secrets, services and configmaps + corev1.AddToScheme(scheme) + + // If one wants to use a local cluster, a KUBECONFIG envvar should be passed, + // otherwise testenv will be used + kubeconfig := os.Getenv("KUBECONFIG") + if kubeconfig != "" { + restConfig, err = clientcmd.BuildConfigFromFlags("", kubeconfig) + if err != nil { + panic(fmt.Sprintf("Failed to get restConfig from BuildConfigFromFlags: %v", err)) + } + } else { + // The version used here MUST reflect the available versions at + // controller-runtime repo: https://raw.githubusercontent.com/kubernetes-sigs/controller-tools/HEAD/envtest-releases.yaml + // If the envvar is not passed, the latest GA will be used + k8sVersion := os.Getenv("K8S_VERSION") + testEnv = &envtest.Environment{ + Scheme: scheme, + ErrorIfCRDPathMissing: true, + DownloadBinaryAssets: true, + DownloadBinaryAssetsVersion: k8sVersion, + CRDInstallOptions: envtest.CRDInstallOptions{ + Paths: []string{ + filepath.Join("..", "..", "..", "config", "crd", "bases"), + }, + CleanUpAfterUse: true, + }, + } + + restConfig, err = testEnv.Start() + if err != nil { + panic(fmt.Sprintf("Error initializing test environment: %v", err)) + } + } + + k8sClient, err = client.New(restConfig, client.Options{ + Scheme: scheme, + }) + if err != nil { + panic(fmt.Sprintf("Error initializing Kubernetes client: %v", err)) + } + + rc := m.Run() + if testEnv != nil { + if err := testEnv.Stop(); err != nil { + panic(fmt.Sprintf("error stopping test environment: %v", err)) + } + } + + os.Exit(rc) +} + +func ptrTo[T any](a T) *T { + return &a +} + +func celErrorStringMatches(got, want string) bool { + gotL := strings.ToLower(got) + wantL := strings.ToLower(want) + + // Starting in k8s v1.32, some CEL error messages changed to use "more" instead of "longer" + alternativeWantL := strings.ReplaceAll(wantL, "longer", "more") + + // Starting in k8s v1.28, CEL error messages stopped adding spec and status prefixes to path names + wantLAdjusted := strings.ReplaceAll(wantL, "spec.", "") + wantLAdjusted = strings.ReplaceAll(wantLAdjusted, "status.", "") + alternativeWantL = strings.ReplaceAll(alternativeWantL, "spec.", "") + alternativeWantL = strings.ReplaceAll(alternativeWantL, "status.", "") + + // Enum validation messages changed in k8s v1.28: + // Before: must be one of ['Exact', 'PathPrefix', 'RegularExpression'] + // After: supported values: "Exact", "PathPrefix", "RegularExpression" + if strings.Contains(wantLAdjusted, "must be one of") { + r := strings.NewReplacer( + "must be one of", "supported values:", + "[", "", + "]", "", + "'", "\"", + ) + wantLAdjusted = r.Replace(wantLAdjusted) + } + return strings.Contains(gotL, wantL) || strings.Contains(gotL, wantLAdjusted) || strings.Contains(gotL, alternativeWantL) +} From f45de61eee99b3296a935d5705b6f995c08d5b05 Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 15:06:32 -0700 Subject: [PATCH 09/11] added cel test --- api/v1/inferencepool_types.go | 2 +- .../v1alpha2/inferencepool_conversion_test.go | 1 + ...ence.networking.k8s.io_inferencepools.yaml | 3 +- test/cel/inferencepool_test.go | 41 ++++++------------- test/cel/main_test.go | 9 ++-- 5 files changed, 23 insertions(+), 33 deletions(-) diff --git a/api/v1/inferencepool_types.go b/api/v1/inferencepool_types.go index 8448adc76..f4f96a39a 100644 --- a/api/v1/inferencepool_types.go +++ b/api/v1/inferencepool_types.go @@ -96,7 +96,7 @@ type Port struct { // EndpointPickerRef specifies a reference to an Endpoint Picker extension and its // associated configuration. -// +kubebuilder:validation:XValidation:rule="self.kind != 'Service' || has(self.port)",message="port is required when kind is 'Service'" +// +kubebuilder:validation:XValidation:rule="self.kind != 'Service' || has(self.port)",message="port is required when kind is 'Service' and unset(default to 'Service')" type EndpointPickerRef struct { // Group is the group of the referent API object. When unspecified, the default value // is "", representing the Core API group. diff --git a/apix/v1alpha2/inferencepool_conversion_test.go b/apix/v1alpha2/inferencepool_conversion_test.go index 1c19b2e36..64b222fed 100644 --- a/apix/v1alpha2/inferencepool_conversion_test.go +++ b/apix/v1alpha2/inferencepool_conversion_test.go @@ -21,6 +21,7 @@ import ( "github.com/google/go-cmp/cmp" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "sigs.k8s.io/gateway-api-inference-extension/api/v1" ) diff --git a/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml b/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml index 4a1a2a577..b412d141b 100644 --- a/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml +++ b/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml @@ -113,7 +113,8 @@ spec: - name type: object x-kubernetes-validations: - - message: port is required when kind is 'Service' + - message: port is required when kind is 'Service' and unset(default + to 'Service') rule: self.kind != 'Service' || has(self.port) selector: description: |- diff --git a/test/cel/inferencepool_test.go b/test/cel/inferencepool_test.go index 3179df4df..78d527698 100644 --- a/test/cel/inferencepool_test.go +++ b/test/cel/inferencepool_test.go @@ -23,15 +23,14 @@ import ( "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/gateway-api-inference-extension/api/v1" + + v1 "sigs.k8s.io/gateway-api-inference-extension/api/v1" ) func TestValidateInferencePool(t *testing.T) { ctx := context.Background() - // baseInferencePool is a valid, minimal InferencePool resource. - // We use a non-Service kind for the picker to ensure the base object is valid - // without needing a port, making it a neutral starting point for mutations. + // baseInferencePool is a valid, InferencePool resource. baseInferencePool := v1.InferencePool{ ObjectMeta: metav1.ObjectMeta{ Name: "base-pool", @@ -43,13 +42,13 @@ func TestValidateInferencePool(t *testing.T) { }, Selector: v1.LabelSelector{ MatchLabels: map[v1.LabelKey]v1.LabelValue{ - "app": "my-model-server", + "app": "model-server", }, }, EndpointPickerRef: v1.EndpointPickerRef{ Name: "epp", Kind: "Service", - Port: ptrTo(v1.Port{Number: 9000}), + Port: ptrTo(v1.Port{Number: 9002}), }, }, } @@ -59,6 +58,12 @@ func TestValidateInferencePool(t *testing.T) { mutate func(ip *v1.InferencePool) wantErrors []string }{ + { + desc: "passes validation with a valid configuration", + mutate: func(ip *v1.InferencePool) { + }, + wantErrors: nil, + }, { desc: "fails validation when kind is unset (defaults to Service) and port is missing", mutate: func(ip *v1.InferencePool) { @@ -67,7 +72,7 @@ func TestValidateInferencePool(t *testing.T) { ip.Spec.EndpointPickerRef.Name = "vllm-llama3-8b-instruct-epp" ip.Spec.EndpointPickerRef.Port = nil }, - wantErrors: []string{"port is required when kind is 'Service'"}, + wantErrors: []string{"port is required when kind is 'Service' and unset(default to 'Service')"}, }, { desc: "fails validation when kind is explicitly 'Service' and port is missing", @@ -76,27 +81,7 @@ func TestValidateInferencePool(t *testing.T) { ip.Spec.EndpointPickerRef.Name = "vllm-llama3-8b-instruct-epp" ip.Spec.EndpointPickerRef.Port = nil }, - wantErrors: []string{"port is required when kind is 'Service'"}, - }, - { - desc: "passes validation when kind is 'Service' and port is present", - mutate: func(ip *v1.InferencePool) { - ip.Spec.EndpointPickerRef.Kind = "Service" - ip.Spec.EndpointPickerRef.Name = "vllm-llama3-8b-instruct-epp" - ip.Spec.EndpointPickerRef.Port = &v1.Port{ - Number: 9002, - } - }, - // No errors expected, so wantErrors is nil or empty. - wantErrors: nil, - }, - { - desc: "passes validation with a valid, minimal configuration", - mutate: func(ip *v1.InferencePool) { - // This mutation just uses the base configuration, which should be valid. - // It's a good sanity check. The base uses a non-Service Kind. - }, - wantErrors: nil, + wantErrors: []string{"port is required when kind is 'Service' and unset(default to 'Service')"}, }, } diff --git a/test/cel/main_test.go b/test/cel/main_test.go index 8da3344f9..2758a5632 100644 --- a/test/cel/main_test.go +++ b/test/cel/main_test.go @@ -29,7 +29,9 @@ import ( "k8s.io/client-go/tools/clientcmd" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" + inferencev1 "sigs.k8s.io/gateway-api-inference-extension/api/v1" + inferencev1alpha2 "sigs.k8s.io/gateway-api-inference-extension/apix/v1alpha2" ) var k8sClient client.Client @@ -40,10 +42,11 @@ func TestMain(m *testing.M) { var testEnv *envtest.Environment var err error - inferencev1.AddToScheme(scheme) + _ = inferencev1.Install(scheme) + _ = inferencev1alpha2.Install(scheme) // Add core APIs in case we refer secrets, services and configmaps - corev1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) // If one wants to use a local cluster, a KUBECONFIG envvar should be passed, // otherwise testenv will be used @@ -65,7 +68,7 @@ func TestMain(m *testing.M) { DownloadBinaryAssetsVersion: k8sVersion, CRDInstallOptions: envtest.CRDInstallOptions{ Paths: []string{ - filepath.Join("..", "..", "..", "config", "crd", "bases"), + filepath.Join("..", "..", "config", "crd", "bases"), }, CleanUpAfterUse: true, }, From da4a889c5c58559d77bec7e95874e0d0f2dee74f Mon Sep 17 00:00:00 2001 From: capri-xiyue <52932582+capri-xiyue@users.noreply.github.com> Date: Wed, 27 Aug 2025 15:20:14 -0700 Subject: [PATCH 10/11] Update api/v1/inferencepool_types.go Co-authored-by: Rob Scott --- api/v1/inferencepool_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1/inferencepool_types.go b/api/v1/inferencepool_types.go index f4f96a39a..2c7b52705 100644 --- a/api/v1/inferencepool_types.go +++ b/api/v1/inferencepool_types.go @@ -96,7 +96,7 @@ type Port struct { // EndpointPickerRef specifies a reference to an Endpoint Picker extension and its // associated configuration. -// +kubebuilder:validation:XValidation:rule="self.kind != 'Service' || has(self.port)",message="port is required when kind is 'Service' and unset(default to 'Service')" +// +kubebuilder:validation:XValidation:rule="self.kind != 'Service' || has(self.port)",message="port is required when kind is 'Service' or unspecified (defaults to 'Service')" type EndpointPickerRef struct { // Group is the group of the referent API object. When unspecified, the default value // is "", representing the Core API group. From e58d8300c8d63e881f234f36d865574d44a908e6 Mon Sep 17 00:00:00 2001 From: Xiyue Yu Date: Wed, 27 Aug 2025 15:24:13 -0700 Subject: [PATCH 11/11] updated test --- .../bases/inference.networking.k8s.io_inferencepools.yaml | 4 ++-- test/cel/inferencepool_test.go | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml b/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml index b412d141b..8f60199f2 100644 --- a/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml +++ b/config/crd/bases/inference.networking.k8s.io_inferencepools.yaml @@ -113,8 +113,8 @@ spec: - name type: object x-kubernetes-validations: - - message: port is required when kind is 'Service' and unset(default - to 'Service') + - message: port is required when kind is 'Service' or unspecified + (defaults to 'Service') rule: self.kind != 'Service' || has(self.port) selector: description: |- diff --git a/test/cel/inferencepool_test.go b/test/cel/inferencepool_test.go index 78d527698..8b3ba3ea5 100644 --- a/test/cel/inferencepool_test.go +++ b/test/cel/inferencepool_test.go @@ -23,7 +23,6 @@ import ( "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "sigs.k8s.io/gateway-api-inference-extension/api/v1" ) @@ -69,19 +68,17 @@ func TestValidateInferencePool(t *testing.T) { mutate: func(ip *v1.InferencePool) { // By setting Kind to an empty string, we rely on the API server's default value of "Service". ip.Spec.EndpointPickerRef.Kind = "" - ip.Spec.EndpointPickerRef.Name = "vllm-llama3-8b-instruct-epp" ip.Spec.EndpointPickerRef.Port = nil }, - wantErrors: []string{"port is required when kind is 'Service' and unset(default to 'Service')"}, + wantErrors: []string{"port is required when kind is 'Service' or unspecified (defaults to 'Service')"}, }, { desc: "fails validation when kind is explicitly 'Service' and port is missing", mutate: func(ip *v1.InferencePool) { ip.Spec.EndpointPickerRef.Kind = "Service" - ip.Spec.EndpointPickerRef.Name = "vllm-llama3-8b-instruct-epp" ip.Spec.EndpointPickerRef.Port = nil }, - wantErrors: []string{"port is required when kind is 'Service' and unset(default to 'Service')"}, + wantErrors: []string{"port is required when kind is 'Service' or unspecified (defaults to 'Service')"}, }, }