From 2b3083ac8836af60c617090f07b8c1e8cc67a410 Mon Sep 17 00:00:00 2001 From: Arthur Cheng Date: Fri, 8 May 2026 12:48:53 -0700 Subject: [PATCH 1/4] deprecate weight since matcher requires strict equality on model format and model architecture --- pkg/runtimeselector/matcher.go | 2 ++ pkg/runtimeselector/scorer.go | 64 +++++++++++++++------------------- 2 files changed, 30 insertions(+), 36 deletions(-) diff --git a/pkg/runtimeselector/matcher.go b/pkg/runtimeselector/matcher.go index 24964709a..4b5f18119 100644 --- a/pkg/runtimeselector/matcher.go +++ b/pkg/runtimeselector/matcher.go @@ -226,6 +226,7 @@ func (m *DefaultRuntimeMatcher) evaluateFormatMatch(model *v1beta1.BaseModelSpec match.FormatMatch = false } + // TODO: stop populating match.Weight; the field is deprecated and no longer read by the scorer. if match.FormatMatch && format.ModelFormat.Weight > 0 { match.Weight += format.ModelFormat.Weight * int64(match.Priority) } @@ -245,6 +246,7 @@ func (m *DefaultRuntimeMatcher) evaluateFormatMatch(model *v1beta1.BaseModelSpec match.FrameworkMatch = false } + // TODO: stop populating match.Weight; the field is deprecated and no longer read by the scorer. if match.FrameworkMatch && format.ModelFramework.Weight > 0 { match.Weight += format.ModelFramework.Weight * int64(match.Priority) } diff --git a/pkg/runtimeselector/scorer.go b/pkg/runtimeselector/scorer.go index 6e1113d45..3ae3a564f 100644 --- a/pkg/runtimeselector/scorer.go +++ b/pkg/runtimeselector/scorer.go @@ -21,12 +21,9 @@ func NewDefaultRuntimeScorer(config *Config) RuntimeScorer { } } -// CalculateScore returns a score for how well a runtime matches a model. -// The score is calculated based on: -// 1. Model format match and weight -// 2. Model framework match and weight -// 3. Priority multiplier -// 4. Model size proximity (if applicable) +// CalculateScore returns the priority of the best matching auto-select format. +// Strict format/framework compatibility is enforced separately by the matcher, +// so a non-zero score here simply means "this runtime matches at this priority". func (s *DefaultRuntimeScorer) CalculateScore(runtime *v1beta1.ServingRuntimeSpec, model *v1beta1.BaseModelSpec) (int64, error) { ctx := context.Background() logger := log.FromContext(ctx) @@ -35,8 +32,7 @@ func (s *DefaultRuntimeScorer) CalculateScore(runtime *v1beta1.ServingRuntimeSpe // Go through all supported model formats in runtime for _, supportedFormat := range runtime.SupportedModelFormats { - // Skip if autoSelect is explicitly false - if supportedFormat.AutoSelect != nil && !(*supportedFormat.AutoSelect) { + if !supportedFormat.IsAutoSelectEnabled() { continue } @@ -64,6 +60,12 @@ func (s *DefaultRuntimeScorer) CalculateScore(runtime *v1beta1.ServingRuntimeSpe // CompareRuntimes compares two runtime matches for a given model. // Returns positive if r1 is better, negative if r2 is better, 0 if equal. +// +// Tie-breaking order (strict): +// 1. Priority (higher wins). +// 2. Model size proximity (closer to the model size wins). +// 3. Scope (namespace-scoped beats cluster-scoped). +// 4. Name (alphabetical, deterministic). func (s *DefaultRuntimeScorer) CompareRuntimes(r1, r2 RuntimeMatch, model *v1beta1.BaseModelSpec) int { // First, compare by score if r1.Score != r2.Score { @@ -99,23 +101,27 @@ func (s *DefaultRuntimeScorer) CompareRuntimes(r1, r2 RuntimeMatch, model *v1bet return 0 } -// calculateFormatScore calculates the score for a specific supported format. -// This matches the exact logic from the original score() function. +// CalculateFormatScore returns the format's Priority when the model is +// compatible with the supported format, and 0 otherwise. Weight is intentionally +// ignored: matcher.go already enforces strict compatibility, so Weight only +// produced scoring collisions. func (s *DefaultRuntimeScorer) CalculateFormatScore(model *v1beta1.BaseModelSpec, supportedFormat v1beta1.SupportedModelFormat, priority int64) int64 { - // Compare model format modelFormatMatches := false if supportedFormat.ModelFormat != nil { if supportedFormat.ModelFormat.Name != model.ModelFormat.Name { return 0 // Format name doesn't match } - // Compare versions if both are specified - if supportedFormat.ModelFormat.Version != nil && model.ModelFormat.Version != nil { + switch { + case supportedFormat.ModelFormat.Version != nil && model.ModelFormat.Version != nil: modelFormatMatches = s.compareVersions(supportedFormat.ModelFormat, &model.ModelFormat) if !modelFormatMatches { return 0 // Version doesn't match } - } else { + case supportedFormat.ModelFormat.Version == nil && model.ModelFormat.Version == nil: modelFormatMatches = true + default: + // Version asymmetry: matcher rejects, so we must too. + return 0 } } @@ -125,39 +131,25 @@ func (s *DefaultRuntimeScorer) CalculateFormatScore(model *v1beta1.BaseModelSpec if supportedFormat.ModelFramework.Name != model.ModelFramework.Name { return 0 // Framework name doesn't match } - // Compare versions if both are specified - if supportedFormat.ModelFramework.Version != nil && model.ModelFramework.Version != nil { + switch { + case supportedFormat.ModelFramework.Version != nil && model.ModelFramework.Version != nil: modelFrameworkMatches = s.compareFrameworkVersions(supportedFormat.ModelFramework, model.ModelFramework) if !modelFrameworkMatches { return 0 // Version doesn't match } - } else { + case supportedFormat.ModelFramework.Version == nil && model.ModelFramework.Version == nil: modelFrameworkMatches = true + default: + return 0 } } - // Check the matching condition (same as original line 223-224) if (modelFormatMatches || supportedFormat.ModelFormat == nil) && (modelFrameworkMatches || (supportedFormat.ModelFramework == nil && model.ModelFramework == nil)) { - - // Calculate weighted score - var currentScore int64 = 0 - if modelFormatMatches && supportedFormat.ModelFormat != nil { - weight := supportedFormat.ModelFormat.Weight - if weight == 0 { - weight = s.config.ModelFormatWeight - } - currentScore += weight * priority + // Require at least one positive match + if modelFormatMatches || modelFrameworkMatches { + return priority } - if modelFrameworkMatches && supportedFormat.ModelFramework != nil { - weight := supportedFormat.ModelFramework.Weight - if weight == 0 { - weight = s.config.ModelFrameworkWeight - } - currentScore += weight * priority - } - - return currentScore } return 0 From f98558bcbc43e744db1cf2e3d60d7de92c37dc23 Mon Sep 17 00:00:00 2001 From: Arthur Cheng Date: Fri, 8 May 2026 13:00:39 -0700 Subject: [PATCH 2/4] enforce scope before size approximity --- pkg/runtimeselector/scorer.go | 22 +++--- pkg/runtimeselector/selector.go | 22 ++---- pkg/runtimeselector/selector_test.go | 112 +++++++++++++++++++++++---- 3 files changed, 115 insertions(+), 41 deletions(-) diff --git a/pkg/runtimeselector/scorer.go b/pkg/runtimeselector/scorer.go index 3ae3a564f..8da76b5d6 100644 --- a/pkg/runtimeselector/scorer.go +++ b/pkg/runtimeselector/scorer.go @@ -63,8 +63,8 @@ func (s *DefaultRuntimeScorer) CalculateScore(runtime *v1beta1.ServingRuntimeSpe // // Tie-breaking order (strict): // 1. Priority (higher wins). -// 2. Model size proximity (closer to the model size wins). -// 3. Scope (namespace-scoped beats cluster-scoped). +// 2. Scope (namespace-scoped beats cluster-scoped). +// 3. Model size proximity (closer to the model size wins). // 4. Name (alphabetical, deterministic). func (s *DefaultRuntimeScorer) CompareRuntimes(r1, r2 RuntimeMatch, model *v1beta1.BaseModelSpec) int { // First, compare by score @@ -72,7 +72,15 @@ func (s *DefaultRuntimeScorer) CompareRuntimes(r1, r2 RuntimeMatch, model *v1bet return int(r1.Score - r2.Score) } - // If scores are equal, compare by model size range if available + // If scores are equal, prefer namespace-scoped runtimes over cluster-scoped + if r1.IsCluster != r2.IsCluster { + if r1.IsCluster { + return -1 // r2 is namespace-scoped, prefer it + } + return 1 // r1 is namespace-scoped, prefer it + } + + // If still equal, compare by model size range if available if model.ModelParameterSize != nil { r1SizeScore := s.calculateSizeScore(r1, model) r2SizeScore := s.calculateSizeScore(r2, model) @@ -83,14 +91,6 @@ func (s *DefaultRuntimeScorer) CompareRuntimes(r1, r2 RuntimeMatch, model *v1bet } } - // If still equal, prefer namespace-scoped runtimes over cluster-scoped - if r1.IsCluster != r2.IsCluster { - if r1.IsCluster { - return -1 // r2 is namespace-scoped, prefer it - } - return 1 // r1 is namespace-scoped, prefer it - } - // Finally, compare by name for deterministic ordering if r1.Name < r2.Name { return 1 diff --git a/pkg/runtimeselector/selector.go b/pkg/runtimeselector/selector.go index 0fa7f4f71..620b0f628 100644 --- a/pkg/runtimeselector/selector.go +++ b/pkg/runtimeselector/selector.go @@ -115,29 +115,21 @@ func (s *defaultSelector) GetCompatibleRuntimes(ctx context.Context, model *v1be return nil, fmt.Errorf("failed to fetch runtimes: %w", err) } - var namespaceMatches []RuntimeMatch - var clusterMatches []RuntimeMatch - - // Process namespace-scoped runtimes + var matches []RuntimeMatch for _, runtime := range collection.NamespaceRuntimes { if match := s.evaluateRuntime(ctx, &runtime.Spec, model, isvc, runtime.Name, false); match != nil { - namespaceMatches = append(namespaceMatches, *match) + matches = append(matches, *match) } } - - // Process cluster-scoped runtimes for _, runtime := range collection.ClusterRuntimes { if match := s.evaluateRuntime(ctx, &runtime.Spec, model, isvc, runtime.Name, true); match != nil { - clusterMatches = append(clusterMatches, *match) + matches = append(matches, *match) } } - // Sort namespace and cluster matches separately - s.sortMatches(namespaceMatches, model) - s.sortMatches(clusterMatches, model) - - // Append cluster matches after namespace matches (namespace-scoped have priority) - matches := append(namespaceMatches, clusterMatches...) + // Sort globally so that scope is only a tie-breaker + // (CompareRuntimes orders by priority -> scope -> size -> name) + s.sortMatches(matches, model) logger.Info("Found compatible runtimes", "model", model.ModelFormat.Name, @@ -321,6 +313,8 @@ func getModelName(model *v1beta1.BaseModelSpec) string { // userSpecifiedRuntime indicates whether the runtime is a user-selected runtime or an automatically selected runtime // if userSpecifiedRuntime is true, the function will consider all supportedModelFormats in the runtime // if userSpecifiedRuntime is false, the function will only consider supportedModelFormats with autoSelect enabled +// +// TODO: pass the format's own Priority (defaulting to DefaultPriority when nil) instead of DefaultPriority for every call, so the highest-priority matching format wins under priority-only scoring. func (s *defaultSelector) GetSupportedModelFormat(ctx context.Context, runtime *v1beta1.ServingRuntimeSpec, model *v1beta1.BaseModelSpec, userSpecifiedRuntime bool) *v1beta1.SupportedModelFormat { if runtime.SupportedModelFormats == nil { return nil diff --git a/pkg/runtimeselector/selector_test.go b/pkg/runtimeselector/selector_test.go index 503b338f3..e9ea42179 100644 --- a/pkg/runtimeselector/selector_test.go +++ b/pkg/runtimeselector/selector_test.go @@ -430,23 +430,23 @@ func TestGetSupportingRuntimes(t *testing.T) { name: "small pytorch model - multiple compatible runtimes", model: baseModels[0].model, expectedRuntimeNames: []string{ - "pytorch-rt", // score: 10 * 2 = 20 (namespace-scoped, higher priority) - "multi-format-rt", // score: 5 * 1 = 5 (namespace-scoped) - "cluster-pytorch-rt", // score: 7 * 1 = 7 (cluster-scoped) + "pytorch-rt", + "multi-format-rt", + "cluster-pytorch-rt", }, - expectedScores: []int64{20, 5, 7}, + expectedScores: []int64{2, 1, 1}, }, { name: "medium onnx model", model: baseModels[1].model, expectedRuntimeNames: []string{"onnx-rt", "multi-format-rt"}, - expectedScores: []int64{8, 5}, + expectedScores: []int64{1, 1}, }, { name: "large tensorflow model", model: baseModels[2].model, expectedRuntimeNames: []string{"tensorflow-rt"}, - expectedScores: []int64{12}, + expectedScores: []int64{1}, }, { name: "model with no compatible runtime", @@ -660,7 +660,7 @@ func TestScore(t *testing.T) { Version: ptr("4.0.0"), }, }, - expectedScore: 45, // (10 * 3) + (5 * 3) = 30 + 15 = 45 + expectedScore: 3, // priority 3; weight is no longer applied }, { name: "format match only", @@ -749,7 +749,7 @@ func TestScore(t *testing.T) { Name: "transformers", }, }, - expectedScore: 36, // Best match: (10 * 2) + (8 * 2) = 20 + 16 = 36 + expectedScore: 2, // best matching format has priority 2 }, { name: "default priority when not specified", @@ -770,7 +770,7 @@ func TestScore(t *testing.T) { Name: "PyTorch", }, }, - expectedScore: 12, // 12 * 1 (default priority) = 12 + expectedScore: 1, // default priority }, { name: "nil model framework in base model", @@ -1345,21 +1345,23 @@ func TestSupportedRuntimeWithAC(t *testing.T) { inferenceService: infereceServices[0], expectedRuntimeNames: []string{ "pytorch-rt", - "pytorch-rt-4", "pytorch-rt-1", + "pytorch-rt-4", }, expectError: false, }, { + // All four runtimes match (no AC requirement on the ISVC). + // p2 first, then p1s ordered by size proximity to 7B, then alphabetically. name: "select without inference service ac", model: baseModels[0].model, inferenceService: infereceServices[1], expectedRuntimeNames: []string{ "pytorch-rt", + "pytorch-rt-1", "pytorch-rt-3", "pytorch-rt-4", - "pytorch-rt-1", }, expectError: false, }, @@ -1381,8 +1383,8 @@ func TestSupportedRuntimeWithAC(t *testing.T) { inferenceService: infereceServices[3], expectedRuntimeNames: []string{ "pytorch-rt", - "pytorch-rt-4", "pytorch-rt-1", + "pytorch-rt-4", }, expectError: false, }, @@ -1393,9 +1395,9 @@ func TestSupportedRuntimeWithAC(t *testing.T) { inferenceService: infereceServices[4], expectedRuntimeNames: []string{ "pytorch-rt", + "pytorch-rt-1", "pytorch-rt-3", "pytorch-rt-4", - "pytorch-rt-1", }, expectError: false, }, @@ -1406,8 +1408,8 @@ func TestSupportedRuntimeWithAC(t *testing.T) { inferenceService: infereceServices[5], expectedRuntimeNames: []string{ "pytorch-rt", - "pytorch-rt-4", "pytorch-rt-1", + "pytorch-rt-4", }, expectError: false, }, @@ -1417,9 +1419,9 @@ func TestSupportedRuntimeWithAC(t *testing.T) { inferenceService: infereceServices[6], expectedRuntimeNames: []string{ "pytorch-rt", + "pytorch-rt-1", "pytorch-rt-3", "pytorch-rt-4", - "pytorch-rt-1", }, expectError: false, }, @@ -1440,8 +1442,8 @@ func TestSupportedRuntimeWithAC(t *testing.T) { inferenceService: infereceServices[8], expectedRuntimeNames: []string{ "pytorch-rt", - "pytorch-rt-4", "pytorch-rt-1", + "pytorch-rt-4", }, expectError: false, }, @@ -1468,3 +1470,81 @@ func TestSupportedRuntimeWithAC(t *testing.T) { }) } } + +// TestHighPriorityClusterRuntimeBeatsLowerPriorityNamespace verifies that +// a cluster-scoped runtime with a higher priority beats a namespace-scoped runtime with a lower priority +func TestHighPriorityClusterRuntimeBeatsLowerPriorityNamespace(t *testing.T) { + fakeClient := createFakeClient() + ctx := context.Background() + + srLow := &v1beta1.ServingRuntime{ + ObjectMeta: metav1.ObjectMeta{Name: "sr-low", Namespace: "default"}, + Spec: v1beta1.ServingRuntimeSpec{ + SupportedModelFormats: []v1beta1.SupportedModelFormat{{ + ModelFormat: &v1beta1.ModelFormat{Name: "pytorch"}, + AutoSelect: ptr(true), + Priority: ptr(int32(1)), + }}, + }, + } + csrHigh := &v1beta1.ClusterServingRuntime{ + ObjectMeta: metav1.ObjectMeta{Name: "csr-high"}, + Spec: v1beta1.ServingRuntimeSpec{ + SupportedModelFormats: []v1beta1.SupportedModelFormat{{ + ModelFormat: &v1beta1.ModelFormat{Name: "pytorch"}, + AutoSelect: ptr(true), + Priority: ptr(int32(10)), + }}, + }, + } + assert.NoError(t, fakeClient.Create(ctx, srLow)) + assert.NoError(t, fakeClient.Create(ctx, csrHigh)) + + selector := New(fakeClient) + model := &v1beta1.BaseModelSpec{ModelFormat: v1beta1.ModelFormat{Name: "pytorch"}} + isvc := &v1beta1.InferenceService{ObjectMeta: metav1.ObjectMeta{Namespace: "default"}} + + selected, err := selector.SelectRuntime(ctx, model, isvc) + assert.NoError(t, err) + assert.Equal(t, "csr-high", selected.Name, "higher priority must win over lower-priority namespace runtime") + assert.True(t, selected.IsCluster) +} + +// TestScorerRejectsVersionAsymmetry mirrors the matcher's strict rejection when +// only one of (runtime format, model) specifies a version. +func TestScorerRejectsVersionAsymmetry(t *testing.T) { + scorer := NewDefaultRuntimeScorer(NewConfig(nil)) + + runtime := &v1beta1.ServingRuntimeSpec{ + SupportedModelFormats: []v1beta1.SupportedModelFormat{{ + ModelFormat: &v1beta1.ModelFormat{Name: "pytorch", Version: ptr("2.0.0")}, + AutoSelect: ptr(true), + Priority: ptr(int32(5)), + }}, + } + // Model has no version — matcher would reject; scorer must too. + model := &v1beta1.BaseModelSpec{ModelFormat: v1beta1.ModelFormat{Name: "pytorch"}} + + score, err := scorer.CalculateScore(runtime, model) + assert.NoError(t, err) + assert.Equal(t, int64(0), score, "version asymmetry must yield score 0") +} + +// TestScorerSkipsNilAutoSelect verifies that a SupportedModelFormat with nil +// AutoSelect is treated as not-auto-selectable (matching IsAutoSelectEnabled). +func TestScorerSkipsNilAutoSelect(t *testing.T) { + scorer := NewDefaultRuntimeScorer(NewConfig(nil)) + + runtime := &v1beta1.ServingRuntimeSpec{ + SupportedModelFormats: []v1beta1.SupportedModelFormat{{ + ModelFormat: &v1beta1.ModelFormat{Name: "pytorch"}, + // AutoSelect intentionally nil + Priority: ptr(int32(7)), + }}, + } + model := &v1beta1.BaseModelSpec{ModelFormat: v1beta1.ModelFormat{Name: "pytorch"}} + + score, err := scorer.CalculateScore(runtime, model) + assert.NoError(t, err) + assert.Equal(t, int64(0), score, "nil AutoSelect must be skipped, yielding score 0") +} From ab21765f653a8dec24d41bff34e11c6a8b9548a1 Mon Sep 17 00:00:00 2001 From: Arthur Cheng Date: Fri, 8 May 2026 13:05:59 -0700 Subject: [PATCH 3/4] deprecate weight and enforce namespace scoped overwrite --- pkg/runtimeselector/scorer.go | 16 ++++++++-------- pkg/runtimeselector/selector.go | 4 ++-- pkg/runtimeselector/selector_test.go | 10 +++++----- pkg/runtimeselector/types.go | 17 ++++++++++++++--- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/pkg/runtimeselector/scorer.go b/pkg/runtimeselector/scorer.go index 8da76b5d6..7929d513d 100644 --- a/pkg/runtimeselector/scorer.go +++ b/pkg/runtimeselector/scorer.go @@ -62,17 +62,12 @@ func (s *DefaultRuntimeScorer) CalculateScore(runtime *v1beta1.ServingRuntimeSpe // Returns positive if r1 is better, negative if r2 is better, 0 if equal. // // Tie-breaking order (strict): -// 1. Priority (higher wins). -// 2. Scope (namespace-scoped beats cluster-scoped). +// 1. Scope (namespace-scoped beats cluster-scoped) +// 2. Priority (higher wins) // 3. Model size proximity (closer to the model size wins). // 4. Name (alphabetical, deterministic). func (s *DefaultRuntimeScorer) CompareRuntimes(r1, r2 RuntimeMatch, model *v1beta1.BaseModelSpec) int { - // First, compare by score - if r1.Score != r2.Score { - return int(r1.Score - r2.Score) - } - - // If scores are equal, prefer namespace-scoped runtimes over cluster-scoped + // Prefer namespace-scoped runtimes over cluster-scoped if r1.IsCluster != r2.IsCluster { if r1.IsCluster { return -1 // r2 is namespace-scoped, prefer it @@ -80,6 +75,11 @@ func (s *DefaultRuntimeScorer) CompareRuntimes(r1, r2 RuntimeMatch, model *v1bet return 1 // r1 is namespace-scoped, prefer it } + // Within the same scope, compare by priority (Score is the priority). + if r1.Score != r2.Score { + return int(r1.Score - r2.Score) + } + // If still equal, compare by model size range if available if model.ModelParameterSize != nil { r1SizeScore := s.calculateSizeScore(r1, model) diff --git a/pkg/runtimeselector/selector.go b/pkg/runtimeselector/selector.go index 620b0f628..0ef962312 100644 --- a/pkg/runtimeselector/selector.go +++ b/pkg/runtimeselector/selector.go @@ -127,8 +127,8 @@ func (s *defaultSelector) GetCompatibleRuntimes(ctx context.Context, model *v1be } } - // Sort globally so that scope is only a tie-breaker - // (CompareRuntimes orders by priority -> scope -> size -> name) + // Sort globally + // CompareRuntimes orders by scope -> priority -> size -> name. s.sortMatches(matches, model) logger.Info("Found compatible runtimes", diff --git a/pkg/runtimeselector/selector_test.go b/pkg/runtimeselector/selector_test.go index e9ea42179..d39bff350 100644 --- a/pkg/runtimeselector/selector_test.go +++ b/pkg/runtimeselector/selector_test.go @@ -1471,9 +1471,9 @@ func TestSupportedRuntimeWithAC(t *testing.T) { } } -// TestHighPriorityClusterRuntimeBeatsLowerPriorityNamespace verifies that -// a cluster-scoped runtime with a higher priority beats a namespace-scoped runtime with a lower priority -func TestHighPriorityClusterRuntimeBeatsLowerPriorityNamespace(t *testing.T) { +// TestNamespaceRuntimeOverridesClusterRuntime verifies that a namespace-scoped +// runtime hard-overrides a cluster-scoped runtime within the namespace +func TestNamespaceRuntimeOverridesClusterRuntime(t *testing.T) { fakeClient := createFakeClient() ctx := context.Background() @@ -1506,8 +1506,8 @@ func TestHighPriorityClusterRuntimeBeatsLowerPriorityNamespace(t *testing.T) { selected, err := selector.SelectRuntime(ctx, model, isvc) assert.NoError(t, err) - assert.Equal(t, "csr-high", selected.Name, "higher priority must win over lower-priority namespace runtime") - assert.True(t, selected.IsCluster) + assert.Equal(t, "sr-low", selected.Name, "namespace runtime must override cluster runtime regardless of priority") + assert.False(t, selected.IsCluster) } // TestScorerRejectsVersionAsymmetry mirrors the matcher's strict rejection when diff --git a/pkg/runtimeselector/types.go b/pkg/runtimeselector/types.go index 1fc683d40..8bbc9fcea 100644 --- a/pkg/runtimeselector/types.go +++ b/pkg/runtimeselector/types.go @@ -82,7 +82,10 @@ type MatchDetails struct { // Priority is the runtime's priority for this model format Priority int32 - // Weight is the total weight used in scoring + // Weight is the total weight used in scoring. + // + // Deprecated: no longer consulted by the scorer; runtime selection uses + // Priority only. Will be removed in a future major version. Weight int64 // AutoSelectEnabled indicates if this runtime can be auto-selected @@ -164,10 +167,18 @@ type Config struct { // DefaultPriority is used when a runtime doesn't specify priority DefaultPriority int32 - // ModelFormatWeight is the default weight for model format matching + // ModelFormatWeight is no longer used by the scorer; runtime selection is + // driven strictly by Priority. Retained for backwards compatibility and + // will be removed in a future major version. + // + // Deprecated: no longer applied; runtime selection uses Priority only. ModelFormatWeight int64 - // ModelFrameworkWeight is the default weight for model framework matching + // ModelFrameworkWeight is no longer used by the scorer; runtime selection + // is driven strictly by Priority. Retained for backwards compatibility and + // will be removed in a future major version. + // + // Deprecated: no longer applied; runtime selection uses Priority only. ModelFrameworkWeight int64 } From 4855ad9004a97d240022a785cdfe1d1598e863d7 Mon Sep 17 00:00:00 2001 From: Arthur Cheng Date: Fri, 8 May 2026 13:30:46 -0700 Subject: [PATCH 4/4] update serving runtime webhook to follow scope priority size and name --- .../servingruntime/servingruntime_webhook.go | 85 ++++--- .../servingruntime_webhook_test.go | 211 +++++++++++++++++- 2 files changed, 262 insertions(+), 34 deletions(-) diff --git a/pkg/webhook/admission/servingruntime/servingruntime_webhook.go b/pkg/webhook/admission/servingruntime/servingruntime_webhook.go index b9fddf501..738a39c60 100644 --- a/pkg/webhook/admission/servingruntime/servingruntime_webhook.go +++ b/pkg/webhook/admission/servingruntime/servingruntime_webhook.go @@ -74,15 +74,15 @@ func (sr *ServingRuntimeValidator) Handle(ctx context.Context, req admission.Req return admission.Denied(err.Error()) } - for i := range ExistingRuntimes.Items { - if err := validateModelFormatPrioritySame(&servingRuntime.Spec); err != nil { - return admission.Denied(fmt.Sprintf(PriorityIsNotSameServingRuntimeError, err.Error(), servingRuntime.Name)) - } + if err := validateModelFormatPrioritySame(&servingRuntime.Spec); err != nil { + return admission.Denied(fmt.Sprintf(PriorityIsNotSameServingRuntimeError, err.Error(), servingRuntime.Name)) + } - if err := validateServingRuntimeAnnotations(&servingRuntime.Spec); err != nil { - return admission.Denied(ChainsawInjectAnnotationNotAllowError) - } + if err := validateServingRuntimeAnnotations(&servingRuntime.Spec); err != nil { + return admission.Denied(ChainsawInjectAnnotationNotAllowError) + } + for i := range ExistingRuntimes.Items { if err := validateServingRuntimePriority(&servingRuntime.Spec, &ExistingRuntimes.Items[i].Spec, servingRuntime.Name, ExistingRuntimes.Items[i].Name); err != nil { return admission.Denied(fmt.Sprintf(InvalidPriorityServingRuntimeError, err.Error(), ExistingRuntimes.Items[i].Name, servingRuntime.Name, servingRuntime.Namespace)) } @@ -120,15 +120,15 @@ func (csr *ClusterServingRuntimeValidator) Handle(ctx context.Context, req admis return admission.Denied(err.Error()) } - for i := range ExistingRuntimes.Items { - if err := validateModelFormatPrioritySame(&clusterServingRuntime.Spec); err != nil { - return admission.Denied(fmt.Sprintf(PriorityIsNotSameClusterServingRuntimeError, err.Error(), clusterServingRuntime.Name)) - } + if err := validateModelFormatPrioritySame(&clusterServingRuntime.Spec); err != nil { + return admission.Denied(fmt.Sprintf(PriorityIsNotSameClusterServingRuntimeError, err.Error(), clusterServingRuntime.Name)) + } - if err := validateServingRuntimeAnnotations(&clusterServingRuntime.Spec); err != nil { - return admission.Denied(ChainsawInjectAnnotationNotAllowError) - } + if err := validateServingRuntimeAnnotations(&clusterServingRuntime.Spec); err != nil { + return admission.Denied(ChainsawInjectAnnotationNotAllowError) + } + for i := range ExistingRuntimes.Items { if err := validateServingRuntimePriority(&clusterServingRuntime.Spec, &ExistingRuntimes.Items[i].Spec, clusterServingRuntime.Name, ExistingRuntimes.Items[i].Name); err != nil { return admission.Denied(fmt.Sprintf(InvalidPriorityClusterServingRuntimeError, err.Error(), ExistingRuntimes.Items[i].Name, clusterServingRuntime.Name)) } @@ -136,12 +136,31 @@ func (csr *ClusterServingRuntimeValidator) Handle(ctx context.Context, req admis return admission.Allowed("") } +// Weight is zeroed before comparison +func modelFormatsCompatibleEqual(a, b *v1beta1.ModelFormat) bool { + if a == nil || b == nil { + return a == b + } + aCopy, bCopy := *a, *b + aCopy.Weight, bCopy.Weight = 0, 0 + return aCopy == bCopy +} + +func modelFrameworksCompatibleEqual(a, b *v1beta1.ModelFrameworkSpec) bool { + if a == nil || b == nil { + return a == b + } + aCopy, bCopy := *a, *b + aCopy.Weight, bCopy.Weight = 0, 0 + return aCopy == bCopy +} + func areSupportedModelFormatsEqual(m1 v1beta1.SupportedModelFormat, m2 v1beta1.SupportedModelFormat) bool { if strings.EqualFold(m1.Name, m2.Name) && ((m1.Version == nil && m2.Version == nil) || (m1.Version != nil && m2.Version != nil && *m1.Version == *m2.Version)) && ((m1.Quantization == nil && m2.Quantization == nil) || (m1.Quantization != nil && m2.Quantization != nil && *m1.Quantization == *m2.Quantization)) && - ((m1.ModelFramework == nil && m2.ModelFramework == nil) || (m1.ModelFramework != nil && m2.ModelFramework != nil && *m1.ModelFramework == *m2.ModelFramework)) && - ((m1.ModelFormat == nil && m2.ModelFormat == nil) || (m1.ModelFormat != nil && m2.ModelFormat != nil && *m1.ModelFormat == *m2.ModelFormat)) && + modelFrameworksCompatibleEqual(m1.ModelFramework, m2.ModelFramework) && + modelFormatsCompatibleEqual(m1.ModelFormat, m2.ModelFormat) && ((m1.ModelArchitecture == nil && m2.ModelArchitecture == nil) || (m1.ModelArchitecture != nil && m2.ModelArchitecture != nil && *m1.ModelArchitecture == *m2.ModelArchitecture)) { return true } @@ -179,29 +198,39 @@ func validateServingRuntimeAnnotations(servingRuntime *v1beta1.ServingRuntimeSpe return nil } +// getPriority defaults a nil priority to 1 +func getPriority(p *int32) int32 { + if p == nil { + return 1 + } + return *p +} + func validateModelFormatPrioritySame(newSpec *v1beta1.ServingRuntimeSpec) error { - nameToPriority := make(map[string]*int32) + nameToPriority := make(map[string]int32) // Validate when same model format has same priority under same runtime. // If the same model format has different priority value then throws the error for _, newModelFormat := range newSpec.SupportedModelFormats { - // Only validate priority if autoselect is true - if newModelFormat.IsAutoSelectEnabled() { - if existingPriority, ok := nameToPriority[newModelFormat.Name]; ok { - if existingPriority != nil && newModelFormat.Priority != nil && (*existingPriority != *newModelFormat.Priority) { - return fmt.Errorf(PriorityIsNotSameError, newModelFormat.Name) - } - } else { - nameToPriority[newModelFormat.Name] = newModelFormat.Priority + if !newModelFormat.IsAutoSelectEnabled() { + continue + } + priority := getPriority(newModelFormat.Priority) + if existing, ok := nameToPriority[newModelFormat.Name]; ok { + if existing != priority { + return fmt.Errorf(PriorityIsNotSameError, newModelFormat.Name) } + } else { + nameToPriority[newModelFormat.Name] = priority } } return nil } func validateServingRuntimePriority(newSpec *v1beta1.ServingRuntimeSpec, existingSpec *v1beta1.ServingRuntimeSpec, existingRuntimeName string, newRuntimeName string) error { - // Skip the runtime if it is disabled or both are not multi model runtime and in update scenario skip the existing runtime if it is same as the new runtime - if (existingSpec.IsDisabled()) || (existingRuntimeName == newRuntimeName) { + // Skip the runtime if it is disabled, or if this is a self-update (the + // existing runtime is the one being updated). + if existingSpec.IsDisabled() || existingRuntimeName == newRuntimeName { return nil } // Only validate for priority if both servingruntimes supports the same protocol version @@ -219,7 +248,7 @@ func validateServingRuntimePriority(newSpec *v1beta1.ServingRuntimeSpec, existin if existingModelFormat.IsAutoSelectEnabled() && newModelFormat.IsAutoSelectEnabled() && areSupportedModelFormatsEqual(existingModelFormat, newModelFormat) && areModelSizeRangesEqual(existingSpec.ModelSizeRange, newSpec.ModelSizeRange) { - if existingModelFormat.Priority != nil && newModelFormat.Priority != nil && *existingModelFormat.Priority == *newModelFormat.Priority { + if getPriority(existingModelFormat.Priority) == getPriority(newModelFormat.Priority) { return fmt.Errorf(InvalidPriorityError, newModelFormat.Name) } } diff --git a/pkg/webhook/admission/servingruntime/servingruntime_webhook_test.go b/pkg/webhook/admission/servingruntime/servingruntime_webhook_test.go index 1eccc731e..aabbac2e2 100644 --- a/pkg/webhook/admission/servingruntime/servingruntime_webhook_test.go +++ b/pkg/webhook/admission/servingruntime/servingruntime_webhook_test.go @@ -2,16 +2,19 @@ package servingruntime import ( "context" + "encoding/json" "fmt" "testing" "github.com/onsi/gomega" "google.golang.org/protobuf/proto" + admissionv1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/sgl-project/ome/pkg/apis/ome/v1beta1" "github.com/sgl-project/ome/pkg/constants" @@ -1194,7 +1197,7 @@ func TestValidateServingRuntimePriority(t *testing.T) { }, expected: gomega.BeNil(), }, - "When priority is nil in both serving runtime then it should return nil": { + "When priority is nil in both serving runtimes it should collide (nil defaults to 1)": { newServingRuntime: &v1beta1.ServingRuntime{ ObjectMeta: metav1.ObjectMeta{ Name: "example-runtime-1", @@ -1263,9 +1266,9 @@ func TestValidateServingRuntimePriority(t *testing.T) { }, }, }, - expected: gomega.BeNil(), + expected: gomega.Equal(fmt.Errorf(InvalidPriorityError, "vllm")), }, - "When priority is nil in new serving runtime and priority is specified in existing serving runtime then it should return nil": { + "When priority is nil in new serving runtime and priority is 1 in existing it should collide": { newServingRuntime: &v1beta1.ServingRuntime{ ObjectMeta: metav1.ObjectMeta{ Name: "example-runtime-1", @@ -1335,9 +1338,9 @@ func TestValidateServingRuntimePriority(t *testing.T) { }, }, }, - expected: gomega.BeNil(), + expected: gomega.Equal(fmt.Errorf(InvalidPriorityError, "vllm")), }, - "When priority is nil in existing serving runtime and priority is specified in new serving runtime then it should return nil": { + "When priority is nil in existing serving runtime and priority is 1 in new it should collide": { newServingRuntime: &v1beta1.ServingRuntime{ ObjectMeta: metav1.ObjectMeta{ Name: "example-runtime-1", @@ -1407,7 +1410,7 @@ func TestValidateServingRuntimePriority(t *testing.T) { }, }, }, - expected: gomega.BeNil(), + expected: gomega.Equal(fmt.Errorf(InvalidPriorityError, "vllm")), }, } @@ -1705,6 +1708,88 @@ func TestValidateModelFormatPrioritySame(t *testing.T) { }, expected: gomega.BeNil(), }, + "When one priority is nil and the other is explicit non-1 for the same model format it should error (nil defaults to 1)": { + newServingRuntime: &v1beta1.ServingRuntime{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-runtime-1", + Namespace: "test", + }, + Spec: v1beta1.ServingRuntimeSpec{ + SupportedModelFormats: []v1beta1.SupportedModelFormat{ + { + Name: "vllm", + AutoSelect: proto.Bool(true), + Priority: nil, + }, + { + Name: "vllm", + AutoSelect: proto.Bool(true), + Priority: proto.Int32(2), + }, + }, + Disabled: proto.Bool(false), + ProtocolVersions: []constants.InferenceServiceProtocol{ + constants.OpenAIProtocol, + constants.OpenAIProtocol, + }, + ServingRuntimePodSpec: v1beta1.ServingRuntimePodSpec{ + Containers: []corev1.Container{ + { + Name: constants.MainContainerName, + Image: "ome/vllm:latest", + Args: []string{ + "--model_name={{.Name}}", + "--model_dir=/mnt/models", + "--http_port=8080", + }, + }, + }, + }, + }, + }, + expected: gomega.Equal(fmt.Errorf(PriorityIsNotSameError, "vllm")), + }, + "When one priority is nil and the other is explicit 1 for the same model format it should return nil (nil defaults to 1)": { + newServingRuntime: &v1beta1.ServingRuntime{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-runtime-1", + Namespace: "test", + }, + Spec: v1beta1.ServingRuntimeSpec{ + SupportedModelFormats: []v1beta1.SupportedModelFormat{ + { + Name: "vllm", + AutoSelect: proto.Bool(true), + Priority: nil, + }, + { + Name: "vllm", + AutoSelect: proto.Bool(true), + Priority: proto.Int32(1), + }, + }, + Disabled: proto.Bool(false), + ProtocolVersions: []constants.InferenceServiceProtocol{ + constants.OpenAIProtocol, + constants.OpenAIProtocol, + }, + ServingRuntimePodSpec: v1beta1.ServingRuntimePodSpec{ + Containers: []corev1.Container{ + { + Name: constants.MainContainerName, + Image: "ome/vllm:latest", + Args: []string{ + "--model_name={{.Name}}", + "--model_dir=/mnt/models", + "--http_port=8080", + }, + }, + }, + }, + }, + }, + expected: gomega.BeNil(), + }, } for name, scenario := range scenarios { @@ -2199,3 +2284,117 @@ func TestValidateAcceleratorClasses(t *testing.T) { }) } } + +func TestServingRuntimeValidator_Handle_CrossScopeOverlapAllowed(t *testing.T) { + scheme := runtime.NewScheme() + _ = v1beta1.AddToScheme(scheme) + + makeSpec := func(priority *int32) v1beta1.ServingRuntimeSpec { + return v1beta1.ServingRuntimeSpec{ + SupportedModelFormats: []v1beta1.SupportedModelFormat{{ + Name: "vllm", + Version: proto.String("1"), + AutoSelect: proto.Bool(true), + Priority: priority, + }}, + Disabled: proto.Bool(false), + ProtocolVersions: []constants.InferenceServiceProtocol{ + constants.OpenAIProtocol, + }, + ServingRuntimePodSpec: v1beta1.ServingRuntimePodSpec{ + Containers: []corev1.Container{{ + Name: constants.MainContainerName, + Image: "ome/vllm:latest", + }}, + }, + } + } + + cases := map[string]struct { + existingCSRPriority *int32 + newSRPriority *int32 + }{ + "explicit equal priorities allowed across scopes": {proto.Int32(1), proto.Int32(1)}, + "nil vs explicit 1 allowed across scopes": {nil, proto.Int32(1)}, + "both nil allowed across scopes": {nil, nil}, + "different priorities allowed across scopes": {proto.Int32(1), proto.Int32(2)}, + "different non-default priorities allowed": {proto.Int32(2), proto.Int32(3)}, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + g := gomega.NewGomegaWithT(t) + + existingCSR := &v1beta1.ClusterServingRuntime{ + ObjectMeta: metav1.ObjectMeta{Name: "existing-csr"}, + Spec: makeSpec(tc.existingCSRPriority), + } + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(existingCSR). + Build() + + validator := &ServingRuntimeValidator{ + Client: fakeClient, + Decoder: admission.NewDecoder(scheme), + } + + newSR := &v1beta1.ServingRuntime{ + ObjectMeta: metav1.ObjectMeta{Name: "new-sr", Namespace: "test"}, + Spec: makeSpec(tc.newSRPriority), + } + raw, err := json.Marshal(newSR) + g.Expect(err).ToNot(gomega.HaveOccurred()) + + req := admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Object: runtime.RawExtension{Raw: raw}, + }} + + resp := validator.Handle(context.Background(), req) + g.Expect(resp.Allowed).To(gomega.BeTrue(), "cross-scope overlaps must be admitted") + }) + } +} + +func TestServingRuntimeValidator_Handle_SameNameAcrossScopesAllowed(t *testing.T) { + g := gomega.NewGomegaWithT(t) + scheme := runtime.NewScheme() + _ = v1beta1.AddToScheme(scheme) + + spec := v1beta1.ServingRuntimeSpec{ + SupportedModelFormats: []v1beta1.SupportedModelFormat{{ + Name: "vllm", + Version: proto.String("1"), + AutoSelect: proto.Bool(true), + Priority: proto.Int32(1), + }}, + Disabled: proto.Bool(false), + ProtocolVersions: []constants.InferenceServiceProtocol{ + constants.OpenAIProtocol, + }, + ServingRuntimePodSpec: v1beta1.ServingRuntimePodSpec{ + Containers: []corev1.Container{{Name: constants.MainContainerName, Image: "ome/vllm:latest"}}, + }, + } + + existingCSR := &v1beta1.ClusterServingRuntime{ + ObjectMeta: metav1.ObjectMeta{Name: "vllm-runtime"}, + Spec: spec, + } + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingCSR).Build() + validator := &ServingRuntimeValidator{Client: fakeClient, Decoder: admission.NewDecoder(scheme)} + + // Same name as the CSR but different scope. + newSR := &v1beta1.ServingRuntime{ + ObjectMeta: metav1.ObjectMeta{Name: "vllm-runtime", Namespace: "test"}, + Spec: spec, + } + raw, err := json.Marshal(newSR) + g.Expect(err).ToNot(gomega.HaveOccurred()) + + resp := validator.Handle(context.Background(), admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create, Object: runtime.RawExtension{Raw: raw}}, + }) + g.Expect(resp.Allowed).To(gomega.BeTrue(), "same-name SR and CSR are distinct objects in distinct scopes; admission must allow") +}