Ensure home persistence can only be enabled when workspace requires persistent storage

controllers/workspace/devworkspace_controller.go

@@ -291,7 +291,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return r.failWorkspace(workspace, fmt.Sprintf("Error provisioning storage: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus), nil
 	}
 
-	if storageProvisioner.NeedsStorage(&workspace.Spec.Template) && home.NeedsPersistentHomeDirectory(workspace) {
+	if home.NeedsPersistentHomeDirectory(workspace) {
 		workspaceWithHomeVolume, err := home.AddPersistentHomeVolume(workspace)
 		if err != nil {
 			reconcileStatus.addWarning(fmt.Sprintf("Info: default persistentHome volume is not being used: %s", err.Error()))

pkg/library/home/persistentHome.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	devfilevalidation "github.com/devfile/api/v2/pkg/validation"
+	"github.com/devfile/devworkspace-operator/pkg/provision/storage"
 	"k8s.io/utils/pointer"
 
 	"github.com/devfile/devworkspace-operator/pkg/common"
@@ -59,13 +60,19 @@ func AddPersistentHomeVolume(workspace *common.DevWorkspaceWithConfig) (*v1alpha
 	return dwTemplateSpecCopy, nil
 }
 
-// Returns true if `persistUserHome` is enabled in the DevWorkspaceOperatorConfig
-// and none of the container components in the DevWorkspace mount a volume to `/home/user/`.
+// Returns true if the following criteria is met:
+// - `persistUserHome` is enabled in the DevWorkspaceOperatorConfig
+// - The storage strategy used by the DevWorkspace supports home persistence
+// - None of the container components in the DevWorkspace mount a volume to `/home/user/`.
+// - Persistent storage is required for the DevWorkspace
 // Returns false otherwise.
 func NeedsPersistentHomeDirectory(workspace *common.DevWorkspaceWithConfig) bool {
 	if !pointer.BoolDeref(workspace.Config.Workspace.PersistUserHome.Enabled, false) {
 		return false
 	}
+	if !storageStrategySupportsPersistentHome(workspace) {
+		return false
+	}
 	for _, component := range workspace.Spec.Template.Components {
 		if component.Container == nil {
 			continue
@@ -78,5 +85,13 @@ func NeedsPersistentHomeDirectory(workspace *common.DevWorkspaceWithConfig) bool
 			}
 		}
 	}
-	return true
+	return storage.WorkspaceNeedsStorage(&workspace.Spec.Template)
+}
+
+// Returns true if the workspace's storage strategy supports persisting the user home directory.
+// The storage strategies which support home persistence are: per-user/common, per-workspace & async.
+// The ephemeral storage strategy does not support home persistence.
+func storageStrategySupportsPersistentHome(workspace *common.DevWorkspaceWithConfig) bool {
+	storageClass := workspace.Spec.Template.Attributes.GetString(constants.DevWorkspaceStorageTypeAttribute, nil)
+	return storageClass != constants.EphemeralStorageClassType
 }

pkg/provision/storage/asyncStorage.go

@@ -45,7 +45,7 @@ type AsyncStorageProvisioner struct{}
 var _ Provisioner = (*AsyncStorageProvisioner)(nil)
 
 func (*AsyncStorageProvisioner) NeedsStorage(workspace *dw.DevWorkspaceTemplateSpec) bool {
-	return needsStorage(workspace)
+	return WorkspaceNeedsStorage(workspace)
 }
 
 func (p *AsyncStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error {

pkg/provision/storage/commonStorage.go

@@ -41,7 +41,7 @@ type CommonStorageProvisioner struct{}
 var _ Provisioner = (*CommonStorageProvisioner)(nil)
 
 func (*CommonStorageProvisioner) NeedsStorage(workspace *dw.DevWorkspaceTemplateSpec) bool {
-	return needsStorage(workspace)
+	return WorkspaceNeedsStorage(workspace)
 }
 
 func (p *CommonStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1.PodAdditions, workspace *common.DevWorkspaceWithConfig, clusterAPI sync.ClusterAPI) error {

pkg/provision/storage/commonStorage_test.go

@@ -288,9 +288,9 @@ func TestNeedsStorage(t *testing.T) {
 			workspace := &dw.DevWorkspaceTemplateSpec{}
 			workspace.Components = tt.Components
 			if tt.NeedsStorage {
-				assert.True(t, needsStorage(workspace), tt.Explanation)
+				assert.True(t, WorkspaceNeedsStorage(workspace), tt.Explanation)
 			} else {
-				assert.False(t, needsStorage(workspace), tt.Explanation)
+				assert.False(t, WorkspaceNeedsStorage(workspace), tt.Explanation)
 			}
 		})
 	}

pkg/provision/storage/perWorkspaceStorage.go

@@ -53,7 +53,7 @@ func (p *PerWorkspaceStorageProvisioner) ProvisionStorage(podAdditions *v1alpha1
 	}
 
 	// If persistent storage is not needed, we're done
-	if !needsStorage(&workspace.Spec.Template) {
+	if !WorkspaceNeedsStorage(&workspace.Spec.Template) {
 		return nil
 	}
 

pkg/provision/storage/perWorkspaceStorage_test.go

@@ -60,7 +60,7 @@ func TestRewriteContainerVolumeMountsForPerWorkspaceStorageClass(t *testing.T) {
 				Logger: zap.New(),
 			}
 
-			if needsStorage(&workspace.Spec.Template) {
+			if WorkspaceNeedsStorage(&workspace.Spec.Template) {
 				err := perWorkspaceStorage.ProvisionStorage(tt.Input.PodAdditions.DeepCopy(), workspace, clusterAPI)
 				if !assert.Error(t, err, "Should get a NotReady error when creating PVC") {
 					return

pkg/provision/storage/shared.go

@@ -37,6 +37,29 @@ import (
 	nsconfig "github.com/devfile/devworkspace-operator/pkg/provision/config"
 )
 
+// WorkspaceNeedsStorage returns true if storage will need to be provisioned for the current workspace. Note that ephemeral volumes
+// do not need to provision storage
+func WorkspaceNeedsStorage(workspace *dw.DevWorkspaceTemplateSpec) bool {
+	projectsVolumeIsEphemeral := false
+	for _, component := range workspace.Components {
+		if component.Volume != nil {
+			// If any non-ephemeral volumes are defined, we need to mount storage
+			if !isEphemeral(component.Volume) {
+				return true
+			}
+			if component.Name == devfileConstants.ProjectsVolumeName {
+				projectsVolumeIsEphemeral = isEphemeral(component.Volume)
+			}
+		}
+	}
+	if projectsVolumeIsEphemeral {
+		// No non-ephemeral volumes, and projects volume mount is ephemeral, so all volumes are ephemeral
+		return false
+	}
+	// Implicit projects volume is non-ephemeral, so any container that mounts sources requires storage
+	return containerlib.AnyMountSources(workspace.Components)
+}
+
 func getPVCSpec(name, namespace string, storageClass *string, size resource.Quantity) (*corev1.PersistentVolumeClaim, error) {
 
 	return &corev1.PersistentVolumeClaim{
@@ -63,29 +86,6 @@ func isEphemeral(volume *dw.VolumeComponent) bool {
 	return volume.Ephemeral != nil && *volume.Ephemeral
 }
 
-// needsStorage returns true if storage will need to be provisioned for the current workspace. Note that ephemeral volumes
-// do not need to provision storage
-func needsStorage(workspace *dw.DevWorkspaceTemplateSpec) bool {
-	projectsVolumeIsEphemeral := false
-	for _, component := range workspace.Components {
-		if component.Volume != nil {
-			// If any non-ephemeral volumes are defined, we need to mount storage
-			if !isEphemeral(component.Volume) {
-				return true
-			}
-			if component.Name == devfileConstants.ProjectsVolumeName {
-				projectsVolumeIsEphemeral = isEphemeral(component.Volume)
-			}
-		}
-	}
-	if projectsVolumeIsEphemeral {
-		// No non-ephemeral volumes, and projects volume mount is ephemeral, so all volumes are ephemeral
-		return false
-	}
-	// Implicit projects volume is non-ephemeral, so any container that mounts sources requires storage
-	return containerlib.AnyMountSources(workspace.Components)
-}
-
 func syncCommonPVC(namespace string, config *v1alpha1.OperatorConfiguration, clusterAPI sync.ClusterAPI) (*corev1.PersistentVolumeClaim, error) {
 	namespacedConfig, err := nsconfig.ReadNamespacedConfig(namespace, clusterAPI)
 	if err != nil {