Skip to content

Commit

Permalink
Merge pull request #1982 from DaoDaoNoCode/upstream-issue-1913
Browse files Browse the repository at this point in the history
Add model serving platform settings
  • Loading branch information
openshift-ci[bot] authored Oct 25, 2023
2 parents bfb3aa8 + 14920ea commit c5c3596
Show file tree
Hide file tree
Showing 9 changed files with 202 additions and 7 deletions.
35 changes: 31 additions & 4 deletions backend/src/routes/api/cluster-settings/clusterSettingsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ const DEFAULT_CLUSTER_SETTINGS: ClusterSettings = {
cullerTimeout: DEFAULT_CULLER_TIMEOUT,
userTrackingEnabled: false,
notebookTolerationSettings: { enabled: false, key: 'NotebooksOnly' },
modelServingPlatformEnabled: {
kServe: true,
modelMesh: false,
},
};

export const updateClusterSettings = async (
Expand All @@ -28,10 +32,30 @@ export const updateClusterSettings = async (
): Promise<{ success: boolean; error: string }> => {
const coreV1Api = fastify.kube.coreV1Api;
const namespace = fastify.kube.namespace;
const { pvcSize, cullerTimeout, userTrackingEnabled, notebookTolerationSettings } = request.body;
const {
pvcSize,
cullerTimeout,
userTrackingEnabled,
notebookTolerationSettings,
modelServingPlatformEnabled,
} = request.body;
const dashConfig = getDashboardConfig();
const isJupyterEnabled = checkJupyterEnabled();
try {
if (
modelServingPlatformEnabled.kServe !== !dashConfig.spec.dashboardConfig.disableKServe ||
modelServingPlatformEnabled.modelMesh !== !dashConfig.spec.dashboardConfig.disableModelMesh
) {
await setDashboardConfig(fastify, {
spec: {
dashboardConfig: {
disableKServe: !modelServingPlatformEnabled.kServe,
disableModelMesh: !modelServingPlatformEnabled.modelMesh,
},
},
});
}

await patchCM(fastify, segmentKeyCfg, {
data: { segmentKeyEnabled: String(userTrackingEnabled) },
}).catch((e) => {
Expand All @@ -41,7 +65,6 @@ export const updateClusterSettings = async (
if (isJupyterEnabled) {
await setDashboardConfig(fastify, {
spec: {
dashboardConfig: dashConfig.spec.dashboardConfig,
notebookController: {
enabled: isJupyterEnabled,
pvcSize: `${pvcSize}Gi`,
Expand Down Expand Up @@ -124,10 +147,14 @@ export const getClusterSettings = async (
): Promise<ClusterSettings | string> => {
const coreV1Api = fastify.kube.coreV1Api;
const namespace = fastify.kube.namespace;
const clusterSettings = {
const dashConfig = getDashboardConfig();
const clusterSettings: ClusterSettings = {
...DEFAULT_CLUSTER_SETTINGS,
modelServingPlatformEnabled: {
kServe: !dashConfig.spec.dashboardConfig.disableKServe,
modelMesh: !dashConfig.spec.dashboardConfig.disableModelMesh,
},
};
const dashConfig = getDashboardConfig();
const isJupyterEnabled = checkJupyterEnabled();
if (!dashConfig.spec.dashboardConfig.disableTracking) {
try {
Expand Down
4 changes: 4 additions & 0 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ export type ClusterSettings = {
cullerTimeout: number;
userTrackingEnabled: boolean;
notebookTolerationSettings: NotebookTolerationSettings | null;
modelServingPlatformEnabled: {
kServe: boolean;
modelMesh: boolean;
};
};

// Add a minimal QuickStart type here as there is no way to get types without pulling in frontend (React) modules
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/__mocks__/mockClusterSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ export const mockClusterSettings = ({
key: 'NotebooksOnlyChange',
enabled: true,
},
modelServingPlatformEnabled = {
kServe: true,
modelMesh: true,
},
}: Partial<ClusterSettingsType>): ClusterSettingsType => ({
userTrackingEnabled,
cullerTimeout,
pvcSize,
notebookTolerationSettings,
modelServingPlatformEnabled,
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,29 @@ test('Cluster settings', async ({ page }) => {
// wait for page to load
await page.waitForSelector('text=Save changes');
const submitButton = page.locator('[data-id="submit-cluster-settings"]');

// check serving platform field
const singlePlatformCheckbox = page.locator(
'[data-id="single-model-serving-platform-enabled-checkbox"]',
);
const multiPlatformCheckbox = page.locator(
'[data-id="multi-model-serving-platform-enabled-checkbox"]',
);
const warningAlert = page.locator('[data-id="serving-platform-warning-alert"]');
await expect(singlePlatformCheckbox).toBeChecked();
await expect(multiPlatformCheckbox).toBeChecked();
await expect(submitButton).toBeDisabled();
await multiPlatformCheckbox.uncheck();
await expect(warningAlert).toBeVisible();
expect(warningAlert.getByLabel('Info Alert')).toBeTruthy();
await expect(submitButton).toBeEnabled();
await singlePlatformCheckbox.uncheck();
expect(warningAlert.getByLabel('Warning Alert')).toBeTruthy();
await singlePlatformCheckbox.check();
await multiPlatformCheckbox.check();
await expect(warningAlert).toBeHidden();
await expect(submitButton).toBeDisabled();

// check PVC size field
const pvcInputField = page.locator('[data-id="pvc-size-input"]');
const pvcHint = page.locator('[data-id="pvc-size-helper-text"]');
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/SettingSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Card, CardBody, CardFooter, CardTitle, Stack, StackItem } from '@patter
type SettingSectionProps = {
children: React.ReactNode;
title: string;
description?: string;
description?: React.ReactNode;
footer?: React.ReactNode;
};

Expand Down
28 changes: 26 additions & 2 deletions frontend/src/pages/clusterSettings/ClusterSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@ import { Button, Stack, StackItem } from '@patternfly/react-core';
import ApplicationsPage from '~/pages/ApplicationsPage';
import { useAppContext } from '~/app/AppContext';
import { fetchClusterSettings, updateClusterSettings } from '~/services/clusterSettingsService';
import { ClusterSettingsType, NotebookTolerationFormSettings } from '~/types';
import {
ClusterSettingsType,
ModelServingPlatformEnabled,
NotebookTolerationFormSettings,
} from '~/types';
import { addNotification } from '~/redux/actions/actions';
import { useCheckJupyterEnabled } from '~/utilities/notebookControllerUtils';
import { useAppDispatch } from '~/redux/hooks';
import PVCSizeSettings from '~/pages/clusterSettings/PVCSizeSettings';
import CullerSettings from '~/pages/clusterSettings/CullerSettings';
import TelemetrySettings from '~/pages/clusterSettings/TelemetrySettings';
import TolerationSettings from '~/pages/clusterSettings/TolerationSettings';
import ModelServingPlatformSettings from '~/pages/clusterSettings/ModelServingPlatformSettings';
import {
DEFAULT_CONFIG,
DEFAULT_PVC_SIZE,
Expand All @@ -35,12 +40,15 @@ const ClusterSettings: React.FC = () => {
enabled: false,
key: isJupyterEnabled ? DEFAULT_TOLERATION_VALUE : '',
});
const [modelServingEnabledPlatforms, setModelServingEnabledPlatforms] =
React.useState<ModelServingPlatformEnabled>(clusterSettings.modelServingPlatformEnabled);
const dispatch = useAppDispatch();

React.useEffect(() => {
fetchClusterSettings()
.then((clusterSettings: ClusterSettingsType) => {
setClusterSettings(clusterSettings);
setModelServingEnabledPlatforms(clusterSettings.modelServingPlatformEnabled);
setLoaded(true);
setLoadError(undefined);
})
Expand All @@ -59,8 +67,16 @@ const ClusterSettings: React.FC = () => {
enabled: notebookTolerationSettings.enabled,
key: notebookTolerationSettings.key,
},
modelServingPlatformEnabled: modelServingEnabledPlatforms,
}),
[pvcSize, cullerTimeout, userTrackingEnabled, clusterSettings, notebookTolerationSettings],
[
pvcSize,
cullerTimeout,
userTrackingEnabled,
clusterSettings,
notebookTolerationSettings,
modelServingEnabledPlatforms,
],
);

const handleSaveButtonClicked = () => {
Expand All @@ -72,6 +88,7 @@ const ClusterSettings: React.FC = () => {
enabled: notebookTolerationSettings.enabled,
key: notebookTolerationSettings.key,
},
modelServingPlatformEnabled: modelServingEnabledPlatforms,
};
if (!_.isEqual(clusterSettings, newClusterSettings)) {
if (
Expand Down Expand Up @@ -123,6 +140,13 @@ const ClusterSettings: React.FC = () => {
provideChildrenPadding
>
<Stack hasGutter>
<StackItem>
<ModelServingPlatformSettings
initialValue={clusterSettings.modelServingPlatformEnabled}
enabledPlatforms={modelServingEnabledPlatforms}
setEnabledPlatforms={setModelServingEnabledPlatforms}
/>
</StackItem>
<StackItem>
<PVCSizeSettings
initialValue={clusterSettings.pvcSize}
Expand Down
102 changes: 102 additions & 0 deletions frontend/src/pages/clusterSettings/ModelServingPlatformSettings.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import * as React from 'react';
import {
Alert,
AlertActionCloseButton,
AlertVariant,
Checkbox,
Stack,
StackItem,
} from '@patternfly/react-core';
import SettingSection from '~/components/SettingSection';
import { ModelServingPlatformEnabled } from '~/types';

type ModelServingPlatformSettingsProps = {
initialValue: ModelServingPlatformEnabled;
enabledPlatforms: ModelServingPlatformEnabled;
setEnabledPlatforms: (platforms: ModelServingPlatformEnabled) => void;
};

const ModelServingPlatformSettings: React.FC<ModelServingPlatformSettingsProps> = ({
initialValue,
enabledPlatforms,
setEnabledPlatforms,
}) => {
const [alert, setAlert] = React.useState<{ variant: AlertVariant; message: string }>();

React.useEffect(() => {
if (!enabledPlatforms.kServe && !enabledPlatforms.modelMesh) {
setAlert({
variant: AlertVariant.warning,
message:
'Disabling both model serving platforms prevents new projects from deploying models. Models can still be deployed from existing projects that already have a serving platform.',
});
} else {
if (initialValue.modelMesh && !enabledPlatforms.modelMesh) {
setAlert({
variant: AlertVariant.info,
message:
'Disabling the multi-model serving platform prevents models deployed in new projects and in existing projects with no deployed models from sharing model servers. Existing projects with deployed models will continue to use multi-model serving.',
});
} else {
setAlert(undefined);
}
}
}, [enabledPlatforms, initialValue]);

return (
<SettingSection
title="Model serving platforms"
description="Select the serving platforms that projects on this cluster can use for deploying models."
>
<Stack hasGutter>
<StackItem>
<Checkbox
label="Single model serving platform"
isChecked={enabledPlatforms.kServe}
onChange={(enabled) => {
const newEnabledPlatforms: ModelServingPlatformEnabled = {
...enabledPlatforms,
kServe: enabled,
};
setEnabledPlatforms(newEnabledPlatforms);
}}
aria-label="Single model serving platform enabled checkbox"
id="single-model-serving-platform-enabled-checkbox"
data-id="single-model-serving-platform-enabled-checkbox"
name="singleModelServingPlatformEnabledCheckbox"
/>
</StackItem>
<StackItem>
<Checkbox
label="Multi-model serving platform"
isChecked={enabledPlatforms.modelMesh}
onChange={(enabled) => {
const newEnabledPlatforms: ModelServingPlatformEnabled = {
...enabledPlatforms,
modelMesh: enabled,
};
setEnabledPlatforms(newEnabledPlatforms);
}}
aria-label="Multi-model serving platform enabled checkbox"
id="multi-model-serving-platform-enabled-checkbox"
data-id="multi-model-serving-platform-enabled-checkbox"
name="multiModelServingPlatformEnabledCheckbox"
/>
</StackItem>
{alert && (
<StackItem>
<Alert
data-id="serving-platform-warning-alert"
variant={alert.variant}
title={alert.message}
isInline
actionClose={<AlertActionCloseButton onClose={() => setAlert(undefined)} />}
/>
</StackItem>
)}
</Stack>
</SettingSection>
);
};

export default ModelServingPlatformSettings;
4 changes: 4 additions & 0 deletions frontend/src/pages/clusterSettings/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ export const DEFAULT_CONFIG: ClusterSettingsType = {
cullerTimeout: DEFAULT_CULLER_TIMEOUT,
userTrackingEnabled: false,
notebookTolerationSettings: null,
modelServingPlatformEnabled: {
kServe: true,
modelMesh: false,
},
};
export const DEFAULT_TOLERATION_VALUE = 'NotebooksOnly';
export const TOLERATION_FORMAT = /^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$/;
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ export type ClusterSettingsType = {
pvcSize: number | string;
cullerTimeout: number;
notebookTolerationSettings: TolerationSettings | null;
modelServingPlatformEnabled: ModelServingPlatformEnabled;
};

export type ModelServingPlatformEnabled = {
kServe: boolean;
modelMesh: boolean;
};

/** @deprecated -- use SDK type */
Expand Down

0 comments on commit c5c3596

Please sign in to comment.