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 Jun 18, 2024
1 parent c9ec3f9 commit 53e0a02
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 7 deletions.
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 @@ -423,6 +423,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 @@ -494,6 +498,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 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>route</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 @@ -330,6 +330,7 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
data={createDataInferenceService}
setData={setCreateDataInferenceService}
allowCreate={allowCreate}
publicRoute
/>
</StackItem>
)}
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/pages/modelServing/screens/projects/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ export const isServingRuntimeRouteEnabled = (servingRuntime: ServingRuntimeKind)
export const isInferenceServiceTokenEnabled = (inferenceService: InferenceServiceKind): boolean =>
inferenceService.metadata.annotations?.['security.opendatahub.io/enable-auth'] === 'true';

export const isInferenceServiceRouteEnabled = (inferenceService: InferenceServiceKind): boolean =>
inferenceService.metadata.labels?.['networking.knative.dev/visibility'] !== 'cluster-local';

export const isGpuDisabled = (servingRuntime: ServingRuntimeKind): boolean =>
servingRuntime.metadata.annotations?.['opendatahub.io/disable-gpu'] === 'true';

Expand Down Expand Up @@ -206,7 +209,8 @@ export const useCreateInferenceServiceObject = (
const existingMaxReplicas =
existingData?.spec.predictor.maxReplicas || existingServingRuntimeData?.spec.replicas || 1;

const existingExternalRoute = false; // TODO: Change this in the future in case we have an External Route
const existingExternalRoute =
existingData?.metadata.labels?.['networking.knative.dev/visibility'] !== 'cluster-local';
const existingTokenAuth =
existingData?.metadata.annotations?.['security.opendatahub.io/enable-auth'] === 'true';

Expand Down

0 comments on commit 53e0a02

Please sign in to comment.