diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 23f264bd8..720e5c6e2 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/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())) diff --git a/pkg/library/home/persistentHome.go b/pkg/library/home/persistentHome.go index 61626971b..5da933a9d 100644 --- a/pkg/library/home/persistentHome.go +++ b/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 } diff --git a/pkg/provision/storage/asyncStorage.go b/pkg/provision/storage/asyncStorage.go index a17d3648d..1a595b6f4 100644 --- a/pkg/provision/storage/asyncStorage.go +++ b/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 { diff --git a/pkg/provision/storage/commonStorage.go b/pkg/provision/storage/commonStorage.go index 3d23f85cf..eef48d987 100644 --- a/pkg/provision/storage/commonStorage.go +++ b/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 { diff --git a/pkg/provision/storage/commonStorage_test.go b/pkg/provision/storage/commonStorage_test.go index f318cefca..b882f8c67 100644 --- a/pkg/provision/storage/commonStorage_test.go +++ b/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) } }) } diff --git a/pkg/provision/storage/perWorkspaceStorage.go b/pkg/provision/storage/perWorkspaceStorage.go index f05cfc53d..5fcd276f4 100644 --- a/pkg/provision/storage/perWorkspaceStorage.go +++ b/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 } diff --git a/pkg/provision/storage/perWorkspaceStorage_test.go b/pkg/provision/storage/perWorkspaceStorage_test.go index b326ac106..355b99f31 100644 --- a/pkg/provision/storage/perWorkspaceStorage_test.go +++ b/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 diff --git a/pkg/provision/storage/shared.go b/pkg/provision/storage/shared.go index 8d06ed626..ca7a9c9aa 100644 --- a/pkg/provision/storage/shared.go +++ b/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 {