Skip to content

Commit

Permalink
Merge pull request #2526 from dpanshug/grpc-kserve
Browse files Browse the repository at this point in the history
Enable GRPC support for model serving routes
  • Loading branch information
openshift-merge-bot[bot] authored Mar 1, 2024
2 parents 30442a5 + 22921fd commit d076658
Show file tree
Hide file tree
Showing 26 changed files with 509 additions and 176 deletions.
4 changes: 4 additions & 0 deletions frontend/src/__mocks__/mockServingRuntimeK8sResource.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { KnownLabels, ServingRuntimeKind } from '~/k8sTypes';
import { ServingRuntimeAPIProtocol } from '~/types';

type MockResourceConfigType = {
name?: string;
Expand All @@ -8,6 +9,7 @@ type MockResourceConfigType = {
auth?: boolean;
route?: boolean;
acceleratorName?: string;
apiProtocol?: ServingRuntimeAPIProtocol;
};

export const mockServingRuntimeK8sResourceLegacy = ({
Expand Down Expand Up @@ -92,6 +94,7 @@ export const mockServingRuntimeK8sResource = ({
route = false,
displayName = 'OVMS Model Serving',
acceleratorName = '',
apiProtocol = ServingRuntimeAPIProtocol.REST,
}: MockResourceConfigType): ServingRuntimeKind => ({
apiVersion: 'serving.kserve.io/v1alpha1',
kind: 'ServingRuntime',
Expand All @@ -108,6 +111,7 @@ export const mockServingRuntimeK8sResource = ({
'enable-auth': auth ? 'true' : 'false',
'enable-route': route ? 'true' : 'false',
'openshift.io/display-name': displayName,
'opendatahub.io/apiProtocol': apiProtocol,
},
name,
namespace,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { TemplateKind } from '~/k8sTypes';
import { ServingRuntimePlatform } from '~/types';
import { ServingRuntimeAPIProtocol, ServingRuntimePlatform } from '~/types';

type MockResourceConfigType = {
name?: string;
namespace?: string;
displayName?: string;
replicas?: number;
platforms?: ServingRuntimePlatform[];
apiProtocol?: ServingRuntimeAPIProtocol;
isModelmesh?: boolean;
};

Expand All @@ -16,6 +17,7 @@ export const mockServingRuntimeTemplateK8sResource = ({
displayName = 'New OVMS Server',
replicas = 1,
isModelmesh = false,
apiProtocol = ServingRuntimeAPIProtocol.REST,
platforms,
}: MockResourceConfigType): TemplateKind => ({
apiVersion: 'template.openshift.io/v1',
Expand All @@ -28,6 +30,7 @@ export const mockServingRuntimeTemplateK8sResource = ({
},
annotations: {
'opendatahub.io/modelServingSupport': JSON.stringify(platforms),
'opendatahub.io/apiProtocol': apiProtocol,
},
},
objects: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { mockProjectK8sResource } from '~/__mocks__/mockProjectK8sResource';
import { mockServingRuntimeTemplateK8sResource } from '~/__mocks__/mockServingRuntimeTemplateK8sResource';
import { mockStatus } from '~/__mocks__/mockStatus';
import { servingRuntimes } from '~/__tests__/cypress/cypress/pages/servingRuntimes';
import { ServingRuntimePlatform } from '~/types';
import { ServingRuntimeAPIProtocol, ServingRuntimePlatform } from '~/types';

describe('Custom serving runtimes', () => {
beforeEach(() => {
Expand All @@ -17,12 +17,13 @@ describe('Custom serving runtimes', () => {
mockServingRuntimeTemplateK8sResource({
name: 'template-1',
displayName: 'Multi Platform',
platforms: [ServingRuntimePlatform.SINGLE, ServingRuntimePlatform.MULTI],
platforms: [ServingRuntimePlatform.SINGLE],
}),
mockServingRuntimeTemplateK8sResource({
name: 'template-2',
displayName: 'Caikit',
platforms: [ServingRuntimePlatform.SINGLE],
apiProtocol: ServingRuntimeAPIProtocol.GRPC,
}),
mockServingRuntimeTemplateK8sResource({
name: 'template-3',
Expand All @@ -48,20 +49,53 @@ describe('Custom serving runtimes', () => {
});

it('should display platform labels in table rows', () => {
servingRuntimes.getRowById('template-1').shouldBeSingleModel(true).shouldBeMultiModel(true);
servingRuntimes.getRowById('template-2').shouldBeSingleModel(true).shouldBeMultiModel(false);
servingRuntimes.getRowById('template-3').shouldBeSingleModel(false).shouldBeMultiModel(true);
servingRuntimes.getRowById('template-4').shouldBeSingleModel(false).shouldBeMultiModel(true);
servingRuntimes.getRowById('template-1').shouldBeSingleModel(true);
servingRuntimes.getRowById('template-2').shouldBeSingleModel(true);
servingRuntimes.getRowById('template-3').shouldBeMultiModel(true);
servingRuntimes.getRowById('template-4').shouldBeMultiModel(true);
});

it('should display api protocol in table row', () => {
servingRuntimes.getRowById('template-1').shouldHaveAPIProtocol(ServingRuntimeAPIProtocol.REST);
servingRuntimes.getRowById('template-2').shouldHaveAPIProtocol(ServingRuntimeAPIProtocol.GRPC);
servingRuntimes.getRowById('template-3').shouldHaveAPIProtocol(ServingRuntimeAPIProtocol.REST);
servingRuntimes.getRowById('template-4').shouldHaveAPIProtocol(ServingRuntimeAPIProtocol.REST);
});

it('should add a new serving runtime', () => {
servingRuntimes.findAddButton().click();
cy.get('h1').should('contain', 'Add serving runtime');
servingRuntimes.shouldDisplayValues([

// Check serving runtime dropdown list
servingRuntimes.shouldDisplayServingRuntimeValues([
'Single-model serving platform',
'Multi-model serving platform',
'Single-model and multi-model serving platforms',
]);
servingRuntimes.findSelectServingPlatformButton().click();

// Create with single model
servingRuntimes.findCreateButton().should('be.disabled');
servingRuntimes.shouldSelectPlatform('Single-model serving platform');
servingRuntimes.shouldDisplayAPIProtocolValues([
ServingRuntimeAPIProtocol.REST,
ServingRuntimeAPIProtocol.GRPC,
]);
servingRuntimes.shouldSelectAPIProtocol(ServingRuntimeAPIProtocol.REST);
servingRuntimes.findStartFromScratchButton().click();
servingRuntimes.shouldEnterData();
servingRuntimes.findCreateButton().should('be.enabled');
servingRuntimes.findCancelButton().click();

servingRuntimes.findAddButton().click();

// Create with multi model
servingRuntimes.findCreateButton().should('be.disabled');
servingRuntimes.shouldSelectPlatform('Multi-model serving platform');
servingRuntimes.findSelectAPIProtocolButton().should('not.be.enabled');
servingRuntimes.findSelectAPIProtocolButton().should('include.text', 'REST');
servingRuntimes.findStartFromScratchButton().click();
servingRuntimes.shouldEnterData();
servingRuntimes.findCreateButton().should('be.enabled');
});

it('should duplicate a serving runtime', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,9 @@ describe('Serving Runtime List', () => {
// Check status of deployed model which loaded successfully after an error
modelServingSection.findStatusTooltip('Loaded model').should('be.visible');
modelServingSection.findStatusTooltipValue('Loaded model', 'Loaded');

// Check API protocol in row
modelServingSection.findAPIProtocol('Loaded model').should('have.text', 'REST');
});
});

Expand Down
6 changes: 6 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/modelServing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,12 @@ class ModelServingSection {
this.findStatusTooltip(name).trigger('mouseleave');
});
}

findAPIProtocol(name: string) {
return this.findInferenceServiceTable()
.contains('tr', name)
.find('td[data-label="API protocol"]');
}
}

export const modelServingGlobal = new ModelServingGlobal();
Expand Down
48 changes: 44 additions & 4 deletions frontend/src/__tests__/cypress/cypress/pages/servingRuntimes.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { appChrome } from '~/__tests__/cypress/cypress/pages/appChrome';
import { ServingRuntimeAPIProtocol } from '~/types';

class ServingRuntimeRow {
constructor(public readonly id: string) {}
Expand All @@ -22,6 +23,10 @@ class ServingRuntimeRow {
.should(enabled ? 'exist' : 'not.exist');
return this;
}

shouldHaveAPIProtocol(apiProtocol: ServingRuntimeAPIProtocol) {
this.find().get('[data-label="API protocol"]').should('include.text', apiProtocol);
}
}

class ServingRuntimes {
Expand Down Expand Up @@ -54,16 +59,51 @@ class ServingRuntimes {
return cy.findByRole('button', { name: 'Add serving runtime' });
}

findSelectValueButton() {
return cy.findByRole('button', { name: 'Select a value' });
findStartFromScratchButton() {
return cy.findByRole('button', { name: 'Start from scratch' });
}

findCreateButton() {
return cy.findByRole('button', { name: 'Create' });
}

findCancelButton() {
return cy.findByRole('button', { name: 'Cancel' });
}

findSelectServingPlatformButton() {
return cy.findByTestId('custom-serving-runtime-selection');
}

findSelectAPIProtocolButton() {
return cy.findByTestId('custom-serving-api-protocol-selection');
}

shouldDisplayValues(values: string[]) {
this.findSelectValueButton().click();
shouldDisplayServingRuntimeValues(values: string[]) {
this.findSelectServingPlatformButton().click();
values.forEach((value) => cy.findByRole('menuitem', { name: value }).should('exist'));
return this;
}

shouldDisplayAPIProtocolValues(values: ServingRuntimeAPIProtocol[]) {
this.findSelectAPIProtocolButton().click();
values.forEach((value) => cy.findByRole('menuitem', { name: value }).should('exist'));
return this;
}

shouldSelectPlatform(value: string) {
this.findSelectServingPlatformButton().click();
cy.findByRole('menuitem', { name: value }).click();
}

shouldSelectAPIProtocol(value: string) {
cy.findByRole('menuitem', { name: value }).click();
}

shouldEnterData() {
cy.get('.view-lines.monaco-mouse-cursor-text').type('test');
}

getRowById(id: string) {
return new ServingRuntimeRow(id);
}
Expand Down
19 changes: 14 additions & 5 deletions frontend/src/api/k8s/__tests__/templates.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { mockServingRuntimeTemplateK8sResource } from '~/__mocks__/mockServingRu
import { assembleServingRuntimeTemplate, deleteTemplate, listTemplates } from '~/api';
import { TemplateModel } from '~/api/models';
import { K8sDSGResource, TemplateKind } from '~/k8sTypes';
import { ServingRuntimePlatform } from '~/types';
import { ServingRuntimeAPIProtocol, ServingRuntimePlatform } from '~/types';
import { genRandomChars } from '~/utilities/string';

jest.mock('@openshift/dynamic-plugin-sdk-utils', () => ({
Expand Down Expand Up @@ -38,6 +38,7 @@ describe('assembleServingRuntimeTemplate', () => {
servingRuntimeMock,
namespace,
[ServingRuntimePlatform.MULTI],
ServingRuntimeAPIProtocol.REST,
'template-1',
);
expect(result).toStrictEqual(
Expand All @@ -47,9 +48,12 @@ describe('assembleServingRuntimeTemplate', () => {
it('should assemble serving runtime template without templateName', () => {
genRandomCharsMock.mockReturnValue('123');
const servingRuntimeMock = JSON.stringify(createServingRuntime('template-123'));
const result = assembleServingRuntimeTemplate(servingRuntimeMock, namespace, [
ServingRuntimePlatform.MULTI,
]);
const result = assembleServingRuntimeTemplate(
servingRuntimeMock,
namespace,
[ServingRuntimePlatform.MULTI],
ServingRuntimeAPIProtocol.REST,
);
expect(result).toStrictEqual(
mockServingRuntimeTemplateK8sResource({
name: 'template-123',
Expand All @@ -61,7 +65,12 @@ describe('assembleServingRuntimeTemplate', () => {
it('should throw an error when servingRuntime name doesnt exist', () => {
const servingRuntimeMock = JSON.stringify(createServingRuntime(''));
expect(() => {
assembleServingRuntimeTemplate(servingRuntimeMock, namespace, [ServingRuntimePlatform.MULTI]);
assembleServingRuntimeTemplate(
servingRuntimeMock,
namespace,
[ServingRuntimePlatform.MULTI],
ServingRuntimeAPIProtocol.REST,
);
}).toThrow('Serving runtime name is required');
});
});
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/api/k8s/templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import { k8sDeleteResource, k8sListResource } from '@openshift/dynamic-plugin-sd
import { ServingRuntimeKind, TemplateKind } from '~/k8sTypes';
import { TemplateModel } from '~/api/models';
import { genRandomChars } from '~/utilities/string';
import { ServingRuntimePlatform } from '~/types';
import { ServingRuntimeAPIProtocol, ServingRuntimePlatform } from '~/types';

export const assembleServingRuntimeTemplate = (
body: string,
namespace: string,
platforms: ServingRuntimePlatform[],
apiProtocol: ServingRuntimeAPIProtocol | undefined,
templateName?: string,
): TemplateKind & { objects: ServingRuntimeKind[] } => {
const servingRuntime: ServingRuntimeKind = YAML.parse(body);
Expand All @@ -30,6 +31,7 @@ export const assembleServingRuntimeTemplate = (
},
annotations: {
'opendatahub.io/modelServingSupport': JSON.stringify(platforms),
...(apiProtocol && { 'opendatahub.io/apiProtocol': apiProtocol }),
},
},
objects: [servingRuntime],
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/k8sTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export type ServingRuntimeAnnotations = Partial<{
'opendatahub.io/disable-gpu': string;
'opendatahub.io/recommended-accelerators': string;
'opendatahub.io/accelerator-name': string;
'opendatahub.io/apiProtocol': string;
'enable-route': string;
'enable-auth': string;
'modelmesh-enabled': 'true' | 'false';
Expand Down Expand Up @@ -1074,6 +1075,7 @@ export type TemplateKind = K8sResourceCommon & {
iconClass?: string;
'opendatahub.io/template-enabled': string;
'opendatahub.io/modelServingSupport': string;
'opendatahub.io/apiProtocol': string;
}>;
name: string;
namespace: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import * as React from 'react';
import { Label, Text, TextVariants } from '@patternfly/react-core';
import { TemplateKind } from '~/k8sTypes';
import {
getAPIProtocolFromTemplate,
getEnabledPlatformsFromTemplate,
} from '~/pages/modelServing/customServingRuntimes/utils';
import { ServingRuntimeAPIProtocol, ServingRuntimePlatform } from '~/types';

type CustomServingRuntimeAPIProtocolLabelProps = {
template: TemplateKind;
};

const CustomServingRuntimeAPIProtocolLabel: React.FC<CustomServingRuntimeAPIProtocolLabelProps> = ({
template,
}) => {
const apiProtocol = getAPIProtocolFromTemplate(template);
const isMultiModel = getEnabledPlatformsFromTemplate(template).includes(
ServingRuntimePlatform.MULTI,
);

// If it is multi-model, we use REST as default
if (!apiProtocol && isMultiModel) {
return <Label color="gold">{ServingRuntimeAPIProtocol.REST}</Label>;
}

if (!apiProtocol || !Object.values(ServingRuntimeAPIProtocol).includes(apiProtocol)) {
return <Text component={TextVariants.small}>Not defined</Text>;
}

return <Label color="gold">{apiProtocol}</Label>;
};

export default CustomServingRuntimeAPIProtocolLabel;
Loading

0 comments on commit d076658

Please sign in to comment.