diff --git a/pkg/library/home/persistentHome.go b/pkg/library/home/persistentHome.go index e4d9db97a..b6a643e82 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" @@ -67,8 +68,10 @@ func StorageStrategySupportsPersistentHome(workspace *common.DevWorkspaceWithCon return storageClass != constants.EphemeralStorageClassType } -// 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 +// - 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) { @@ -86,5 +89,5 @@ func NeedsPersistentHomeDirectory(workspace *common.DevWorkspaceWithConfig) bool } } } - return true + return storage.WorkspaceNeedsStorage(&workspace.Spec.Template) } 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 {