Skip to content

Commit 0a2751e

Browse files
capri-xiyuerobscott
authored and
kfswain
committed
cleanup: make port definitions symmetric and clean endpointPickerRef.port (#1484)
* make port definitions symmetric * fixed linter Signed-off-by: Xiyue Yu <[email protected]> * change spec * clean comment * fixed port * change port default * fixed test * draft cel test * added cel test * Update api/v1/inferencepool_types.go Co-authored-by: Rob Scott <[email protected]> * updated test --------- Signed-off-by: Xiyue Yu <[email protected]> Co-authored-by: Rob Scott <[email protected]>
1 parent dc82925 commit 0a2751e

14 files changed

+307
-29
lines changed

‎api/v1/inferencepool_types.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ type Port struct {
9696

9797
// EndpointPickerRef specifies a reference to an Endpoint Picker extension and its
9898
// associated configuration.
99+
// +kubebuilder:validation:XValidation:rule="self.kind != 'Service' || has(self.port)",message="port is required when kind is 'Service' or unspecified (defaults to 'Service')"
99100
type EndpointPickerRef struct {
100101
// Group is the group of the referent API object. When unspecified, the default value
101102
// is "", representing the Core API group.
@@ -125,13 +126,15 @@ type EndpointPickerRef struct {
125126
// +required
126127
Name ObjectName `json:"name,omitempty"`
127128

128-
// PortNumber is the port number of the Endpoint Picker extension service. When unspecified,
129-
// implementations SHOULD infer a default value of 9002 when the kind field is "Service" or
130-
// unspecified (defaults to "Service").
129+
// Port is the port of the Endpoint Picker extension service.
130+
//
131+
// Port is required when the referent is a Kubernetes Service. In this
132+
// case, the port number is the service port number, not the target port.
133+
// For other resources, destination port might be derived from the referent
134+
// resource or this field.
131135
//
132136
// +optional
133-
//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.
134-
PortNumber *PortNumber `json:"portNumber,omitempty"`
137+
Port *Port `json:"port,omitempty"`
135138

136139
// FailureMode configures how the parent handles the case when the Endpoint Picker extension
137140
// is non-responsive. When unspecified, defaults to "FailClose".

‎api/v1/zz_generated.deepcopy.go‎

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎apix/v1alpha2/inferencepool_conversion.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ func convertExtensionRefToV1(src *Extension) (v1.EndpointPickerRef, error) {
254254
}
255255
endpointPickerRef.Name = v1.ObjectName(src.Name)
256256
if src.PortNumber != nil {
257-
endpointPickerRef.PortNumber = ptr.To(v1.PortNumber(*src.PortNumber))
257+
endpointPickerRef.Port = ptr.To(v1.Port{Number: v1.PortNumber(*src.PortNumber)})
258258
}
259259
if src.FailureMode != nil {
260260
endpointPickerRef.FailureMode = v1.EndpointPickerFailureMode(*src.FailureMode)
@@ -275,8 +275,8 @@ func convertEndpointPickerRefFromV1(src *v1.EndpointPickerRef) (Extension, error
275275
extension.Kind = ptr.To(Kind(src.Kind))
276276
}
277277
extension.Name = ObjectName(src.Name)
278-
if src.PortNumber != nil {
279-
extension.PortNumber = ptr.To(PortNumber(*src.PortNumber))
278+
if src.Port != nil {
279+
extension.PortNumber = ptr.To(PortNumber(src.Port.Number))
280280
}
281281
if src.FailureMode != "" {
282282
extension.FailureMode = ptr.To(ExtensionFailureMode(src.FailureMode))

‎apix/v1alpha2/inferencepool_conversion_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"github.com/google/go-cmp/cmp"
2323
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
2425
v1 "sigs.k8s.io/gateway-api-inference-extension/api/v1"
2526
)
2627

@@ -34,7 +35,7 @@ var (
3435
v1Group = v1.Group("my-group")
3536
v1Kind = v1.Kind("MyKind")
3637
v1FailureMode = v1.EndpointPickerFailureMode("Deny")
37-
v1PortNumber = v1.PortNumber(9000)
38+
v1Port = v1.Port{Number: 9000}
3839
)
3940

4041
func TestInferencePoolConvertTo(t *testing.T) {
@@ -110,7 +111,7 @@ func TestInferencePoolConvertTo(t *testing.T) {
110111
Group: &v1Group,
111112
Kind: v1Kind,
112113
Name: "my-epp-service",
113-
PortNumber: &v1PortNumber,
114+
Port: &v1Port,
114115
FailureMode: v1FailureMode,
115116
},
116117
},
@@ -433,7 +434,7 @@ func TestInferencePoolConvertFrom(t *testing.T) {
433434
Group: &v1Group,
434435
Kind: v1Kind,
435436
Name: "my-epp-service",
436-
PortNumber: &v1PortNumber,
437+
Port: &v1Port,
437438
FailureMode: v1FailureMode,
438439
},
439440
},

‎client-go/applyconfiguration/api/v1/endpointpickerref.go‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/inference.networking.k8s.io_inferencepools.yaml‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,18 +89,33 @@ spec:
8989
maxLength: 253
9090
minLength: 1
9191
type: string
92-
portNumber:
92+
port:
9393
description: |-
94-
PortNumber is the port number of the Endpoint Picker extension service. When unspecified,
95-
implementations SHOULD infer a default value of 9002 when the kind field is "Service" or
96-
unspecified (defaults to "Service").
97-
format: int32
98-
maximum: 65535
99-
minimum: 1
100-
type: integer
94+
Port is the port of the Endpoint Picker extension service.
95+
96+
Port is required when the referent is a Kubernetes Service. In this
97+
case, the port number is the service port number, not the target port.
98+
For other resources, destination port might be derived from the referent
99+
resource or this field.
100+
properties:
101+
number:
102+
description: |-
103+
Number defines the port number to access the selected model server Pods.
104+
The number must be in the range 1 to 65535.
105+
format: int32
106+
maximum: 65535
107+
minimum: 1
108+
type: integer
109+
required:
110+
- number
111+
type: object
101112
required:
102113
- name
103114
type: object
115+
x-kubernetes-validations:
116+
- message: port is required when kind is 'Service' or unspecified
117+
(defaults to 'Service')
118+
rule: self.kind != 'Service' || has(self.port)
104119
selector:
105120
description: |-
106121
Selector determines which Pods are members of this inference pool.

‎config/manifests/inferencepool-resources.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ spec:
1515
app: vllm-llama3-8b-instruct
1616
endpointPickerRef:
1717
name: vllm-llama3-8b-instruct-epp
18+
kind: Service
19+
port:
20+
number: 9002
1821
---
1922
apiVersion: v1
2023
kind: Service

‎conformance/tests/inferencepool_invalid_epp_service.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ spec:
1111
- number: 3000
1212
endpointPickerRef:
1313
name: non-existent-epp-svc
14+
kind: Service
15+
port:
16+
number: 9002
1417
---
1518
apiVersion: gateway.networking.k8s.io/v1
1619
kind: HTTPRoute

‎site-src/reference/spec.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ _Appears in:_
5151
| `group` _[Group](#group)_ | Group is the group of the referent API object. When unspecified, the default value<br />is "", representing the Core API group. | | MaxLength: 253 <br />MinLength: 0 <br />Pattern: `^$\|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` <br /> |
5252
| `kind` _[Kind](#kind)_ | Kind is the Kubernetes resource kind of the referent.<br />Required if the referent is ambiguous, e.g. service with multiple ports.<br />Defaults to "Service" when not specified.<br />ExternalName services can refer to CNAME DNS records that may live<br />outside of the cluster and as such are difficult to reason about in<br />terms of conformance. They also may not be safe to forward to (see<br />CVE-2021-25740 for more information). Implementations MUST NOT<br />support ExternalName Services. | Service | MaxLength: 63 <br />MinLength: 1 <br />Pattern: `^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$` <br /> |
5353
| `name` _[ObjectName](#objectname)_ | Name is the name of the referent API object. | | MaxLength: 253 <br />MinLength: 1 <br /> |
54-
| `portNumber` _[PortNumber](#portnumber)_ | PortNumber is the port number of the Endpoint Picker extension service. When unspecified,<br />implementations SHOULD infer a default value of 9002 when the kind field is "Service" or<br />unspecified (defaults to "Service"). | | Maximum: 65535 <br />Minimum: 1 <br /> |
54+
| `port` _[Port](#port)_ | Port is the port of the Endpoint Picker extension service.<br />Port is required when the referent is a Kubernetes Service. In this<br />case, the port number is the service port number, not the target port.<br />For other resources, destination port might be derived from the referent<br />resource or this field. | | |
5555
| `failureMode` _[EndpointPickerFailureMode](#endpointpickerfailuremode)_ | FailureMode configures how the parent handles the case when the Endpoint Picker extension<br />is non-responsive. When unspecified, defaults to "FailClose". | FailClose | Enum: [FailOpen FailClose] <br /> |
5656

5757

@@ -340,6 +340,7 @@ Port defines the network port that will be exposed by this InferencePool.
340340

341341

342342
_Appears in:_
343+
- [EndpointPickerRef](#endpointpickerref)
343344
- [InferencePoolSpec](#inferencepoolspec)
344345

345346
| Field | Description | Default | Validation |
@@ -358,7 +359,6 @@ _Validation:_
358359
- Minimum: 1
359360

360361
_Appears in:_
361-
- [EndpointPickerRef](#endpointpickerref)
362362
- [Port](#port)
363363

364364

‎test/cel/inferencepool_test.go‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package main
18+
19+
import (
20+
"context"
21+
"fmt"
22+
"testing"
23+
"time"
24+
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
v1 "sigs.k8s.io/gateway-api-inference-extension/api/v1"
27+
)
28+
29+
func TestValidateInferencePool(t *testing.T) {
30+
ctx := context.Background()
31+
32+
// baseInferencePool is a valid, InferencePool resource.
33+
baseInferencePool := v1.InferencePool{
34+
ObjectMeta: metav1.ObjectMeta{
35+
Name: "base-pool",
36+
Namespace: metav1.NamespaceDefault,
37+
},
38+
Spec: v1.InferencePoolSpec{
39+
TargetPorts: []v1.Port{
40+
{Number: 8000},
41+
},
42+
Selector: v1.LabelSelector{
43+
MatchLabels: map[v1.LabelKey]v1.LabelValue{
44+
"app": "model-server",
45+
},
46+
},
47+
EndpointPickerRef: v1.EndpointPickerRef{
48+
Name: "epp",
49+
Kind: "Service",
50+
Port: ptrTo(v1.Port{Number: 9002}),
51+
},
52+
},
53+
}
54+
55+
testCases := []struct {
56+
desc string
57+
mutate func(ip *v1.InferencePool)
58+
wantErrors []string
59+
}{
60+
{
61+
desc: "passes validation with a valid configuration",
62+
mutate: func(ip *v1.InferencePool) {
63+
},
64+
wantErrors: nil,
65+
},
66+
{
67+
desc: "fails validation when kind is unset (defaults to Service) and port is missing",
68+
mutate: func(ip *v1.InferencePool) {
69+
// By setting Kind to an empty string, we rely on the API server's default value of "Service".
70+
ip.Spec.EndpointPickerRef.Kind = ""
71+
ip.Spec.EndpointPickerRef.Port = nil
72+
},
73+
wantErrors: []string{"port is required when kind is 'Service' or unspecified (defaults to 'Service')"},
74+
},
75+
{
76+
desc: "fails validation when kind is explicitly 'Service' and port is missing",
77+
mutate: func(ip *v1.InferencePool) {
78+
ip.Spec.EndpointPickerRef.Kind = "Service"
79+
ip.Spec.EndpointPickerRef.Port = nil
80+
},
81+
wantErrors: []string{"port is required when kind is 'Service' or unspecified (defaults to 'Service')"},
82+
},
83+
}
84+
85+
for _, tc := range testCases {
86+
t.Run(tc.desc, func(t *testing.T) {
87+
ip := baseInferencePool.DeepCopy()
88+
// Use a unique name for each test case to avoid conflicts.
89+
ip.Name = fmt.Sprintf("test-pool-%v", time.Now().UnixNano())
90+
91+
if tc.mutate != nil {
92+
tc.mutate(ip)
93+
}
94+
err := k8sClient.Create(ctx, ip)
95+
96+
// This is a boolean XOR. It's true if one is true, but not both.
97+
// It ensures that an error is returned if and only if we expect one.
98+
if (len(tc.wantErrors) != 0) != (err != nil) {
99+
t.Fatalf("Unexpected response while creating InferencePool; got err=\n%v\n; want error=%v", err, tc.wantErrors != nil)
100+
}
101+
102+
// If we got an error, check that it contains the expected substrings.
103+
var missingErrorStrings []string
104+
for _, wantError := range tc.wantErrors {
105+
if !celErrorStringMatches(err.Error(), wantError) {
106+
missingErrorStrings = append(missingErrorStrings, wantError)
107+
}
108+
}
109+
if len(missingErrorStrings) != 0 {
110+
t.Errorf("Unexpected response while creating InferencePool; got err=\n%v\n; missing strings within error=%q", err, missingErrorStrings)
111+
}
112+
})
113+
}
114+
}

0 commit comments

Comments
 (0)