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

Allow to configure workspaces persistent storage from Eclipse Che CR #21405

Closed
Tracked by #21438 ...
ibuziuk opened this issue May 17, 2022 · 29 comments
Closed
Tracked by #21438 ...

Allow to configure workspaces persistent storage from Eclipse Che CR #21405

ibuziuk opened this issue May 17, 2022 · 29 comments
Assignees
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/dashboard area/devworkspace-operator kind/task Internal things, technical debt, and to-do tasks to be performed. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes sprint/current
Milestone

Comments

@ibuziuk
Copy link
Member

ibuziuk commented May 17, 2022

Is your task related to a problem? Please describe

CheCluster API v2 [1] is going to have the dedicated properties for workspace storage configuration:

devEnvironments:
  storage:
      pvcStrategy: <strategy> // 'per-user' or 'per-workspace'
      perUserStrategyPvcConfig:
        claimSize: <size> // will only be applied for 'per-user'
        storageClass: <classname> // will only be applied for 'per-user'

once specified those properties should be used as defaults for DevWorkspaces creation

[1] eclipse-che/che-operator#1324

Describe the solution you'd like

Configure storage related properties from the Eclipse Che CR

Release Notes Text

Workspaces persistent storage options like the strategy and the size are now configurable using CheCluster CR. It's also possible to set the storage strategy to per-workspace now so that developers will be able to run multiple workspaces concurrently.

@ibuziuk ibuziuk added kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system. area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/devworkspace-operator area/dashboard sprint/next labels May 17, 2022
@ibuziuk ibuziuk changed the title As an Admin I want to configure pvc stroage properties from the Eclipse Che CR As an Admin I want to configure default pvc stroage properties from the Eclipse Che CR May 17, 2022
@ibuziuk ibuziuk changed the title As an Admin I want to configure default pvc stroage properties from the Eclipse Che CR As an Admin I want to configure default pvc storage properties from the Eclipse Che CR May 17, 2022
@ibuziuk ibuziuk mentioned this issue May 18, 2022
49 tasks
@amisevsk
Copy link
Contributor

Related: devfile/devworkspace-operator#843

@amisevsk
Copy link
Contributor

A few additional notes:

  • claimSize is configured in the DevWorkspaceOperatorConfig. However, the DWOC has separate configuration based on workspace storage-type (you can set a different default for per-workspace vs common). It's not clear which default the CheCluster config refers to
  • storageClass is configured in the DWOC as well.
  • pvcStrategy would need to be consumed by whichever component creates DevWorkspaces (e.g. the Dashboard) as DWO does not provide a way to configure the default storage type used for workspaces. Also in terms of naming, pvcStrategy may be somewhat confusing as the DevWorkspace attribute is controller.devfile.io/storage-type

@ibuziuk ibuziuk mentioned this issue Jun 2, 2022
64 tasks
@AObuchow
Copy link

AObuchow commented Jun 21, 2022

claimSize is configured in the DevWorkspaceOperatorConfig. However, the DWOC has separate configuration based on workspace storage-type (you can set a different default for per-workspace vs common). It's not clear which default the CheCluster config refers to

This ambiguity has been confusing me a bit, too. Something that came to mind, however, is perhaps we need to use the pvcStrategy and claimSize in combination to determine which default PVC size should be set on DWO?
E.g. if pvcStrategy is set to per-workspace and claimSize is set to 7Gi then the per-workspace PVC size will be set to 7Gi on the DWOC.

pvcStrategy would need to be consumed by whichever component creates DevWorkspaces (e.g. the Dashboard) as DWO does not provide a way to configure the default storage type used for workspaces. Also in terms of naming, pvcStrategy may be somewhat confusing as the DevWorkspace attribute is controller.devfile.io/storage-type

This question still holds - from what I understand, pvcStrategy should be consumed by the Dashboard to set the storage-type attribute in the workspace.

@ibuziuk
Copy link
Member Author

ibuziuk commented Jun 23, 2022

@amisevsk @AObuchow folks, wdyt about adding default strategy to the DWOC as part of this issue e.g.

  workspace:
    defaultStorageStrategy: 'common| per-workspace | ephemeral'  

This would allow configuring storage from the Che Cluster CR without a need of adding extra attributes on the dashboard level

@amisevsk
Copy link
Contributor

On the DevWorkspace Operator level, we can't change the default. Currently, DWO treats an empty/blank storage type as common -- if it was possible to configure this default then we would risk orphaning workspace data on the cluster whenever it was changed. For example, if the default is changed from common to per-workspace, all existing workspaces would need to be re-provisioned with per-workspace storage, and all storage kept in the common PVC would be untracked.

In fact, the DevWorkspace Operator does not allow changing the storage type of a DevWorkspace once it has been used for this reason. Otherwise, flows like

  • Create common storage DevWorkspace, clone a large repository
  • Switch storage-type to ephemeral
  • Delete the DevWorkspace

would result in all the data saved in the common PVC to be left in-tact with no clear way to clean it up short of doing it manually or deleting the DevWorkspace. Eventually, this would result in the common PVC being full of old data, preventing new workspaces from functioning correctly.

@ibuziuk ibuziuk mentioned this issue Jun 27, 2022
68 tasks
@ibuziuk
Copy link
Member Author

ibuziuk commented Jun 30, 2022

After some discussion, it was decided to change the CR properties to make it more straightforward and do NOT use DWOC for configuring any of the storage properties from the Eclipse Che CR, since it complicates the flow and would create a global DevWorkspace Operator configuration which in some cases might not be desired. The current plan is:

  1. Need to make the CR more straightforward. Based on the feedback common is sometimes wrongly associated with the common PVC provisioned for all users, which is not the case. The suggested CR structure that should make the configuration less ambiguous:
devEnvironments:
  storage:
    pvcStrategy: per-user | per-workspace // `per-user` - single PVC per-user, per-workspace - sinle PVC per-workspace
    claimSize: <size> // applied only for the `per-user`
    storageClass: <classname> // applied only `per-user`

claimSize and storageClass will be relevant only for per-user strategy. (per-workspace properties like size and class can be manually configured by the cluster-admin by creating a dedicated DWOC manually)

  1. In the case of the per-user strategy, PVC with the given storage class and size will be created by the che-server during the namespace configuration process [1]

Caveats:

  • for per-user strategy PVC will not be deleted once all user workspaces are deleted since it is not going to be managed by the DWO directly.
  • if the admin wants to configure per-workspace properties (size / class) they could achieve it by manually creating the DWOC

[1] https://github.com/eclipse-che/che-server/blob/main/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java#L566-L572

@tolusha
Copy link
Contributor

tolusha commented Jul 3, 2022

I don't understand how it correlates with the current CheCluster CR configuration

workspaces:
  storage:
    pvc:
      claimSize: <size>
      storageClass: <classname>
    pvcStrategy: <strategy>

@l0rd
Copy link
Contributor

l0rd commented Jul 4, 2022

I don't understand how it correlates with the current CheCluster CR configuration

workspaces:
  storage:
    pvc:
      claimSize: <size>
      storageClass: <classname>
    pvcStrategy: <strategy>

The idea is to:

  1. rename the PVC strategy common as per-user
  2. apply claimSize and storageClass only if the strategy is per-user

That's why I would rather add a new field:

devEnvironments:
  storage:
    pvcStrategy: <strategy>
-    pvc:
-      claimSize: <size>
-      storageClass: <classname>
+    perUserStrategyPvcConfig:
+      claimSize: <size>
+      storageClass: <classname>

@amisevsk
Copy link
Contributor

amisevsk commented Jul 4, 2022

The issue is that since checluster v2 has released, the pvc field cannot be removed safely.

@tolusha
Copy link
Contributor

tolusha commented Jul 5, 2022

Catalog source to test: docker.io/abazko/catalog:21405
Started subscription:

apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
metadata:
  name: eclipse-che-subscription
  namespace: openshift-operators
spec:
  channel: next
  installPlanApproval: Manual
  name: eclipse-che-preview-openshift
  source: eclipse-che-custom-catalog-source
  sourceNamespace: openshift-operators
  startingCSV: eclipse-che-preview-openshift.v7.50.0-621.next

CheCluster CR:

...
  devEnvironments:
    defaultNamespace:
      template: <username>-che
    storage:
      pvc:
        claimSize: 10Gi
        storageClass: default
      pvcStrategy: common
...      

After upgrading Eclipse Che to eclipse-che-preview-openshift.v7.51.0-622.next where devEnvironments.storage.pvc field is removed from a CRD, CheCluster CR looks like:

...
  devEnvironments:
    defaultNamespace:
      template: <username>-che
    storage:
      pvcStrategy: common
...      

So, there are no issues with upgrading.

@ibuziuk
Copy link
Member Author

ibuziuk commented Jul 5, 2022

On my end, I have verified on the dogfooding instance that once namespace is removed it will be recreated by che-server on the next login, which means that namespace configuration [1] routine will be triggered

[1] https://github.com/eclipse-che/che-server/blob/main/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesNamespaceFactory.java#L566-L572

@amisevsk
Copy link
Contributor

amisevsk commented Jul 5, 2022

I've verified @tolusha's sample -- seems to work. Don't know what problem I ran into in the past 🤷

AObuchow added a commit to AObuchow/devworkspace-operator that referenced this issue Jul 15, 2022
The 'per-user' storage type is an alternate storage-class name for the 'common' storage class.
In other words, it is an alias for the 'common' storage class and behaves in the same manner.

Part of eclipse-che/che#21405

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
AObuchow added a commit to AObuchow/devworkspace-operator that referenced this issue Jul 15, 2022
The 'per-user' storage-type is an alternate storage-type name for the 'common' storage-type.
In other words, it is an alias for the 'common' storage-type and behaves in the same manner
as the 'common' storage-type.

Part of eclipse-che/che#21405

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
@ibuziuk ibuziuk mentioned this issue Jul 19, 2022
51 tasks
AObuchow added a commit to AObuchow/devworkspace-operator that referenced this issue Aug 30, 2022
Allow specifying an external DWOC that gets merged with the workspace's
internal DWOC.

The external DWOC's name and namespace are specified with the `controller.devfile.io/devworkspace-config` DevWorkspace attribute,
which has the following structure:

attributes:
  controller.devfile.io/devworkspace-config:
    name: <string>
    namespace: <string>

Part of eclipse-che/che#21405

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
AObuchow added a commit to AObuchow/devworkspace-operator that referenced this issue Aug 30, 2022
Allow specifying an external DWOC that gets merged with the workspace's
internal DWOC.

The external DWOC's name and namespace are specified with the `controller.devfile.io/devworkspace-config` DevWorkspace attribute,
which has the following structure:

attributes:
  controller.devfile.io/devworkspace-config:
    name: <string>
    namespace: <string>

Part of eclipse-che/che#21405

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

AObuchow commented Aug 30, 2022

@olexii4 I apologize for the late notice, I realized another attribute needs to be added to devfiles for this feature to work.

The attribute is controller.devfile.io/devworkspace-config, which has the following structure:

attributes:
  controller.devfile.io/devworkspace-config:
    name: <string>
    namespace: <string>

It specifies the name and namespace of the external, che-operator-provided devworkspace-operator configuration that should be used for the workspace.

My task today will be to work on the che-operator patch that creates this devworkspace-operator configuration that the attribute references.

There are 2 ambiguities, however, @ibuziuk @l0rd:

  • What should be the name of the che-provided DWOC? che-DWOC?
  • In which namespace should this DWOC exist in? I assume the same namespace as che is deployed in. @tolusha Is there a way for the dashboard to find out which namespace che is deployed in?

@ibuziuk
Copy link
Member Author

ibuziuk commented Aug 30, 2022

What should be the name of the che-provided DWOC? che-DWOC?

I would not prefix it with che- since it will be also used by Dev Spaces. Could be just devworkspace-config.yaml I think

in which namespace should this DWOC exist in?

We are going to have a single DWOC used by all users, right? in this case it makes sense to have it in the namespace where Eclipse Che CR is created.

@l0rd
Copy link
Contributor

l0rd commented Aug 30, 2022

👍 I agree with @ibuziuk answers

@tolusha
Copy link
Contributor

tolusha commented Aug 30, 2022

7.52.x: eclipse-che/che-operator#1490

@AObuchow
Copy link

We are going to have a single DWOC used by all users, right?

Correct

in this case it makes sense to have it in the namespace where Eclipse Che CR is created.

Sounds good :)

@ibuziuk ibuziuk removed the severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code. label Aug 31, 2022
AObuchow added a commit to devfile/devworkspace-operator that referenced this issue Sep 2, 2022
Allow specifying an external DWOC that gets merged with the global DWOC for use by a workspace.
During the merge, the fields which are set in the external DWOC will overwrite those existing in the internal/global DWOC,
resulting in a merged DWOC to be used by the workspace. The internal/global DWOC will remain unchanged after the merge.

The external DWOC's name and namespace are specified with the `controller.devfile.io/devworkspace-config` DevWorkspace attribute,
which has the following structure:

attributes:
  controller.devfile.io/devworkspace-config:
    name: <string>
    namespace: <string>

Part of eclipse-che/che#21405

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
amisevsk pushed a commit to devfile/devworkspace-operator that referenced this issue Sep 10, 2022
Allow specifying an external DWOC that gets merged with the global DWOC for use by a workspace.
During the merge, the fields which are set in the external DWOC will overwrite those existing in the internal/global DWOC,
resulting in a merged DWOC to be used by the workspace. The internal/global DWOC will remain unchanged after the merge.

The external DWOC's name and namespace are specified with the `controller.devfile.io/devworkspace-config` DevWorkspace attribute,
which has the following structure:

attributes:
  controller.devfile.io/devworkspace-config:
    name: <string>
    namespace: <string>

Part of eclipse-che/che#21405

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
AObuchow added a commit to devfile/devworkspace-operator that referenced this issue Sep 12, 2022
Allow specifying an external DWOC that gets merged with the global DWOC for use by a workspace.
During the merge, the fields which are set in the external DWOC will overwrite those existing in the internal/global DWOC,
resulting in a merged DWOC to be used by the workspace. The internal/global DWOC will remain unchanged after the merge.

The external DWOC's name and namespace are specified with the `controller.devfile.io/devworkspace-config` DevWorkspace attribute,
which has the following structure:

attributes:
  controller.devfile.io/devworkspace-config:
    name: <string>
    namespace: <string>

Part of eclipse-che/che#21405

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

ibuziuk commented Sep 16, 2022

Today, after merging all the relevant PRs and testing on dogfooding 2 more issues have been created that need to be resolved before closing this story:

@l0rd l0rd changed the title As an Admin I want to configure default pvc storage properties from the Eclipse Che CR Allow to configure workspaces persistent storage from Eclipse Che CR Sep 22, 2022
@l0rd l0rd added new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes status/release-notes-review-needed Issues that needs to be reviewed by the doc team for the Release Notes wording labels Sep 22, 2022
@l0rd l0rd added this to the 7.54 milestone Sep 22, 2022
@l0rd
Copy link
Contributor

l0rd commented Sep 29, 2022

Closing as completed

@l0rd l0rd closed this as completed Sep 29, 2022
@max-cx
Copy link

max-cx commented Dec 15, 2022

Sync'd with Red Hat JIRA https://issues.redhat.com/browse/CRW-3630

@max-cx max-cx removed the status/release-notes-review-needed Issues that needs to be reviewed by the doc team for the Release Notes wording label Jan 12, 2023
@devstudio-release
Copy link

sync'd to Red Hat JIRA https://issues.redhat.com/browse/CRW-3854

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/dashboard area/devworkspace-operator kind/task Internal things, technical debt, and to-do tasks to be performed. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes sprint/current
Projects
None yet
Development

No branches or pull requests

8 participants