Skip to content

Commit

Permalink
fix: disable automount resources for the init-persistent-home initcon…
Browse files Browse the repository at this point in the history
…tainer if persistent home is enabled

Fix #1257

Disabling automount resources for the init-persistent-home initconatiner
is done to avoid possible stow conflicts which can be introduced by
automount resources.

Signed-off-by: David Kwon <dakwon@redhat.com>
  • Loading branch information
dkwon17 committed Jun 24, 2024
1 parent f31d8e8 commit ab269ce
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 8 deletions.
2 changes: 1 addition & 1 deletion controllers/workspace/devworkspace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/library/home/persistentHome.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions pkg/library/lifecycle/prestart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 8 additions & 2 deletions pkg/provision/automount/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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...)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/provision/automount/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ab269ce

Please sign in to comment.