Skip to content

Commit 3eb86ae

Browse files
tolushatolusha
committed
feat: Mouning resources should not cause workspace restart
Signed-off-by: Anatolii Bazko <[email protected]>
1 parent b61eaed commit 3eb86ae

File tree

10 files changed

+393
-23
lines changed

10 files changed

+393
-23
lines changed

‎controllers/workspace/devworkspace_controller.go‎

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

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

‎pkg/constants/attributes.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,4 +151,10 @@ const (
151151
// of a cloned project. If the bootstrap process is successful, project-clone will automatically remove this attribute
152152
// from the DevWorkspace
153153
BootstrapDevWorkspaceAttribute = "controller.devfile.io/bootstrap-devworkspace"
154+
155+
// MountOnStartOnlyAttribute is an attribute applied to Kubernetes resources to indicate that they should only
156+
// be mounted to a workspace when it starts from a stopped state. When this attribute is set to "true", newly created
157+
// or updated resources will not be automatically mounted to running workspaces, preventing unwanted workspace
158+
// restarts.
159+
MountOnStartOnlyAttribute = "controller.devfile.io/mount-on-start-only"
154160
)

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

‎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",
@@ -41,6 +41,13 @@ func getDevWorkspaceConfigmaps(namespace string, api sync.ClusterAPI) (*Resource
4141
if msg := checkAutomountVolumeForPotentialError(&configmap); msg != "" {
4242
return nil, &dwerrors.FailError{Message: msg}
4343
}
44+
45+
// Skip mounting if mount-on-start-only is set to "true" and workspace has been already started
46+
mountOnStartOnly := configmap.Annotations[constants.MountOnStartOnlyAttribute] == "true"
47+
if isWorkspaceStarted && mountOnStartOnly {
48+
continue
49+
}
50+
4451
mountAs := configmap.Annotations[constants.DevWorkspaceMountAsAnnotation]
4552
mountPath := configmap.Annotations[constants.DevWorkspaceMountPathAnnotation]
4653
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)