Disable automount resources for the init-container-home init container

controllers/workspace/devworkspace_controller.go

@@ -353,7 +353,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	// Add automount resources into devfile containers
-	err = automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace)
+	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
 	}

pkg/library/home/persistentHome.go

@@ -92,10 +92,7 @@ func AddPersistentHomeVolume(workspace *common.DevWorkspaceWithConfig) (*v1alpha
 // - 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) {
+	if !PersistUserHomeEnabled(workspace) || !storageStrategySupportsPersistentHome(workspace) {
 		return false
 	}
 	for _, component := range workspace.Spec.Template.Components {
@@ -113,6 +110,10 @@ func NeedsPersistentHomeDirectory(workspace *common.DevWorkspaceWithConfig) bool
 	return storage.WorkspaceNeedsStorage(&workspace.Spec.Template)
 }
 
+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.

pkg/library/lifecycle/prestart.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+	"github.com/devfile/devworkspace-operator/pkg/constants"
 )
 
 // GetInitContainers partitions the components in a devfile's flattened spec into initContainer and non-initContainer lists
@@ -71,9 +72,25 @@ func GetInitContainers(devfile dw.DevWorkspaceTemplateSpecContent) (initContaine
 		}
 	}
 
+	// This is necessary because if there is another initcontainer that runs before it,
+	// an automount resource can mount within /home/user and cause stow conflicts
+	// in init-persistent-home since the automount resource would be saved in the PVC.
+	initContainers = setHomeInitContainerToFront(initContainers)
 	return initContainers, mainComponents, nil
 }
 
+// Takes a slice of components which are intended to run as initContainers and moves the
+// init-persistent-home component to the start of the slice.
+func setHomeInitContainerToFront(initContainers []dw.Component) []dw.Component {
+	for i, container := range initContainers {
+		if container.Name == constants.HomeInitComponentName {
+			initContainers = append(initContainers[:i], initContainers[i+1:]...)
+			return append([]dw.Component{container}, initContainers...)
+		}
+	}
+	return initContainers
+}
+
 func checkPreStartEventCommandsValidity(initCommands []dw.Command) error {
 	for _, cmd := range initCommands {
 		commandType, err := getCommandType(cmd)

pkg/library/lifecycle/prestart_test.go

@@ -56,6 +56,9 @@ func TestGetInitContainers(t *testing.T) {
 		loadPreStartTestCaseOrPanic(t, "prestart_exec_command.yaml"),
 		loadPreStartTestCaseOrPanic(t, "prestart_apply_command.yaml"),
 		loadPreStartTestCaseOrPanic(t, "init_and_main_container.yaml"),
+		loadPreStartTestCaseOrPanic(t, "persistent_home_initcontainer_first_initcontainer.yaml"),
+		loadPreStartTestCaseOrPanic(t, "persistent_home_initcontainer_second_initcontainer.yaml"),
+		loadPreStartTestCaseOrPanic(t, "persistent_home_initcontainer_only_initcontainer.yaml"),
 	}
 
 	for _, tt := range tests {

pkg/library/lifecycle/testdata/prestart/persistent_home_initcontainer_first_initcontainer.yaml

@@ -0,0 +1,45 @@
+name: "Should set the init-persistent-home init container when init-persistent-home is the first preStart event"
+
+input:
+  components:
+    - name: test-container1
+      container:
+        image: my-image
+    - name: test-container2
+      container:
+        image: my-image
+    - name: init-persistent-home
+      container:
+        image: my-image
+  commands:
+    - id: test_preStart_command
+      apply:
+        component: test-container1
+    - id: test_regular_command
+      exec:
+        component: test-container1
+        command: "test_command"
+    - id: init-persistent-home
+      apply:
+        component: init-persistent-home
+  events:
+    preStart:
+      - "init-persistent-home"
+      - "test_preStart_command"
+
+output:
+  initContainers:
+    - name: init-persistent-home
+      container:
+        image: my-image
+    - name: test-container1
+      container:
+        image: my-image
+  mainContainers:
+    - name: test-container1
+      container:
+        image: my-image
+    - name: test-container2
+      container:
+        image: my-image
+  errRegexp:

pkg/library/lifecycle/testdata/prestart/persistent_home_initcontainer_only_initcontainer.yaml

@@ -0,0 +1,38 @@
+name: "Should set the init-persistent-home init container when it is the only init container"
+
+input:
+  components:
+    - name: test-container1
+      container:
+        image: my-image
+    - name: test-container2
+      container:
+        image: my-image
+    - name: init-persistent-home
+      container:
+        image: my-image
+  commands:
+    - id: test_regular_command
+      exec:
+        component: test-container1
+        command: "test_command"
+    - id: init-persistent-home
+      apply:
+        component: init-persistent-home
+  events:
+    preStart:
+      - "init-persistent-home"
+
+output:
+  initContainers:
+    - name: init-persistent-home
+      container:
+        image: my-image
+  mainContainers:
+    - name: test-container1
+      container:
+        image: my-image
+    - name: test-container2
+      container:
+        image: my-image
+  errRegexp:

pkg/library/lifecycle/testdata/prestart/persistent_home_initcontainer_second_initcontainer.yaml

@@ -0,0 +1,45 @@
+name: "Should set the init-persistent-home init container when init-persistent-home is the second preStart event"
+
+input:
+  components:
+    - name: test-container1
+      container:
+        image: my-image
+    - name: test-container2
+      container:
+        image: my-image
+    - name: init-persistent-home
+      container:
+        image: my-image
+  commands:
+    - id: test_preStart_command
+      apply:
+        component: test-container1
+    - id: test_regular_command
+      exec:
+        component: test-container1
+        command: "test_command"
+    - id: init-persistent-home
+      apply:
+        component: init-persistent-home
+  events:
+    preStart:
+      - "test_preStart_command"
+      - "init-persistent-home"
+
+output:
+  initContainers:
+    - name: init-persistent-home
+      container:
+        image: my-image
+    - name: test-container1
+      container:
+        image: my-image
+  mainContainers:
+    - name: test-container1
+      container:
+        image: my-image
+    - name: test-container2
+      container:
+        image: my-image
+  errRegexp:

pkg/provision/automount/common.go

@@ -42,7 +42,7 @@ type Resources struct {
 	EnvFromSource []corev1.EnvFromSource
 }
 
-func ProvisionAutoMountResourcesInto(podAdditions *v1alpha1.PodAdditions, api sync.ClusterAPI, namespace string) error {
+func ProvisionAutoMountResourcesInto(podAdditions *v1alpha1.PodAdditions, api sync.ClusterAPI, namespace string, persistentHome bool) error {
 	resources, err := getAutomountResources(api, namespace)
 
 	if err != nil {
@@ -59,10 +59,16 @@ func ProvisionAutoMountResourcesInto(podAdditions *v1alpha1.PodAdditions, api sy
 	}
 
 	for idx, initContainer := range podAdditions.InitContainers {
+
+		// Don't add automount resources if persistent home init container exists, because
+		// automount resources can conflict with the init container's stow command
+		if persistentHome && initContainer.Name == constants.HomeInitComponentName {
+			continue
+		}
+
 		podAdditions.InitContainers[idx].VolumeMounts = append(initContainer.VolumeMounts, resources.VolumeMounts...)
 		podAdditions.InitContainers[idx].EnvFrom = append(initContainer.EnvFrom, resources.EnvFromSource...)
 	}
-
 	if resources.Volumes != nil {
 		podAdditions.Volumes = append(podAdditions.Volumes, resources.Volumes...)
 	}

pkg/provision/automount/common_persistenthome_test.go

@@ -0,0 +1,93 @@
+//
+// Copyright (c) 2019-2024 Red Hat, Inc.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package automount
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
+	"github.com/devfile/devworkspace-operator/pkg/provision/sync"
+)
+
+func TestProvisionAutomountResourcesIntoPersistentHomeEnabled(t *testing.T) {
+	tests := []testCase{
+		loadTestCaseOrPanic(t, "testdata/testProvisionsConfigmaps.yaml"),
+		loadTestCaseOrPanic(t, "testdata/testProvisionsProjectedVolumes.yaml"),
+		loadTestCaseOrPanic(t, "testdata/testProvisionsSecrets.yaml"),
+	}
+
+	testPodAdditions := &v1alpha1.PodAdditions{
+		Containers: []corev1.Container{{
+			Name:  "test-container",
+			Image: "test-image",
+		}},
+		InitContainers: []corev1.Container{{
+			Name:  "init-persistent-home",
+			Image: "test-image",
+		}, {
+			Name:  "test-container",
+			Image: "test-image",
+		}},
+	}
+
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("%s (%s)", tt.Name, tt.TestPath), func(t *testing.T) {
+			podAdditions := testPodAdditions.DeepCopy()
+			testAPI := sync.ClusterAPI{
+				Client: fake.NewClientBuilder().WithObjects(tt.Input.allObjects...).Build(),
+			}
+
+			err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, true)
+
+			if !assert.NoError(t, err, "Unexpected error") {
+				return
+			}
+			assert.Truef(t, cmp.Equal(tt.Output.Volumes, podAdditions.Volumes, testDiffOpts),
+				"Volumes should match expected output:\n%s",
+				cmp.Diff(tt.Output.Volumes, podAdditions.Volumes, testDiffOpts))
+
+			for _, container := range podAdditions.Containers {
+				assert.Truef(t, cmp.Equal(tt.Output.VolumeMounts, container.VolumeMounts, testDiffOpts),
+					"Container VolumeMounts should match expected output:\n%s",
+					cmp.Diff(tt.Output.VolumeMounts, container.VolumeMounts, testDiffOpts))
+				assert.Truef(t, cmp.Equal(tt.Output.EnvFrom, container.EnvFrom, testDiffOpts),
+					"Container EnvFrom should match expected output:\n%s",
+					cmp.Diff(tt.Output.EnvFrom, container.EnvFrom, testDiffOpts))
+			}
+
+			for _, container := range podAdditions.InitContainers {
+				if container.Name == "init-persistent-home" {
+					assert.Truef(t, container.VolumeMounts == nil || len(container.VolumeMounts) == 0,
+						"The init-persistent-home container should not have any volume mounts if persistent home is enabled")
+				} else {
+					assert.Truef(t, cmp.Equal(tt.Output.VolumeMounts, container.VolumeMounts, testDiffOpts),
+						"Container VolumeMounts should match expected output:\n%s",
+						cmp.Diff(tt.Output.VolumeMounts, container.VolumeMounts, testDiffOpts))
+					assert.Truef(t, cmp.Equal(tt.Output.EnvFrom, container.EnvFrom, testDiffOpts),
+						"Container EnvFrom should match expected output:\n%s",
+						cmp.Diff(tt.Output.EnvFrom, container.EnvFrom, testDiffOpts))
+				}
+			}
+
+		})
+	}
+}

pkg/provision/automount/common_test.go

@@ -125,7 +125,7 @@ func TestProvisionAutomountResourcesInto(t *testing.T) {
 			}
 			// Note: this test does not allow for returning AutoMountError with isFatal: false (i.e. no retrying)
 			// and so is not suitable for testing automount features that provision cluster resources (yet)
-			err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace)
+			err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, false)
 			if tt.Output.ErrRegexp != nil {
 				if !assert.Error(t, err, "Expected an error but got none") {
 					return