From 72e4c40081268d428383e4ae878fa353dd0a1677 Mon Sep 17 00:00:00 2001 From: Mariah Holder Date: Tue, 7 Oct 2025 09:22:45 -0400 Subject: [PATCH 1/7] expose the actual model name instead of the resource name for the /v1/models endpoint --- maas-api/internal/handlers/models_test.go | 43 +++++++++++++++++++++++ maas-api/internal/models/kserve.go | 16 +++++++-- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/maas-api/internal/handlers/models_test.go b/maas-api/internal/handlers/models_test.go index bb95e5c30..53f6b3d94 100644 --- a/maas-api/internal/handlers/models_test.go +++ b/maas-api/internal/handlers/models_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/openai/openai-go/v2" "github.com/openai/openai-go/v2/packages/pagination" @@ -13,6 +14,7 @@ import ( "github.com/opendatahub-io/maas-billing/maas-api/test/fixtures" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "knative.dev/pkg/apis" ) @@ -51,6 +53,33 @@ func TestListingModels(t *testing.T) { } llmInferenceServices := fixtures.CreateLLMInferenceServices(llmTestScenarios...) + //Add a model where .spec.model.name is unset; should default to metadata.name + unsetSpecModelName := "unset-spec-model-name" + unsetSpecModelNameNamespace := fixtures.TestNamespace + + unsetSpecModelNameISVC := unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "serving.kserve.io/v1beta1", + "kind": "InferenceService", + "metadata": map[string]any{ + "name": unsetSpecModelName, + "namespace": unsetSpecModelNameNamespace, + "creationTimestamp": time.Now().UTC().Format(time.RFC3339), + }, + "spec": map[string]any{ + "model": map[string]any{ + //left out the "name" key + }, + }, + "status": map[string]any{ + "url": "http://" + unsetSpecModelName + "." + unsetSpecModelNameNamespace + ".acme.com/v1", + }, + }, + } + + // Append the model to the objects used by the fake server + llmInferenceServices = append(llmInferenceServices, unsetSpecModelNameISVC) + config := fixtures.TestServerConfig{ Objects: llmInferenceServices, } @@ -102,6 +131,20 @@ func TestListingModels(t *testing.T) { }) } + //After loop that builds testCases from llmTestScenarios completes, append an expectedModel for the unsetSpecModelName case: + testCases = append(testCases, expectedModel{ + name: unsetSpecModelName, // key used to pull from modelsByName + expectedModel: models.Model{ + Model: openai.Model{ + ID: unsetSpecModelName, // should equal metadata.name because spec.model.name is missing + Object: "model", + OwnedBy: unsetSpecModelNameNamespace, + }, + URL: mustParseURL("http://" + unsetSpecModelName + "." + unsetSpecModelNameNamespace + ".acme.com/v1"), + Ready: false, + }, + }) + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { actualModel, exists := modelsByName[tc.name] diff --git a/maas-api/internal/models/kserve.go b/maas-api/internal/models/kserve.go index 5a237b5c7..8a9c40e5d 100644 --- a/maas-api/internal/models/kserve.go +++ b/maas-api/internal/models/kserve.go @@ -78,12 +78,24 @@ func toModels(list *unstructured.UnstructuredList) ([]Model, error) { for _, item := range list.Items { url, errURL := findURL(item) if errURL != nil { - log.Printf("DEBUG: Failed to find URL for %s: %v", item.GetKind(), errURL) + log.Printf("DEBUG: Failed to find URL for %s %s/%s: %v", + item.GetKind(), item.GetNamespace(), item.GetName(), errURL) + } + + // Default to metadata.name + modelID := item.GetName() + + // Check if .spec.model.name exists + if name, found, err := unstructured.NestedString(item.Object, "spec", "model", "name"); err != nil { + log.Printf("DEBUG: Error reading spec.model.name for %s %s/%s: %v", + item.GetKind(), item.GetNamespace(), item.GetName(), err) + } else if found { + modelID = name } models = append(models, Model{ Model: openai.Model{ - ID: item.GetName(), + ID: modelID, Object: "model", OwnedBy: item.GetNamespace(), Created: item.GetCreationTimestamp().Unix(), From 2d94afad198659d487cff8fdc0503682206669c6 Mon Sep 17 00:00:00 2001 From: Mariah Holder <94134625+mholder6@users.noreply.github.com> Date: Tue, 7 Oct 2025 09:28:38 -0400 Subject: [PATCH 2/7] Update maas-api/internal/models/kserve.go Co-authored-by: Pierangelo Di Pilato --- maas-api/internal/models/kserve.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maas-api/internal/models/kserve.go b/maas-api/internal/models/kserve.go index 8a9c40e5d..51286cf12 100644 --- a/maas-api/internal/models/kserve.go +++ b/maas-api/internal/models/kserve.go @@ -89,7 +89,7 @@ func toModels(list *unstructured.UnstructuredList) ([]Model, error) { if name, found, err := unstructured.NestedString(item.Object, "spec", "model", "name"); err != nil { log.Printf("DEBUG: Error reading spec.model.name for %s %s/%s: %v", item.GetKind(), item.GetNamespace(), item.GetName(), err) - } else if found { + } else if found && name != "" { modelID = name } From 12b14e52dc805a0b40c62354b49e9a078d87ad4a Mon Sep 17 00:00:00 2001 From: Mariah Holder Date: Tue, 7 Oct 2025 09:38:45 -0400 Subject: [PATCH 3/7] updated testing isvc to be llmisvc --- maas-api/internal/handlers/models_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maas-api/internal/handlers/models_test.go b/maas-api/internal/handlers/models_test.go index 53f6b3d94..47e12d3f7 100644 --- a/maas-api/internal/handlers/models_test.go +++ b/maas-api/internal/handlers/models_test.go @@ -60,7 +60,7 @@ func TestListingModels(t *testing.T) { unsetSpecModelNameISVC := unstructured.Unstructured{ Object: map[string]any{ "apiVersion": "serving.kserve.io/v1beta1", - "kind": "InferenceService", + "kind": "LLMInferenceService", "metadata": map[string]any{ "name": unsetSpecModelName, "namespace": unsetSpecModelNameNamespace, From 0e514fcfbebac10132508c761df929aeab93087c Mon Sep 17 00:00:00 2001 From: Mariah Holder Date: Tue, 7 Oct 2025 09:45:19 -0400 Subject: [PATCH 4/7] typo --- maas-api/internal/handlers/models_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maas-api/internal/handlers/models_test.go b/maas-api/internal/handlers/models_test.go index 47e12d3f7..c1efc2f1c 100644 --- a/maas-api/internal/handlers/models_test.go +++ b/maas-api/internal/handlers/models_test.go @@ -57,7 +57,7 @@ func TestListingModels(t *testing.T) { unsetSpecModelName := "unset-spec-model-name" unsetSpecModelNameNamespace := fixtures.TestNamespace - unsetSpecModelNameISVC := unstructured.Unstructured{ + unsetSpecModelNameISVC := &unstructured.Unstructured{ Object: map[string]any{ "apiVersion": "serving.kserve.io/v1beta1", "kind": "LLMInferenceService", From 9163e0549d9b30b2758a86836a6aa6bf488d727a Mon Sep 17 00:00:00 2001 From: Mariah Holder Date: Tue, 7 Oct 2025 14:24:44 -0400 Subject: [PATCH 5/7] squash --- maas-api/internal/handlers/models_test.go | 52 ++++------------------- maas-api/test/fixtures/llm_models.go | 39 ++++++++++++++--- 2 files changed, 42 insertions(+), 49 deletions(-) diff --git a/maas-api/internal/handlers/models_test.go b/maas-api/internal/handlers/models_test.go index c1efc2f1c..7e829edd2 100644 --- a/maas-api/internal/handlers/models_test.go +++ b/maas-api/internal/handlers/models_test.go @@ -5,7 +5,6 @@ import ( "net/http" "net/http/httptest" "testing" - "time" "github.com/openai/openai-go/v2" "github.com/openai/openai-go/v2/packages/pagination" @@ -14,11 +13,12 @@ import ( "github.com/opendatahub-io/maas-billing/maas-api/test/fixtures" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "knative.dev/pkg/apis" ) func TestListingModels(t *testing.T) { + strptr := func(s string) *string { return &s } + llmTestScenarios := []fixtures.LLMTestScenario{ { Name: "llama-7b", @@ -50,35 +50,15 @@ func TestListingModels(t *testing.T) { URL: fixtures.PublicURL(""), Ready: false, }, - } - llmInferenceServices := fixtures.CreateLLMInferenceServices(llmTestScenarios...) - - //Add a model where .spec.model.name is unset; should default to metadata.name - unsetSpecModelName := "unset-spec-model-name" - unsetSpecModelNameNamespace := fixtures.TestNamespace - - unsetSpecModelNameISVC := &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "serving.kserve.io/v1beta1", - "kind": "LLMInferenceService", - "metadata": map[string]any{ - "name": unsetSpecModelName, - "namespace": unsetSpecModelNameNamespace, - "creationTimestamp": time.Now().UTC().Format(time.RFC3339), - }, - "spec": map[string]any{ - "model": map[string]any{ - //left out the "name" key - }, - }, - "status": map[string]any{ - "url": "http://" + unsetSpecModelName + "." + unsetSpecModelNameNamespace + ".acme.com/v1", - }, + { + Name: "empty-spec-model-name", + Namespace: fixtures.TestNamespace, + URL: fixtures.PublicURL("http://empty-spec-model-name." + fixtures.TestNamespace + ".acme.com/v1"), + Ready: true, + SpecModelName: strptr(""), }, } - - // Append the model to the objects used by the fake server - llmInferenceServices = append(llmInferenceServices, unsetSpecModelNameISVC) + llmInferenceServices := fixtures.CreateLLMInferenceServices(llmTestScenarios...) config := fixtures.TestServerConfig{ Objects: llmInferenceServices, @@ -131,20 +111,6 @@ func TestListingModels(t *testing.T) { }) } - //After loop that builds testCases from llmTestScenarios completes, append an expectedModel for the unsetSpecModelName case: - testCases = append(testCases, expectedModel{ - name: unsetSpecModelName, // key used to pull from modelsByName - expectedModel: models.Model{ - Model: openai.Model{ - ID: unsetSpecModelName, // should equal metadata.name because spec.model.name is missing - Object: "model", - OwnedBy: unsetSpecModelNameNamespace, - }, - URL: mustParseURL("http://" + unsetSpecModelName + "." + unsetSpecModelNameNamespace + ".acme.com/v1"), - Ready: false, - }, - }) - for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { actualModel, exists := modelsByName[tc.name] diff --git a/maas-api/test/fixtures/llm_models.go b/maas-api/test/fixtures/llm_models.go index 98dbd06da..4dbc39892 100644 --- a/maas-api/test/fixtures/llm_models.go +++ b/maas-api/test/fixtures/llm_models.go @@ -42,8 +42,17 @@ func (i AddressEntry) AddTo(obj *unstructured.Unstructured) { }, "status", "addresses") } +type LLMInferenceServiceOption func(*unstructured.Unstructured) + +// WithSpecModelName sets .spec.model.name (can be an empty string "" to test fallback logic). +func WithSpecModelName(name string) LLMInferenceServiceOption { + return func(obj *unstructured.Unstructured) { + _ = unstructured.SetNestedField(obj.Object, name, "spec", "model", "name") + } +} + // CreateLLMInferenceService creates a test LLMInferenceService unstructured object -func CreateLLMInferenceService(name, namespace string, url ModelURL, ready bool) *unstructured.Unstructured { +func CreateLLMInferenceService(name, namespace string, url ModelURL, ready bool, opts ...LLMInferenceServiceOption) *unstructured.Unstructured { obj := &unstructured.Unstructured{} obj.Object = map[string]any{} obj.SetAPIVersion("serving.kserve.io/v1alpha1") @@ -114,22 +123,40 @@ func CreateLLMInferenceService(name, namespace string, url ModelURL, ready bool) _ = unstructured.SetNestedSlice(obj.Object, conditions, "status", "conditions") + // Apply options (e.g., WithSpecModelName) + for _, opt := range opts { + opt(obj) + } + return obj } // LLMTestScenario defines a test scenario for LLM models type LLMTestScenario struct { - Name string - Namespace string - URL ModelURL - Ready bool + Name string + Namespace string + URL ModelURL + Ready bool + SpecModelName *string } // CreateLLMInferenceServices creates a set of test LLM objects for testing func CreateLLMInferenceServices(scenarios ...LLMTestScenario) []runtime.Object { var objects []runtime.Object for _, scenario := range scenarios { - obj := CreateLLMInferenceService(scenario.Name, scenario.Namespace, scenario.URL, scenario.Ready) + var opts []LLMInferenceServiceOption + if scenario.SpecModelName != nil { + opts = append(opts, WithSpecModelName(*scenario.SpecModelName)) + } + + obj := CreateLLMInferenceService( + scenario.Name, + scenario.Namespace, + scenario.URL, + scenario.Ready, + opts..., // apply any options (e.g., WithSpecModelName) + ) + objects = append(objects, obj) } From 867e0cf54fb4fbc2e96f438da1c1bfce6157b9c6 Mon Sep 17 00:00:00 2001 From: Mariah Holder <94134625+mholder6@users.noreply.github.com> Date: Fri, 10 Oct 2025 09:38:49 -0400 Subject: [PATCH 6/7] Update maas-api/internal/handlers/models_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Edgar Hernández --- maas-api/internal/handlers/models_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maas-api/internal/handlers/models_test.go b/maas-api/internal/handlers/models_test.go index 7e829edd2..b9e55d426 100644 --- a/maas-api/internal/handlers/models_test.go +++ b/maas-api/internal/handlers/models_test.go @@ -55,7 +55,7 @@ func TestListingModels(t *testing.T) { Namespace: fixtures.TestNamespace, URL: fixtures.PublicURL("http://empty-spec-model-name." + fixtures.TestNamespace + ".acme.com/v1"), Ready: true, - SpecModelName: strptr(""), + SpecModelName: strptr("my-model-name"), }, } llmInferenceServices := fixtures.CreateLLMInferenceServices(llmTestScenarios...) From 97d5c85dced08bb69c741619da22768c48e82e2d Mon Sep 17 00:00:00 2001 From: Mariah Holder Date: Fri, 10 Oct 2025 09:59:02 -0400 Subject: [PATCH 7/7] updated the test to test the fallback logic instead --- maas-api/internal/handlers/models_test.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/maas-api/internal/handlers/models_test.go b/maas-api/internal/handlers/models_test.go index b9e55d426..66408af0e 100644 --- a/maas-api/internal/handlers/models_test.go +++ b/maas-api/internal/handlers/models_test.go @@ -51,11 +51,11 @@ func TestListingModels(t *testing.T) { Ready: false, }, { - Name: "empty-spec-model-name", + Name: "fallback-model-name", Namespace: fixtures.TestNamespace, - URL: fixtures.PublicURL("http://empty-spec-model-name." + fixtures.TestNamespace + ".acme.com/v1"), + URL: fixtures.PublicURL("http://fallback-model-name." + fixtures.TestNamespace + ".acme.com/v1"), Ready: true, - SpecModelName: strptr("my-model-name"), + SpecModelName: strptr("fallback-model-name"), }, } llmInferenceServices := fixtures.CreateLLMInferenceServices(llmTestScenarios...) @@ -97,11 +97,17 @@ func TestListingModels(t *testing.T) { var testCases []expectedModel for _, llmTestScenario := range llmTestScenarios { + // expected ID mirrors toModels(): fallback to metadata.name unless spec.model.name is non-empty + expectedModelID := llmTestScenario.Name + if llmTestScenario.SpecModelName != nil && *llmTestScenario.SpecModelName != "" { + expectedModelID = *llmTestScenario.SpecModelName + } + testCases = append(testCases, expectedModel{ - name: llmTestScenario.Name, + name: expectedModelID, expectedModel: models.Model{ Model: openai.Model{ - ID: llmTestScenario.Name, + ID: expectedModelID, Object: "model", OwnedBy: llmTestScenario.Namespace, },