Skip to content

Commit 716818b

Browse files
authored
fix(backend): Remove the default workspace configuration (#12369)
This was added as a mistake as this is environment specific and not a safe default. Signed-off-by: mprahl <[email protected]>
1 parent 6051a05 commit 716818b

File tree

6 files changed

+98
-60
lines changed

6 files changed

+98
-60
lines changed

backend/src/apiserver/config/config.json

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,5 @@
2424
"CacheEnabled": "true",
2525
"CRON_SCHEDULE_TIMEZONE": "UTC",
2626
"CACHE_IMAGE": "ghcr.io/containerd/busybox",
27-
"CACHE_NODE_RESTRICTIONS": "false",
28-
"Workspace": {
29-
"VolumeClaimTemplateSpec": {
30-
"accessModes": [
31-
"ReadWriteOnce"
32-
],
33-
"storageClassName": "standard-csi"
34-
}
35-
}
27+
"CACHE_NODE_RESTRICTIONS": "false"
3628
}

backend/src/v2/compiler/argocompiler/argo.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,17 @@ func GetWorkspacePVC(
542542
}
543543
}
544544

545+
// Access modes define the read/write semantics for the underlying volume and
546+
// gate whether multiple pods can mount it concurrently. It is required when creating the
547+
// underlying PVC for the workspace.
548+
if len(pvcSpec.AccessModes) == 0 {
549+
return k8score.PersistentVolumeClaim{}, fmt.Errorf("workspace PVC spec must specify accessModes")
550+
}
551+
552+
if pvcSpec.StorageClassName == nil || *pvcSpec.StorageClassName == "" {
553+
return k8score.PersistentVolumeClaim{}, fmt.Errorf("workspace PVC spec must specify storageClassName")
554+
}
555+
545556
quantity, err := k8sres.ParseQuantity(sizeStr)
546557
if err != nil {
547558
return k8score.PersistentVolumeClaim{}, fmt.Errorf("invalid size value for workspace PVC: %v", err)

backend/src/v2/compiler/argocompiler/argo_workspace_test.go

Lines changed: 78 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,28 @@ func TestGetWorkspacePVC(t *testing.T) {
3535
expectError bool
3636
}{
3737
{
38-
name: "workspace with size specified",
38+
name: "workspace with size specified and default workspace",
3939
workspace: &pipelinespec.WorkspaceConfig{
40-
Size: "5Gi",
40+
Size: "5Gi",
41+
Kubernetes: nil,
42+
},
43+
opts: &argocompiler.Options{
44+
DefaultWorkspace: &k8score.PersistentVolumeClaimSpec{
45+
AccessModes: []k8score.PersistentVolumeAccessMode{
46+
k8score.ReadWriteOnce,
47+
},
48+
StorageClassName: stringPtr("standard"),
49+
},
4150
},
42-
opts: nil,
4351
expectedPVC: k8score.PersistentVolumeClaim{
4452
ObjectMeta: k8smeta.ObjectMeta{
4553
Name: "kfp-workspace",
4654
},
4755
Spec: k8score.PersistentVolumeClaimSpec{
56+
AccessModes: []k8score.PersistentVolumeAccessMode{
57+
k8score.ReadWriteOnce,
58+
},
59+
StorageClassName: stringPtr("standard"),
4860
Resources: k8score.VolumeResourceRequirements{
4961
Requests: map[k8score.ResourceName]resource.Quantity{
5062
k8score.ResourceStorage: resource.MustParse("5Gi"),
@@ -68,7 +80,14 @@ func TestGetWorkspacePVC(t *testing.T) {
6880
workspace: &pipelinespec.WorkspaceConfig{
6981
Size: "", // no size specified
7082
},
71-
opts: nil, // no options
83+
opts: &argocompiler.Options{
84+
DefaultWorkspace: &k8score.PersistentVolumeClaimSpec{
85+
AccessModes: []k8score.PersistentVolumeAccessMode{
86+
k8score.ReadWriteOnce,
87+
},
88+
StorageClassName: stringPtr("standard"),
89+
},
90+
},
7291
expectedPVC: k8score.PersistentVolumeClaim{},
7392
expectError: true,
7493
},
@@ -108,6 +127,17 @@ func TestGetWorkspacePVC(t *testing.T) {
108127
},
109128
expectError: false,
110129
},
130+
{
131+
name: "default workspace missing required fields should fail",
132+
workspace: &pipelinespec.WorkspaceConfig{
133+
Size: "10Gi",
134+
},
135+
opts: &argocompiler.Options{
136+
DefaultWorkspace: &k8score.PersistentVolumeClaimSpec{},
137+
},
138+
expectedPVC: k8score.PersistentVolumeClaim{},
139+
expectError: true,
140+
},
111141
{
112142
name: "workspace with Kubernetes PVC spec patch",
113143
workspace: &pipelinespec.WorkspaceConfig{
@@ -160,6 +190,24 @@ func TestGetWorkspacePVC(t *testing.T) {
160190
expectedPVC: k8score.PersistentVolumeClaim{},
161191
expectError: true,
162192
},
193+
{
194+
name: "workspace patch missing accessModes should fail",
195+
workspace: &pipelinespec.WorkspaceConfig{
196+
Size: "20Gi",
197+
Kubernetes: &pipelinespec.KubernetesWorkspaceConfig{
198+
PvcSpecPatch: &structpb.Struct{
199+
Fields: map[string]*structpb.Value{
200+
"storageClassName": structpb.NewStringValue("fast-ssd"),
201+
},
202+
},
203+
},
204+
},
205+
opts: &argocompiler.Options{
206+
DefaultWorkspace: &k8score.PersistentVolumeClaimSpec{},
207+
},
208+
expectedPVC: k8score.PersistentVolumeClaim{},
209+
expectError: true,
210+
},
163211
{
164212
name: "workspace with invalid PVC spec patch",
165213
workspace: &pipelinespec.WorkspaceConfig{
@@ -221,50 +269,6 @@ func TestGetWorkspacePVC(t *testing.T) {
221269
},
222270
expectError: false,
223271
},
224-
{
225-
name: "workspace with nil Kubernetes config",
226-
workspace: &pipelinespec.WorkspaceConfig{
227-
Size: "30Gi",
228-
Kubernetes: nil,
229-
},
230-
opts: nil,
231-
expectedPVC: k8score.PersistentVolumeClaim{
232-
ObjectMeta: k8smeta.ObjectMeta{
233-
Name: "kfp-workspace",
234-
},
235-
Spec: k8score.PersistentVolumeClaimSpec{
236-
Resources: k8score.VolumeResourceRequirements{
237-
Requests: map[k8score.ResourceName]resource.Quantity{
238-
k8score.ResourceStorage: resource.MustParse("30Gi"),
239-
},
240-
},
241-
},
242-
},
243-
expectError: false,
244-
},
245-
{
246-
name: "workspace with empty Kubernetes config",
247-
workspace: &pipelinespec.WorkspaceConfig{
248-
Size: "40Gi",
249-
Kubernetes: &pipelinespec.KubernetesWorkspaceConfig{
250-
PvcSpecPatch: nil,
251-
},
252-
},
253-
opts: nil,
254-
expectedPVC: k8score.PersistentVolumeClaim{
255-
ObjectMeta: k8smeta.ObjectMeta{
256-
Name: "kfp-workspace",
257-
},
258-
Spec: k8score.PersistentVolumeClaimSpec{
259-
Resources: k8score.VolumeResourceRequirements{
260-
Requests: map[k8score.ResourceName]resource.Quantity{
261-
k8score.ResourceStorage: resource.MustParse("40Gi"),
262-
},
263-
},
264-
},
265-
},
266-
expectError: false,
267-
},
268272
}
269273

270274
for _, tt := range tests {
@@ -299,17 +303,41 @@ func TestGetWorkspacePVC_EdgeCases(t *testing.T) {
299303
name: "workspace with very large size",
300304
workspace: &pipelinespec.WorkspaceConfig{
301305
Size: "1000Ti",
306+
Kubernetes: &pipelinespec.KubernetesWorkspaceConfig{
307+
PvcSpecPatch: &structpb.Struct{
308+
Fields: map[string]*structpb.Value{
309+
"accessModes": structpb.NewListValue(&structpb.ListValue{
310+
Values: []*structpb.Value{
311+
structpb.NewStringValue("ReadWriteOnce"),
312+
},
313+
}),
314+
"storageClassName": structpb.NewStringValue("gp2"),
315+
},
316+
},
317+
},
302318
},
303319
opts: nil,
304-
expectError: false, // should be valid
320+
expectError: false,
305321
},
306322
{
307323
name: "workspace with decimal size",
308324
workspace: &pipelinespec.WorkspaceConfig{
309325
Size: "1.5Gi",
326+
Kubernetes: &pipelinespec.KubernetesWorkspaceConfig{
327+
PvcSpecPatch: &structpb.Struct{
328+
Fields: map[string]*structpb.Value{
329+
"accessModes": structpb.NewListValue(&structpb.ListValue{
330+
Values: []*structpb.Value{
331+
structpb.NewStringValue("ReadWriteOnce"),
332+
},
333+
}),
334+
"storageClassName": structpb.NewStringValue("standard"),
335+
},
336+
},
337+
},
310338
},
311339
opts: nil,
312-
expectError: false, // should be valid
340+
expectError: false,
313341
},
314342
{
315343
name: "workspace with invalid size format",

test_data/compiled-workflows/pipeline_with_workspace.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,8 @@ spec:
387387
resources:
388388
requests:
389389
storage: 1Gi
390+
accessModes:
391+
- ReadWriteOnce
390392
storageClassName: standard
391393
status: {}
392394
status:

test_data/sdk_compiled_pipelines/valid/critical/pipeline_with_workspace.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ def read_from_workspace(file_path: str) -> str:
5656
workspace=dsl.WorkspaceConfig(
5757
size='1Gi',
5858
kubernetes=dsl.KubernetesWorkspaceConfig(
59-
pvcSpecPatch={'storageClassName': 'standard'}
59+
pvcSpecPatch={
60+
'storageClassName': 'standard',
61+
'accessModes': ['ReadWriteOnce'],
62+
}
6063
)
6164
),
6265
),

test_data/sdk_compiled_pipelines/valid/critical/pipeline_with_workspace.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,5 +146,7 @@ platforms:
146146
workspace:
147147
kubernetes:
148148
pvcSpecPatch:
149+
accessModes:
150+
- ReadWriteOnce
149151
storageClassName: standard
150152
size: 1Gi

0 commit comments

Comments
 (0)