Skip to content

Commit ec869a7

Browse files
tolushatolusha
committed
feat: Mouning resources should not cause workspace restart
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
1 parent 943ae8e commit ec869a7

10 files changed

Lines changed: 398 additions & 23 deletions

File tree

‎controllers/workspace/devworkspace_controller.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,10 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
455455
}
456456

457457
// Add automount resources into devfile containers
458-
err = automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace, home.PersistUserHomeEnabled(workspace))
458+
// Determine if workspace is transitioning from stopped state
459+
// This is true when the workspace phase is not Starting or Running (i.e., it's Stopped, Failed, or empty)
460+
isWorkspaceStarted := workspace.Status.Phase == dw.DevWorkspaceStatusStarting || workspace.Status.Phase == dw.DevWorkspaceStatusRunning
461+
err = automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace, home.PersistUserHomeEnabled(workspace), isWorkspaceStarted)
459462
if shouldReturn, reconcileResult, reconcileErr := r.checkDWError(workspace, err, "Failed to process automount resources", metrics.ReasonBadRequest, reqLogger, &reconcileStatus); shouldReturn {
460463
return reconcileResult, reconcileErr
461464
}

‎pkg/constants/attributes.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,4 +171,10 @@ const (
171171
// controller.devfile.io/restore-source-image: "registry.example.com/backups/my-workspace:20241111-123456"
172172
//
173173
WorkspaceRestoreSourceImageAttribute = "controller.devfile.io/restore-source-image"
174+
175+
// MountOnStartOnlyAttribute is an attribute applied to Kubernetes resources to indicate that they should only
176+
// be mounted to a workspace when it starts from a stopped state. When this attribute is set to "true", newly created
177+
// or updated resources will not be automatically mounted to running workspaces, preventing unwanted workspace
178+
// restarts.
179+
MountOnStartOnlyAttribute = "controller.devfile.io/mount-on-start-only"
174180
)

‎pkg/provision/automount/common.go‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,14 @@ type Resources struct {
4242
EnvFromSource []corev1.EnvFromSource
4343
}
4444

45-
func ProvisionAutoMountResourcesInto(podAdditions *v1alpha1.PodAdditions, api sync.ClusterAPI, namespace string, persistentHome bool) error {
46-
resources, err := getAutomountResources(api, namespace)
45+
func ProvisionAutoMountResourcesInto(
46+
podAdditions *v1alpha1.PodAdditions,
47+
api sync.ClusterAPI,
48+
namespace string,
49+
persistentHome bool,
50+
isWorkspaceStarted bool,
51+
) error {
52+
resources, err := getAutomountResources(api, namespace, isWorkspaceStarted)
4753

4854
if err != nil {
4955
return err
@@ -76,18 +82,18 @@ func ProvisionAutoMountResourcesInto(podAdditions *v1alpha1.PodAdditions, api sy
7682
return nil
7783
}
7884

79-
func getAutomountResources(api sync.ClusterAPI, namespace string) (*Resources, error) {
80-
gitCMAutoMountResources, err := ProvisionGitConfiguration(api, namespace)
85+
func getAutomountResources(api sync.ClusterAPI, namespace string, isWorkspaceStarted bool) (*Resources, error) {
86+
gitCMAutoMountResources, err := ProvisionGitConfiguration(api, namespace, isWorkspaceStarted)
8187
if err != nil {
8288
return nil, err
8389
}
8490

85-
cmAutoMountResources, err := getDevWorkspaceConfigmaps(namespace, api)
91+
cmAutoMountResources, err := getDevWorkspaceConfigmaps(namespace, api, isWorkspaceStarted)
8692
if err != nil {
8793
return nil, err
8894
}
8995

90-
secretAutoMountResources, err := getDevWorkspaceSecrets(namespace, api)
96+
secretAutoMountResources, err := getDevWorkspaceSecrets(namespace, api, isWorkspaceStarted)
9197
if err != nil {
9298
return nil, err
9399
}
@@ -104,7 +110,7 @@ func getAutomountResources(api sync.ClusterAPI, namespace string) (*Resources, e
104110
}
105111
dropItemsFieldFromVolumes(mergedResources.Volumes)
106112

107-
pvcAutoMountResources, err := getAutoMountPVCs(namespace, api)
113+
pvcAutoMountResources, err := getAutoMountPVCs(namespace, api, isWorkspaceStarted)
108114
if err != nil {
109115
return nil, err
110116
}

‎pkg/provision/automount/common_persistenthome_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func TestProvisionAutomountResourcesIntoPersistentHomeEnabled(t *testing.T) {
5656
Client: fake.NewClientBuilder().WithObjects(tt.Input.allObjects...).Build(),
5757
}
5858

59-
err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, true)
59+
err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, true, false)
6060

6161
if !assert.NoError(t, err, "Unexpected error") {
6262
return

‎pkg/provision/automount/common_test.go‎

Lines changed: 219 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/google/go-cmp/cmp/cmpopts"
2727
"github.com/stretchr/testify/assert"
2828
corev1 "k8s.io/api/core/v1"
29+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"sigs.k8s.io/controller-runtime/pkg/client"
3031
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3132
"sigs.k8s.io/yaml"
@@ -126,7 +127,7 @@ func TestProvisionAutomountResourcesInto(t *testing.T) {
126127
}
127128
// Note: this test does not allow for returning AutoMountError with isFatal: false (i.e. no retrying)
128129
// and so is not suitable for testing automount features that provision cluster resources (yet)
129-
err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, false)
130+
err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, false, false)
130131
if tt.Output.ErrRegexp != nil {
131132
if !assert.Error(t, err, "Expected an error but got none") {
132133
return
@@ -407,3 +408,220 @@ func loadTestCaseOrPanic(t *testing.T, testPath string) testCase {
407408
test.TestPath = testPath
408409
return test
409410
}
411+
412+
func TestShouldNotMountSecretWithMountOnStartOnlyIfWorkspaceStarted(t *testing.T) {
413+
testSecret := corev1.Secret{
414+
ObjectMeta: metav1.ObjectMeta{
415+
Name: "test-secret",
416+
Namespace: testNamespace,
417+
Labels: map[string]string{
418+
"controller.devfile.io/mount-to-devworkspace": "true",
419+
},
420+
Annotations: map[string]string{
421+
"controller.devfile.io/mount-as": "file",
422+
"controller.devfile.io/mount-path": "/test/path",
423+
"controller.devfile.io/mount-on-start-only": "true",
424+
},
425+
},
426+
Data: map[string][]byte{
427+
"data": []byte("test"),
428+
},
429+
}
430+
431+
testAPI := sync.ClusterAPI{
432+
Client: fake.NewClientBuilder().WithObjects(&testSecret).Build(),
433+
}
434+
435+
testPodAdditions := &v1alpha1.PodAdditions{
436+
Containers: []corev1.Container{{
437+
Name: "test-container",
438+
Image: "test-image",
439+
}},
440+
}
441+
442+
// When workspace is started (isWorkspaceStarted=true), secret with mount-on-start-only should be skipped
443+
err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, true)
444+
assert.NoError(t, err)
445+
assert.Empty(t, testPodAdditions.Volumes)
446+
assert.Empty(t, testPodAdditions.Containers[0].VolumeMounts)
447+
}
448+
449+
func TestMountSecretWithMountOnStartOnlyIfWorkspaceNotStarted(t *testing.T) {
450+
testSecret := corev1.Secret{
451+
ObjectMeta: metav1.ObjectMeta{
452+
Name: "test-secret",
453+
Namespace: testNamespace,
454+
Labels: map[string]string{
455+
"controller.devfile.io/mount-to-devworkspace": "true",
456+
},
457+
Annotations: map[string]string{
458+
"controller.devfile.io/mount-as": "file",
459+
"controller.devfile.io/mount-path": "/test/path",
460+
"controller.devfile.io/mount-on-start-only": "true",
461+
},
462+
},
463+
Data: map[string][]byte{
464+
"data": []byte("test"),
465+
},
466+
}
467+
468+
testAPI := sync.ClusterAPI{
469+
Client: fake.NewClientBuilder().WithObjects(&testSecret).Build(),
470+
}
471+
472+
testPodAdditions := &v1alpha1.PodAdditions{
473+
Containers: []corev1.Container{{
474+
Name: "test-container",
475+
Image: "test-image",
476+
}},
477+
}
478+
479+
// When workspace is not started (isWorkspaceStarted=false), secret with mount-on-start-only should be mounted
480+
err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, false)
481+
assert.NoError(t, err)
482+
assert.Len(t, testPodAdditions.Volumes, 1)
483+
assert.Len(t, testPodAdditions.Containers[0].VolumeMounts, 1)
484+
assert.Equal(t, "test-secret", testPodAdditions.Volumes[0].Name)
485+
}
486+
487+
func TestShouldNotMountConfigMapWithMountOnStartOnlyIfWorkspaceStarted(t *testing.T) {
488+
testConfigMap := corev1.ConfigMap{
489+
ObjectMeta: metav1.ObjectMeta{
490+
Name: "test-cm",
491+
Namespace: testNamespace,
492+
Labels: map[string]string{
493+
"controller.devfile.io/mount-to-devworkspace": "true",
494+
},
495+
Annotations: map[string]string{
496+
"controller.devfile.io/mount-as": "file",
497+
"controller.devfile.io/mount-path": "/test/path",
498+
"controller.devfile.io/mount-on-start-only": "true",
499+
},
500+
},
501+
Data: map[string]string{
502+
"data": "test",
503+
},
504+
}
505+
506+
testAPI := sync.ClusterAPI{
507+
Client: fake.NewClientBuilder().WithObjects(&testConfigMap).Build(),
508+
}
509+
510+
testPodAdditions := &v1alpha1.PodAdditions{
511+
Containers: []corev1.Container{{
512+
Name: "test-container",
513+
Image: "test-image",
514+
}},
515+
}
516+
517+
// When workspace is started (isWorkspaceStarted=true), configmap with mount-on-start-only should be skipped
518+
err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, true)
519+
assert.NoError(t, err)
520+
assert.Empty(t, testPodAdditions.Volumes)
521+
assert.Empty(t, testPodAdditions.Containers[0].VolumeMounts)
522+
}
523+
524+
func TestMountConfigMapWithMountOnStartOnlyIfWorkspaceNotStarted(t *testing.T) {
525+
testConfigMap := corev1.ConfigMap{
526+
ObjectMeta: metav1.ObjectMeta{
527+
Name: "test-cm",
528+
Namespace: testNamespace,
529+
Labels: map[string]string{
530+
"controller.devfile.io/mount-to-devworkspace": "true",
531+
},
532+
Annotations: map[string]string{
533+
"controller.devfile.io/mount-as": "file",
534+
"controller.devfile.io/mount-path": "/test/path",
535+
"controller.devfile.io/mount-on-start-only": "true",
536+
},
537+
},
538+
Data: map[string]string{
539+
"data": "test",
540+
},
541+
}
542+
543+
testAPI := sync.ClusterAPI{
544+
Client: fake.NewClientBuilder().WithObjects(&testConfigMap).Build(),
545+
}
546+
547+
testPodAdditions := &v1alpha1.PodAdditions{
548+
Containers: []corev1.Container{{
549+
Name: "test-container",
550+
Image: "test-image",
551+
}},
552+
}
553+
554+
// When workspace is not started (isWorkspaceStarted=false), configmap with mount-on-start-only should be mounted
555+
err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, false)
556+
assert.NoError(t, err)
557+
assert.Len(t, testPodAdditions.Volumes, 1)
558+
assert.Len(t, testPodAdditions.Containers[0].VolumeMounts, 1)
559+
assert.Equal(t, "test-cm", testPodAdditions.Volumes[0].Name)
560+
}
561+
562+
func TestShouldNotMountPVCWithMountOnStartOnlyIfWorkspaceStarted(t *testing.T) {
563+
testPVC := corev1.PersistentVolumeClaim{
564+
ObjectMeta: metav1.ObjectMeta{
565+
Name: "test-pvc",
566+
Namespace: testNamespace,
567+
Labels: map[string]string{
568+
"controller.devfile.io/mount-to-devworkspace": "true",
569+
},
570+
Annotations: map[string]string{
571+
"controller.devfile.io/mount-path": "/test/path",
572+
"controller.devfile.io/mount-on-start-only": "true",
573+
},
574+
},
575+
}
576+
577+
testAPI := sync.ClusterAPI{
578+
Client: fake.NewClientBuilder().WithObjects(&testPVC).Build(),
579+
}
580+
581+
testPodAdditions := &v1alpha1.PodAdditions{
582+
Containers: []corev1.Container{{
583+
Name: "test-container",
584+
Image: "test-image",
585+
}},
586+
}
587+
588+
// When workspace is started (isWorkspaceStarted=true), PVC with mount-on-start-only should be skipped
589+
err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, true)
590+
assert.NoError(t, err)
591+
assert.Empty(t, testPodAdditions.Volumes)
592+
assert.Empty(t, testPodAdditions.Containers[0].VolumeMounts)
593+
}
594+
595+
func TestMountPVCWithMountOnStartOnlyIfWorkspaceNotStarted(t *testing.T) {
596+
testPVC := corev1.PersistentVolumeClaim{
597+
ObjectMeta: metav1.ObjectMeta{
598+
Name: "test-pvc",
599+
Namespace: testNamespace,
600+
Labels: map[string]string{
601+
"controller.devfile.io/mount-to-devworkspace": "true",
602+
},
603+
Annotations: map[string]string{
604+
"controller.devfile.io/mount-path": "/test/path",
605+
"controller.devfile.io/mount-on-start-only": "true",
606+
},
607+
},
608+
}
609+
610+
testAPI := sync.ClusterAPI{
611+
Client: fake.NewClientBuilder().WithObjects(&testPVC).Build(),
612+
}
613+
614+
testPodAdditions := &v1alpha1.PodAdditions{
615+
Containers: []corev1.Container{{
616+
Name: "test-container",
617+
Image: "test-image",
618+
}},
619+
}
620+
621+
// When workspace is not started (isWorkspaceStarted=false), PVC with mount-on-start-only should be mounted
622+
err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, false)
623+
assert.NoError(t, err)
624+
assert.Len(t, testPodAdditions.Volumes, 1)
625+
assert.Len(t, testPodAdditions.Containers[0].VolumeMounts, 1)
626+
assert.Equal(t, common.AutoMountPVCVolumeName("test-pvc"), testPodAdditions.Volumes[0].Name)
627+
}

‎pkg/provision/automount/configmap.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import (
2929
"github.com/devfile/devworkspace-operator/pkg/constants"
3030
)
3131

32-
func getDevWorkspaceConfigmaps(namespace string, api sync.ClusterAPI) (*Resources, error) {
32+
func getDevWorkspaceConfigmaps(namespace string, api sync.ClusterAPI, isWorkspaceStarted bool) (*Resources, error) {
3333
configmaps := &corev1.ConfigMapList{}
3434
if err := api.Client.List(api.Ctx, configmaps, k8sclient.InNamespace(namespace), k8sclient.MatchingLabels{
3535
constants.DevWorkspaceMountLabel: "true",
@@ -42,6 +42,13 @@ func getDevWorkspaceConfigmaps(namespace string, api sync.ClusterAPI) (*Resource
4242
if msg := checkAutomountVolumeForPotentialError(&configmap); msg != "" {
4343
return nil, &dwerrors.FailError{Message: msg}
4444
}
45+
46+
// Skip mounting if mount-on-start-only is set to "true" and workspace has been already started
47+
mountOnStartOnly := configmap.Annotations[constants.MountOnStartOnlyAttribute] == "true"
48+
if isWorkspaceStarted && mountOnStartOnly {
49+
continue
50+
}
51+
4552
mountAs := configmap.Annotations[constants.DevWorkspaceMountAsAnnotation]
4653
mountPath := configmap.Annotations[constants.DevWorkspaceMountPathAnnotation]
4754
if mountPath == "" {

‎pkg/provision/automount/gitconfig.go‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ import (
2626
const mergedGitCredentialsMountPath = "/.git-credentials/"
2727

2828
// ProvisionGitConfiguration takes care of mounting git credentials and a gitconfig into a devworkspace.
29-
func ProvisionGitConfiguration(api sync.ClusterAPI, namespace string) (*Resources, error) {
30-
credentialsSecrets, tlsConfigMaps, err := getGitResources(api, namespace)
29+
func ProvisionGitConfiguration(api sync.ClusterAPI, namespace string, isWorkspaceStarted bool) (*Resources, error) {
30+
credentialsSecrets, tlsConfigMaps, err := getGitResources(api, namespace, isWorkspaceStarted)
3131
if err != nil {
3232
return nil, err
3333
}
@@ -69,7 +69,7 @@ func ProvisionGitConfiguration(api sync.ClusterAPI, namespace string) (*Resource
6969
return &resources, nil
7070
}
7171

72-
func getGitResources(api sync.ClusterAPI, namespace string) (credentialSecrets []corev1.Secret, tlsConfigMaps []corev1.ConfigMap, err error) {
72+
func getGitResources(api sync.ClusterAPI, namespace string, isWorkspaceStarted bool) (credentialSecrets []corev1.Secret, tlsConfigMaps []corev1.ConfigMap, err error) {
7373
credentialsLabelSelector := k8sclient.MatchingLabels{
7474
constants.DevWorkspaceGitCredentialLabel: "true",
7575
}
@@ -82,8 +82,13 @@ func getGitResources(api sync.ClusterAPI, namespace string) (credentialSecrets [
8282
return nil, nil, err
8383
}
8484
var secrets []corev1.Secret
85-
if len(secretList.Items) > 0 {
86-
secrets = secretList.Items
85+
for _, secret := range secretList.Items {
86+
// Skip mounting if mount-on-start-only is set to "true" and workspace has been already started
87+
mountOnStartOnly := secret.Annotations[constants.MountOnStartOnlyAttribute] == "true"
88+
if isWorkspaceStarted && mountOnStartOnly {
89+
continue
90+
}
91+
secrets = append(secrets, secret)
8792
}
8893
sortSecrets(secrets)
8994

@@ -92,8 +97,13 @@ func getGitResources(api sync.ClusterAPI, namespace string) (credentialSecrets [
9297
return nil, nil, err
9398
}
9499
var configmaps []corev1.ConfigMap
95-
if len(configmapList.Items) > 0 {
96-
configmaps = configmapList.Items
100+
for _, configmap := range configmapList.Items {
101+
// Skip mounting if mount-on-start-only is set to "true" and workspace has been already started
102+
mountOnStartOnly := configmap.Annotations[constants.MountOnStartOnlyAttribute] == "true"
103+
if isWorkspaceStarted && mountOnStartOnly {
104+
continue
105+
}
106+
configmaps = append(configmaps, configmap)
97107
}
98108
sortConfigmaps(configmaps)
99109

0 commit comments

Comments
 (0)