Allow enabling persistence of /home/user/ in workspaces #1737

AObuchow · 2023-07-20T18:33:49Z

What does this PR do?

This PR adds 2 new fields to the Che Cluster CR:

spec.devEnvironments.persistUserHome: holds config settings related to the persistence of /home/user/ in workspaces
spec.devEnvironments.persistUserHome.enabled determines whether /home/user/ will persist in workspaces. Disabled by default.

Note: this PR depends on DWO 0.22.0 which has currently only been released upstream.

Screenshot/screencast of this PR

n/a

What issues does this PR fix or reference?

eclipse-che/che#22102

How to test this PR?

Ensure that changes to spec.devEnvironments.persistUserHome.enabled from the Che Cluster CR are propagated to the Che-owned DWOC's config.workspace.persistUserHome:

Setting spec.devEnvironments.persistUserHome.enabled: true in the Che Cluster CR results in config.workspace.persistUserHome.enabled: true for the Che-owned DWOC
Setting spec.devEnvironments.persistUserHome.enabled: false in the Che Cluster CR results in config.workspace.persistUserHome.enabled: false for the Che-owned DWOC
Removing spec.devEnvironments.persistUserHome in the Che Cluster CR (i.e. setting it to nil) removes config.workspace.persistUserHome in the Che-owned DWOC

For example, for test case 1. do:

kubectl edit checluster -n eclipse-che

(...)
spec:
(...)
  devEnvironments:
    containerBuildConfiguration:
      openShiftSecurityContextConstraint: container-build
    defaultNamespace:
      autoProvision: true
      template: <username>-che
    maxNumberOfWorkspacesPerUser: -1
+    persistUserHome:
+      enabled: true
    secondsOfInactivityBeforeIdling: 1800
    secondsOfRunBeforeIdling: -1
    startTimeoutSeconds: 300
    storage:
      pvcStrategy: per-user

kubectl describe dwoc -n eclipse-che and ensure persistUserHome is now enabled:

Name:         devworkspace-config  
Namespace:    eclipse-che  
Labels:       <none>  
Annotations:  <none>  
API Version:  controller.devfile.io/v1alpha1  
Config:  
 Workspace:  
   Container Security Context:  
     Allow Privilege Escalation:  true  
     Capabilities:  
       Add:  
         SETGID  
         SETUID  
   Deployment Strategy:  Recreate  
+   Persist User Home:  
+     Enabled:         true  
   Progress Timeout:  300s  
   Service Account:  
     Disable Creation:  false  
Kind:                    DevWorkspaceOperatorConfig

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

openshift-ci · 2023-07-20T18:34:01Z

Hi @AObuchow. Thanks for your PR.

I'm waiting for a eclipse-che member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

codecov · 2023-07-20T18:37:19Z

Codecov Report

Merging #1737 (a63572c) into main (923972e) will decrease coverage by 0.10%.
The diff coverage is 29.62%.

❗ Current head a63572c differs from pull request most recent head 3e2c49e. Consider uploading reports for the commit 3e2c49e to get more accurate results

@@            Coverage Diff             @@
##             main    #1737      +/-   ##
==========================================
- Coverage   60.06%   59.96%   -0.10%     
==========================================
  Files          71       71              
  Lines        8423     8450      +27     
==========================================
+ Hits         5059     5067       +8     
- Misses       3017     3036      +19     
  Partials      347      347

Impacted Files	Coverage Δ
api/v2/checluster_types.go	`22.64% <ø> (ø)`
api/v2/zz_generated.deepcopy.go	`0.00% <0.00%> (ø)`
...eploy/dev-workspace-config/dev_workspace_config.go	`94.36% <100.00%> (+0.33%)`	⬆️

amisevsk

Generally LGTM; IIRC this is one we were considering backporting to 7.72.x

pkg/deploy/dev-workspace-config/dev_workspace_config.go

tolusha · 2023-07-21T07:26:02Z

api/v2/checluster_types.go

@@ -85,6 +85,10 @@ type CheClusterDevEnvironments struct {
 	// +optional
 	// +kubebuilder:default:={pvcStrategy: per-user}
 	Storage WorkspaceStorage `json:"storage,omitempty"`
+	// PersistUserHome defines configuration options for persisting the `/home/user/`


Can we not to mention specifically /home/user?
For instance: Defined configuration options for persisting a home user directory in a workspace

That's okay with me, will make the appropriate change 👍

I did not modify the documentation for persistUserHome.enabled to remove the mention of /home/user/ as it felt appropriate to explicitly state which directory would persist. However, I am open to reconsidering this decision.

tolusha · 2023-07-24T09:26:15Z

@AObuchow
Could you run make update-dev-resources and commit changes

AObuchow · 2023-07-24T14:14:04Z

@AObuchow Could you run make update-dev-resources and commit changes

Done 👍 I removed the original update dev resources commit and re-ran make update-dev-resources

AObuchow · 2023-07-24T14:22:52Z

api/v2/checluster_types.go

-    // Must be used with the 'per-user' or 'per-workspace' PVC strategy in order to take effect.
-    // Disabled by default.
-    Enabled *bool `json:"enabled,omitempty"`
+	// Determines whether the `/home/user/` directory in workspaces should persist between


@tolusha is this formatting problematic?

That's not a problem.
Could you remove references to /home/user from the comments in checluster_types.go and regenerate dev resources?

Will do 👍

Fix eclipse-che/che#22102 Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

…config Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

AObuchow · 2023-07-24T16:07:34Z

I believe the failing tests are due to che-server:next currently having a bug that's causing a crash loop backoff

tolusha · 2023-07-25T11:17:23Z

/retest

openshift-ci · 2023-07-25T13:57:04Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: amisevsk, AObuchow, tolusha

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

devstudio-release · 2023-07-27T14:39:21Z

Build 3.9 :: operator-bundle_3.x/1628: Console, Changes, Git Data

devstudio-release · 2023-07-27T14:45:01Z

Build 3.9 :: sync-to-downstream_3.x/3815: Console, Changes, Git Data

devstudio-release · 2023-07-27T14:49:54Z

Build 3.9 :: get-sources-rhpkg-container-build_3.x/3665: Console, Changes, Git Data

devstudio-release · 2023-07-27T15:06:33Z

Build 3.9 :: push-latest-container-to-quay_3.x/2763: Console, Changes, Git Data

devstudio-release · 2023-07-27T15:11:23Z

Build 3.9 :: get-sources-rhpkg-container-build_3.x/3665:

devspaces-operator-bundle : 3.x :: Build 54231547 : quay.io/devspaces/devspaces-operator-bundle:3.9-6
SUCCESS; copied to quay; /DS_CI/push-latest-container-to-quay_3.x/2763 triggered;
Bundle CSV related images:
- quay.io/devspaces/code-rhel8:3.9-1
- quay.io/devspaces/configbump-rhel8:3.9-4
- quay.io/devspaces/dashboard-rhel8:3.9-2
- quay.io/devspaces/devfileregistry-rhel8:3.9-4
- quay.io/devspaces/devspaces-rhel8-operator:3.9-4
- quay.io/devspaces/idea-rhel8:3.9-4
- quay.io/devspaces/pluginregistry-rhel8:3.9-5
- quay.io/devspaces/server-rhel8:3.9-3
- quay.io/devspaces/traefik-rhel8:3.9-2
- quay.io/devspaces/udi-rhel8:3.9-2
= registry.redhat.io/openshift4/ose-kube-rbac-proxy:v4.12.0-202307170916.p0.g94f3fde.assembly.stream
= registry.redhat.io/openshift4/ose-oauth-proxy:v4.12.0-202307071529.p0.g03e5b13.assembly.stream
= registry.redhat.io/rhscl/mongodb-36-rhel7:1-50
= registry.redhat.io/ubi8/ubi-minimal:8.8-1014

devstudio-release · 2023-07-27T15:11:26Z

Build 3.9 :: copyIIBsToQuay/1554: Console, Changes, Git Data

devstudio-release · 2023-07-27T15:11:27Z

Build 3.9 :: sync-to-downstream_3.x/3815:

Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.x/3665 triggered; /job/DS_CI/job/dsc_3.x triggered;

devstudio-release · 2023-07-27T15:11:28Z

Build 3.9 :: operator-bundle_3.x/1628:

Upstream sync done; /DS_CI/sync-to-downstream_3.x/3815 triggered

devstudio-release · 2023-07-27T15:13:56Z

Build 3.9 :: dsc_3.x/1053: Console, Changes, Git Data

devstudio-release · 2023-07-27T15:20:53Z

Build 3.9 :: dsc_3.x/1053:

3.9.0 CI

AObuchow requested review from nickboldt, tolusha, ibuziuk and l0rd as code owners July 20, 2023 18:33

openshift-ci bot added the needs-ok-to-test label Jul 20, 2023

AObuchow mentioned this pull request Jul 20, 2023

Add a CheCluster configuration to persist workspaces $HOME eclipse-che/che#22102

Closed

amisevsk reviewed Jul 20, 2023

View reviewed changes

pkg/deploy/dev-workspace-config/dev_workspace_config.go Outdated Show resolved Hide resolved

pkg/deploy/dev-workspace-config/dev_workspace_config.go Outdated Show resolved Hide resolved

tolusha reviewed Jul 21, 2023

View reviewed changes

tolusha added the ok-to-test label Jul 21, 2023

AObuchow force-pushed the persist_user_home_rebased branch from a63572c to 7f8863e Compare July 21, 2023 21:13

openshift-ci bot removed the needs-ok-to-test label Jul 21, 2023

AObuchow force-pushed the persist_user_home_rebased branch from 7f8863e to b728c1e Compare July 24, 2023 14:13

AObuchow commented Jul 24, 2023

View reviewed changes

AObuchow added 4 commits July 24, 2023 15:24

feat: add persistUserHome Che Cluster CR field

ebe3340

Fix eclipse-che/che#22102 Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

feat: allow configuring persistent workspace home directory

c1245c5

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

chore: add tests for configuring persistent workspace home directory …

f9d80eb

…config Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

chore: update dev resources for persistUserHome Che Cluster CR field

3e2c49e

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>

AObuchow force-pushed the persist_user_home_rebased branch from b728c1e to 3e2c49e Compare July 24, 2023 15:26

amisevsk approved these changes Jul 24, 2023

View reviewed changes

openshift-ci bot assigned amisevsk Jul 24, 2023

openshift-ci bot added the lgtm label Jul 24, 2023

tolusha approved these changes Jul 25, 2023

View reviewed changes

openshift-ci bot assigned tolusha Jul 25, 2023

ibuziuk merged commit cbf068f into eclipse-che:main Jul 27, 2023
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow enabling persistence of /home/user/ in workspaces #1737

Allow enabling persistence of /home/user/ in workspaces #1737

AObuchow commented Jul 20, 2023 •

edited by tolusha

Loading

openshift-ci bot commented Jul 20, 2023

codecov bot commented Jul 20, 2023 •

edited

Loading

amisevsk left a comment

tolusha Jul 21, 2023

AObuchow Jul 21, 2023

AObuchow Jul 21, 2023

tolusha commented Jul 24, 2023

AObuchow commented Jul 24, 2023

AObuchow Jul 24, 2023

tolusha Jul 24, 2023

AObuchow Jul 24, 2023

AObuchow commented Jul 24, 2023

tolusha commented Jul 25, 2023

openshift-ci bot commented Jul 25, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

Allow enabling persistence of /home/user/ in workspaces #1737

Allow enabling persistence of /home/user/ in workspaces #1737

Conversation

AObuchow commented Jul 20, 2023 • edited by tolusha Loading

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

openshift-ci bot commented Jul 20, 2023

codecov bot commented Jul 20, 2023 • edited Loading

Codecov Report

amisevsk left a comment

Choose a reason for hiding this comment

tolusha Jul 21, 2023

Choose a reason for hiding this comment

AObuchow Jul 21, 2023

Choose a reason for hiding this comment

AObuchow Jul 21, 2023

Choose a reason for hiding this comment

tolusha commented Jul 24, 2023

AObuchow commented Jul 24, 2023

AObuchow Jul 24, 2023

Choose a reason for hiding this comment

tolusha Jul 24, 2023

Choose a reason for hiding this comment

AObuchow Jul 24, 2023

Choose a reason for hiding this comment

AObuchow commented Jul 24, 2023

tolusha commented Jul 25, 2023

openshift-ci bot commented Jul 25, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

devstudio-release commented Jul 27, 2023

AObuchow commented Jul 20, 2023 •

edited by tolusha

Loading

codecov bot commented Jul 20, 2023 •

edited

Loading