From 008b03d13623f9e389bcf7fad0df93195bd29e93 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Mon, 10 Feb 2025 15:48:16 +0200 Subject: [PATCH 1/4] fix: auto-mount path collisions Signed-off-by: Oleksii Kurinnyi --- controllers/workspace/devworkspace_controller.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index c505e55d7..4c5f8baf8 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -371,7 +371,9 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } // Add automount resources into devfile containers - err = automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace, home.PersistUserHomeEnabled(workspace)) + if err := automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace, home.PersistUserHomeEnabled(workspace)); err != nil { + return r.failWorkspace(workspace, fmt.Sprintf("Failed to mount automount resources to workspace: %s", err), metrics.ReasonWorkspaceEngineFailure, reqLogger, &reconcileStatus), nil + } if shouldReturn, reconcileResult, reconcileErr := r.checkDWError(workspace, err, "Failed to process automount resources", metrics.ReasonBadRequest, reqLogger, &reconcileStatus); shouldReturn { return reconcileResult, reconcileErr } From 45a6e2f4c06431042c4372cf05980f5ef916c887 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Mon, 17 Feb 2025 14:46:41 +0200 Subject: [PATCH 2/4] fixup! fix: auto-mount path collisions Signed-off-by: Oleksii Kurinnyi --- controllers/workspace/devworkspace_controller.go | 4 +--- pkg/provision/automount/projected.go | 5 ++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 4c5f8baf8..c505e55d7 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -371,9 +371,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } // Add automount resources into devfile containers - if err := automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace, home.PersistUserHomeEnabled(workspace)); err != nil { - return r.failWorkspace(workspace, fmt.Sprintf("Failed to mount automount resources to workspace: %s", err), metrics.ReasonWorkspaceEngineFailure, reqLogger, &reconcileStatus), nil - } + err = automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace, home.PersistUserHomeEnabled(workspace)) if shouldReturn, reconcileResult, reconcileErr := r.checkDWError(workspace, err, "Failed to process automount resources", metrics.ReasonBadRequest, reqLogger, &reconcileStatus); shouldReturn { return reconcileResult, reconcileErr } diff --git a/pkg/provision/automount/projected.go b/pkg/provision/automount/projected.go index 99a7ab03e..3f09ddaf3 100644 --- a/pkg/provision/automount/projected.go +++ b/pkg/provision/automount/projected.go @@ -19,6 +19,7 @@ import ( "strings" "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/dwerrors" corev1 "k8s.io/api/core/v1" "k8s.io/utils/pointer" ) @@ -164,7 +165,9 @@ func checkCanUseProjectedVolumes(volumeMounts []corev1.VolumeMount, volumeNameTo for _, vm := range volumeMounts { problemNames = append(problemNames, formatVolumeDescription(volumeNameToVolume[vm.Name])) } - return fmt.Errorf("auto-mounted volumes from (%s) have the same mount path", strings.Join(problemNames, ", ")) + return &dwerrors.FailError{ + Message: fmt.Sprintf("auto-mounted volumes from (%s) have the same mount path", strings.Join(problemNames, ", ")), + } } return nil } From 1325c96bcee604869f33303f036a539af33cf804 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Tue, 18 Feb 2025 16:33:32 +0200 Subject: [PATCH 3/4] fix: merge projected volumes Signed-off-by: Oleksii Kurinnyi --- pkg/provision/automount/projected.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/provision/automount/projected.go b/pkg/provision/automount/projected.go index 3f09ddaf3..b36510a61 100644 --- a/pkg/provision/automount/projected.go +++ b/pkg/provision/automount/projected.go @@ -54,6 +54,8 @@ func mergeProjectedVolumes(resources *Resources) (*Resources, error) { volumeNameToVolume[volume.Name] = volume } + // Map of merged volume names -> bool, for not merging the same volume twice + mergedVolumeNames := map[string]bool{} for _, mountPath := range mountPathOrder { volumeMounts := mountPathToVolumeMounts[mountPath] switch len(volumeMounts) { @@ -63,7 +65,12 @@ func mergeProjectedVolumes(resources *Resources) (*Resources, error) { // No projected volume necessary mergedResources.VolumeMounts = append(mergedResources.VolumeMounts, volumeMounts[0]) volume := volumeNameToVolume[volumeMounts[0].Name] - mergedResources.Volumes = append(mergedResources.Volumes, volume) + + _, isMerged := mergedVolumeNames[volume.Name] + if !isMerged { + mergedResources.Volumes = append(mergedResources.Volumes, volume) + mergedVolumeNames[volume.Name] = true + } default: vm, vol, err := generateProjectedVolume(mountPath, volumeMounts, volumeNameToVolume) if err != nil { From 1b6cfb2ee376cb1036190299362646e2dff348f1 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Thu, 27 Feb 2025 16:53:35 +0200 Subject: [PATCH 4/4] fixup! fix: merge projected volumes Signed-off-by: Oleksii Kurinnyi --- pkg/provision/automount/projected.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/provision/automount/projected.go b/pkg/provision/automount/projected.go index b36510a61..30fd7f882 100644 --- a/pkg/provision/automount/projected.go +++ b/pkg/provision/automount/projected.go @@ -55,6 +55,7 @@ func mergeProjectedVolumes(resources *Resources) (*Resources, error) { } // Map of merged volume names -> bool, for not merging the same volume twice + // This can happen due to different subpath volume mounts, where the mount path is the same. In this case, there should be only one volume. mergedVolumeNames := map[string]bool{} for _, mountPath := range mountPathOrder { volumeMounts := mountPathToVolumeMounts[mountPath]