Skip to content

Commit c751590

Browse files
ashley-o0oashley-o0o
committed
Fallback to YAML on edit
1 parent f0b4499 commit c751590

File tree

7 files changed

+348
-16
lines changed

7 files changed

+348
-16
lines changed

‎packages/cypress/cypress/tests/mocked/modelServing/modelServingDeploy.cy.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1946,6 +1946,52 @@ describe('Model Serving Deploy Wizard', () => {
19461946
expect(interceptions).to.have.length(0);
19471947
});
19481948
});
1949+
1950+
it('should auto-fallback to YAML edit mode if the form data extraction fails', () => {
1951+
initIntercepts({});
1952+
initMockConnectionSecretIntercepts({});
1953+
initMockModelAuthIntercepts({});
1954+
1955+
const unparseableDeployment = mockLLMInferenceServiceK8sResource({
1956+
name: 'unparseable-model',
1957+
displayName: 'Unparseable Model',
1958+
modelUri: 's3://my-bucket/my-model',
1959+
});
1960+
delete unparseableDeployment.metadata.annotations?.['opendatahub.io/model-type'];
1961+
1962+
cy.interceptK8sList(
1963+
{ model: LLMInferenceServiceModel, ns: 'test-project' },
1964+
mockK8sResourceList([unparseableDeployment]),
1965+
);
1966+
cy.interceptK8sList(
1967+
{ model: ServingRuntimeModel, ns: 'test-project' },
1968+
mockK8sResourceList([]),
1969+
);
1970+
cy.interceptK8s(
1971+
'GET',
1972+
{ model: LLMInferenceServiceModel, ns: 'test-project', name: 'unparseable-model' },
1973+
unparseableDeployment,
1974+
);
1975+
cy.interceptK8s(
1976+
'PUT',
1977+
{ model: LLMInferenceServiceModel, ns: 'test-project', name: 'unparseable-model' },
1978+
{
1979+
statusCode: 200,
1980+
body: unparseableDeployment,
1981+
},
1982+
).as('updateLLMInferenceService');
1983+
1984+
modelServingGlobal.visit('test-project');
1985+
modelServingGlobal.getModelRow('Unparseable Model').findKebabAction('Edit').click();
1986+
modelServingWizard.findYAMLViewerToggle('YAML').should('have.attr', 'aria-pressed', 'true');
1987+
modelServingWizard.findYAMLViewerToggle('Form').should('be.disabled');
1988+
1989+
cy.findByTestId('yaml-fallback-alert').should('exist');
1990+
cy.findByTestId('yaml-fallback-alert').should(
1991+
'contain.text',
1992+
'This deployment contains custom configuration that cannot be displayed in the form',
1993+
);
1994+
});
19491995
});
19501996

19511997
describe('redirect from v2 to v3 route', () => {

‎packages/model-serving/src/components/deploymentWizard/ModelDeploymentWizard.tsx‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
Wizard,
77
WizardStep,
88
} from '@patternfly/react-core';
9+
import { stringify } from 'yaml';
910
import ApplicationsPage from '@odh-dashboard/internal/pages/ApplicationsPage';
1011
import { ProjectKind } from '@odh-dashboard/internal/k8sTypes';
1112
import {
@@ -62,7 +63,9 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
6263
useExitDeploymentWizard({ returnRoute, cancelReturnRoute });
6364

6465
const isYAMLViewerEnabled = useIsAreaAvailable(SupportedArea.YAML_VIEWER).status;
65-
const [viewMode, setViewMode] = React.useState<ModelDeploymentWizardViewMode>('form');
66+
const [viewMode, setViewMode] = React.useState<ModelDeploymentWizardViewMode>(
67+
existingData?.viewMode ?? 'form',
68+
);
6669

6770
// External data state - loaded by ExternalDataLoader component
6871
const [externalData, setExternalData] = React.useState<ExternalDataMap>({});
@@ -119,6 +122,13 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
119122
error: yamlError,
120123
} = useFormYamlResources(formResources);
121124

125+
const isAutoFallback = existingData?.viewMode === 'yaml-edit';
126+
React.useEffect(() => {
127+
if (isAutoFallback && existingDeployment && !yaml) {
128+
setYaml(stringify(existingDeployment.model));
129+
}
130+
}, [isAutoFallback, existingDeployment, yaml, setYaml]);
131+
122132
const { onSave, onOverwrite, isLoading, submitError, clearSubmitError } =
123133
useModelDeploymentSubmit(
124134
wizardFormData.state,
@@ -172,7 +182,7 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
172182
buttonId="form-view"
173183
isSelected={viewMode === 'form'}
174184
onChange={() => setViewMode('form')}
175-
isDisabled={viewMode === 'yaml-edit'}
185+
isDisabled={viewMode === 'yaml-edit' || existingData?.viewMode === 'yaml-edit'}
176186
/>
177187
<ToggleGroupItem
178188
data-testid="yaml-view"
@@ -202,6 +212,8 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
202212
viewMode={viewMode}
203213
setViewMode={setViewMode}
204214
canEnterYAMLEditMode={existingDeployment?.model.kind !== 'InferenceService'}
215+
existingDeployment={existingDeployment}
216+
isAutoFallback={existingData?.viewMode === 'yaml-edit'}
205217
/>
206218
</PageSection>
207219
<PageSection hasBodyWrapper={false} isFilled={false} style={{ paddingTop: 0 }}>

‎packages/model-serving/src/components/deploymentWizard/extractionValidation.ts‎

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
import type { InitialWizardFormData } from './types';
2+
import type { Deployment } from '../../../extension-points';
3+
4+
export enum ExtractionIncompleteReason {
5+
MISSING_MODEL_TYPE = 'missing-model-type',
6+
MISSING_CONNECTION = 'missing-connection',
7+
AUTOSCALING_NOT_SUPPORTED = 'autoscaling-not-supported',
8+
TOLERATIONS_NOT_SUPPORTED = 'tolerations-not-supported',
9+
NODE_SELECTOR_NOT_SUPPORTED = 'node-selector-not-supported',
10+
MULTIPLE_CONTAINERS_NOT_SUPPORTED = 'multiple-containers-not-supported',
11+
CUSTOM_ROUTER_NOT_SUPPORTED = 'custom-router-not-supported',
12+
MISSING_CRITICAL_FIELD = 'missing-critical-field',
13+
EXTRACTION_ERROR = 'extraction-error',
14+
}
15+
16+
export type ExtractionIncompleteDetail = {
17+
reason: ExtractionIncompleteReason;
18+
message: string;
19+
fieldName?: string;
20+
};
21+
22+
export type ExtractionValidationResult = {
23+
isComplete: boolean;
24+
details: ExtractionIncompleteDetail[];
25+
};
26+
27+
/**
28+
* Validates whether a deployment's extracted form data is complete and parseable.
29+
* Returns details about any missing or unsupported configurations that would prevent
30+
* proper form display.
31+
*
32+
* @param deployment - The deployment being edited
33+
* @param extractedFormData - The form data extracted from the deployment
34+
* @param extractionError - Any error that occurred during extraction
35+
* @returns Validation result with completeness status and details
36+
*/
37+
export const validateExtractionCompleteness = (
38+
deployment: Deployment | undefined | null,
39+
extractedFormData: InitialWizardFormData | undefined,
40+
extractionError: Error | undefined,
41+
): ExtractionValidationResult => {
42+
const details: ExtractionIncompleteDetail[] = [];
43+
44+
// If there's no deployment, extraction is "complete" (new deployment)
45+
if (!deployment) {
46+
return { isComplete: true, details: [] };
47+
}
48+
49+
// If there was an extraction error, mark as incomplete
50+
if (extractionError) {
51+
details.push({
52+
reason: ExtractionIncompleteReason.EXTRACTION_ERROR,
53+
message: `Failed to extract deployment data: ${extractionError.message}`,
54+
});
55+
return { isComplete: false, details };
56+
}
57+
58+
// If extraction produced no data, mark as incomplete
59+
if (!extractedFormData) {
60+
details.push({
61+
reason: ExtractionIncompleteReason.EXTRACTION_ERROR,
62+
message: 'No form data could be extracted from deployment',
63+
});
64+
return { isComplete: false, details };
65+
}
66+
67+
// Check for missing model type annotation
68+
if (!extractedFormData.modelTypeField) {
69+
details.push({
70+
reason: ExtractionIncompleteReason.MISSING_MODEL_TYPE,
71+
message:
72+
'Missing model type annotation (opendatahub.io/model-type). The wizard requires this to determine if the model is predictive or generative.',
73+
fieldName: 'modelTypeField',
74+
});
75+
}
76+
77+
// Check for missing connection when model location type is EXISTING
78+
if (
79+
extractedFormData.modelLocationData?.type === 'existing' &&
80+
!extractedFormData.modelLocationData.connection
81+
) {
82+
details.push({
83+
reason: ExtractionIncompleteReason.MISSING_CONNECTION,
84+
message:
85+
'Missing connection annotation. The deployment references an existing connection but the annotation is missing.',
86+
fieldName: 'modelLocationData.connection',
87+
});
88+
}
89+
90+
// Check for KServe autoscaling (minReplicas !== maxReplicas)
91+
// For KServe deployments, check if the deployment has different min/max replicas
92+
if (deployment.model.kind === 'InferenceService' && 'spec' in deployment.model) {
93+
const { spec } = deployment.model;
94+
if (
95+
spec &&
96+
typeof spec === 'object' &&
97+
'predictor' in spec &&
98+
spec.predictor &&
99+
typeof spec.predictor === 'object'
100+
) {
101+
const { predictor } = spec;
102+
const minReplicas = 'minReplicas' in predictor ? predictor.minReplicas : undefined;
103+
const maxReplicas = 'maxReplicas' in predictor ? predictor.maxReplicas : undefined;
104+
105+
if (
106+
typeof minReplicas === 'number' &&
107+
typeof maxReplicas === 'number' &&
108+
minReplicas !== maxReplicas
109+
) {
110+
details.push({
111+
reason: ExtractionIncompleteReason.AUTOSCALING_NOT_SUPPORTED,
112+
message: `Autoscaling is configured (minReplicas: ${minReplicas}, maxReplicas: ${maxReplicas}) but is not supported in the wizard form.`,
113+
fieldName: 'numReplicas',
114+
});
115+
}
116+
117+
// Check for KServe tolerations
118+
if (
119+
'tolerations' in predictor &&
120+
Array.isArray(predictor.tolerations) &&
121+
predictor.tolerations.length > 0
122+
) {
123+
details.push({
124+
reason: ExtractionIncompleteReason.TOLERATIONS_NOT_SUPPORTED,
125+
message: `Tolerations are configured (${predictor.tolerations.length} toleration(s)) but are not supported in the wizard form.`,
126+
fieldName: 'hardwareProfile',
127+
});
128+
}
129+
130+
// Check for KServe node selectors
131+
if (
132+
'nodeSelector' in predictor &&
133+
predictor.nodeSelector &&
134+
typeof predictor.nodeSelector === 'object' &&
135+
Object.keys(predictor.nodeSelector).length > 0
136+
) {
137+
details.push({
138+
reason: ExtractionIncompleteReason.NODE_SELECTOR_NOT_SUPPORTED,
139+
message: `Node selectors are configured but are not supported in the wizard form.`,
140+
fieldName: 'hardwareProfile',
141+
});
142+
}
143+
}
144+
}
145+
146+
// Check for LLMD-specific unsupported configurations
147+
if (deployment.model.kind === 'LLMInferenceService' && 'spec' in deployment.model) {
148+
const { spec } = deployment.model;
149+
if (spec && typeof spec === 'object') {
150+
// Check for multiple containers
151+
if ('template' in spec && spec.template && typeof spec.template === 'object') {
152+
const { template } = spec;
153+
if (
154+
'containers' in template &&
155+
Array.isArray(template.containers) &&
156+
template.containers.length > 1
157+
) {
158+
details.push({
159+
reason: ExtractionIncompleteReason.MULTIPLE_CONTAINERS_NOT_SUPPORTED,
160+
message: `Multiple containers (${template.containers.length}) are configured. The wizard form only supports a single main container.`,
161+
fieldName: 'template.containers',
162+
});
163+
}
164+
165+
// Check for LLMD tolerations
166+
if (
167+
'tolerations' in template &&
168+
Array.isArray(template.tolerations) &&
169+
template.tolerations.length > 0
170+
) {
171+
details.push({
172+
reason: ExtractionIncompleteReason.TOLERATIONS_NOT_SUPPORTED,
173+
message: `Tolerations are configured (${template.tolerations.length} toleration(s)) but are not supported in the wizard form.`,
174+
fieldName: 'hardwareProfile',
175+
});
176+
}
177+
178+
// Check for LLMD node selectors
179+
if (
180+
'nodeSelector' in template &&
181+
template.nodeSelector &&
182+
typeof template.nodeSelector === 'object' &&
183+
Object.keys(template.nodeSelector).length > 0
184+
) {
185+
details.push({
186+
reason: ExtractionIncompleteReason.NODE_SELECTOR_NOT_SUPPORTED,
187+
message: `Node selectors are configured but are not supported in the wizard form.`,
188+
fieldName: 'hardwareProfile',
189+
});
190+
}
191+
}
192+
193+
// Check for custom router configuration
194+
if ('router' in spec && spec.router && typeof spec.router === 'object') {
195+
// Check if router has any non-empty configuration
196+
const hasCustomRouter = Object.entries(spec.router).some(([, value]) => {
197+
// Check if the value is not undefined, null, or an empty object
198+
if (value == null) {
199+
return false;
200+
}
201+
if (typeof value === 'object' && Object.keys(value).length === 0) {
202+
return false;
203+
}
204+
return true;
205+
});
206+
207+
if (hasCustomRouter) {
208+
details.push({
209+
reason: ExtractionIncompleteReason.CUSTOM_ROUTER_NOT_SUPPORTED,
210+
message:
211+
'Custom router configuration is present but is not supported in the wizard form.',
212+
fieldName: 'router',
213+
});
214+
}
215+
}
216+
}
217+
}
218+
219+
return {
220+
isComplete: details.length === 0,
221+
details,
222+
};
223+
};

‎packages/model-serving/src/components/deploymentWizard/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ export type InitialWizardFormData = {
104104
// wizard
105105
wizardStartIndex?: number;
106106
isEditing?: boolean;
107+
viewMode?: 'form' | 'yaml-preview' | 'yaml-edit';
107108
// fields
108109
project?: ProjectKind | null;
109110
modelTypeField?: ModelTypeFieldData;

‎packages/model-serving/src/components/deploymentWizard/useExtractFormDataFromDeployment.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React from 'react';
22
import { setupDefaults } from '@odh-dashboard/internal/concepts/k8s/K8sNameDescriptionField/utils';
33
import { useDashboardNamespace } from '@odh-dashboard/internal/redux/selectors/project';
4+
import { validateExtractionCompleteness } from './extractionValidation';
45
import { type InitialWizardFormData } from './types';
56
import {
67
getModelTypeFromDeployment,
@@ -81,7 +82,7 @@ export const useExtractFormDataFromDeployment = (
8182
!deployment || (formDataExtensionLoaded && deploymentSecretsLoaded && extractorsLoaded);
8283

8384
// Memoize error computation to prevent unnecessary recalculations
84-
const error = React.useMemo((): Error | undefined => {
85+
const loadingError = React.useMemo((): Error | undefined => {
8586
if (!deployment) {
8687
return undefined;
8788
}
@@ -102,8 +103,8 @@ export const useExtractFormDataFromDeployment = (
102103

103104
// Memoize the form data extraction to prevent unnecessary recalculations
104105
const formData = React.useMemo((): InitialWizardFormData | undefined => {
105-
// Only extract form data if everything is loaded and there are no errors
106-
if (!deployment || !loaded || error) {
106+
// Only extract form data if everything is loaded and there are no loading errors
107+
if (!deployment || !loaded || loadingError) {
107108
return undefined;
108109
}
109110

@@ -172,10 +173,25 @@ export const useExtractFormDataFromDeployment = (
172173
deploymentSecrets,
173174
dashboardNamespace,
174175
loaded,
175-
error,
176+
loadingError,
176177
extractedFieldData,
177178
]);
178179

180+
const validationError = React.useMemo((): Error | undefined => {
181+
if (!deployment || loadingError) {
182+
return undefined;
183+
}
184+
const validationResult = validateExtractionCompleteness(deployment, formData, loadingError);
185+
if (!validationResult.isComplete) {
186+
return new Error(
187+
'This deployment contains custom configuration that cannot be displayed in the form.',
188+
);
189+
}
190+
return undefined;
191+
}, [deployment, formData, loadingError]);
192+
193+
const error = loadingError || validationError;
194+
179195
return {
180196
formData,
181197
loaded,

0 commit comments

Comments
 (0)