Skip to content

Commit

Permalink
Improve Model Server modal error handling with dynamic spec (#934)
Browse files Browse the repository at this point in the history 
Add error handling for the dynamic spec of the ServingRuntime object.
Fix https://issues.redhat.com/browse/RHODS-6538
  • Loading branch information
@lucferbux
lucferbux authored Feb 17, 2023
1 parent b989852 commit 01da86e
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 96 deletions.
61 changes: 23 additions & 38 deletions frontend/src/api/network/servingRuntimes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,43 +7,14 @@ import {
k8sUpdateResource,
} from '@openshift/dynamic-plugin-sdk-utils';
import { ServingRuntimeModel } from '../models';
import { ServingRuntimeKind } from '../../k8sTypes';
import { ConfigMapKind, ServingRuntimeKind } from '../../k8sTypes';
import { CreatingServingRuntimeObject } from 'pages/modelServing/screens/types';
import { getModelServingRuntimeName } from 'pages/modelServing/utils';
import { getModelServingProjects } from './projects';
import { getConfigMap } from './configMaps';
import { DEFAULT_MODEL_SERVING_TEMPLATE } from 'pages/modelServing/screens/const';
import YAML from 'yaml';
import { assemblePodSpecOptions } from './utils';
import { ContainerResources } from '../../types';

const fetchServingRuntime = (
data: CreatingServingRuntimeObject,
configNamespace: string,
namespace: string,
): Promise<ServingRuntimeKind> => {
//TODO: remove fetch servingruntimes-config here and pass in the previously fetched valued.
// Leaving it in for now to keep the original behavior when no GPU is configured.
return getConfigMap(configNamespace, 'servingruntimes-config')
.then((configmap) => {
const overrideServingRuntime = YAML.parse(configmap.data?.['override-config'] || '');
if (overrideServingRuntime) {
return assembleServingRuntime(data, namespace, overrideServingRuntime);
}
const defaultServingRuntime = YAML.parse(configmap.data?.['default-config'] || '');
if (defaultServingRuntime === null) {
throw new Error(
'servingruntimes-config is misconfigured or key might be missing from the ConfigMap',
);
}
return assembleServingRuntime(data, namespace, defaultServingRuntime);
})
.catch((e) => {
console.error(`${e}, using default config.`);
const servingRuntime = DEFAULT_MODEL_SERVING_TEMPLATE;
return assembleServingRuntime(data, namespace, servingRuntime);
});
};
import { getDefaultServingRuntime } from 'pages/modelServing/screens/projects/utils';
import { DEFAULT_MODEL_SERVING_TEMPLATE } from 'pages/modelServing/screens/const';

const assembleServingRuntime = (
data: CreatingServingRuntimeObject,
Expand Down Expand Up @@ -163,16 +134,30 @@ export const updateServingRuntime = (
});
};

const getAssembledServingRuntime = (
data: CreatingServingRuntimeObject,
servingRuntimeConfig: ConfigMapKind | undefined,
namespace: string,
): ServingRuntimeKind => {
const servingRuntime = getDefaultServingRuntime(servingRuntimeConfig);

try {
return assembleServingRuntime(data, namespace, servingRuntime);
} catch {
return assembleServingRuntime(data, namespace, DEFAULT_MODEL_SERVING_TEMPLATE);
}
};

export const createServingRuntime = (
data: CreatingServingRuntimeObject,
configNamespace: string,
servingRuntimeConfig: ConfigMapKind | undefined,
namespace: string,
): Promise<ServingRuntimeKind> => {
return fetchServingRuntime(data, configNamespace, namespace).then((servingRuntime) => {
return k8sCreateResource<ServingRuntimeKind>({
model: ServingRuntimeModel,
resource: servingRuntime,
});
const assembledServingRuntime = getAssembledServingRuntime(data, servingRuntimeConfig, namespace);

return k8sCreateResource<ServingRuntimeKind>({
model: ServingRuntimeModel,
resource: assembledServingRuntime,
});
};

Expand Down
117 changes: 71 additions & 46 deletions ...src/pages/modelServing/screens/projects/ServingRuntimeModal/ManageServingRuntimeModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import ServingRuntimeReplicaSection from './ServingRuntimeReplicaSection';
import ServingRuntimeSizeSection from './ServingRuntimeSizeSection';
import ServingRuntimeTokenSection from './ServingRuntimeTokenSection';
import { translateDisplayNameForK8s } from 'pages/projects/utils';
import { useDashboardNamespace } from 'redux/selectors';
import { requestsUnderLimits, resourcesArePositive } from '../../../utils';

type ManageServingRuntimeModalProps = {
Expand All @@ -58,23 +57,36 @@ const ManageServingRuntimeModal: React.FC<ManageServingRuntimeModalProps> = ({
const [actionInProgress, setActionInProgress] = React.useState(false);
const [error, setError] = React.useState<Error | undefined>();

const { currentProject } = React.useContext(ProjectDetailsContext);

const { dashboardNamespace } = useDashboardNamespace();
const {
currentProject,
servingRuntimesConfig: {
servingRuntimesConfig,
refresh: servingRuntimeConfigRefresh,
loaded: servingRuntimeConfigLoaded,
},
} = React.useContext(ProjectDetailsContext);

const namespace = currentProject.metadata.name;

const tokenErrors = createData.tokens.filter((token) => token.error !== '').length > 0;

const inputValueValid =
servingRuntimeConfigLoaded &&
createData.numReplicas >= 0 &&
resourcesArePositive(createData.modelSize.resources) &&
requestsUnderLimits(createData.modelSize.resources);

const canCreate = !actionInProgress && !tokenErrors && inputValueValid;

React.useEffect(() => {
if (isOpen) {
servingRuntimeConfigRefresh();
}
}, [isOpen, servingRuntimeConfigRefresh]);

const onBeforeClose = (submitted: boolean) => {
onClose(submitted);
setError(undefined);
setActionInProgress(false);
resetData();
};
Expand Down Expand Up @@ -121,7 +133,7 @@ const ManageServingRuntimeModal: React.FC<ManageServingRuntimeModalProps> = ({
const deletedSecrets = (editInfo?.secrets || [])
.map((secret) => secret.metadata.name)
.filter((token) => !createData.tokens.some((tokenEdit) => tokenEdit.editName === token));
allSettledPromises<K8sStatus | SecretKind, Error>([
Promise.all<K8sStatus | SecretKind>([
...createData.tokens
.filter((token) => translateDisplayNameForK8s(token.name) !== token.editName)
.map((token) => {
Expand Down Expand Up @@ -164,11 +176,11 @@ const ManageServingRuntimeModal: React.FC<ManageServingRuntimeModalProps> = ({
})
.catch((e) => setErrorModal(e));
} else {
allSettledPromises<ServingRuntimeKind | string | void, Error>([
Promise.all<ServingRuntimeKind | string | void>([
...(currentProject.metadata.labels?.['modelmesh-enabled']
? [addSupportModelMeshProject(currentProject.metadata.name)]
: []),
createServingRuntime(createData, dashboardNamespace, namespace),
createServingRuntime(createData, servingRuntimesConfig, namespace),
enableTokenAuth(),
])
.then(() => {
Expand All @@ -187,55 +199,68 @@ const ManageServingRuntimeModal: React.FC<ManageServingRuntimeModalProps> = ({
onClose={() => onBeforeClose(false)}
showClose
actions={[
<Button key="submit-model" variant="primary" isDisabled={!canCreate} onClick={submit}>
<Button
key="submit-model"
variant="primary"
isDisabled={!canCreate}
onClick={submit}
isLoading={actionInProgress}
>
Configure
</Button>,
<Button key="cancel-model" variant="secondary" onClick={() => onBeforeClose(false)}>
Cancel
</Button>,
]}
>
<Form
onSubmit={(e) => {
e.preventDefault();
submit();
}}
>
<Stack hasGutter>
<StackItem>
<ServingRuntimeReplicaSection data={createData} setData={setCreateData} />
</StackItem>
<StackItem>
<ServingRuntimeSizeSection
data={createData}
setData={setCreateData}
sizes={sizes}
gpuSetting={gpuSetting}
/>
</StackItem>
<StackItem>
<FormSection title="Model route" titleElement="div">
<FormGroup>
<Checkbox
label="Make deployed models available through an external route"
id="alt-form-checkbox-route"
name="alt-form-checkbox-route"
isChecked={createData.externalRoute}
onChange={(check) => setCreateData('externalRoute', check)}
<Stack hasGutter>
<StackItem>
<Form
onSubmit={(e) => {
e.preventDefault();
submit();
}}
>
<Stack hasGutter>
<StackItem>
<ServingRuntimeReplicaSection data={createData} setData={setCreateData} />
</StackItem>
<StackItem>
<ServingRuntimeSizeSection
data={createData}
setData={setCreateData}
sizes={sizes}
gpuSetting={gpuSetting}
/>
</FormGroup>
</FormSection>
</StackItem>
</StackItem>
<StackItem>
<FormSection title="Model route" titleElement="div">
<FormGroup>
<Checkbox
label="Make deployed models available through an external route"
id="alt-form-checkbox-route"
name="alt-form-checkbox-route"
isChecked={createData.externalRoute}
onChange={(check) => setCreateData('externalRoute', check)}
/>
</FormGroup>
</FormSection>
</StackItem>
<StackItem>
<ServingRuntimeTokenSection data={createData} setData={setCreateData} />
</StackItem>
</Stack>
</Form>
</StackItem>

{error && (
<StackItem>
<ServingRuntimeTokenSection data={createData} setData={setCreateData} />
<Alert isInline variant="danger" title="Error creating model server">
{error.message}
</Alert>
</StackItem>
</Stack>
</Form>
{error && (
<Alert isInline variant="danger" title="Error creating model server">
{error.message}
</Alert>
)}
)}
</Stack>
</Modal>
);
};
Expand Down
29 changes: 17 additions & 12 deletions frontend/src/pages/modelServing/screens/projects/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,27 @@ export const isServingRuntimeTokenEnabled = (servingRuntime: ServingRuntimeKind)
export const isServingRuntimeRouteEnabled = (servingRuntime: ServingRuntimeKind): boolean =>
servingRuntime.metadata.annotations?.['enable-route'] === 'true';

const getDefaultServingRuntime = (servingRuntimesConfig?: ConfigMapKind): ServingRuntimeKind => {
export const getDefaultServingRuntime = (
servingRuntimesConfig?: ConfigMapKind,
): ServingRuntimeKind => {
if (!servingRuntimesConfig) {
return DEFAULT_MODEL_SERVING_TEMPLATE;
}

const overrideServingRuntime = YAML.parse(servingRuntimesConfig?.data?.['override-config'] || '');
if (overrideServingRuntime) {
return overrideServingRuntime;
}
const defaultServingRuntime = YAML.parse(servingRuntimesConfig?.data?.['default-config'] || '');
if (defaultServingRuntime) {
return defaultServingRuntime;
try {
const overrideServingRuntime = YAML.parse(
servingRuntimesConfig?.data?.['override-config'] || '',
);
if (overrideServingRuntime) {
return overrideServingRuntime;
}
const defaultServingRuntime = YAML.parse(servingRuntimesConfig?.data?.['default-config'] || '');
if (defaultServingRuntime) {
return defaultServingRuntime;
}
} catch {
return DEFAULT_MODEL_SERVING_TEMPLATE;
}
console.error(
'servingruntimes-config is misconfigured or key might be missing from the ConfigMap, using default config.',
);

return DEFAULT_MODEL_SERVING_TEMPLATE;
};

Expand Down

0 comments on commit 01da86e

Please sign in to comment.