Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a CheCluster configuration to persist workspaces $HOME #22102

Closed
l0rd opened this issue Mar 27, 2023 · 31 comments
Closed

Add a CheCluster configuration to persist workspaces $HOME #22102

l0rd opened this issue Mar 27, 2023 · 31 comments
Assignees
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/udi Issues and PRs related to the universal developer image https://github.com/devfile/developer-images engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.

Comments

@l0rd
Copy link
Contributor

l0rd commented Mar 27, 2023

Is your enhancement related to a problem? Please describe

We want to allow users to automatically persist files and folders in their workspaces home directory. Folders such as ~/.m2 would be persisted without any extra setup.

Describe the solution you'd like

kind: CheCluster
apiVersion: org.eclipse.che/v2
spec:
  devEnvironments:
    storage:
      pvcStrategy: per-workspace
      persistUserHomeDirectory: true

A new field persistUserHomeDirectory in CheCluster. If set to true than the workspaces home folders will be persisted as the /projects/ folder.

The pvcStrategy (per-workspace, per-user, ephemeral) will be honored.

The universal developer image should be updated so that files will be copied or linked to $HOME at startup (using the entrypoint). We should avoid to copy files to $HOME when possible as this slows down the workspace startup.

The default is not to persist the home directory. That's because allowing developers to persist changes of the home directory may break the reproducibility of the development environments.

⚠️ We should verify that projected volumes such as git credentials can be mounted even if the whole home directory is a volume.

@l0rd l0rd added the kind/enhancement A feature request - must adhere to the feature request template. label Mar 27, 2023
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Mar 27, 2023
@l0rd l0rd added severity/P1 Has a major impact to usage or development of the system. area/udi Issues and PRs related to the universal developer image https://github.com/devfile/developer-images engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Mar 27, 2023
@amisevsk
Copy link
Contributor

I haven't tested this, but are there any potential issues with workspaces that already mount subdirectories within the home directory? E.g. for our Java sample, we currently explicitly mount .m2 to /home/user/.m2 (I'll test this when I have a moment)

For some reference, the Web Terminal tooling container currently does the suggested copy-files-to-$HOME step in its entrypoint, based off of an initial config directory: source. We do this to allow user-specified containers that use alternate dotfiles (e.g. .bashrc).

@AObuchow
Copy link

I haven't tested this, but are there any potential issues with workspaces that already mount subdirectories within the home directory? E.g. for our Java sample, we currently explicitly mount .m2 to /home/user/.m2 (I'll test this when I have a moment)

In my testing, this should be okay. I was able to mount a PVC within a PVC used by a devworkspace. The resulting volumeMounts in the devworkspace pod spec looked like:

      volumeMounts:
      (...)
        - name: my-pvc
          mountPath: /home/user/my-pvc
        - name: my-inner-pvc
          mountPath: /home/user/my-pvc/my-inner-pvc

@AObuchow
Copy link

@l0rd some questions:

  • For the first iteration of this feature, do we want devEnvironments.persistUserHomeDirectory to affect all workspaces? As opposed to allowing users to toggle this feature on a per-workspace basis (which would most likely require a patch on the dashboard)
  • Does each workspace get their own persistent home directory? Or do all workspaces in a namespace share the same home directory, for example? Is this something we should allow configuration for (e.g. devEnvironments.persistUserHomeDirectory.strategy: per-user/per-workspace) ?
    • In my personal experience, I would like to be able to configure my .bashrc once and have it be used in all my workspaces

@l0rd
Copy link
Contributor Author

l0rd commented Apr 24, 2023

@AObuchow

  • allowing users to toggle this feature is not required for this first iteration
  • having a specific strategy for the home directory is not required neither at this stage: if the pvc strategy is per user we should mount the same home on every workspace, if it's per-worksapce then home should be different for every workspace. If we don't do that users won't be able to run multiple workspaces at the same time even if the PVC strategy is per-workspace and that would be a regression.

@AObuchow
Copy link

@l0rd Thank you for the clarification 😎

Some other questions & thoughts regarding this feature:

  • Should we have a field for configuring the home directory PVC size?
    • If so, should there be 3 separate size fields (one for when per-user storage is used, one for ephemeral and another for per-workspace)?
    • We could also reuse the per-user/per-workspace storage size for the home directory, though that may be suboptimal and lead to wasted space. This also makes it ambiguous of which storage size field to use when ephemeral storage is used.
    • If we decide on a static size value (that could be changed by interacting with the cluster, manually), we will have to come up with a reasonable size.
  • When should the home directory PVC be deleted?
    • I assume the home directory is not supposed to be deleted when the workspace is deleted (thus, being persistent)? This would make sense to me, assuming the home directory PVC will contain user configuration files, build dependencies that can be reused between projects (e.g. binaries/tooling, .m2 cache), etc. and the per-user/per-workspace PVC will contain project source code, build artifacts, etc.
    • For a first iteration, maybe the home directory PVC will require manual cleanup (with kubectl/oc)? In a future release, the dashboard could provide a UI to delete the home directory PVCs (as well as launching a shell or editor to modify the contents of the home directory)?
  • Can you clarify what you meant by "If set to true than the workspaces home folders will be persisted as the /projects/ folder."? Do you mean the projects folder should persist as well (as opposed to populating it by cloning projects)? Or that the home directory will be persistent, similar to how the projects folders seem to persist?

@cgruver
Copy link

cgruver commented Apr 29, 2023

@l0rd @AObuchow FWIW, I would prefer for the $HOME directory to be deleted when the workspace is deleted. I think it might cause unintended problems if it's reused across different workspaces which might have/need conflicting configurations.

My preference would be for a persisted $HOME to be workspace unique like /projects

If I intentionally delete a workspace, then I am stating that I no longer want its state to be retained.

My $0.02... That's in USD... ;-)

@amisevsk
Copy link
Contributor

amisevsk commented May 1, 2023

I'll throw in my C$0.02 and agree with @cgruver. Sharing $HOME across all workspaces in a namespace could lead to unforeseen issues depending on what gets stored there. A potential issue that comes to mind is locally-installed python modules, which would be stored in ~/.local/bin.

It also helps that persisting $HOME per-workspace is a lot easier to implement on our end -- DWO already supports all the pieces (including cleanup) necessary to do this.

To hopefully add some clarity here, the two main approaches being discussed can be applied manually in current Che versions:

  • $HOME directory per-workspace, deleted when workspace is deleted --> Edit your devfile to have a home volume and mount it to all containers at /home/user.
  • $HOME directory shared across all workspaces --> Create a PVC and use automount functionality to mount it to /home/user for all workspaces in the namespace. PVC is never cleaned up and must be deleted by the user.

For dotfiles/configuration that you truly would want to share for all workspaces (e.g. .bashrc), I still think the best approach would be an auto-mounted configmap.

@AObuchow
Copy link

AObuchow commented May 8, 2023

It seems like the common consensus is to not persist $HOME across workspaces (even when using the common/per-user storage strategy), so I will go with that approach.

However, we may want to enable sharing $HOME across workspaces in the future (or add other persistent $HOME settings) if requested by users.

So I propose the Che Cluster CR field be a struct to hold the persistent $HOME settings:

kind: CheCluster
apiVersion: org.eclipse.che/v2
spec:
  devEnvironments:
    storage:
      pvcStrategy: per-workspace
      persistentUserHomeDirectoryConfig:
        enabled: <bool>

If anyone has a better field name than persistentUserHomeDirectoryConfig (which is very long) please share it :)

@cgruver
Copy link

cgruver commented May 8, 2023

persistUserHome:

;-)

@AObuchow
Copy link

Update: I've submitted the DWO-side PR. Once this is merged, I will have to get a Che-Operator PR merged as well for this issue to be resolved.

@AObuchow
Copy link

AObuchow commented Jun 9, 2023

The DWO-side PR is now merged. Once the next DWO release happens, the required Che-Operator PR can be merged (still need to submit it) and this issue will be resolved.

@aufbakanleitung
Copy link

So the issue has been solved? So how do I implement it?
The issue suggests I should edit the dev-file to contain this:

config:
  workspace:
    persistentHomeUserDirectoryConfig:
      enabled: <bool>
+     shareHomePerUser: <bool>

But my dev-file in Openshift DevSpaces doesn't seem to have those variable names. I do see a reference to repo/-/tree/main with a file called .che/che-editor.yaml. But that only contains the line id: che-incubator/che-idea/latest.

Could anyone explain to me how to implement this persistent config?

@amisevsk
Copy link
Contributor

@aufbakanleitung sorry for the confusion. The .config.workspace.persistsentHome setting is going to be included in DevWorkspace Operator v0.22, which should be releasing today. Once that is done, we intend to add functionality to the Che Operator to support these fields.

AObuchow added a commit to AObuchow/che-operator that referenced this issue Jul 20, 2023
Fix eclipse-che/che#22102

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

The Che Operator PR has now been submitted, once merged we can mark this issue as resolved.

AObuchow added a commit to AObuchow/che-operator that referenced this issue Jul 21, 2023
Fix eclipse-che/che#22102

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
AObuchow added a commit to AObuchow/che-operator that referenced this issue Jul 24, 2023
Fix eclipse-che/che#22102

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
AObuchow added a commit to AObuchow/developer-images that referenced this issue Jul 25, 2023
Part of eclipse-che/che#22102

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

Ignore the reference to devfile/developer-images#108, this was done by accident

@AObuchow
Copy link

AObuchow commented Jul 27, 2023

This feature is now merged into upstream Che. To try it out, edit the checluster CR (i.e. kubectl edit checluster -n eclipse-che) and set spec.devEnvironments.persistUserHome.enabled: true.

When enabled, this feature will automatically add a volume mount to all workspace pods that mounts to your PVC at /home/user/ (requires the per-user or per-workspace PVC strategy to be in-use).

Note: the workspace image's /home/user/ directory will be overridden. For example, if you are using the UDI image which has some tools in /home/user/, they will no longer be accessible. This is something we will have to address in the future, which is why part of the reason that this feature is disabled by default.

@aufbakanleitung
Copy link

Really nice that this (pretty important) feature could be integrated so quickly!
I do have some trouble actually implementing the solution though. Not sure if this is the place to ask this, but I'll try anyway:
I'm running Red Hat OpenShift Dev Spaces, which uses CheCluster. When setting up a new CheCluster you can edit the kind: CheCluster yaml and add spec.devEnvironments.persistUserHome.enabled: true. It is unclear to me if they use the latest Che release (with this persistent workspace feature). However I already run into a bigger issue, it doesn't seem to be allowed to edit the Che Cluster configuration of a running instance:

The CheCluster custom resource allows defining and managing Red Hat OpenShift Dev Spaces server installation. Based on these settings, the Operator automatically creates and maintains several ConfigMaps: che, plugin-registry, devfile-registry that will contain the appropriate environment variables of the various components of the installation. These generated ConfigMaps must NOT be updated manually.

Also the oc edit checluster/eclipse-che -n eclipse-che command doesn't work simply because that namespace doesn't exist, nor does the eclipse-che configmap exist in the -n openshift-devspaces namespace.

I'm at a loss about what to do now. Does anyone here have experience with configuration in DevSpaces, and perhaps a solution?

@amisevsk
Copy link
Contributor

amisevsk commented Aug 1, 2023

@aufbakanleitung You should be able to find the CheCluster for your install by just checking all namespaces -- it may not be present in a default namespace: oc get checlusters --all-namespaces. Once you know the namespace, you should be able to edit the object.

The section of documentation you linked is a warning against editing the downstream objects (configmaps, secrets, etc.) as they are defined by the CheCluster, which you can (and should) edit to suit your needs.

@aufbakanleitung
Copy link

Thanks @amisevsk!

Indeed your command found the CheCluster, and with oc edit checluster -n openshift-devspaces I was able to edit it. Though ironically this change to the CheCluster file itself doesn't persist.

spec:
  devEnvironments:
    persistUserHome:
      enabled: true

I also tried the more common enable: true (without the d), alas to no avail.
Any idea what I'm doing wrong?

I'm considering removing the entire CheCluster and spinning up a new one with this setting; to make sure everything is up to date.

@l0rd
Copy link
Contributor Author

l0rd commented Aug 2, 2023

Not sure what version of Dev Spaces you are using but this configuration will be in 3.8 that is going to be GA at the end of the month (i.e. you can only test it with latest Eclipse Che or Dev Spaces 3.8 preview).

@aufbakanleitung
Copy link

Ah I already removed the current one and spun up a new one, but indeed no effect.
Alright, then I'll patiently wait until the next release. Thanks for the help!

@l0rd
Copy link
Contributor Author

l0rd commented Aug 2, 2023

@aufbakanleitung if you have a non-prod cluster to start playing with it I had prepared these scripts to enable it in 3.7.

@aufbakanleitung
Copy link

aufbakanleitung commented Aug 4, 2023

Thanks @l0rd !
What a service!
Alas it didn't quite work for me, perhaps I don't understand exactly what is supposed to happen.

See here my log:

chmod +x 1_create_catalog_source.sh
chmod +x 2_patch_subscription.sh
chmod +x 3_patch_dwoc.sh

./1_create_catalog_source.sh
# catalogsource.operators.coreos.com/devworkspace-operator-catalog created

./2_patch_subscription.sh
# Error from server (NotFound): subscriptions.operators.coreos.com "devworkspace-operator-fast-redhat-operators-openshift-marketplace" not found

oc get subscription --all-namespaces

NAMESPACE             NAME                     PACKAGE                  SOURCE                CHANNEL
nvidia-gpu-operator   gpu-operator-certified   gpu-operator-certified   certified-operators   v23.3
openshift-nfd         nfd                      nfd                      redhat-operators      stable
openshift-operators   devspaces-subscription   devspaces                redhat-operators      stable
pyflask-demo          rhsso-operator           rhsso-operator           redhat-operators      stable
sso                   rhsso-operator           rhsso-operator           redhat-operators      stable

# Renamed variable: SUB_NAME="devspaces-subscription"

./2_patch_subscription.sh
# subscription.operators.coreos.com/devspaces-subscription patched

./3_patch_dwoc.sh
# devworkspaceoperatorconfig.controller.devfile.io/devworkspace-config patched (no change)

oc edit dwoc  # Decide to manually "add config.workspace.persistUserHome.enabled: true"
# doesn't persist

Once again editting the config and adding config.workspace.persistUserHome.enabled: true doesn't persist.

@aufbakanleitung
Copy link

Aii I just noticed I'm not even running 3.7 😅

Dashboard Version: 7.60.2
Server Version: 3.5 @ 4202c #10 :: Eclipse Che Dashboard 7.60.1 @ be8aa

Okay, I'll fix that first then. Sorry for the confusion.

@l0rd
Copy link
Contributor Author

l0rd commented Aug 4, 2023

My bad: the SUB_NAME should be devspaces-subscription yes.

I am not sure why it doesn't persist the DWOC change but between the 2nd and 3rd step you should wait that the operator gets updated.

The procedure should work even if you are using 3.5 but then you deployment may get broken in unpredictable ways. So yes you had better test that on a cluster where you have 3.7.

@aufbakanleitung
Copy link

aufbakanleitung commented Aug 8, 2023

Hey @l0rd,

I've updated to DevSpaces 3.7 but the patching of the DWOC still doesn't work; nor does changing it manually. Changing something else, like setting config.workspace.containerSecurityContext.allowPrivilegeEscalation to false does persist. This makes sense to me, because the persistUserHome variable doesn't exist in 3.7.

For my understanding, how is changing the upstream catalog source and the patching the subscription supposed to do?

I expected your scripts to create a pvc and link the home directories for all the DevSpaces instances to it.

@l0rd
Copy link
Contributor Author

l0rd commented Aug 8, 2023

The first 2 scripts upgrade the DevWorkspace operator to v0.22. The third script changes the configuration of that operator to automatically persist /home/user but only works if the version is 0.22.

@aufbakanleitung
Copy link

So Redhat finally pushed the 3.9 Che version (3.9 :: Eclipse Che Dashboard 7.74.0) and now I was able to add the PersistUserHome.enabled: true variable. It had an effect, because now when I start a terminal in DevSpaces it looks like this:

[user@workspace10b0cf1173bb4c08-ff54d5d65-zq5cb documentation]$ echo $HISTFILE
/home/user/.bash_history

However when I close and open a workspace and press arrow up in the Terminal I still don't see my history.

Any idea what I should do to make it work?

@amisevsk
Copy link
Contributor

@aufbakanleitung it may be due to how the in-browser terminal handles history. We had a similar issue in the Web Terminal Operator and fixed it by adding the following to our default .bashrc:

# Since xterm doesn't save history on exit, we manually sync history on each command
shopt -s histappend
# Append lines from this session to history, clear the session's history, re-read the history file
PROMPT_COMMAND="history -a; history -c; history -r; $PROMPT_COMMAND"

(see here)

@aufbakanleitung
Copy link

Ah nice! Good solution, thanks @amisevsk!
Except I copied your rc files and now my terminal doesn't boot. Any idea what could be the issue?

@amisevsk
Copy link
Contributor

That .bashrc is for the Web Terminal, it's not expected to work in other enviornments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/udi Issues and PRs related to the universal developer image https://github.com/devfile/developer-images engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

6 participants