Skip to content

Commit a076001

Browse files
LucifergeneLucifergene
authored
RHOAIENG-49131: LLMInferenceService not showing up URLs in the AI Assets page (opendatahub-io#6219)
* llminferenceservice url fix Signed-off-by: Avik Kundu <akundu@redhat.com> * review comments Signed-off-by: Avik Kundu <akundu@redhat.com> --------- Signed-off-by: Avik Kundu <akundu@redhat.com>
1 parent fbfc28b commit a076001

File tree

5 files changed

+312
-16
lines changed

5 files changed

+312
-16
lines changed

‎frontend/src/__mocks__/mockLLMInferenceServiceK8sResource.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ type MockLLMInferenceServiceConfigType = {
1515
lastTransitionTime?: string;
1616
isReady?: boolean;
1717
url?: string;
18+
addresses?: { name?: string; url?: string }[];
1819
additionalLabels?: Record<string, string>;
1920
isNonDashboardItem?: boolean;
2021
modelType?: ServingRuntimeModelType;
@@ -33,6 +34,7 @@ export const mockLLMInferenceServiceK8sResource = ({
3334
lastTransitionTime = '2023-03-17T16:12:41Z',
3435
isReady = true,
3536
url,
37+
addresses,
3638
isStopped = false,
3739
}: MockLLMInferenceServiceConfigType): LLMInferenceServiceKind => ({
3840
apiVersion: 'serving.kserve.io/v1alpha1',
@@ -124,6 +126,7 @@ export const mockLLMInferenceServiceK8sResource = ({
124126
},
125127
],
126128
url: url || `http://us-east-1.elb.amazonaws.com/${namespace}/${name}`,
129+
...(addresses ? { addresses } : {}),
127130
observedGeneration: 1,
128131
},
129132
});

‎packages/gen-ai/bff/internal/integrations/kubernetes/token_k8s_client.go‎

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -892,31 +892,50 @@ func (kc *TokenKubernetesClient) extractUseCaseFromLLMInferenceService(llmSvc *k
892892
return llmSvc.Annotations[InferenceServiceUseCaseAnnotation]
893893
}
894894

895-
// Helper method to extract endpoints from LLMInferenceService
895+
// Helper method to extract endpoints from LLMInferenceService.
896+
// Uses Address (singular) and URL as primary sources, falls back to Addresses (plural).
896897
func (kc *TokenKubernetesClient) extractEndpointsFromLLMInferenceService(llmSvc *kservev1alpha1.LLMInferenceService) []string {
897-
endpoints := []string{}
898-
899898
if llmSvc == nil {
900-
return endpoints
899+
return []string{}
901900
}
902901

903-
// Extract internal endpoint from Address
902+
var internalURL, externalURL string
903+
904+
// Primary: Address (singular) for internal, URL for external
904905
if llmSvc.Status.Address != nil && llmSvc.Status.Address.URL != nil {
905-
internal := llmSvc.Status.Address.URL.String()
906-
endpoints = append(endpoints, fmt.Sprintf("internal: %s", internal))
906+
internalURL = llmSvc.Status.Address.URL.String()
907907
}
908-
909-
// Extract external endpoint from URL
910908
if llmSvc.Status.URL != nil {
911-
external := llmSvc.Status.URL.String()
912-
if strings.Contains(external, ".svc.cluster.local") {
913-
return endpoints
909+
u := llmSvc.Status.URL.String()
910+
if !strings.Contains(u, ".svc.cluster.local") {
911+
externalURL = u
914912
}
915-
// Only add if it's different from internal and not internal service
916-
if len(endpoints) == 0 || !strings.Contains(endpoints[0], external) {
917-
endpoints = append(endpoints, fmt.Sprintf("external: %s", external))
913+
}
914+
915+
// Fallback: Addresses (plural) for any missing endpoints
916+
if internalURL == "" || externalURL == "" {
917+
for _, addr := range llmSvc.Status.Addresses {
918+
if addr.URL == nil {
919+
continue
920+
}
921+
u := addr.URL.String()
922+
if strings.Contains(u, ".svc.cluster.local") {
923+
if internalURL == "" {
924+
internalURL = u
925+
}
926+
} else if externalURL == "" {
927+
externalURL = u
928+
}
918929
}
919930
}
931+
932+
var endpoints []string
933+
if internalURL != "" {
934+
endpoints = append(endpoints, fmt.Sprintf("internal: %s", internalURL))
935+
}
936+
if externalURL != "" {
937+
endpoints = append(endpoints, fmt.Sprintf("external: %s", externalURL))
938+
}
920939
return endpoints
921940
}
922941

‎packages/gen-ai/bff/internal/integrations/kubernetes/token_k8s_client_test.go‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ import (
1414
"github.com/stretchr/testify/assert"
1515
authv1 "k8s.io/api/authorization/v1"
1616
"k8s.io/client-go/rest"
17+
"knative.dev/pkg/apis"
18+
duckv1 "knative.dev/pkg/apis/duck/v1"
19+
20+
kservev1alpha1 "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
1721
)
1822

1923
func TestCanListLlamaStackDistributions(t *testing.T) {
@@ -428,3 +432,154 @@ func TestHeadlessServicePortLogic(t *testing.T) {
428432
})
429433
}
430434
}
435+
436+
// mustParseURL is a test helper that parses a URL string and panics on failure.
437+
func mustParseURL(u string) *apis.URL {
438+
parsed, err := apis.ParseURL(u)
439+
if err != nil {
440+
panic(err)
441+
}
442+
return parsed
443+
}
444+
445+
func TestExtractEndpointsFromLLMInferenceService(t *testing.T) {
446+
client := &TokenKubernetesClient{
447+
Logger: slog.Default(),
448+
}
449+
450+
t.Run("nil LLMInferenceService returns empty endpoints", func(t *testing.T) {
451+
endpoints := client.extractEndpointsFromLLMInferenceService(nil)
452+
assert.Empty(t, endpoints)
453+
})
454+
455+
t.Run("empty status returns empty endpoints", func(t *testing.T) {
456+
llmSvc := &kservev1alpha1.LLMInferenceService{}
457+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
458+
assert.Empty(t, endpoints)
459+
})
460+
461+
t.Run("status.url set with external URL returns external endpoint", func(t *testing.T) {
462+
llmSvc := &kservev1alpha1.LLMInferenceService{
463+
Status: kservev1alpha1.LLMInferenceServiceStatus{
464+
URL: mustParseURL("https://my-model.apps.example.com/v1"),
465+
},
466+
}
467+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
468+
assert.Len(t, endpoints, 1)
469+
assert.Equal(t, "external: https://my-model.apps.example.com/v1", endpoints[0])
470+
})
471+
472+
t.Run("status.url with svc.cluster.local is not added as external", func(t *testing.T) {
473+
llmSvc := &kservev1alpha1.LLMInferenceService{
474+
Status: kservev1alpha1.LLMInferenceServiceStatus{
475+
URL: mustParseURL("https://my-model.namespace.svc.cluster.local:8080/v1"),
476+
},
477+
}
478+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
479+
// svc.cluster.local URL in Status.URL is ignored; no Addresses to fall back to.
480+
assert.Empty(t, endpoints)
481+
})
482+
483+
t.Run("only status.addresses with internal URL returns internal endpoint", func(t *testing.T) {
484+
// This is the real-world scenario: KServe controller sets addresses but not url or address (singular)
485+
llmSvc := &kservev1alpha1.LLMInferenceService{
486+
Status: kservev1alpha1.LLMInferenceServiceStatus{
487+
AddressStatus: duckv1.AddressStatus{
488+
Addresses: []duckv1.Addressable{
489+
{URL: mustParseURL("https://openshift-ai-inference.openshift-ingress.svc.cluster.local/ns/my-model")},
490+
},
491+
},
492+
},
493+
}
494+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
495+
assert.Len(t, endpoints, 1)
496+
assert.Equal(t, "internal: https://openshift-ai-inference.openshift-ingress.svc.cluster.local/ns/my-model", endpoints[0])
497+
})
498+
499+
t.Run("only status.addresses with external URL returns external endpoint", func(t *testing.T) {
500+
llmSvc := &kservev1alpha1.LLMInferenceService{
501+
Status: kservev1alpha1.LLMInferenceServiceStatus{
502+
AddressStatus: duckv1.AddressStatus{
503+
Addresses: []duckv1.Addressable{
504+
{URL: mustParseURL("https://my-model.apps.example.com/v1")},
505+
},
506+
},
507+
},
508+
}
509+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
510+
assert.Len(t, endpoints, 1)
511+
assert.Equal(t, "external: https://my-model.apps.example.com/v1", endpoints[0])
512+
})
513+
514+
t.Run("status.addresses with both internal and external URLs", func(t *testing.T) {
515+
llmSvc := &kservev1alpha1.LLMInferenceService{
516+
Status: kservev1alpha1.LLMInferenceServiceStatus{
517+
AddressStatus: duckv1.AddressStatus{
518+
Addresses: []duckv1.Addressable{
519+
{URL: mustParseURL("https://my-model.namespace.svc.cluster.local:8080/v1")},
520+
{URL: mustParseURL("https://my-model.apps.example.com/v1")},
521+
},
522+
},
523+
},
524+
}
525+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
526+
assert.Len(t, endpoints, 2)
527+
assert.Equal(t, "internal: https://my-model.namespace.svc.cluster.local:8080/v1", endpoints[0])
528+
assert.Equal(t, "external: https://my-model.apps.example.com/v1", endpoints[1])
529+
})
530+
531+
t.Run("status.url and status.addresses both set does not duplicate", func(t *testing.T) {
532+
llmSvc := &kservev1alpha1.LLMInferenceService{
533+
Status: kservev1alpha1.LLMInferenceServiceStatus{
534+
URL: mustParseURL("https://my-model.apps.example.com/v1"),
535+
AddressStatus: duckv1.AddressStatus{
536+
Addresses: []duckv1.Addressable{
537+
{URL: mustParseURL("https://my-model.namespace.svc.cluster.local:8080/v1")},
538+
{URL: mustParseURL("https://my-model.apps.example.com/v1")},
539+
},
540+
},
541+
},
542+
}
543+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
544+
// Internal from Addresses fallback, external from Status.URL, no duplicate external
545+
assert.Len(t, endpoints, 2)
546+
assert.Equal(t, "internal: https://my-model.namespace.svc.cluster.local:8080/v1", endpoints[0])
547+
assert.Equal(t, "external: https://my-model.apps.example.com/v1", endpoints[1])
548+
})
549+
550+
t.Run("status.address singular and status.addresses both set does not duplicate", func(t *testing.T) {
551+
internalURL := mustParseURL("https://my-model.namespace.svc.cluster.local:8080/v1")
552+
llmSvc := &kservev1alpha1.LLMInferenceService{
553+
Status: kservev1alpha1.LLMInferenceServiceStatus{
554+
AddressStatus: duckv1.AddressStatus{
555+
Address: &duckv1.Addressable{URL: internalURL},
556+
Addresses: []duckv1.Addressable{
557+
{URL: mustParseURL("https://my-model.namespace.svc.cluster.local:8080/v1")},
558+
{URL: mustParseURL("https://my-model.apps.example.com/v1")},
559+
},
560+
},
561+
},
562+
}
563+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
564+
// Should have internal from Address (singular) + external from Addresses, no duplicate internal
565+
assert.Len(t, endpoints, 2)
566+
assert.Equal(t, "internal: https://my-model.namespace.svc.cluster.local:8080/v1", endpoints[0])
567+
assert.Equal(t, "external: https://my-model.apps.example.com/v1", endpoints[1])
568+
})
569+
570+
t.Run("status.addresses with nil URL entries are skipped", func(t *testing.T) {
571+
llmSvc := &kservev1alpha1.LLMInferenceService{
572+
Status: kservev1alpha1.LLMInferenceServiceStatus{
573+
AddressStatus: duckv1.AddressStatus{
574+
Addresses: []duckv1.Addressable{
575+
{URL: nil},
576+
{URL: mustParseURL("https://my-model.apps.example.com/v1")},
577+
},
578+
},
579+
},
580+
}
581+
endpoints := client.extractEndpointsFromLLMInferenceService(llmSvc)
582+
assert.Len(t, endpoints, 1)
583+
assert.Equal(t, "external: https://my-model.apps.example.com/v1", endpoints[0])
584+
})
585+
}

‎packages/llmd-serving/src/deployments/__tests__/endpoints.spec.ts‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import { mockLLMInferenceServiceK8sResource } from '@odh-dashboard/internal/__mocks__/mockLLMInferenceServiceK8sResource';
2+
import { getLLMdDeploymentEndpoints } from '../endpoints';
3+
4+
describe('getLLMdDeploymentEndpoints', () => {
5+
it('should return external endpoint from status.url', () => {
6+
const llmSvc = mockLLMInferenceServiceK8sResource({
7+
url: 'https://my-model.apps.example.com/v1',
8+
});
9+
const endpoints = getLLMdDeploymentEndpoints(llmSvc);
10+
expect(endpoints).toHaveLength(1);
11+
expect(endpoints[0]).toEqual({
12+
type: 'external',
13+
url: 'https://my-model.apps.example.com/v1',
14+
});
15+
});
16+
17+
it('should return internal endpoint from status.addresses when status.url is not set', () => {
18+
const llmSvc = mockLLMInferenceServiceK8sResource({});
19+
llmSvc.status = {
20+
addresses: [
21+
{
22+
url: 'https://openshift-ai-inference.openshift-ingress.svc.cluster.local/ns/my-model',
23+
},
24+
],
25+
};
26+
const endpoints = getLLMdDeploymentEndpoints(llmSvc);
27+
expect(endpoints).toHaveLength(1);
28+
expect(endpoints[0]).toEqual({
29+
type: 'internal',
30+
url: 'https://openshift-ai-inference.openshift-ingress.svc.cluster.local/ns/my-model',
31+
});
32+
});
33+
34+
it('should return external endpoint from status.addresses when URL does not contain svc.cluster.local', () => {
35+
const llmSvc = mockLLMInferenceServiceK8sResource({});
36+
llmSvc.status = {
37+
addresses: [{ url: 'https://my-model.apps.example.com/v1' }],
38+
};
39+
const endpoints = getLLMdDeploymentEndpoints(llmSvc);
40+
expect(endpoints).toHaveLength(1);
41+
expect(endpoints[0]).toEqual({
42+
type: 'external',
43+
url: 'https://my-model.apps.example.com/v1',
44+
});
45+
});
46+
47+
it('should return both internal and external from status.addresses', () => {
48+
const llmSvc = mockLLMInferenceServiceK8sResource({});
49+
llmSvc.status = {
50+
addresses: [
51+
{ url: 'https://my-model.namespace.svc.cluster.local:8080/v1' },
52+
{ url: 'https://my-model.apps.example.com/v1' },
53+
],
54+
};
55+
const endpoints = getLLMdDeploymentEndpoints(llmSvc);
56+
expect(endpoints).toHaveLength(2);
57+
expect(endpoints[0]).toEqual({
58+
type: 'internal',
59+
url: 'https://my-model.namespace.svc.cluster.local:8080/v1',
60+
});
61+
expect(endpoints[1]).toEqual({
62+
type: 'external',
63+
url: 'https://my-model.apps.example.com/v1',
64+
});
65+
});
66+
67+
it('should not use addresses fallback when status.url is already set', () => {
68+
const llmSvc = mockLLMInferenceServiceK8sResource({
69+
url: 'https://my-model.apps.example.com/v1',
70+
addresses: [
71+
{ url: 'https://my-model.namespace.svc.cluster.local:8080/v1' },
72+
{ url: 'https://my-model.apps.example.com/v1' },
73+
],
74+
});
75+
const endpoints = getLLMdDeploymentEndpoints(llmSvc);
76+
expect(endpoints).toHaveLength(1);
77+
expect(endpoints[0]).toEqual({
78+
type: 'external',
79+
url: 'https://my-model.apps.example.com/v1',
80+
});
81+
});
82+
83+
it('should skip addresses with no url', () => {
84+
const llmSvc = mockLLMInferenceServiceK8sResource({});
85+
llmSvc.status = {
86+
addresses: [{ name: 'no-url-entry' }, { url: 'https://my-model.apps.example.com/v1' }],
87+
};
88+
const endpoints = getLLMdDeploymentEndpoints(llmSvc);
89+
expect(endpoints).toHaveLength(1);
90+
expect(endpoints[0]).toEqual({
91+
type: 'external',
92+
url: 'https://my-model.apps.example.com/v1',
93+
});
94+
});
95+
96+
it('should return empty array when status has empty addresses and no url', () => {
97+
const llmSvc = mockLLMInferenceServiceK8sResource({});
98+
llmSvc.status = { addresses: [] };
99+
const endpoints = getLLMdDeploymentEndpoints(llmSvc);
100+
expect(endpoints).toHaveLength(0);
101+
});
102+
103+
it('should return empty array when status is undefined', () => {
104+
const llmSvc = mockLLMInferenceServiceK8sResource({});
105+
llmSvc.status = undefined;
106+
const endpoints = getLLMdDeploymentEndpoints(llmSvc);
107+
expect(endpoints).toHaveLength(0);
108+
});
109+
});

‎packages/llmd-serving/src/deployments/endpoints.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,22 @@ export const getLLMdDeploymentEndpoints = (
66
): DeploymentEndpoint[] => {
77
const endpoints: DeploymentEndpoint[] = [];
88

9-
// `status.url` is the public facing URL (from the gateway)
9+
// Use status.url (external) if available, otherwise fall back to status.addresses.
1010
if (llmInferenceService.status?.url) {
1111
endpoints.push({
1212
type: 'external',
1313
url: llmInferenceService.status.url,
1414
});
15+
} else if (llmInferenceService.status?.addresses) {
16+
for (const addr of llmInferenceService.status.addresses) {
17+
if (addr.url) {
18+
const isInternal = addr.url.includes('.svc.cluster.local');
19+
endpoints.push({
20+
type: isInternal ? 'internal' : 'external',
21+
url: addr.url,
22+
});
23+
}
24+
}
1525
}
1626

1727
return endpoints;

0 commit comments

Comments
 (0)