che
che copied to clipboard
eclipse-che
As an Admin I want to configure default pvc storage properties from the Eclipse Che CR
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] https://github.com/eclipse-che/che-operator/pull/1324
Describe the solution you'd like
Configure storage related properties from the Eclipse Che CR
Describe alternatives you've considered
N/A
Additional context
No response
A few additional notes:
-
claimSizeis configured in the DevWorkspaceOperatorConfig. However, the DWOC has separate configuration based on workspace storage-type (you can set a different default forper-workspacevscommon). It's not clear which default the CheCluster config refers to -
storageClassis configured in the DWOC as well. -
pvcStrategywould 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,pvcStrategymay be somewhat confusing as the DevWorkspace attribute iscontroller.devfile.io/storage-type
claimSizeis configured in the DevWorkspaceOperatorConfig. However, the DWOC has separate configuration based on workspace storage-type (you can set a different default forper-workspacevscommon). 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.
pvcStrategywould 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,pvcStrategymay be somewhat confusing as the DevWorkspace attribute iscontroller.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.
@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
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.
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:
- Need to make the CR more straightforward. Based on the feedback
commonis 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)
- In the case of the
per-userstrategy, PVC with the given storage class and size will be created by the che-server during the namespace configuration process [1]
Caveats:
- for
per-userstrategy 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-workspaceproperties (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
I don't understand how it correlates with the current CheCluster CR configuration
workspaces:
storage:
pvc:
claimSize: <size>
storageClass: <classname>
pvcStrategy: <strategy>
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:
- rename the PVC strategy
commonasper-user - apply
claimSizeandstorageClassonly if the strategy isper-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>
The issue is that since checluster v2 has released, the pvc field cannot be removed safely.
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.
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
I've verified @tolusha's sample -- seems to work. Don't know what problem I ran into in the past :shrug:
@l0rd @AObuchow @amisevsk @tolusha based on the customer requests I believe we need to make this story a blocker for 3.2 release. All the changes need to be backported to 7.52.x branch
I think that's important to mention that there is an alternative: pre-create the PVC in the developers namespaces. This is a powerful alternative as it let the admin to carefully specify any properties of the developers' PVs.
@l0rd right, but I think you can only pre-create it for per-user (common) strategy e.g. it is not possible to pre-create it for per-workspace
@AObuchow Let me know if you need any help with che-operator part.
@tolusha I'm not sure if this addition to the Che-Operator CRD is required for the 3.2 release, but if it is, then it might be best if you make this change as I am currently still working on the required DWO changes for PVC-configuration-through-Che, and afterwards I need to finish the che-operator patch that will make use of these changes to DWO.
I apologize for asking so late. However, if you are busy with other tasks just let me know, and I'll find some time to get it done :)
There is definitely still time for 3.2 respins, so if you push changes to 7.52.x branch, we'll get 'er built.
@olexii4 Part of this issue requires a change in the Che-Dashboard. The requirement (from my understanding) is to read the Che-Operator CR's pvcStrategy field and inject this value into the controller.devfile.io/storage-type attribute of a user's devfile when starting a workspace.
Something like:
schemaVersion: 2.1.0
metadata:
name: some-devfile
+ attributes:
+ controller.devfile.io/storage-type: per-workspace
projects:
...
The valid values for this attribute should currently be common, per-user and per-workspace, though I'm not sure if validation of these values is required on the dashboard end, as they are validated by the Che-Operator CRD.
The valid values for this attribute should currently be common, per-user and per-workspace, though I'm not sure if validation of these values is required on the dashboard end, as they are validated by the Che-Operator CRD.
We don't have to validate those values on Dashboard side.
@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?
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.
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 :)
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:
- https://github.com/eclipse/che/issues/21707
- https://github.com/eclipse/che/issues/21710
