From 994cb4d395e90c4cb42be0818cc8d4e913793133 Mon Sep 17 00:00:00 2001 From: Andrew Obuchowicz Date: Mon, 1 May 2023 13:47:53 -0400 Subject: [PATCH] Support persistent home directory This commit allows the `/home/user/` directory in workspaces to persist if persistent storage is enabled (through the use of the 'per-user' ('common') or 'per-workspace' storage class). To enable this feature, modify the DWOC and set `config.workspace.persistUserHome.enabled` to `true`. When enabled, a Devfile volume named `persistentHome` will be added to DevWorkspaces. All DevWorkspace container components will mount the `persistentHome` volume at `/home/user/`. If a container component of a DevWorkspace already mounts to `/home/user/`, the DWO-provisioned `persistentHome` volume will not be added to the DevWorkspace, and the DevWorkspace will be given a warning condition. Fix #1097 Signed-off-by: Andrew Obuchowicz --- .../workspace/devworkspace_controller.go | 5 + pkg/constants/constants.go | 4 + pkg/provision/workspace/persistentHome.go | 87 +++++++++++++++ .../workspace/persistentHome_test.go | 105 ++++++++++++++++++ 4 files changed, 201 insertions(+) create mode 100644 pkg/provision/workspace/persistentHome.go create mode 100644 pkg/provision/workspace/persistentHome_test.go diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 866bca868..3c12ebdc0 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -289,6 +289,11 @@ 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 } + err = wsprovision.ProvisionPersistentUserHome(workspace) + if shouldReturn, reconcileResult, reconcileErr := r.checkDWError(workspace, err, "Failed to add persistentHome volume", metrics.ReasonBadRequest, reqLogger, &reconcileStatus); shouldReturn { + return reconcileResult, reconcileErr + } + // Set finalizer on DevWorkspace if necessary // Note: we need to check the flattened workspace to see if a finalizer is needed, as plugins could require storage if storageProvisioner.NeedsStorage(&workspace.Spec.Template) && !controllerutil.ContainsFinalizer(clusterWorkspace, constants.StorageCleanupFinalizer) { diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 71659976c..6e50c15df 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -37,6 +37,10 @@ var ( const ( DefaultProjectsSourcesRoot = "/projects" + HomeUserDirectory = "/home/user/" + + HomeVolumeName = "persistentHome" + ServiceAccount = "devworkspace" SidecarDefaultMemoryLimit = "128M" diff --git a/pkg/provision/workspace/persistentHome.go b/pkg/provision/workspace/persistentHome.go new file mode 100644 index 000000000..529cf9381 --- /dev/null +++ b/pkg/provision/workspace/persistentHome.go @@ -0,0 +1,87 @@ +// +// Copyright (c) 2019-2023 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 workspace + +import ( + "fmt" + + "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/dwerrors" +) + +// Adds a Devfile volume to the given DevWorkspace and mounts the volume to `/home/user/` for every +// container component defined in the DevWorkspace by adding additional volumeMounts. +// Has no effect if `persistUserHome` is disabled in the DevWorkspaceOperatorConfig. +// Returns an error if `persistUserHome` is enabled in the DWOC and at least one of the container components +// in the DevWorkspace already mounts to `/home/user/`. +func ProvisionPersistentUserHome(workspace *common.DevWorkspaceWithConfig) error { + shouldPersistHome, err := needsPersistentHomeDirectory(workspace) + if err != nil { + return err + } + if !shouldPersistHome { + return nil + } + + homeVolume := v1alpha2.Component{ + Name: constants.HomeVolumeName, + ComponentUnion: v1alpha2.ComponentUnion{ + Volume: &v1alpha2.VolumeComponent{}, + }, + } + homeVolumeMount := v1alpha2.VolumeMount{ + Name: constants.HomeVolumeName, + Path: constants.HomeUserDirectory, + } + + workspace.Spec.Template.Components = append(workspace.Spec.Template.Components, homeVolume) + for _, component := range workspace.Spec.Template.Components { + if component.Container == nil { + continue + } + component.Container.VolumeMounts = append(component.Container.VolumeMounts, homeVolumeMount) + } + + return 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 false if `persistUserHome` is disabled. +// Returns an error if at least one of the DevWorkspace's container components already mount a volume to `/home/user/`. +func needsPersistentHomeDirectory(workspace *common.DevWorkspaceWithConfig) (bool, error) { + if !*workspace.Config.Workspace.PersistUserHome.Enabled { + return false, nil + } + for _, component := range workspace.Spec.Template.Components { + if component.Container == nil { + continue + } + for _, volumeMount := range component.Container.VolumeMounts { + if volumeMount.Path == constants.HomeUserDirectory { + // If a volume is already being mounted to /home/user/, it takes precedence + // over the DWO-provisioned home directory volume. + return false, &dwerrors.WarningError{ + Message: fmt.Sprintf("Unable to add persistentHome volume: volume '%s' already mounts to %s", volumeMount.Name, constants.HomeUserDirectory), + } + } + } + } + return true, nil +} diff --git a/pkg/provision/workspace/persistentHome_test.go b/pkg/provision/workspace/persistentHome_test.go new file mode 100644 index 000000000..5941b3f05 --- /dev/null +++ b/pkg/provision/workspace/persistentHome_test.go @@ -0,0 +1,105 @@ +// +// Copyright (c) 2019-2023 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 workspace + +import ( + "os" + "path/filepath" + "testing" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/google/go-cmp/cmp" + "sigs.k8s.io/yaml" + + "github.com/stretchr/testify/assert" +) + +type testCase struct { + Name string `json:"name,omitempty"` + Input testInput `json:"input,omitempty"` + Output testOutput `json:"output,omitempty"` +} + +type testInput struct { + DevWorkspaceID string `json:"devworkspaceId,omitempty"` + Workspace *dw.DevWorkspaceTemplateSpec `json:"workspace,omitempty"` + Config *v1alpha1.OperatorConfiguration `json:"config,omitempty"` +} + +type testOutput struct { + Workspace *dw.DevWorkspaceTemplateSpec `json:"workspace,omitempty"` +} + +func loadTestCaseOrPanic(t *testing.T, testFilepath string) testCase { + bytes, err := os.ReadFile(testFilepath) + if err != nil { + t.Fatal(err) + } + var test testCase + if err := yaml.Unmarshal(bytes, &test); err != nil { + t.Fatal(err) + } + return test +} + +func loadAllTestCasesOrPanic(t *testing.T, fromDir string) []testCase { + files, err := os.ReadDir(fromDir) + if err != nil { + t.Fatal(err) + } + var tests []testCase + for _, file := range files { + if file.IsDir() { + continue + } + tests = append(tests, loadTestCaseOrPanic(t, filepath.Join(fromDir, file.Name()))) + } + return tests +} + +func getDevWorkspaceWithConfig(input testInput) *common.DevWorkspaceWithConfig { + return &common.DevWorkspaceWithConfig{ + DevWorkspace: &dw.DevWorkspace{ + Spec: dw.DevWorkspaceSpec{ + Template: *input.Workspace, + }, + }, + Config: input.Config, + } +} + +func TestPersistentHomeVolume(t *testing.T) { + tests := loadAllTestCasesOrPanic(t, "testdata/persistent-home") + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + // sanity check that file is read correctly. + assert.NotNil(t, tt.Input.Workspace, "Input does not define workspace") + assert.NotNil(t, tt.Input.Config, "Input does not define a config") + workspace := getDevWorkspaceWithConfig(tt.Input) + + if NeedsPersistentHomeDirectory(workspace) { + AddHomeVolume(&workspace.Spec.Template) + } + + assert.Equal(t, tt.Output.Workspace.DevWorkspaceTemplateSpecContent, workspace.Spec.Template.DevWorkspaceTemplateSpecContent, + "DevWorkspace Template Spec should match expected output: Diff: %s", + cmp.Diff(tt.Output.Workspace.DevWorkspaceTemplateSpecContent, workspace.Spec.Template.DevWorkspaceTemplateSpecContent)) + }) + } + +}