Skip to content

Commit 3650186

Browse files
andyatmiamiandyatmiami
committed
feat: implement Workspace Update API method
This commit delivers the following: - Complete the UpdateWorkspace repository method which was previously a stub (GH #848). - Apply all mutable spec fields from the update model to the K8s Workspace object and persist via client.Update(). - Add handler tests for successful update, 409 stale revision, and 404 not found. Not included in this PR and to be addressed in a follow up: - Retrying on 409 response from k8s server to account for stale cache - TODO comment still in code calling out this need Signed-off-by: Andy Stoneberg <astonebe@redhat.com>
1 parent 455b333 commit 3650186

File tree

2 files changed

+351
-6
lines changed

2 files changed

+351
-6
lines changed

‎workspaces/backend/api/workspaces_handler_test.go‎

Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,303 @@ var _ = Describe("Workspaces Handler", func() {
987987
Expect(createdWorkspace.Spec.PodTemplate.Volumes.Secrets).To(Equal(expected))
988988
})
989989

990+
It("should update a Workspace successfully", func() {
991+
992+
By("creating a Workspace via the API")
993+
workspaceCreate := &models.WorkspaceCreate{
994+
Name: workspaceName,
995+
Kind: workspaceKindName,
996+
Paused: false,
997+
PodTemplate: models.PodTemplateMutate{
998+
PodMetadata: models.PodMetadataMutate{
999+
Labels: map[string]string{
1000+
"app": "testing",
1001+
},
1002+
Annotations: map[string]string{
1003+
"example.com/testing": "true",
1004+
},
1005+
},
1006+
Volumes: models.PodVolumesMutate{
1007+
Home: ptr.To("my-home-pvc"),
1008+
Data: []models.PodVolumeMount{
1009+
{
1010+
PVCName: "my-data-pvc",
1011+
MountPath: "/data/1",
1012+
ReadOnly: false,
1013+
},
1014+
},
1015+
},
1016+
Options: models.PodTemplateOptionsMutate{
1017+
ImageConfig: "jupyterlab_scipy_180",
1018+
PodConfig: "tiny_cpu",
1019+
},
1020+
},
1021+
}
1022+
createEnvelope := WorkspaceCreateEnvelope{Data: workspaceCreate}
1023+
createJSON, err := json.Marshal(createEnvelope)
1024+
Expect(err).NotTo(HaveOccurred())
1025+
1026+
path := strings.Replace(WorkspacesByNamespacePath, ":"+NamespacePathParam, namespaceNameCrud, 1)
1027+
req, err := http.NewRequest(http.MethodPost, path, strings.NewReader(string(createJSON)))
1028+
Expect(err).NotTo(HaveOccurred())
1029+
req.Header.Set("Content-Type", MediaTypeJson)
1030+
req.Header.Set(userIdHeader, adminUser)
1031+
1032+
rr := httptest.NewRecorder()
1033+
ps := httprouter.Params{
1034+
httprouter.Param{Key: NamespacePathParam, Value: namespaceNameCrud},
1035+
}
1036+
a.CreateWorkspaceHandler(rr, req, ps)
1037+
rs := rr.Result()
1038+
defer rs.Body.Close()
1039+
Expect(rs.StatusCode).To(Equal(http.StatusCreated), descUnexpectedHTTPStatus, rr.Body.String())
1040+
1041+
By("getting the Workspace from the Kubernetes API to obtain its current revision")
1042+
createdWorkspace := &kubefloworgv1beta1.Workspace{}
1043+
Expect(k8sClient.Get(ctx, workspaceKey, createdWorkspace)).To(Succeed())
1044+
originalRevision := models.CalculateWorkspaceRevision(createdWorkspace)
1045+
1046+
By("building a WorkspaceUpdate model with changed fields")
1047+
workspaceUpdate := &models.WorkspaceUpdate{
1048+
Revision: originalRevision,
1049+
Paused: true,
1050+
PodTemplate: models.PodTemplateMutate{
1051+
PodMetadata: models.PodMetadataMutate{
1052+
Labels: map[string]string{
1053+
"app": "testing",
1054+
"version": "v2",
1055+
},
1056+
Annotations: map[string]string{
1057+
"example.com/testing": "false",
1058+
},
1059+
},
1060+
Volumes: models.PodVolumesMutate{
1061+
Home: ptr.To("my-home-pvc"),
1062+
Data: []models.PodVolumeMount{
1063+
{
1064+
PVCName: "my-data-pvc",
1065+
MountPath: "/data/updated",
1066+
ReadOnly: true,
1067+
},
1068+
},
1069+
},
1070+
Options: models.PodTemplateOptionsMutate{
1071+
ImageConfig: "jupyterlab_scipy_180",
1072+
PodConfig: "small_cpu",
1073+
},
1074+
},
1075+
}
1076+
updateEnvelope := WorkspaceEnvelope{Data: workspaceUpdate}
1077+
updateJSON, err := json.Marshal(updateEnvelope)
1078+
Expect(err).NotTo(HaveOccurred())
1079+
1080+
By("executing UpdateWorkspaceHandler")
1081+
path = strings.Replace(WorkspacesByNamePath, ":"+NamespacePathParam, namespaceNameCrud, 1)
1082+
path = strings.Replace(path, ":"+ResourceNamePathParam, workspaceName, 1)
1083+
req, err = http.NewRequest(http.MethodPut, path, strings.NewReader(string(updateJSON)))
1084+
Expect(err).NotTo(HaveOccurred())
1085+
req.Header.Set("Content-Type", MediaTypeJson)
1086+
req.Header.Set(userIdHeader, adminUser)
1087+
1088+
rr = httptest.NewRecorder()
1089+
ps = httprouter.Params{
1090+
httprouter.Param{Key: NamespacePathParam, Value: namespaceNameCrud},
1091+
httprouter.Param{Key: ResourceNamePathParam, Value: workspaceName},
1092+
}
1093+
a.UpdateWorkspaceHandler(rr, req, ps)
1094+
rs = rr.Result()
1095+
defer rs.Body.Close()
1096+
1097+
By("verifying the HTTP response status code")
1098+
Expect(rs.StatusCode).To(Equal(http.StatusOK), descUnexpectedHTTPStatus, rr.Body.String())
1099+
1100+
By("reading the HTTP response body")
1101+
body, err := io.ReadAll(rs.Body)
1102+
Expect(err).NotTo(HaveOccurred())
1103+
1104+
By("unmarshalling the response JSON to WorkspaceEnvelope")
1105+
var response WorkspaceEnvelope
1106+
err = json.Unmarshal(body, &response)
1107+
Expect(err).NotTo(HaveOccurred())
1108+
1109+
By("verifying the response contains a new revision")
1110+
Expect(response.Data.Revision).NotTo(Equal(originalRevision))
1111+
1112+
By("getting the updated Workspace from the Kubernetes API")
1113+
updatedWorkspace := &kubefloworgv1beta1.Workspace{}
1114+
Expect(k8sClient.Get(ctx, workspaceKey, updatedWorkspace)).To(Succeed())
1115+
1116+
By("verifying all fields were applied")
1117+
Expect(ptr.Deref(updatedWorkspace.Spec.Paused, false)).To(BeTrue())
1118+
Expect(updatedWorkspace.Spec.PodTemplate.PodMetadata.Labels).To(Equal(workspaceUpdate.PodTemplate.PodMetadata.Labels))
1119+
Expect(updatedWorkspace.Spec.PodTemplate.PodMetadata.Annotations).To(Equal(workspaceUpdate.PodTemplate.PodMetadata.Annotations))
1120+
Expect(updatedWorkspace.Spec.PodTemplate.Options.PodConfig).To(Equal("small_cpu"))
1121+
Expect(updatedWorkspace.Spec.PodTemplate.Volumes.Data).To(Equal([]kubefloworgv1beta1.PodVolumeMount{
1122+
{
1123+
PVCName: "my-data-pvc",
1124+
MountPath: "/data/updated",
1125+
ReadOnly: ptr.To(true),
1126+
},
1127+
}))
1128+
1129+
By("cleaning up the Workspace")
1130+
Expect(k8sClient.Delete(ctx, updatedWorkspace)).To(Succeed())
1131+
})
1132+
1133+
It("should return 409 for a stale revision", func() {
1134+
1135+
By("creating a Workspace via the API")
1136+
workspaceCreate := &models.WorkspaceCreate{
1137+
Name: workspaceName,
1138+
Kind: workspaceKindName,
1139+
Paused: false,
1140+
PodTemplate: models.PodTemplateMutate{
1141+
PodMetadata: models.PodMetadataMutate{
1142+
Labels: map[string]string{},
1143+
Annotations: map[string]string{},
1144+
},
1145+
Volumes: models.PodVolumesMutate{
1146+
Home: ptr.To("my-home-pvc"),
1147+
Data: []models.PodVolumeMount{
1148+
{
1149+
PVCName: "my-data-pvc",
1150+
MountPath: "/data/1",
1151+
ReadOnly: false,
1152+
},
1153+
},
1154+
},
1155+
Options: models.PodTemplateOptionsMutate{
1156+
ImageConfig: "jupyterlab_scipy_180",
1157+
PodConfig: "tiny_cpu",
1158+
},
1159+
},
1160+
}
1161+
createEnvelope := WorkspaceCreateEnvelope{Data: workspaceCreate}
1162+
createJSON, err := json.Marshal(createEnvelope)
1163+
Expect(err).NotTo(HaveOccurred())
1164+
1165+
path := strings.Replace(WorkspacesByNamespacePath, ":"+NamespacePathParam, namespaceNameCrud, 1)
1166+
req, err := http.NewRequest(http.MethodPost, path, strings.NewReader(string(createJSON)))
1167+
Expect(err).NotTo(HaveOccurred())
1168+
req.Header.Set("Content-Type", MediaTypeJson)
1169+
req.Header.Set(userIdHeader, adminUser)
1170+
1171+
rr := httptest.NewRecorder()
1172+
ps := httprouter.Params{
1173+
httprouter.Param{Key: NamespacePathParam, Value: namespaceNameCrud},
1174+
}
1175+
a.CreateWorkspaceHandler(rr, req, ps)
1176+
rs := rr.Result()
1177+
defer rs.Body.Close()
1178+
Expect(rs.StatusCode).To(Equal(http.StatusCreated), descUnexpectedHTTPStatus, rr.Body.String())
1179+
1180+
By("getting the Workspace to obtain the current revision")
1181+
workspace := &kubefloworgv1beta1.Workspace{}
1182+
Expect(k8sClient.Get(ctx, workspaceKey, workspace)).To(Succeed())
1183+
staleRevision := models.CalculateWorkspaceRevision(workspace)
1184+
1185+
By("modifying the Workspace directly to change its revision")
1186+
workspace.Spec.PodTemplate.Options.PodConfig = "small_cpu"
1187+
Expect(k8sClient.Update(ctx, workspace)).To(Succeed())
1188+
1189+
By("attempting to update via the API with the stale revision")
1190+
workspaceUpdate := &models.WorkspaceUpdate{
1191+
Revision: staleRevision,
1192+
Paused: false,
1193+
PodTemplate: models.PodTemplateMutate{
1194+
PodMetadata: models.PodMetadataMutate{
1195+
Labels: map[string]string{},
1196+
Annotations: map[string]string{},
1197+
},
1198+
Volumes: models.PodVolumesMutate{
1199+
Home: ptr.To("my-home-pvc"),
1200+
Data: []models.PodVolumeMount{
1201+
{
1202+
PVCName: "my-data-pvc",
1203+
MountPath: "/data/1",
1204+
ReadOnly: false,
1205+
},
1206+
},
1207+
},
1208+
Options: models.PodTemplateOptionsMutate{
1209+
ImageConfig: "jupyterlab_scipy_180",
1210+
PodConfig: "tiny_cpu",
1211+
},
1212+
},
1213+
}
1214+
updateEnvelope := WorkspaceEnvelope{Data: workspaceUpdate}
1215+
updateJSON, err := json.Marshal(updateEnvelope)
1216+
Expect(err).NotTo(HaveOccurred())
1217+
1218+
path = strings.Replace(WorkspacesByNamePath, ":"+NamespacePathParam, namespaceNameCrud, 1)
1219+
path = strings.Replace(path, ":"+ResourceNamePathParam, workspaceName, 1)
1220+
req, err = http.NewRequest(http.MethodPut, path, strings.NewReader(string(updateJSON)))
1221+
Expect(err).NotTo(HaveOccurred())
1222+
req.Header.Set("Content-Type", MediaTypeJson)
1223+
req.Header.Set(userIdHeader, adminUser)
1224+
1225+
rr = httptest.NewRecorder()
1226+
ps = httprouter.Params{
1227+
httprouter.Param{Key: NamespacePathParam, Value: namespaceNameCrud},
1228+
httprouter.Param{Key: ResourceNamePathParam, Value: workspaceName},
1229+
}
1230+
a.UpdateWorkspaceHandler(rr, req, ps)
1231+
rs = rr.Result()
1232+
defer rs.Body.Close()
1233+
1234+
By("verifying the HTTP response status code is 409")
1235+
Expect(rs.StatusCode).To(Equal(http.StatusConflict), descUnexpectedHTTPStatus, rr.Body.String())
1236+
1237+
By("cleaning up the Workspace")
1238+
Expect(k8sClient.Delete(ctx, workspace)).To(Succeed())
1239+
})
1240+
1241+
It("should return 404 for a non-existent Workspace update", func() {
1242+
missingWorkspaceName := "non-existent-workspace"
1243+
1244+
By("building an update request for a workspace that doesn't exist")
1245+
workspaceUpdate := &models.WorkspaceUpdate{
1246+
Revision: "fake-revision",
1247+
Paused: false,
1248+
PodTemplate: models.PodTemplateMutate{
1249+
PodMetadata: models.PodMetadataMutate{
1250+
Labels: map[string]string{},
1251+
Annotations: map[string]string{},
1252+
},
1253+
Volumes: models.PodVolumesMutate{
1254+
Data: []models.PodVolumeMount{},
1255+
},
1256+
Options: models.PodTemplateOptionsMutate{
1257+
ImageConfig: "jupyterlab_scipy_180",
1258+
PodConfig: "tiny_cpu",
1259+
},
1260+
},
1261+
}
1262+
updateEnvelope := WorkspaceEnvelope{Data: workspaceUpdate}
1263+
updateJSON, err := json.Marshal(updateEnvelope)
1264+
Expect(err).NotTo(HaveOccurred())
1265+
1266+
By("executing UpdateWorkspaceHandler")
1267+
path := strings.Replace(WorkspacesByNamePath, ":"+NamespacePathParam, namespaceNameCrud, 1)
1268+
path = strings.Replace(path, ":"+ResourceNamePathParam, missingWorkspaceName, 1)
1269+
req, err := http.NewRequest(http.MethodPut, path, strings.NewReader(string(updateJSON)))
1270+
Expect(err).NotTo(HaveOccurred())
1271+
req.Header.Set("Content-Type", MediaTypeJson)
1272+
req.Header.Set(userIdHeader, adminUser)
1273+
1274+
rr := httptest.NewRecorder()
1275+
ps := httprouter.Params{
1276+
httprouter.Param{Key: NamespacePathParam, Value: namespaceNameCrud},
1277+
httprouter.Param{Key: ResourceNamePathParam, Value: missingWorkspaceName},
1278+
}
1279+
a.UpdateWorkspaceHandler(rr, req, ps)
1280+
rs := rr.Result()
1281+
defer rs.Body.Close()
1282+
1283+
By("verifying the HTTP response status code is 404")
1284+
Expect(rs.StatusCode).To(Equal(http.StatusNotFound), descUnexpectedHTTPStatus, rr.Body.String())
1285+
})
1286+
9901287
// TODO: test when fail to create a Workspace when:
9911288
// - body payload invalid (missing name/kind, and/or non RCF 1123 name)
9921289
// - invalid namespace HTTP path parameter (also test for other API handlers)

‎workspaces/backend/internal/repositories/workspaces/repo.go‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -257,17 +257,65 @@ func (r *WorkspaceRepository) UpdateWorkspace(ctx context.Context, workspaceUpda
257257
// TODO: validate the requested updates (e.g. validate new home PVC is mountable, etc.)
258258
// ...
259259

260-
// TODO: update workspace fields from workspaceUpdate model
261-
// ...
260+
// NOTE: currently all Workspaces are "podTemplate"-type, if other types are
261+
// introduced in the future, this logic will need to be updated.
262+
263+
// convert data volumes from model
264+
dataVolumeMounts := make([]kubefloworgv1beta1.PodVolumeMount, len(workspaceUpdate.PodTemplate.Volumes.Data))
265+
for i, dataVolume := range workspaceUpdate.PodTemplate.Volumes.Data {
266+
dataVolumeMounts[i] = kubefloworgv1beta1.PodVolumeMount{
267+
PVCName: dataVolume.PVCName,
268+
MountPath: dataVolume.MountPath,
269+
ReadOnly: ptr.To(dataVolume.ReadOnly),
270+
}
271+
}
272+
273+
// convert secrets from model
274+
secretMounts := make([]kubefloworgv1beta1.PodSecretMount, len(workspaceUpdate.PodTemplate.Volumes.Secrets))
275+
for i, secret := range workspaceUpdate.PodTemplate.Volumes.Secrets {
276+
secretMounts[i] = kubefloworgv1beta1.PodSecretMount{
277+
SecretName: secret.SecretName,
278+
MountPath: secret.MountPath,
279+
DefaultMode: secret.DefaultMode,
280+
}
281+
}
282+
283+
// apply model fields to workspace spec
284+
workspace.Spec.Paused = ptr.To(workspaceUpdate.Paused)
285+
workspace.Spec.PodTemplate = kubefloworgv1beta1.WorkspacePodTemplate{
286+
PodMetadata: &kubefloworgv1beta1.WorkspacePodMetadata{
287+
Labels: workspaceUpdate.PodTemplate.PodMetadata.Labels,
288+
Annotations: workspaceUpdate.PodTemplate.PodMetadata.Annotations,
289+
},
290+
Volumes: kubefloworgv1beta1.WorkspacePodVolumes{
291+
Home: workspaceUpdate.PodTemplate.Volumes.Home,
292+
Data: dataVolumeMounts,
293+
Secrets: secretMounts,
294+
},
295+
Options: kubefloworgv1beta1.WorkspacePodOptions{
296+
ImageConfig: workspaceUpdate.PodTemplate.Options.ImageConfig,
297+
PodConfig: workspaceUpdate.PodTemplate.Options.PodConfig,
298+
},
299+
}
262300

263301
// set audit annotations
264302
modelsCommon.UpdateObjectMetaForUpdate(&workspace.ObjectMeta, actor, now)
265303

266-
// TODO: update the workspace in K8s
267304
// TODO: if the update fails due to a kubernetes conflict, this implies our cache is stale.
268-
// we should retry the entire update operation a few times (including recalculating clusterRevision)
269-
// before returning a 500 error to the caller (DO NOT return a 409, as it's not the caller's fault)
270-
// ...
305+
// we should wrap this operation in retry.RetryOnConflict to retry the entire update
306+
// (including re-fetching and recalculating clusterRevision) before returning a 500
307+
// error to the caller (DO NOT return a 409, as it's not the caller's fault)
308+
if err := r.client.Update(ctx, workspace); err != nil {
309+
if apierrors.IsNotFound(err) {
310+
return nil, ErrWorkspaceNotFound
311+
}
312+
if apierrors.IsInvalid(err) {
313+
// NOTE: we don't wrap this error so we can unpack it in the caller
314+
// and extract the validation errors returned by the Kubernetes API server
315+
return nil, err
316+
}
317+
return nil, fmt.Errorf("failed to update workspace: %w", err)
318+
}
271319

272320
workspaceUpdateModel := models.NewWorkspaceUpdateModelFromWorkspace(workspace)
273321
return workspaceUpdateModel, nil

0 commit comments

Comments
 (0)