Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for public routes in kserve #2924

Merged
merged 1 commit into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions frontend/src/__mocks__/mockInferenceServiceK8sResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ type MockResourceConfigType = {
maxReplicas?: number;
lastFailureInfoMessage?: string;
resources?: ContainerResources;
kserveInternalUrl?: string;
statusPredictor?: Record<string, string>;
kserveInternalLabel?: boolean;
};

type InferenceServicek8sError = K8sStatus & {
Expand Down Expand Up @@ -76,6 +78,8 @@ export const mockInferenceServiceK8sResource = ({
lastFailureInfoMessage = 'Waiting for runtime Pod to become available',
resources,
statusPredictor = undefined,
kserveInternalUrl = '',
kserveInternalLabel = false,
}: MockResourceConfigType): InferenceServiceKind => ({
apiVersion: 'serving.kserve.io/v1beta1',
kind: 'InferenceService',
Expand All @@ -96,6 +100,7 @@ export const mockInferenceServiceK8sResource = ({
labels: {
name,
[KnownLabels.DASHBOARD_RESOURCE]: 'true',
...(kserveInternalLabel && { 'networking.knative.dev/visibility': 'cluster-local' }),
},
name,
namespace,
Expand Down Expand Up @@ -167,5 +172,10 @@ export const mockInferenceServiceK8sResource = ({
},
transitionStatus: '',
},
...(kserveInternalUrl && {
address: {
url: kserveInternalUrl,
},
}),
},
});
8 changes: 8 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/modelServing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,14 @@ class ModelServingRow extends TableRow {
.should(enabled ? 'not.exist' : 'exist');
return this;
}

findInternalServiceButton() {
return this.find().findByTestId('internal-service-button');
}

findInternalServicePopover() {
return cy.findByTestId('internal-service-popover');
}
}

class ModelMeshRow extends ModelServingRow {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1894,7 +1894,7 @@ describe('Serving Runtime List', () => {
});

describe('Internal service', () => {
it('Check internal service is rendered when the model is loaded', () => {
it('Check internal service is rendered when the model is loaded in Modelmesh', () => {
initIntercepts({
projectEnableModelMesh: true,
disableKServeConfig: false,
Expand Down Expand Up @@ -1948,5 +1948,57 @@ describe('Serving Runtime List', () => {
.findByText('Could not find any internal service enabled')
.should('exist');
});

it('Check internal service is rendered when the model is loaded in Kserve', () => {
initIntercepts({
projectEnableModelMesh: false,
disableKServeConfig: true,
disableModelMeshConfig: false,
servingRuntimes: [
mockServingRuntimeK8sResource({
name: 'test-model',
auth: true,
route: false,
}),
mockServingRuntimeK8sResource({
name: 'test-model-not-loaded',
auth: true,
route: false,
}),
],
inferenceServices: [
mockInferenceServiceK8sResource({
name: 'model-loaded',
modelName: 'test-model',
displayName: 'Loaded model',
isModelMesh: false,
kserveInternalUrl: 'http://test.kserve.svc.cluster.local',
kserveInternalLabel: true,
}),
mockInferenceServiceK8sResource({
name: 'model-not-loaded',
modelName: 'est-model-not-loaded',
displayName: 'Model Not loaded',
isModelMesh: false,
kserveInternalLabel: true,
}),
],
});

projectDetails.visitSection('test-project', 'model-server');

// Get modal of inference service when is loaded
const kserveRowModelLoaded = modelServingSection.getKServeRow('Loaded model');
kserveRowModelLoaded.findInternalServiceButton().click();
kserveRowModelLoaded.findInternalServicePopover().findByText('url').should('exist');

// Get modal of inference service when is not loaded
const kserveRowModelNotLoaded = modelServingSection.getKServeRow('Model Not loaded');
kserveRowModelNotLoaded.findInternalServiceButton().click();
kserveRowModelLoaded
.findInternalServicePopover()
.findByText('Could not find any internal service enabled')
.should('exist');
});
});
});
31 changes: 31 additions & 0 deletions frontend/src/api/k8s/__tests__/inferenceServices.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,37 @@ describe('assembleInferenceService', () => {
);
});

it('should have the right labels when creating for Kserve with public route', async () => {
const inferenceService = assembleInferenceService(
mockInferenceServiceModalData({ externalRoute: false }),
);

expect(inferenceService.metadata.labels?.['networking.knative.dev/visibility']).toBe(
'cluster-local',
);

const missingExternalRoute = assembleInferenceService(
mockInferenceServiceModalData({ externalRoute: true }),
);

expect(missingExternalRoute.metadata.labels?.['networking.knative.dev/visibility']).toBe(
undefined,
);
});

it('should have the right labels when creating for Modelmesh with public route', async () => {
const missingExternalRoute = assembleInferenceService(
mockInferenceServiceModalData({ externalRoute: true }),
undefined,
undefined,
true,
);

expect(missingExternalRoute.metadata.labels?.['networking.knative.dev/visibility']).toBe(
undefined,
);
});

it('should handle name and display name', async () => {
const displayName = 'Llama model';

Expand Down
12 changes: 12 additions & 0 deletions frontend/src/api/k8s/inferenceServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const assembleInferenceService = (
maxReplicas,
minReplicas,
tokenAuth,
externalRoute,
} = data;
const name = editName || translateDisplayNameForK8s(data.name);
const { path, dataConnection } = storage;
Expand All @@ -55,6 +56,11 @@ export const assembleInferenceService = (
...(tokenAuth && { 'security.opendatahub.io/enable-auth': 'true' }),
}),
},
labels: {
...inferenceService.metadata.labels,
...(!isModelMesh &&
!externalRoute && { 'networking.knative.dev/visibility': 'cluster-local' }),
},
},
spec: {
predictor: {
Expand Down Expand Up @@ -82,6 +88,8 @@ export const assembleInferenceService = (
namespace: project,
labels: {
[KnownLabels.DASHBOARD_RESOURCE]: 'true',
...(!isModelMesh &&
!externalRoute && { 'networking.knative.dev/visibility': 'cluster-local' }),
},
annotations: {
'openshift.io/display-name': data.name.trim(),
Expand Down Expand Up @@ -118,6 +126,10 @@ export const assembleInferenceService = (
delete updateInferenceService.metadata.annotations['serving.knative.openshift.io/token-auth'];
}

if (externalRoute && updateInferenceService.metadata.labels) {
delete updateInferenceService.metadata.labels['networking.knative.dev/visibility'];
}

// Resource and Accelerator support for KServe
if (!isModelMesh) {
const resourceSettings: ContainerResources = {
Expand Down
10 changes: 10 additions & 0 deletions frontend/src/k8sTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,10 @@ export type InferenceServiceAnnotations = Partial<{
'security.opendatahub.io/enable-auth': string;
}>;

export type InferenceServiceLabels = Partial<{
'networking.knative.dev/visibility': string;
}>;

export type InferenceServiceKind = K8sResourceCommon & {
metadata: {
name: string;
Expand Down Expand Up @@ -495,6 +499,12 @@ export type InferenceServiceKind = K8sResourceCommon & {
transitionStatus: string;
};
url: string;
address?: {
CACerts?: string;
audience?: string;
name?: string;
url?: string;
};
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import {
Skeleton,
} from '@patternfly/react-core';
import { InferenceServiceKind, ServingRuntimeKind } from '~/k8sTypes';
import { isServingRuntimeRouteEnabled } from '~/pages/modelServing/screens/projects/utils';
import {
isServingRuntimeRouteEnabled,
isInferenceServiceRouteEnabled,
} from '~/pages/modelServing/screens/projects/utils';
import useRouteForInferenceService from './useRouteForInferenceService';
import InternalServicePopoverContent from './InternalServicePopoverContent';

Expand All @@ -23,22 +26,25 @@ const InferenceServiceEndpoint: React.FC<InferenceServiceEndpointProps> = ({
servingRuntime,
isKserve,
}) => {
const isRouteEnabled =
servingRuntime !== undefined && isServingRuntimeRouteEnabled(servingRuntime);
const isRouteEnabled = !isKserve
? servingRuntime !== undefined && isServingRuntimeRouteEnabled(servingRuntime)
: isInferenceServiceRouteEnabled(inferenceService);

const [routeLink, loaded, loadError] = useRouteForInferenceService(
inferenceService,
isRouteEnabled,
isKserve,
);

if (!isKserve && !isRouteEnabled) {
if (!isRouteEnabled) {
return (
<Popover
data-testid="internal-service-popover"
headerContent="Internal Service can be accessed inside the cluster"
aria-label="Internal Service Info"
bodyContent={<InternalServicePopoverContent inferenceService={inferenceService} />}
bodyContent={
<InternalServicePopoverContent inferenceService={inferenceService} isKserve={isKserve} />
}
>
<Button data-testid="internal-service-button" isInline variant="link">
Internal Service
Expand All @@ -47,10 +53,6 @@ const InferenceServiceEndpoint: React.FC<InferenceServiceEndpointProps> = ({
);
}

if (!loaded) {
return <Skeleton />;
}

if (loadError) {
return (
<HelperText>
Expand All @@ -61,6 +63,10 @@ const InferenceServiceEndpoint: React.FC<InferenceServiceEndpointProps> = ({
);
}

if (!loaded || !routeLink) {
return <Skeleton />;
}

return (
<ClipboardCopy hoverTip="Copy" clickTip="Copied" isReadOnly>
{isKserve ? routeLink : `${routeLink}/infer`}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import * as React from 'react';
import {
ClipboardCopy,
ClipboardCopyVariant,
DescriptionList,
DescriptionListDescription,
DescriptionListGroup,
Expand All @@ -9,23 +11,54 @@ import { InferenceServiceKind } from '~/k8sTypes';

type InternalServicePopoverContentProps = {
inferenceService: InferenceServiceKind;
isKserve?: boolean;
};

const InternalServicePopoverContent: React.FC<InternalServicePopoverContentProps> = ({
inferenceService,
isKserve,
}) => {
const isInternalServiceEnabled = inferenceService.status?.components?.predictor;
const isInternalServiceEnabled = !isKserve
? inferenceService.status?.components?.predictor
: inferenceService.status?.address?.url;

if (!isInternalServiceEnabled) {
return <>Could not find any internal service enabled</>;
}

if (isKserve) {
return (
<DescriptionList isCompact>
<DescriptionListGroup>
<DescriptionListTerm>url</DescriptionListTerm>
<DescriptionListDescription>
<ClipboardCopy
hoverTip="Copy"
clickTip="Copied"
variant={ClipboardCopyVariant.inlineCompact}
>
{inferenceService.status?.address?.url}
</ClipboardCopy>
</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>
);
}

return (
<DescriptionList isCompact>
{Object.entries(isInternalServiceEnabled).map(([route, value]) => (
<DescriptionListGroup key={route}>
<DescriptionListTerm>{route}</DescriptionListTerm>
<DescriptionListDescription>{value}</DescriptionListDescription>
<DescriptionListDescription>
<ClipboardCopy
hoverTip="Copy"
clickTip="Copied"
variant={ClipboardCopyVariant.inlineCompact}
>
{value}
</ClipboardCopy>
</DescriptionListDescription>
</DescriptionListGroup>
))}
</DescriptionList>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ import { mockProjectK8sResource } from '~/__mocks__/mockProjectK8sResource';
import {
filterOutConnectionsWithoutBucket,
getProjectModelServingPlatform,
getUrlFromKserveInferenceService,
} from '~/pages/modelServing/screens/projects/utils';
import { DataConnection } from '~/pages/projects/types';
import { ServingPlatformStatuses } from '~/pages/modelServing/screens/types';
import { ServingRuntimePlatform } from '~/types';
import { mockInferenceServiceK8sResource } from '~/__mocks__/mockInferenceServiceK8sResource';

describe('filterOutConnectionsWithoutBucket', () => {
it('should return an empty array if input connections array is empty', () => {
Expand Down Expand Up @@ -125,3 +127,27 @@ describe('getProjectModelServingPlatform', () => {
).toStrictEqual({ platform: ServingRuntimePlatform.MULTI });
});
});

describe('getUrlsFromKserveInferenceService', () => {
it('should return the url from the inference service status', () => {
const url = 'https://test-kserve.apps.kserve-pm.dev.com';
const inferenceService = mockInferenceServiceK8sResource({
url,
});
expect(getUrlFromKserveInferenceService(inferenceService)).toBe(url);
});
it('should return undefined if the inference service status does not have an address', () => {
const url = '';
const inferenceService = mockInferenceServiceK8sResource({
url,
});
expect(getUrlFromKserveInferenceService(inferenceService)).toBeUndefined();
});
it('should return undefined if the inference service status is an internal service', () => {
const url = 'http://test.kserve.svc.cluster.local';
const inferenceService = mockInferenceServiceK8sResource({
url,
});
expect(getUrlFromKserveInferenceService(inferenceService)).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
data={createDataInferenceService}
setData={setCreateDataInferenceService}
allowCreate={allowCreate}
publicRoute
/>
</StackItem>
)}
Expand Down
Loading
Loading