Skip to content

Commit 261403c

Browse files
katieperry4katieperry4
committed
adding dry runs
1 parent da02c26 commit 261403c

File tree

14 files changed

+436
-38
lines changed

14 files changed

+436
-38
lines changed

‎packages/cypress/cypress/support/commands/odh.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,6 +1174,11 @@ declare global {
11741174
type: 'DELETE /maas/api/v1/maasmodel/:namespace/:name',
11751175
options: { path: { namespace: string; name: string } },
11761176
response: OdhResponse<{ message: string }>,
1177+
) => Cypress.Chainable<null>) &
1178+
((
1179+
type: 'PUT /maas/api/v1/maasmodel/:namespace/:name',
1180+
options: { path: { namespace: string; name: string } },
1181+
response: OdhResponse<MaaSModelRef>,
11771182
) => Cypress.Chainable<null>);
11781183
}
11791184
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,12 @@ const initIntercepts = ({
143143
body: mockLLMInferenceServiceK8sResource({ name: 'test-llmd-model' }),
144144
},
145145
).as('createLLMInferenceService');
146+
// MaaS is enabled so we need to intercept this for edit scenarios
147+
cy.interceptOdh(
148+
'DELETE /maas/api/v1/maasmodel/:namespace/:name',
149+
{ path: { namespace: '*', name: '*' } },
150+
{ message: 'Deleted successfully' },
151+
).as('deleteMaaSModelRef');
146152
};
147153

148154
describe('Model Serving LLMD', () => {
@@ -774,6 +780,12 @@ describe('Model Serving LLMD', () => {
774780
cy.intercept('PUT', '**/llminferenceserviceconfigs/test-vllm-gpu*', (req) => {
775781
req.reply({ statusCode: 200, body: req.body });
776782
}).as('updateLLMInferenceServiceConfig');
783+
// MaaS is enabled so we need to intercept this for edit scenarios
784+
cy.interceptOdh(
785+
'DELETE /maas/api/v1/maasmodel/:namespace/:name',
786+
{ path: { namespace: '*', name: '*' } },
787+
{ message: 'Deleted successfully' },
788+
).as('deleteMaaSModelRef');
777789
};
778790

779791
it('should display serving runtime name and version, then pre-fill when editing', () => {

‎packages/cypress/cypress/tests/mocked/modelsAsAService/maasDeploymentWizard.cy.ts‎

Lines changed: 128 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,15 @@ describe('MaaS Deployment Wizard', () => {
126126
{ path: { namespace: 'test-project', name: 'test-llm-inference-service' } },
127127
{ message: 'Deleted successfully' },
128128
).as('deleteMaaSModelRef');
129+
cy.interceptOdh(
130+
'PUT /maas/api/v1/maasmodel/:namespace/:name',
131+
{ path: { namespace: '*', name: '*' } },
132+
mockMaaSModelRef({
133+
name: 'test-maas-model-ref',
134+
namespace: 'test-project',
135+
modelRef: { name: 'test-llm-inference-service', kind: 'LLMInferenceService' },
136+
}),
137+
).as('updateMaaSModelRef');
129138
};
130139

131140
it('should create an LLMD deployment with MaaS enabled and create a MaaSModelRef', () => {
@@ -165,6 +174,12 @@ describe('MaaS Deployment Wizard', () => {
165174
// Submit and verify MaaS-specific annotations and gateway refs
166175
modelServingWizard.findSubmitButton().click();
167176

177+
cy.wait('@createMaaSModelRef').then((interception) => {
178+
expect(interception.request.url).to.include('?dryRun=true');
179+
expect(interception.request.body.name).to.equal('test-llm-inference-service');
180+
expect(interception.request.body.namespace).to.equal('test-project');
181+
});
182+
168183
cy.wait('@createLLMInferenceService').then((interception) => {
169184
expect(interception.request.url).to.include('?dryRun=All');
170185

@@ -179,16 +194,66 @@ describe('MaaS Deployment Wizard', () => {
179194
cy.wait('@createLLMInferenceService');
180195
cy.get('@createLLMInferenceService.all').should('have.length', 2);
181196
cy.wait('@createMaaSModelRef').then((interception) => {
197+
expect(interception.request.url).not.to.include('?dryRun=true');
182198
expect(interception.request.body.name).to.equal('test-llm-inference-service');
183199
expect(interception.request.body.namespace).to.equal('test-project');
184200
expect(interception.request.body.displayName).to.equal('Test LLM Inference Service');
185201
expect(interception.request.body.description).to.equal(
186202
'Test LLM Inference Service Description',
187203
);
188-
expect(interception.request.body.modelRef).to.deep.equal({
189-
name: 'test-llm-inference-service',
190-
kind: 'LLMInferenceService',
191-
});
204+
});
205+
cy.get('@createMaaSModelRef.all').should('have.length', 2);
206+
});
207+
it('should update the MaaSModelRef when editing an existing deployment', () => {
208+
initMaaSDeploymentIntercepts();
209+
const savedURIModel = mockLLMInferenceServiceK8sResource({
210+
isMaaS: true,
211+
replicas: 2,
212+
});
213+
cy.interceptK8sList(
214+
{ model: LLMInferenceServiceModel, ns: 'test-project' },
215+
mockK8sResourceList([savedURIModel]),
216+
);
217+
modelServingGlobal.visit('test-project');
218+
modelServingGlobal.getModelRow('Test LLM Inference Service').findKebabAction('Edit').click();
219+
modelServingWizardEdit
220+
.findModelLocationSelectOption('Existing connection')
221+
.should('exist')
222+
.click();
223+
modelServingWizardEdit.findNextButton().should('be.enabled').click();
224+
modelServingWizardEdit.findModelDeploymentNameInput().clear().type('test-llmd-model-2');
225+
modelServingWizardEdit.findModelDeploymentDescriptionInput().type('test-llmd-description-2');
226+
hardwareProfileSection.findSelect().should('exist');
227+
hardwareProfileSection.findSelect().should('contain.text', 'Small');
228+
hardwareProfileSection.selectProfile(
229+
'Large Profile Compatible CPU: Request = 4 Cores; Limit = 4 Cores; Memory: Request = 8 GiB; Limit = 8 GiB',
230+
);
231+
modelServingWizardEdit.findNextButton().should('be.enabled').click();
232+
maasWizardField.findSaveAsMaaSCheckbox().should('exist').should('be.checked');
233+
modelServingWizardEdit.findNextButton().should('be.enabled').click();
234+
modelServingWizardEdit.findSubmitButton().click();
235+
cy.wait('@updateMaaSModelRef').then((interception) => {
236+
expect(interception.request.url).to.include('?dryRun=true');
237+
expect(interception.request.url).to.include('/test-project/test-llm-inference-service');
238+
expect(interception.request.body.displayName).to.equal('test-llmd-model-2');
239+
expect(interception.request.body.description).to.equal('test-llmd-description-2');
240+
});
241+
cy.wait('@updateLLMInferenceService').then((interception) => {
242+
expect(interception.request.url).to.include('?dryRun=All');
243+
});
244+
245+
cy.wait('@updateLLMInferenceService').then((interception) => {
246+
expect(interception.request.url).not.to.include('?dryRun=All');
247+
});
248+
249+
cy.get('@updateLLMInferenceService.all').then((interceptions) => {
250+
expect(interceptions).to.have.length(2);
251+
});
252+
cy.wait('@updateMaaSModelRef').then((interception) => {
253+
expect(interception.request.url).not.to.include('?dryRun=true');
254+
});
255+
cy.get('@updateMaaSModelRef.all').then((interceptions) => {
256+
expect(interceptions).to.have.length(2);
192257
});
193258
});
194259
it('should delete the MaaSModelRef when the MaaS checkbox is unchecked', () => {
@@ -227,6 +292,10 @@ describe('MaaS Deployment Wizard', () => {
227292
maasWizardField.findSaveAsMaaSCheckbox().should('not.be.checked');
228293
modelServingWizardEdit.findNextButton().should('be.enabled').click();
229294
modelServingWizardEdit.findSubmitButton().click();
295+
cy.wait('@deleteMaaSModelRef').then((interception) => {
296+
expect(interception.request.url).to.include('?dryRun=true');
297+
expect(interception.request.url).to.include('/test-project/test-llm-inference-service');
298+
});
230299
cy.wait('@updateLLMInferenceService').then((interception) => {
231300
expect(interception.request.url).to.include('?dryRun=All');
232301
expect(interception.request.body.spec.router.gateway.refs).to.deep.equal(undefined);
@@ -239,8 +308,60 @@ describe('MaaS Deployment Wizard', () => {
239308
cy.get('@updateLLMInferenceService.all').then((interceptions) => {
240309
expect(interceptions).to.have.length(2);
241310
});
242-
cy.wait('@deleteMaaSModelRef').then((interception) => {
243-
expect(interception.request.url).to.include('/test-project/test-llm-inference-service');
244-
});
311+
cy.get('@deleteMaaSModelRef.all').should('have.length', 2);
312+
});
313+
it('should show an error if the MaaSModelRef dry run fails', () => {
314+
initMaaSDeploymentIntercepts();
315+
cy.intercept(
316+
{ method: 'POST', url: '**/maas/api/v1/maasmodel*', query: { dryRun: 'true' } },
317+
{
318+
statusCode: 409,
319+
body: {
320+
error: {
321+
code: '409',
322+
message: "MaaSModelRef 'test-llm-inference-service' already exists",
323+
},
324+
},
325+
},
326+
).as('createMaaSModelRefDryRun');
327+
328+
// Navigate to wizard and set up basic deployment
329+
modelServingGlobal.visit('test-project');
330+
modelServingGlobal.findDeployModelButton().click();
331+
332+
// Quick setup: Model source and deployment
333+
modelServingWizard.findModelLocationSelectOption(ModelLocationSelectOption.EXISTING).click();
334+
modelServingWizard.findExistingConnectionValue().should('have.value', 'test-s3-secret');
335+
modelServingWizard.findModelTypeSelectOption(ModelTypeLabel.GENERATIVE).click();
336+
modelServingWizard.findNextButton().click();
337+
338+
modelServingWizard.findModelDeploymentNameInput().type('test-llm-inference-service');
339+
modelServingWizard
340+
.findModelDeploymentDescriptionInput()
341+
.type('Test LLM Inference Service Description');
342+
modelServingWizard.findServingRuntimeTemplateSearchSelector().click();
343+
modelServingWizard.findGlobalScopedTemplateOption('Distributed inference with llm-d').click();
344+
modelServingWizard.findNextButton().click();
345+
346+
// Focus on MaaS feature testing
347+
// uncheck token auth to simplify test
348+
modelServingWizard.findTokenAuthenticationCheckbox().click();
349+
350+
// Verify MaaS checkbox is unchecked by default
351+
maasWizardField.findSaveAsMaaSCheckbox().should('exist').should('not.be.checked');
352+
353+
// Check the MaaS checkbox
354+
maasWizardField.findSaveAsMaaSCheckbox().click();
355+
maasWizardField.findSaveAsMaaSCheckbox().should('be.checked');
356+
357+
modelServingWizard.findNextButton().should('be.enabled').click();
358+
359+
// Submit and verify MaaS-specific annotations and gateway refs
360+
modelServingWizard.findSubmitButton().click();
361+
cy.wait('@createMaaSModelRefDryRun');
362+
// Wizard stayed open and shows the error
363+
modelServingWizard.findErrorMessageAlert().should('be.visible').contains('Error');
364+
// no LLMInferenceService should be created
365+
cy.get('@createLLMInferenceService.all').should('have.length', 0);
245366
});
246367
});

‎packages/maas/bff/internal/api/maasmodelref_handlers.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ func CreateMaaSModelRefHandler(app *App, w http.ResponseWriter, r *http.Request,
4444
return
4545
}
4646

47-
response, err := app.repositories.MaaSModelRefs.CreateMaaSModelRef(ctx, request)
47+
dryRun := r.URL.Query().Get("dryRun") == "true"
48+
response, err := app.repositories.MaaSModelRefs.CreateMaaSModelRef(ctx, request, dryRun)
4849
if err != nil {
4950
if strings.Contains(err.Error(), "already exists") {
5051
app.errorResponse(w, r, &HTTPError{
@@ -88,7 +89,8 @@ func UpdateMaaSModelRefHandler(app *App, w http.ResponseWriter, r *http.Request,
8889
return
8990
}
9091

91-
response, err := app.repositories.MaaSModelRefs.UpdateMaaSModelRef(ctx, namespace, name, request)
92+
dryRun := r.URL.Query().Get("dryRun") == "true"
93+
response, err := app.repositories.MaaSModelRefs.UpdateMaaSModelRef(ctx, namespace, name, request, dryRun)
9294
if err != nil {
9395
if strings.Contains(err.Error(), "not found") {
9496
app.errorResponse(w, r, &HTTPError{
@@ -121,7 +123,8 @@ func DeleteMaaSModelRefHandler(app *App, w http.ResponseWriter, r *http.Request,
121123
return
122124
}
123125

124-
if err := app.repositories.MaaSModelRefs.DeleteMaaSModelRef(ctx, namespace, name); err != nil {
126+
dryRun := r.URL.Query().Get("dryRun") == "true"
127+
if err := app.repositories.MaaSModelRefs.DeleteMaaSModelRef(ctx, namespace, name, dryRun); err != nil {
125128
if strings.Contains(err.Error(), "not found") {
126129
app.errorResponse(w, r, &HTTPError{
127130
StatusCode: http.StatusNotFound,

‎packages/maas/bff/internal/api/maasmodelref_handlers_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,47 @@ var _ = Describe("MaaSModelRefHandlers", Ordered, func() {
124124
Expect(err).NotTo(HaveOccurred())
125125
Expect(rs.StatusCode).To(Equal(http.StatusBadRequest))
126126
})
127+
128+
It("returns 201 for a dry-run and does not persist the resource", func() {
129+
dryRunName := fmt.Sprintf("dry-run-model-%d", GinkgoRandomSeed())
130+
131+
// Dry-run create should succeed
132+
actual, rs, err := setupApiTest[models.MaaSModelRefSummary](
133+
http.MethodPost,
134+
"/api/v1/maasmodel?dryRun=true",
135+
models.CreateMaaSModelRefRequest{
136+
Name: dryRunName,
137+
Namespace: "maas-models",
138+
ModelRef: models.ModelReference{
139+
Kind: "LLMInferenceService",
140+
Name: "dry-run-llm",
141+
},
142+
},
143+
k8Factory,
144+
identity,
145+
)
146+
Expect(err).NotTo(HaveOccurred())
147+
Expect(rs.StatusCode).To(Equal(http.StatusCreated))
148+
Expect(actual.Name).To(Equal(dryRunName))
149+
150+
// A subsequent real create should also succeed, showing the dry-run did not persist
151+
_, rs2, err := setupApiTest[models.MaaSModelRefSummary](
152+
http.MethodPost,
153+
"/api/v1/maasmodel",
154+
models.CreateMaaSModelRefRequest{
155+
Name: dryRunName,
156+
Namespace: "maas-models",
157+
ModelRef: models.ModelReference{
158+
Kind: "LLMInferenceService",
159+
Name: "dry-run-llm",
160+
},
161+
},
162+
k8Factory,
163+
identity,
164+
)
165+
Expect(err).NotTo(HaveOccurred())
166+
Expect(rs2.StatusCode).To(Equal(http.StatusCreated))
167+
})
127168
})
128169

129170
var _ = Describe("UpdateMaaSModelRefHandler", Ordered, func() {

‎packages/maas/bff/internal/repositories/maasmodelrefs.go‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,10 @@ func NewMaaSModelRefsRepository(logger *slog.Logger, k8sFactory kubernetes.Kuber
2929
}
3030
}
3131

32-
// CreateMaaSModelRef creates a MaaSModelRef resource.
33-
func (r *MaaSModelRefsRepository) CreateMaaSModelRef(ctx context.Context, request models.CreateMaaSModelRefRequest) (*models.MaaSModelRefSummary, error) {
34-
r.logger.Debug("Creating MaaSModelRef", slog.String("name", request.Name), slog.String("namespace", request.Namespace))
32+
// CreateMaaSModelRef creates a MaaSModelRef resource. When dryRun is true the request is
33+
// validated by the API server but not persisted.
34+
func (r *MaaSModelRefsRepository) CreateMaaSModelRef(ctx context.Context, request models.CreateMaaSModelRefRequest, dryRun bool) (*models.MaaSModelRefSummary, error) {
35+
r.logger.Debug("Creating MaaSModelRef", slog.String("name", request.Name), slog.String("namespace", request.Namespace), slog.Bool("dryRun", dryRun))
3536

3637
client, err := r.k8sFactory.GetClient(ctx)
3738
if err != nil {
@@ -40,8 +41,13 @@ func (r *MaaSModelRefsRepository) CreateMaaSModelRef(ctx context.Context, reques
4041

4142
kubeClient := client.GetDynamicClient()
4243

44+
createOpts := metav1.CreateOptions{}
45+
if dryRun {
46+
createOpts.DryRun = []string{metav1.DryRunAll}
47+
}
48+
4349
obj := buildModelRefUnstructured(request.Name, request.Namespace, request.ModelRef, request.EndpointOverride, request.Uid, request.DisplayName, request.Description)
44-
created, err := kubeClient.Resource(constants.MaaSModelRefGvr).Namespace(request.Namespace).Create(ctx, obj, metav1.CreateOptions{})
50+
created, err := kubeClient.Resource(constants.MaaSModelRefGvr).Namespace(request.Namespace).Create(ctx, obj, createOpts)
4551
if err != nil {
4652
if k8sErrors.IsAlreadyExists(err) {
4753
return nil, fmt.Errorf("MaaSModelRef '%s' already exists", request.Name)
@@ -52,9 +58,10 @@ func (r *MaaSModelRefsRepository) CreateMaaSModelRef(ctx context.Context, reques
5258
return convertUnstructuredToModelRefSummary(created)
5359
}
5460

55-
// UpdateMaaSModelRef updates a MaaSModelRef resource.
56-
func (r *MaaSModelRefsRepository) UpdateMaaSModelRef(ctx context.Context, namespace, name string, request models.UpdateMaaSModelRefRequest) (*models.MaaSModelRefSummary, error) {
57-
r.logger.Debug("Updating MaaSModelRef", slog.String("namespace", namespace), slog.String("name", name))
61+
// UpdateMaaSModelRef updates a MaaSModelRef resource. When dryRun is true the request is
62+
// validated by the API server but not persisted.
63+
func (r *MaaSModelRefsRepository) UpdateMaaSModelRef(ctx context.Context, namespace, name string, request models.UpdateMaaSModelRefRequest, dryRun bool) (*models.MaaSModelRefSummary, error) {
64+
r.logger.Debug("Updating MaaSModelRef", slog.String("namespace", namespace), slog.String("name", name), slog.Bool("dryRun", dryRun))
5865

5966
client, err := r.k8sFactory.GetClient(ctx)
6067
if err != nil {
@@ -105,25 +112,35 @@ func (r *MaaSModelRefsRepository) UpdateMaaSModelRef(ctx context.Context, namesp
105112
existing.SetAnnotations(annotations)
106113
existing.Object["spec"] = existingSpec
107114

108-
updated, err := kubeClient.Resource(constants.MaaSModelRefGvr).Namespace(namespace).Update(ctx, existing, metav1.UpdateOptions{})
115+
updateOpts := metav1.UpdateOptions{}
116+
if dryRun {
117+
updateOpts.DryRun = []string{metav1.DryRunAll}
118+
}
119+
120+
updated, err := kubeClient.Resource(constants.MaaSModelRefGvr).Namespace(namespace).Update(ctx, existing, updateOpts)
109121
if err != nil {
110122
return nil, fmt.Errorf("failed to update MaaSModelRef: %w", err)
111123
}
112124

113125
return convertUnstructuredToModelRefSummary(updated)
114126
}
115127

116-
// DeleteMaaSModelRef deletes a MaaSModelRef resource by namespace and name.
117-
func (r *MaaSModelRefsRepository) DeleteMaaSModelRef(ctx context.Context, namespace, name string) error {
118-
r.logger.Debug("Deleting MaaSModelRef", slog.String("namespace", namespace), slog.String("name", name))
128+
// DeleteMaaSModelRef deletes a MaaSModelRef resource by namespace and name. When dryRun is true
129+
// the request is validated by the API server but not persisted.
130+
func (r *MaaSModelRefsRepository) DeleteMaaSModelRef(ctx context.Context, namespace, name string, dryRun bool) error {
131+
r.logger.Debug("Deleting MaaSModelRef", slog.String("namespace", namespace), slog.String("name", name), slog.Bool("dryRun", dryRun))
119132

120133
client, err := r.k8sFactory.GetClient(ctx)
121134
if err != nil {
122135
return err
123136
}
124137

125138
kubeClient := client.GetDynamicClient()
126-
err = kubeClient.Resource(constants.MaaSModelRefGvr).Namespace(namespace).Delete(ctx, name, metav1.DeleteOptions{})
139+
deleteOpts := metav1.DeleteOptions{}
140+
if dryRun {
141+
deleteOpts.DryRun = []string{metav1.DryRunAll}
142+
}
143+
err = kubeClient.Resource(constants.MaaSModelRefGvr).Namespace(namespace).Delete(ctx, name, deleteOpts)
127144
if err != nil {
128145
if k8sErrors.IsNotFound(err) {
129146
return fmt.Errorf("MaaSModelRef '%s' not found", name)

0 commit comments

Comments
 (0)