Skip to content

Commit 6454ca1

Browse files
committed
Remove the default workspace configuration
This was added as a mistake as this is environment specific and not a safe default. Signed-off-by: mprahl <[email protected]>
1 parent de3feb5 commit 6454ca1

File tree

6 files changed

+85
-63
lines changed

6 files changed

+85
-63
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: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,13 @@ func GetWorkspacePVC(
534534
}
535535
}
536536

537+
if len(pvcSpec.AccessModes) == 0 {
538+
return k8score.PersistentVolumeClaim{}, fmt.Errorf("workspace PVC spec must specify accessModes")
539+
}
540+
if pvcSpec.StorageClassName == nil || *pvcSpec.StorageClassName == "" {
541+
return k8score.PersistentVolumeClaim{}, fmt.Errorf("workspace PVC spec must specify storageClassName")
542+
}
543+
537544
quantity, err := resource.ParseQuantity(sizeStr)
538545
if err != nil {
539546
return k8score.PersistentVolumeClaim{}, fmt.Errorf("invalid size value for workspace PVC: %v", err)

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

Lines changed: 69 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +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{
48-
Resources: k8score.VolumeResourceRequirements{
49-
Requests: map[k8score.ResourceName]resource.Quantity{
50-
k8score.ResourceStorage: resource.MustParse("5Gi"),
51-
},
56+
AccessModes: []k8score.PersistentVolumeAccessMode{
57+
k8score.ReadWriteOnce,
5258
},
59+
StorageClassName: stringPtr("standard"),
5360
},
5461
},
5562
expectError: false,
@@ -108,6 +115,17 @@ func TestGetWorkspacePVC(t *testing.T) {
108115
},
109116
expectError: false,
110117
},
118+
{
119+
name: "default workspace missing required fields should fail",
120+
workspace: &pipelinespec.WorkspaceConfig{
121+
Size: "10Gi",
122+
},
123+
opts: &argocompiler.Options{
124+
DefaultWorkspace: &k8score.PersistentVolumeClaimSpec{},
125+
},
126+
expectedPVC: k8score.PersistentVolumeClaim{},
127+
expectError: true,
128+
},
111129
{
112130
name: "workspace with Kubernetes PVC spec patch",
113131
workspace: &pipelinespec.WorkspaceConfig{
@@ -160,6 +178,24 @@ func TestGetWorkspacePVC(t *testing.T) {
160178
expectedPVC: k8score.PersistentVolumeClaim{},
161179
expectError: true,
162180
},
181+
{
182+
name: "workspace patch missing accessModes should fail",
183+
workspace: &pipelinespec.WorkspaceConfig{
184+
Size: "20Gi",
185+
Kubernetes: &pipelinespec.KubernetesWorkspaceConfig{
186+
PvcSpecPatch: &structpb.Struct{
187+
Fields: map[string]*structpb.Value{
188+
"storageClassName": structpb.NewStringValue("fast-ssd"),
189+
},
190+
},
191+
},
192+
},
193+
opts: &argocompiler.Options{
194+
DefaultWorkspace: &k8score.PersistentVolumeClaimSpec{},
195+
},
196+
expectedPVC: k8score.PersistentVolumeClaim{},
197+
expectError: true,
198+
},
163199
{
164200
name: "workspace with invalid PVC spec patch",
165201
workspace: &pipelinespec.WorkspaceConfig{
@@ -221,50 +257,6 @@ func TestGetWorkspacePVC(t *testing.T) {
221257
},
222258
expectError: false,
223259
},
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-
},
268260
}
269261

270262
for _, tt := range tests {
@@ -299,17 +291,41 @@ func TestGetWorkspacePVC_EdgeCases(t *testing.T) {
299291
name: "workspace with very large size",
300292
workspace: &pipelinespec.WorkspaceConfig{
301293
Size: "1000Ti",
294+
Kubernetes: &pipelinespec.KubernetesWorkspaceConfig{
295+
PvcSpecPatch: &structpb.Struct{
296+
Fields: map[string]*structpb.Value{
297+
"accessModes": structpb.NewListValue(&structpb.ListValue{
298+
Values: []*structpb.Value{
299+
structpb.NewStringValue("ReadWriteOnce"),
300+
},
301+
}),
302+
"storageClassName": structpb.NewStringValue("gp2"),
303+
},
304+
},
305+
},
302306
},
303307
opts: nil,
304-
expectError: false, // should be valid
308+
expectError: false,
305309
},
306310
{
307311
name: "workspace with decimal size",
308312
workspace: &pipelinespec.WorkspaceConfig{
309313
Size: "1.5Gi",
314+
Kubernetes: &pipelinespec.KubernetesWorkspaceConfig{
315+
PvcSpecPatch: &structpb.Struct{
316+
Fields: map[string]*structpb.Value{
317+
"accessModes": structpb.NewListValue(&structpb.ListValue{
318+
Values: []*structpb.Value{
319+
structpb.NewStringValue("ReadWriteOnce"),
320+
},
321+
}),
322+
"storageClassName": structpb.NewStringValue("standard"),
323+
},
324+
},
325+
},
310326
},
311327
opts: nil,
312-
expectError: false, // should be valid
328+
expectError: false,
313329
},
314330
{
315331
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)