diff --git a/charts/modeldeployment/README.md b/charts/modeldeployment/README.md index fe04f07..306f353 100644 --- a/charts/modeldeployment/README.md +++ b/charts/modeldeployment/README.md @@ -124,8 +124,9 @@ letting KEDA fail later when it creates the HPA: ## Naming conventions -- InferencePool: `-inferencepool` -- EPP Deployment / Service / SA / Role / ConfigMap: `-inferencepool-epp` +- InferencePool, EPP Service and the EPP pods' `inferencepool` label: `` + (these must fit 63 characters, so `name` is limited to 63 characters) +- EPP Deployment / SA / Role / RoleBinding / ConfigMap: `-inferencepool-epp` - HTTPRoute: `-route` ## Example diff --git a/charts/modeldeployment/templates/_helpers.tpl b/charts/modeldeployment/templates/_helpers.tpl index 917045e..df6bbbe 100644 --- a/charts/modeldeployment/templates/_helpers.tpl +++ b/charts/modeldeployment/templates/_helpers.tpl @@ -24,17 +24,27 @@ Gateway that modelharness provisions in the workload namespace. {{- end }} {{/* -Derived InferencePool name, matching KAITO's naming convention. +InferencePool name. Equals the deployment name: Istio stamps it as the +istio.io/inferencepool-name label value on the InferencePool's shadow +Service, so it must fit the 63-character label value limit. */}} {{- define "modeldeployment.inferencePoolName" -}} -{{ include "modeldeployment.name" . }}-inferencepool +{{- include "modeldeployment.name" . -}} {{- end }} {{/* -Derived EPP service name (used as the DestinationRule host). +EPP Service name, also used as the `inferencepool` selector label value. +Equals the deployment name so it fits the 63-character limit of both. */}} {{- define "modeldeployment.eppServiceName" -}} -{{ include "modeldeployment.inferencePoolName" . }}-epp +{{- include "modeldeployment.name" . -}} +{{- end }} + +{{/* +Name of the EPP Deployment / ServiceAccount / Role / RoleBinding / ConfigMap. +*/}} +{{- define "modeldeployment.eppName" -}} +{{ include "modeldeployment.name" . }}-inferencepool-epp {{- end }} {{/* diff --git a/charts/modeldeployment/templates/epp-configmap.yaml b/charts/modeldeployment/templates/epp-configmap.yaml index a2c7ef1..5b9e549 100644 --- a/charts/modeldeployment/templates/epp-configmap.yaml +++ b/charts/modeldeployment/templates/epp-configmap.yaml @@ -1,7 +1,7 @@ apiVersion: v1 kind: ConfigMap metadata: - name: {{ include "modeldeployment.eppServiceName" . }} + name: {{ include "modeldeployment.eppName" . }} namespace: {{ include "modeldeployment.namespace" . }} labels: {{- include "modeldeployment.labels" . | nindent 4 }} diff --git a/charts/modeldeployment/templates/epp-deployment.yaml b/charts/modeldeployment/templates/epp-deployment.yaml index 7f4195c..a85ba0f 100644 --- a/charts/modeldeployment/templates/epp-deployment.yaml +++ b/charts/modeldeployment/templates/epp-deployment.yaml @@ -1,7 +1,7 @@ apiVersion: apps/v1 kind: Deployment metadata: - name: {{ include "modeldeployment.eppServiceName" . }} + name: {{ include "modeldeployment.eppName" . }} namespace: {{ include "modeldeployment.namespace" . }} labels: {{- include "modeldeployment.labels" . | nindent 4 }} @@ -36,7 +36,7 @@ spec: checksum/epp-flow-control: {{ include "modeldeployment.eppFlowControlSettings" . | sha256sum }} {{- end }} spec: - serviceAccountName: {{ include "modeldeployment.eppServiceName" . }} + serviceAccountName: {{ include "modeldeployment.eppName" . }} terminationGracePeriodSeconds: 130 {{- with .Values.epp.tolerations }} tolerations: @@ -94,4 +94,4 @@ spec: volumes: - name: plugins-config-volume configMap: - name: {{ include "modeldeployment.eppServiceName" . }} + name: {{ include "modeldeployment.eppName" . }} diff --git a/charts/modeldeployment/templates/epp-rbac.yaml b/charts/modeldeployment/templates/epp-rbac.yaml index fab74fa..812fa62 100644 --- a/charts/modeldeployment/templates/epp-rbac.yaml +++ b/charts/modeldeployment/templates/epp-rbac.yaml @@ -1,7 +1,7 @@ apiVersion: v1 kind: ServiceAccount metadata: - name: {{ include "modeldeployment.eppServiceName" . }} + name: {{ include "modeldeployment.eppName" . }} namespace: {{ include "modeldeployment.namespace" . }} labels: {{- include "modeldeployment.labels" . | nindent 4 }} @@ -9,7 +9,7 @@ metadata: apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: - name: {{ include "modeldeployment.eppServiceName" . }} + name: {{ include "modeldeployment.eppName" . }} namespace: {{ include "modeldeployment.namespace" . }} labels: {{- include "modeldeployment.labels" . | nindent 4 }} @@ -27,15 +27,15 @@ rules: apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: - name: {{ include "modeldeployment.eppServiceName" . }} + name: {{ include "modeldeployment.eppName" . }} namespace: {{ include "modeldeployment.namespace" . }} labels: {{- include "modeldeployment.labels" . | nindent 4 }} subjects: - kind: ServiceAccount - name: {{ include "modeldeployment.eppServiceName" . }} + name: {{ include "modeldeployment.eppName" . }} namespace: {{ include "modeldeployment.namespace" . }} roleRef: apiGroup: rbac.authorization.k8s.io kind: Role - name: {{ include "modeldeployment.eppServiceName" . }} + name: {{ include "modeldeployment.eppName" . }} diff --git a/charts/modeldeployment/values.schema.json b/charts/modeldeployment/values.schema.json index 47b4e86..e1ff896 100644 --- a/charts/modeldeployment/values.schema.json +++ b/charts/modeldeployment/values.schema.json @@ -64,7 +64,8 @@ "properties": { "name": { "type": "string", - "description": "Deployment / InferenceSet name. Empty falls back to .Release.Name." + "maxLength": 63, + "description": "Deployment / InferenceSet name, also used as the InferencePool and EPP Service name and the EPP `inferencepool` selector label value, hence at most 63 characters. Empty falls back to .Release.Name." }, "namespace": { "type": "string", diff --git a/test/e2e/README.md b/test/e2e/README.md index 4cfe146..3b8e87e 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -32,7 +32,7 @@ the production App Routing provisioning contract. - [`setup.go`](utils/setup.go) — `EnsureNamespace` (provisions the workload namespace + modelharness), `DeleteNamespace`, `SetupInferenceSetsWithRouting`, `TeardownInferenceSetsWithRouting`. - [`http.go`](utils/http.go) — authenticated `SendGatewayRequest` with `SendChat` / `SendModels` shortcuts against a `deploy.GatewayEndpoint`, taking `RequestOption`s (`WithPrompt`, `WithAuth`, `WithoutAuth`, `WithHeader`, `WithMethod`, `WithTransportRetry`, `WithOriginPath`, `WithTimeout`). - [`deployer.go`](utils/deployer.go) — `SetDeployer` / `CurrentDeployer` plus the `InstallModelDeployment`, `UpgradeModelDeployment`, `UninstallModelDeployment`, `InstallModelHarness`, `UninstallModelHarness`, `NamespaceAuthHeaders` helpers that delegate to the active backend (see [Deployment backends](#deployment-backends)). -- [`inference.go`](utils/inference.go) — `WaitForInferenceSetReady`, `EPPServiceName`, snapshot/diff helpers. +- [`inference.go`](utils/inference.go) — `WaitForInferenceSetReady`, `EPPDeploymentName`, `EPPSelector`, snapshot/diff helpers. - [`metrics.go`](utils/metrics.go), [`cluster.go`](utils/cluster.go), [`dynamic.go`](utils/dynamic.go), [`ginkgo.go`](utils/ginkgo.go). `cases.go`: diff --git a/test/e2e/cases.go b/test/e2e/cases.go index 8f886ae..a7eb239 100644 --- a/test/e2e/cases.go +++ b/test/e2e/cases.go @@ -225,9 +225,10 @@ const ( // unique across the entire table so Helm releases never collide. // // Conventions: -// - Name: deployment / Helm release name. AND the value carried in the -// `model` field of OpenAI-style requests (matched by the HTTPRoute as -// X-Gateway-Model-Name). +// - Name: deployment name (the Helm backend uses it as the release name, +// shortened when it exceeds Helm's 53-character limit). AND the value +// carried in the `model` field of OpenAI-style requests (matched by the +// HTTPRoute as X-Gateway-Model-Name). // - Model: inference preset name, written to the InferenceSet's // spec.template.inference.preset.name. // - Namespace: per-case namespace — the suite installs into this diff --git a/test/e2e/control_plane_error_spec.go b/test/e2e/control_plane_error_spec.go index 7a2e247..53027ab 100644 --- a/test/e2e/control_plane_error_spec.go +++ b/test/e2e/control_plane_error_spec.go @@ -60,7 +60,7 @@ var _ = Describe("Control-plane Error Reporter", }) It("emits inferencesetEPPNotReady when the InferenceSet EPP is scaled to zero", func() { - eppName := utils.EPPServiceName(modelName) + eppName := utils.EPPDeploymentName(modelName) desired, _, err := utils.GetDeploymentReplicas(ctx, caseNS, eppName) Expect(err).NotTo(HaveOccurred()) baseline, err := utils.ReporterEventBaseline(ctx, "inferencesetEPPNotReady", caseNS) diff --git a/test/e2e/deploy/helm/chart_test.go b/test/e2e/deploy/helm/chart_test.go index aa76981..b84248e 100644 --- a/test/e2e/deploy/helm/chart_test.go +++ b/test/e2e/deploy/helm/chart_test.go @@ -17,15 +17,22 @@ limitations under the License. package helm import ( + "encoding/json" + "maps" "os/exec" "path/filepath" + "regexp" + "slices" "strings" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/yaml" ) +var maxLengthErrorRe = regexp.MustCompile(`name: String length must be less than or equal to 63|'/name': maxLength: got \d+, want 63`) + func TestModelDeploymentScalingAnnotations(t *testing.T) { if _, err := exec.LookPath("helm"); err != nil { t.Skip("helm is required for chart rendering tests") @@ -188,6 +195,89 @@ func TestModelDeploymentScalingValidation(t *testing.T) { } } +// The deployment name feeds the EPP Service name, the `inferencepool` +// selector label value and the InferencePool name (which Istio copies into a +// label on the pool's shadow Service), all capped at 63 characters. A +// 63-character name must render objects that satisfy those limits, and a +// 64-character name must be rejected by the chart. +func TestModelDeploymentNameLength(t *testing.T) { + if _, err := exec.LookPath("helm"); err != nil { + t.Skip("helm is required for chart rendering tests") + } + chart := filepath.Join("..", "..", "..", "..", "charts", "modeldeployment") + render := func(name string) ([]byte, error) { + return exec.Command("helm", "template", "model", chart, + "--set", "model=test-model", "--set", "namespace=ns", "--set", "name="+name).CombinedOutput() + } + + // Helm's JSON schema validator words the error differently across releases: + // "name: String length must be less than or equal to 63" before v3.18, + // "at '/name': maxLength: got 64, want 63" from v3.18 on. + output, err := render(strings.Repeat("a", 64)) + if err == nil || !maxLengthErrorRe.Match(output) { + t.Errorf("expected a 64-character name to be rejected, got %v\n%s", err, output) + } + + output, err = render(strings.Repeat("a", 63)) + if err != nil { + t.Fatalf("render with a 63-character name: %v\n%s", err, output) + } + var kinds []string + for _, doc := range strings.Split(string(output), "\n---") { + var obj struct { + Kind string `json:"kind"` + Metadata metav1.ObjectMeta `json:"metadata"` + Spec struct { + Selector json.RawMessage `json:"selector"` + Template struct { + Metadata metav1.ObjectMeta `json:"metadata"` + } `json:"template"` + } `json:"spec"` + } + if err := yaml.Unmarshal([]byte(doc), &obj); err != nil { + t.Fatalf("parse rendered object: %v\n%s", err, doc) + } + if obj.Kind == "" { + continue + } + kinds = append(kinds, obj.Kind) + labels := map[string]string{} + maps.Copy(labels, obj.Metadata.Labels) + maps.Copy(labels, obj.Spec.Template.Metadata.Labels) + switch obj.Kind { + case "Service": + if errs := validation.IsDNS1035Label(obj.Metadata.Name); len(errs) > 0 { + t.Errorf("Service name %q: %v", obj.Metadata.Name, errs) + } + var selector map[string]string + if err := json.Unmarshal(obj.Spec.Selector, &selector); err != nil { + t.Fatalf("parse Service selector: %v", err) + } + maps.Copy(labels, selector) + case "Deployment": + var selector metav1.LabelSelector + if err := json.Unmarshal(obj.Spec.Selector, &selector); err != nil { + t.Fatalf("parse Deployment selector: %v", err) + } + maps.Copy(labels, selector.MatchLabels) + case "InferencePool": + if len(obj.Metadata.Name) > 63 { + t.Errorf("InferencePool name %q exceeds 63 characters", obj.Metadata.Name) + } + } + for key, value := range labels { + if len(value) > 63 { + t.Errorf("%s %s: label %s=%q exceeds 63 characters", obj.Kind, obj.Metadata.Name, key, value) + } + } + } + for _, kind := range []string{"Service", "Deployment", "InferencePool"} { + if !slices.Contains(kinds, kind) { + t.Errorf("rendered chart has no %s; rendered kinds: %v", kind, kinds) + } + } +} + func renderModelDeployment(t *testing.T, values ...string) ([]byte, error) { t.Helper() chart := filepath.Join("..", "..", "..", "..", "charts", "modeldeployment") diff --git a/test/e2e/deploy/helm/helm.go b/test/e2e/deploy/helm/helm.go index 726cf59..3f17f2a 100644 --- a/test/e2e/deploy/helm/helm.go +++ b/test/e2e/deploy/helm/helm.go @@ -20,7 +20,9 @@ package helm import ( "context" + "crypto/sha256" "encoding/base64" + "encoding/hex" "encoding/json" "fmt" "os" @@ -287,8 +289,9 @@ func isNotFound(out []byte) bool { // InstallModelDeployment runs `helm upgrade --install` for the modeldeployment // chart with the supplied values. It is idempotent — re-running it on an -// existing release reconciles to the new values. The release name equals -// values.Name and the release namespace equals values.Namespace. +// existing release reconciles to the new values. The release name is +// modelDeploymentReleaseName(values.Name) and the release namespace equals +// values.Namespace. func (d *Deployer) InstallModelDeployment(ctx context.Context, values deploy.ModelDeploymentValues) error { if err := values.Validate(); err != nil { return err @@ -304,7 +307,7 @@ func (d *Deployer) InstallModelDeployment(ctx context.Context, values deploy.Mod } args := []string{ - "upgrade", "--install", values.Name, chart, + "upgrade", "--install", modelDeploymentReleaseName(values.Name), chart, "--namespace", values.Namespace, "--create-namespace", } @@ -335,7 +338,7 @@ func (d *Deployer) UpgradeModelDeployment(ctx context.Context, values deploy.Mod return err } - args := []string{"upgrade", values.Name, chart, "--namespace", values.Namespace} + args := []string{"upgrade", modelDeploymentReleaseName(values.Name), chart, "--namespace", values.Namespace} args = append(args, setArgs(values)...) args = append(args, gatewaySetArgs(values.Gateway)...) @@ -357,13 +360,14 @@ func gatewaySetArgs(values deploy.GatewayValues) []string { return args } -// UninstallModelDeployment runs `helm uninstall` for the named release. -// Missing releases are treated as success (so cleanup is idempotent). +// UninstallModelDeployment runs `helm uninstall` for the named model +// deployment's release. Missing releases are treated as success (so cleanup +// is idempotent). func (d *Deployer) UninstallModelDeployment(ctx context.Context, name, namespace string) error { if name == "" { return fmt.Errorf("modeldeployment: name is required") } - args := []string{"uninstall", name, "--namespace", namespace, "--ignore-not-found", "--wait"} + args := []string{"uninstall", modelDeploymentReleaseName(name), "--namespace", namespace, "--ignore-not-found", "--wait"} if out, err := d.run(ctx, args...); err != nil { return fmt.Errorf("helm uninstall %s failed: %w\n%s", name, err, string(out)) @@ -371,6 +375,19 @@ func (d *Deployer) UninstallModelDeployment(ctx context.Context, name, namespace return nil } +// modelDeploymentReleaseName returns the Helm release name for a model +// deployment. It is the deployment name, except that names longer than Helm's +// 53-character release name limit (deployment names may reach 63 characters) +// are truncated and suffixed with a hash of the full name so they stay unique. +func modelDeploymentReleaseName(name string) string { + const maxReleaseNameLength, hashLength = 53, 8 + if len(name) <= maxReleaseNameLength { + return name + } + sum := sha256.Sum256([]byte(name)) + return name[:maxReleaseNameLength-hashLength-1] + "-" + hex.EncodeToString(sum[:])[:hashLength] +} + // setArgs builds the `--set` arguments for the modeldeployment chart. func setArgs(v deploy.ModelDeploymentValues) []string { args := []string{ diff --git a/test/e2e/deploy/helm/helm_test.go b/test/e2e/deploy/helm/helm_test.go index 9b25cac..a86e58e 100644 --- a/test/e2e/deploy/helm/helm_test.go +++ b/test/e2e/deploy/helm/helm_test.go @@ -742,6 +742,47 @@ func assertNamespaceLabelled(t *testing.T, calls [][]string, namespace string) { // Upgrade must not double as create: a mistyped release name has to fail // rather than silently provision a second deployment. +// Deployment names may reach 63 characters, but Helm release names are capped +// at 53. Install, upgrade and uninstall must all target the same shortened +// release, while the chart still receives the full deployment name. +func TestLongDeploymentNameUsesShortenedRelease(t *testing.T) { + d, calls := newTestDeployer(t, nil) + + name := strings.Repeat("a", 63) + values := deploy.ModelDeploymentValues{ + Name: name, Namespace: "e2e-ns", Model: "phi-4-mini-instruct", Replicas: 1, + } + if err := d.InstallModelDeployment(context.Background(), values); err != nil { + t.Fatalf("InstallModelDeployment: %v", err) + } + if err := d.UpgradeModelDeployment(context.Background(), values); err != nil { + t.Fatalf("UpgradeModelDeployment: %v", err) + } + if err := d.UninstallModelDeployment(context.Background(), name, "e2e-ns"); err != nil { + t.Fatalf("UninstallModelDeployment: %v", err) + } + + release := modelDeploymentReleaseName(name) + if len(release) != 53 || !strings.HasPrefix(release, strings.Repeat("a", 44)+"-") { + t.Fatalf("release name %q is not a 53-character hash-suffixed prefix", release) + } + if got := modelDeploymentReleaseName(strings.Repeat("a", 62) + "b"); got == release { + t.Errorf("names differing past the truncation point share release %q", got) + } + if got := modelDeploymentReleaseName("phi"); got != "phi" { + t.Errorf("short name release = %q, want %q", got, "phi") + } + + install, upgrade, uninstall := (*calls)[0], (*calls)[1], (*calls)[2] + if install[2] != release || upgrade[1] != release || uninstall[1] != release { + t.Errorf("release mismatch: install %q, upgrade %q, uninstall %q, want %q", + install[2], upgrade[1], uninstall[1], release) + } + if !hasArg(install, "name="+name) || !hasArg(upgrade, "name="+name) { + t.Errorf("chart must receive the full deployment name: install %v, upgrade %v", install, upgrade) + } +} + func TestUpgradeModelDeploymentDoesNotInstall(t *testing.T) { d, calls := newTestDeployer(t, nil) diff --git a/test/e2e/epp_outage_spec.go b/test/e2e/epp_outage_spec.go index cbf3f9b..ccc0c62 100644 --- a/test/e2e/epp_outage_spec.go +++ b/test/e2e/epp_outage_spec.go @@ -74,8 +74,8 @@ var _ = Describe("EPP outage (fail-closed InferencePool ext_proc)", caseGateway = InstallCase(CaseEPPOutage) modelName = CaseDeployments[CaseEPPOutage][0].Name // The EPP Deployment is named "-inferencepool-epp" by the - // modeldeployment chart (modeldeployment.eppServiceName). - replicas = prepareDeploymentOutage(ctx, CaseNamespace(CaseEPPOutage), utils.EPPServiceName(modelName)) + // modeldeployment chart (modeldeployment.eppName). + replicas = prepareDeploymentOutage(ctx, CaseNamespace(CaseEPPOutage), utils.EPPDeploymentName(modelName)) // Sanity: a valid request must succeed BEFORE we induce the // outage, otherwise a 502 below would be meaningless. diff --git a/test/e2e/gpu_mocker_spec.go b/test/e2e/gpu_mocker_spec.go index c43c2c5..5a41435 100644 --- a/test/e2e/gpu_mocker_spec.go +++ b/test/e2e/gpu_mocker_spec.go @@ -93,10 +93,10 @@ var _ = Describe("GPU Mocker E2E", Ordered, func() { Expect(err).NotTo(HaveOccurred()) for _, d := range suiteDeployments { - eppName := utils.EPPServiceName(d.Name) - By(fmt.Sprintf("checking EPP pods for %q", eppName)) + selector := utils.EPPSelector(d.Name) + By(fmt.Sprintf("checking EPP pods for %q", selector)) pods, err := clientset.CoreV1().Pods(caseNamespace).List(context.Background(), metav1.ListOptions{ - LabelSelector: fmt.Sprintf("inferencepool=%s", eppName), + LabelSelector: selector, }) Expect(err).NotTo(HaveOccurred()) var runningEPP int @@ -106,7 +106,7 @@ var _ = Describe("GPU Mocker E2E", Ordered, func() { } } Expect(runningEPP).To(BeNumerically(">=", 1), - "at least one EPP pod should be Running for %q", eppName) + "at least one EPP pod should be Running for %q", selector) } }) diff --git a/test/e2e/network_policy_spec.go b/test/e2e/network_policy_spec.go index bc48ccc..7e2d411 100644 --- a/test/e2e/network_policy_spec.go +++ b/test/e2e/network_policy_spec.go @@ -828,8 +828,8 @@ func readyModelPodEndpoint(ctx context.Context, clientset *kubernetes.Clientset, // InferenceSetReadyTimeout. func readyEPPPodEndpoint(ctx context.Context, clientset *kubernetes.Clientset, ns, deploymentName string) (string, int32) { // Matches charts/modeldeployment/templates/_helpers.tpl: - // inferencepool: -inferencepool-epp - selector := fmt.Sprintf("inferencepool=%s-inferencepool-epp", deploymentName) + // inferencepool: + selector := utils.EPPSelector(deploymentName) Eventually(func() (string, error) { pods, err := clientset.CoreV1().Pods(ns).List(ctx, metav1.ListOptions{LabelSelector: selector}) diff --git a/test/e2e/production-stack-E2E-test-scenarios.md b/test/e2e/production-stack-E2E-test-scenarios.md index 29a41fd..533c3a1 100644 --- a/test/e2e/production-stack-E2E-test-scenarios.md +++ b/test/e2e/production-stack-E2E-test-scenarios.md @@ -8,6 +8,7 @@ | **Infrastructure** | Fake Node / Shadow Pod Lifecycle | PR | gpu-node-mocker two-phase pipeline validation | | | InferenceSet / InferencePool Lifecycle | PR | KAITO controller and routing infrastructure health | | | NodeClaim / Fake Node Cleanup | PR | Reverse-path teardown and reconcile idempotency | +| | Long Deployment Name | PR | 63-character deployment name serves inference; 64 characters is rejected | | | InferenceSet Scaling (E2E) | Daily (mocker) + Nightly (H100) | End-to-end scale up/down driven by KEDA + traffic verification | | | InferenceSet Scale To Zero | Daily (mocker) + Nightly (H100) | EPP-driven `1 -> 0 -> 1 -> 0` activation and post-cold-start serving | | **Routing** | Model-Based Routing | PR | BBR → HTTPRoute → EPP → Pod routing correctness | diff --git a/test/e2e/scaling_spec.go b/test/e2e/scaling_spec.go index e9d39a3..8dcd185 100644 --- a/test/e2e/scaling_spec.go +++ b/test/e2e/scaling_spec.go @@ -1085,7 +1085,7 @@ func assertEPPReady(ctx context.Context, namespace, modelName string) error { if err != nil { return err } - eppName := utils.EPPServiceName(modelName) + eppName := utils.EPPDeploymentName(modelName) deployment, err := clientset.AppsV1().Deployments(namespace).Get(ctx, eppName, metav1.GetOptions{}) if err != nil { return err @@ -1093,7 +1093,7 @@ func assertEPPReady(ctx context.Context, namespace, modelName string) error { if deployment.Status.ReadyReplicas < 1 { return fmt.Errorf("EPP Deployment %s/%s has no ready replicas", namespace, eppName) } - selector := "inferencepool=" + eppName + selector := utils.EPPSelector(modelName) pods, err := clientset.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: selector}) if err != nil { return err diff --git a/test/e2e/utils/inference.go b/test/e2e/utils/inference.go index ea1951d..c450a86 100644 --- a/test/e2e/utils/inference.go +++ b/test/e2e/utils/inference.go @@ -42,13 +42,19 @@ const ( // deployment (InferenceSet) name. Matches the modeldeployment chart's // naming convention. func InferencePoolName(deploymentName string) string { - return deploymentName + "-inferencepool" + return deploymentName } -// EPPServiceName returns the EPP service name derived from the deployment -// name. Matches the modeldeployment chart's naming convention. -func EPPServiceName(deploymentName string) string { - return InferencePoolName(deploymentName) + "-epp" +// EPPDeploymentName returns the EPP Deployment name derived from the +// deployment name. Matches the modeldeployment chart's naming convention. +func EPPDeploymentName(deploymentName string) string { + return deploymentName + "-inferencepool-epp" +} + +// EPPSelector returns the label selector matching the EPP pods of the given +// deployment. Matches the modeldeployment chart's `inferencepool` label. +func EPPSelector(deploymentName string) string { + return "inferencepool=" + deploymentName } // WaitForInferenceSetReady waits until the modeldeployment can actually serve: @@ -72,7 +78,7 @@ func WaitForInferenceSetReady(ctx context.Context, values deploy.ModelDeployment } func inferenceSetReady(ctx context.Context, clientset kubernetes.Interface, values deploy.ModelDeploymentValues) error { - namespace, eppName := values.Namespace, EPPServiceName(values.Name) + namespace, eppName := values.Namespace, EPPDeploymentName(values.Name) epp, err := clientset.AppsV1().Deployments(namespace).Get(ctx, eppName, metav1.GetOptions{}) if err != nil { @@ -119,7 +125,7 @@ func RestartEPP(ctx context.Context, name, namespace string) error { if err != nil { return fmt.Errorf("init clientset: %w", err) } - eppName := EPPServiceName(name) + eppName := EPPDeploymentName(name) patch := fmt.Sprintf( `{"spec":{"template":{"metadata":{"annotations":{"kubectl.kubernetes.io/restartedAt":%q}}}}}`, time.Now().Format(time.RFC3339Nano)) @@ -139,7 +145,7 @@ func WaitForEPPRollout(ctx context.Context, name, namespace string, timeout time if err != nil { return fmt.Errorf("init clientset: %w", err) } - eppName := EPPServiceName(name) + eppName := EPPDeploymentName(name) return pollUntilReady(ctx, timeout, fmt.Sprintf("EPP Deployment %s/%s to roll out", namespace, eppName), func(ctx context.Context) error { return eppRolledOut(ctx, clientset, namespace, eppName) diff --git a/test/e2e/utils/metrics.go b/test/e2e/utils/metrics.go index 6ce508b..3806cb6 100644 --- a/test/e2e/utils/metrics.go +++ b/test/e2e/utils/metrics.go @@ -292,9 +292,8 @@ func inferenceSimPort(pod corev1.Pod) int { // GetEPPPods returns the EPP pods for the given deployment. func GetEPPPods(ctx context.Context, clientset *kubernetes.Clientset, deploymentName, namespace string) ([]corev1.Pod, error) { - eppName := EPPServiceName(deploymentName) pods, err := clientset.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{ - LabelSelector: fmt.Sprintf("inferencepool=%s", eppName), + LabelSelector: EPPSelector(deploymentName), FieldSelector: "status.phase=Running", }) if err != nil { diff --git a/test/e2e/utils/utils.go b/test/e2e/utils/utils.go index 02ced42..28bdb41 100644 --- a/test/e2e/utils/utils.go +++ b/test/e2e/utils/utils.go @@ -168,7 +168,13 @@ func pollUntilReady(ctx context.Context, timeout time.Duration, description stri if err := ctx.Err(); err != nil { return false, err } - lastErr = check(ctx) + checkErr := check(ctx) + // A check cut short by the deadline only reports the cancellation; + // keep the previous diagnostic so the timeout says what was not ready. + if checkErr != nil && ctx.Err() != nil && lastErr != nil { + return false, nil + } + lastErr = checkErr return lastErr == nil, nil }) if err != nil { diff --git a/test/e2e/utils/utils_test.go b/test/e2e/utils/utils_test.go new file mode 100644 index 0000000..a761b89 --- /dev/null +++ b/test/e2e/utils/utils_test.go @@ -0,0 +1,63 @@ +/* +Copyright 2026 The KAITO 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 utils + +import ( + "context" + "errors" + "strings" + "testing" + "time" +) + +// A check interrupted by the deadline must not hide what the previous checks +// reported as not ready. +func TestPollUntilReadyKeepsLastDiagnosticOnTimeout(t *testing.T) { + interval := PollInterval + PollInterval = 20 * time.Millisecond + t.Cleanup(func() { PollInterval = interval }) + + calls := 0 + err := pollUntilReady(context.Background(), 100*time.Millisecond, "thing", func(ctx context.Context) error { + calls++ + if calls == 1 { + return errors.New("inference pod is Pending") + } + <-ctx.Done() + return ctx.Err() + }) + if err == nil { + t.Fatal("expected a timeout error") + } + if !strings.Contains(err.Error(), "last check: inference pod is Pending") { + t.Fatalf("timeout error lost the last diagnostic: %v", err) + } +} + +func TestPollUntilReadyReportsDeadlineWithoutEarlierDiagnostic(t *testing.T) { + interval := PollInterval + PollInterval = 20 * time.Millisecond + t.Cleanup(func() { PollInterval = interval }) + + err := pollUntilReady(context.Background(), 50*time.Millisecond, "thing", func(ctx context.Context) error { + <-ctx.Done() + return ctx.Err() + }) + if err == nil || !strings.Contains(err.Error(), "last check: context deadline exceeded") { + t.Fatalf("expected the deadline as the last check, got %v", err) + } +}