From 16dc1423e82ee1cd530de5299d07ec5476b2e866 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. Fix #1097 Signed-off-by: Andrew Obuchowicz --- .../workspace/devworkspace_controller.go | 10 ++ pkg/constants/constants.go | 4 + pkg/library/home/persistentHome.go | 82 ++++++++++++++ .../workspace/persistentHome_test.go | 105 ++++++++++++++++++ 4 files changed, 201 insertions(+) create mode 100644 pkg/library/home/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..38a43ea70 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -34,6 +34,7 @@ import ( containerlib "github.com/devfile/devworkspace-operator/pkg/library/container" "github.com/devfile/devworkspace-operator/pkg/library/env" "github.com/devfile/devworkspace-operator/pkg/library/flatten" + "github.com/devfile/devworkspace-operator/pkg/library/home" kubesync "github.com/devfile/devworkspace-operator/pkg/library/kubernetes" "github.com/devfile/devworkspace-operator/pkg/library/projects" "github.com/devfile/devworkspace-operator/pkg/library/status" @@ -289,6 +290,15 @@ 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 } + if home.NeedsPersistentHomeDirectory(workspace) { + workspaceWithHomeVolume, err := home.AddPersistentHomeVolume(workspace) + if err != nil { + reconcileStatus.addWarning(fmt.Sprintf("Info: default persistentHome volume is not being used: %s", err.Error())) + } else { + workspace.Spec.Template = *workspaceWithHomeVolume + } + } + // 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/library/home/persistentHome.go b/pkg/library/home/persistentHome.go new file mode 100644 index 000000000..94574d2e3 --- /dev/null +++ b/pkg/library/home/persistentHome.go @@ -0,0 +1,82 @@ +// +// 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 home + +import ( + "fmt" + + "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + devfilevalidation "github.com/devfile/api/v2/pkg/validation" + "k8s.io/utils/pointer" + + "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" +) + +// Returns a modified copy of the given DevWorkspace's Template Spec which contains an additional +// Devfile volume 'persistentHome' that is mounted to `/home/user/` for every container component defined in the DevWorkspace. +// An error is returned if the addition of the 'persistentHome' volume would result +// in an invalid DevWorkspace. +func AddPersistentHomeVolume(workspace *common.DevWorkspaceWithConfig) (*v1alpha2.DevWorkspaceTemplateSpec, error) { + dwTemplateSpecCopy := workspace.Spec.Template.DeepCopy() + homeVolume := v1alpha2.Component{ + Name: constants.HomeVolumeName, + ComponentUnion: v1alpha2.ComponentUnion{ + Volume: &v1alpha2.VolumeComponent{}, + }, + } + homeVolumeMount := v1alpha2.VolumeMount{ + Name: constants.HomeVolumeName, + Path: constants.HomeUserDirectory, + } + + dwTemplateSpecCopy.Components = append(dwTemplateSpecCopy.Components, homeVolume) + for _, component := range dwTemplateSpecCopy.Components { + if component.Container == nil { + continue + } + component.Container.VolumeMounts = append(component.Container.VolumeMounts, homeVolumeMount) + } + + err := devfilevalidation.ValidateComponents(dwTemplateSpecCopy.Components) + if err != nil { + return nil, fmt.Errorf("addition of %s volume would render DevWorkspace invalid: %w", constants.HomeVolumeName, err) + } + + return dwTemplateSpecCopy, 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 otherwise. +func NeedsPersistentHomeDirectory(workspace *common.DevWorkspaceWithConfig) bool { + if !pointer.BoolDeref(workspace.Config.Workspace.PersistUserHome.Enabled, false) { + return false + } + 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 + } + } + } + return true +} 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)) + }) + } + +}