diff --git a/pkg/library/home/persistentHome.go b/pkg/library/home/persistentHome.go index 534fea9dc..d1b3e2a91 100644 --- a/pkg/library/home/persistentHome.go +++ b/pkg/library/home/persistentHome.go @@ -20,7 +20,6 @@ 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" corev1 "k8s.io/api/core/v1" "k8s.io/utils/pointer" @@ -87,20 +86,22 @@ func AddPersistentHomeVolume(workspace *common.DevWorkspaceWithConfig) (*v1alpha return dwTemplateSpecCopy, nil } -// 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. +// Returns true if persistUserHome is enabled, the workspace has at least one container component, +// and no container already mounts a volume to /home/user/. +// For ephemeral storage workspaces, only returns true if init-persistent-home init container +// is configured in the DevWorkspaceOperatorConfig. This is to allow consistent behaviour +// between ephemeral and non-ephemeral workspaces for setups with a custom init-persistent-home. func NeedsPersistentHomeDirectory(workspace *common.DevWorkspaceWithConfig) bool { - if !PersistUserHomeEnabled(workspace) || !storageStrategySupportsPersistentHome(workspace) { + if !PersistUserHomeEnabled(workspace) { return false } + + hasContainerComponents := false for _, component := range workspace.Spec.Template.Components { if component.Container == nil { continue } + hasContainerComponents = true for _, volumeMount := range component.Container.VolumeMounts { if volumeMount.Path == constants.HomeUserDirectory { // If a volume is already being mounted to /home/user/, it takes precedence @@ -109,19 +110,36 @@ func NeedsPersistentHomeDirectory(workspace *common.DevWorkspaceWithConfig) bool } } } - return storage.WorkspaceNeedsStorage(&workspace.Spec.Template) + + if !hasContainerComponents { + return false + } + + // For ephemeral storage, only add persistent home if init-persistent-home is configured in DWOC + storageType := workspace.Spec.Template.Attributes.GetString(constants.DevWorkspaceStorageTypeAttribute, nil) + if storageType == constants.EphemeralStorageClassType { + return hasInitPersistentHomeInConfig(workspace) + } + + return true } func PersistUserHomeEnabled(workspace *common.DevWorkspaceWithConfig) bool { return pointer.BoolDeref(workspace.Config.Workspace.PersistUserHome.Enabled, false) } -// 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 +func hasInitPersistentHomeInConfig(workspace *common.DevWorkspaceWithConfig) bool { + if workspace.Config == nil || workspace.Config.Workspace == nil { + return false + } + + for _, container := range workspace.Config.Workspace.InitContainers { + if container.Name == constants.HomeInitComponentName { + return true + } + } + + return false } func addInitContainer(dwTemplateSpec *v1alpha2.DevWorkspaceTemplateSpec) error { diff --git a/pkg/library/home/testdata/persistent-home/creates-home-vm-with-ephemeral-storage.yaml b/pkg/library/home/testdata/persistent-home/creates-home-vm-with-ephemeral-storage.yaml new file mode 100644 index 000000000..dfac73415 --- /dev/null +++ b/pkg/library/home/testdata/persistent-home/creates-home-vm-with-ephemeral-storage.yaml @@ -0,0 +1,42 @@ +name: "Creates persistent home volume when persistUserHome is enabled with ephemeral storage and init-persistent-home is explicitly configured" + +input: + devworkspaceId: "test-workspaceid" + config: + workspace: + persistUserHome: + enabled: true + disableInitContainer: true + initContainers: + - name: init-persistent-home + image: testing-image-1 + workspace: + attributes: + controller.devfile.io/storage-type: ephemeral + components: + - name: testing-container-1 + container: + image: testing-image-1 + volumeMounts: + - name: my-defined-volume + path: /my-defined-volume-path + - name: my-defined-volume + volume: {} + +output: + workspace: + attributes: + controller.devfile.io/storage-type: ephemeral + components: + - name: testing-container-1 + container: + image: testing-image-1 + volumeMounts: + - name: my-defined-volume + path: /my-defined-volume-path + - name: persistent-home + path: /home/user/ + - name: my-defined-volume + volume: {} + - name: persistent-home + volume: {} diff --git a/pkg/library/home/testdata/persistent-home/noop-ephemeral-storage-without-init-container.yaml b/pkg/library/home/testdata/persistent-home/noop-ephemeral-storage-without-init-container.yaml new file mode 100644 index 000000000..007f073db --- /dev/null +++ b/pkg/library/home/testdata/persistent-home/noop-ephemeral-storage-without-init-container.yaml @@ -0,0 +1,34 @@ +name: "Does not create persistent home volume for ephemeral storage when init-persistent-home is not explicitly configured" + +input: + devworkspaceId: "test-workspaceid" + config: + workspace: + persistUserHome: + enabled: true + workspace: + attributes: + controller.devfile.io/storage-type: ephemeral + components: + - name: testing-container-1 + container: + image: testing-image-1 + volumeMounts: + - name: my-defined-volume + path: /my-defined-volume-path + - name: my-defined-volume + volume: {} + +output: + workspace: + attributes: + controller.devfile.io/storage-type: ephemeral + components: + - name: testing-container-1 + container: + image: testing-image-1 + volumeMounts: + - name: my-defined-volume + path: /my-defined-volume-path + - name: my-defined-volume + volume: {} diff --git a/pkg/library/home/testdata/persistent-home/noop-if-init-prestartevent-already-defined.yaml b/pkg/library/home/testdata/persistent-home/noop-if-init-prestartevent-already-defined.yaml index 33fd9f541..46dfa09f6 100644 --- a/pkg/library/home/testdata/persistent-home/noop-if-init-prestartevent-already-defined.yaml +++ b/pkg/library/home/testdata/persistent-home/noop-if-init-prestartevent-already-defined.yaml @@ -12,7 +12,7 @@ input: - init-persistent-home output: - error: "failed to add init container for home persistence setup: command with id init-persistent-home already exists in the devworkspace" + error: "failed to add init container for home persistence setup: event with id init-persistent-home already exists in the devworkspace" workspace: events: prestart: