Skip to content

Commit

Permalink
Add support for public routes in kserve
Browse files Browse the repository at this point in the history
  • Loading branch information
lucferbux committed Jul 10, 2024
1 parent 3e07400 commit c5cc583
Show file tree
Hide file tree
Showing 11 changed files with 196 additions and 13 deletions.
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
Expand Up @@ -9,17 +9,34 @@ 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>
{inferenceService.status?.address?.url}
</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>
);
}

return (
<DescriptionList isCompact>
{Object.entries(isInternalServiceEnabled).map(([route, value]) => (
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

0 comments on commit c5cc583

Please sign in to comment.