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

Extra option --PersistentVolumeName and --HostPathSource to CheCTL #15157

Closed
gattytto opened this issue Nov 10, 2019 · 4 comments
Closed

Extra option --PersistentVolumeName and --HostPathSource to CheCTL #15157

gattytto opened this issue Nov 10, 2019 · 4 comments
Assignees
Labels
area/chectl Issues related to chectl, the CLI of Che 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.
Milestone

Comments

@gattytto
Copy link

Is your enhancement related to a problem? Please describe.

yes, it is related to this issue #15065 in which the first layer of che (postgres data) loses its data when running inside a minikube node.

Describe the solution you'd like

as far as I can see the problem, there is no "PersistentVolume" reference to pass as an argument when issuing the chectl server:start command to start the che server deployment in a minikube server. Neither there is a property in cr.yaml to specify a PersistentVolume name or a specific path in the host.

This leads to PersistentVolumeClaim(s) created by che letting minikube choose the path for the files (both first layer postgres data and 2nd layer workspaces data), which for the time of making this issue is a folder in /tmp/hostpath-provisioner that gets emptied when kubelet service starts.

Describe alternatives you've considered

for the main postgres instance called postgres-data I have made source changes for a proof of concept that implements the creation of a PersistentVolume using the HostPath type to specify a non-tmp location for the data. The source changes are present in the issue conversation of #15065

as for the workspaces portion, there seems to be a PersistentVolumeClaims strategy property with options to choose from, that creates a single persistent volume claim for each workspace or a common one for all of them so I don't know maybe there could be an extra option for the single-persistent-volume strategy to make sure it uses a custom persitent volume and an extra option to specify the hostPath source (final directory in the node filesystem).

Additional context

there are screenshots posted in the issue mentioned above showing the behauvoir of che when a custom persistent volume is used for the general che postgres data (to have it persisted after a minikube node reboot) and how workspaces are unable to be brought back up when the node gets restarted, even tho the info about them in postgres is persisted, because the workspaces folders in filesystem get deleted from /tmp.

@gattytto gattytto added the kind/enhancement A feature request - must adhere to the feature request template. label Nov 10, 2019
@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 Nov 10, 2019
@tolusha tolusha added area/install Issues related to installation, including offline/air gap and initial setup severity/P1 Has a major impact to usage or development of the system. area/chectl Issues related to chectl, the CLI of Che and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Nov 11, 2019
@tolusha tolusha added this to the Backlog - Deploy milestone Dec 17, 2019
@tolusha tolusha added team/deploy and removed area/install Issues related to installation, including offline/air gap and initial setup labels Dec 18, 2019
@tolusha tolusha mentioned this issue Dec 18, 2019
12 tasks
@AndrienkoAleksandr AndrienkoAleksandr added the status/in-progress This issue has been taken by an engineer and is under active development. label Dec 18, 2019
@tolusha tolusha mentioned this issue Jan 8, 2020
16 tasks
@AndrienkoAleksandr
Copy link
Contributor

AndrienkoAleksandr commented Jan 10, 2020

Hello. I started discussion about current issue in the che-dev mail list.

User asks for implementing for chectl an ability to configurе Eclipse Che host volume paths.

Why do we need this change?
 
User can deploy Eclipse Che on the minikube locally(without virtual machine, it is an option "minikube start ...options... --vm-driver=none"). Also user can deploy Eclipse Che on the LXD container with pre-installed minikube(without VM too, because LXD container it is a light alternative VM). Minikube has hard-coded host volume path to the /tmp/hostpath-provisioner folder. So if application(like Eclipse Che) doesn't create own persisted volume, then minikube provide volumes from `tmp` folder. When user restart computer or lxd container, Eclipse Che loses all volume data with postgres DB and workspaces source code, because system clean up `/tmp` folder. 

Options to solve user's issue:
1. It seems to be not really Che issue but minikube since Kubernetes declares that persistent volumes must keep data even after Deployment or Cluster restart. So, quite logical proposal here could be provide an ability for minikube to configure root folder for PVs, now it's hard-coded to /tmp/hostpath-provisioner[2]. But it's more long term solution since it needs participation of minikube team and minikube release then.

2. As an alternative solution (it's even more workaround) - implement the option user asks for: Che could try to manage host paths manually instead of relying on default minikube hostpath-provisioner.
I've already created pull request[3] to the che-operator to fix this issue for Postgres DB where Che Operator creates Persisted Volume with non /tmp path and then reference it from Persistent Volume Claim for postgres.
But more difficult part here is Che Server part, Che Server itself doesn't need persisted volumes, but they're needed by Workspaces. 
So Che Server can create persistent volume(PV) per workspace/per user and bind Persistent Volume Claim to pre-created PV by class name.
And questions here:
- then Che Server would need cluster-admin rights, now for K8s platform it has but I heard willing that we want to get rid of it;
- then Che Server would do additional logic with managing PV, except preparing we should also think about cleaning them up after workspace removing.
So, do we really need such quite fast but dirty solution to workaround platform issue? It seems it costs for us quite much to implement it and later support.

Before creating an issue for minikube and initing such discussion there I would like to hear opinions

[1] https://github.com/eclipse/che/issues/15157
[2] https://github.com/kubernetes/minikube/blob/master/pkg/storage/storage_provisioner.go#L50
[3] https://github.com/eclipse/che-operator/pull/144
_______________________________________________
che-dev mailing list
che-dev@eclipse.org
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/che-dev

After that I found that minikube has pr to configure host volume path. So I temporary stop work on this issue and I am tracking minikube pr's:

kubernetes/minikube#6156 -> pvc folder path readability
kubernetes/minikube#5400 -> resolves our issue, by customizing host volume path. Work stopped due 2 months. And Ci test fails.
kubernetes/minikube#6257 -> looks like this one should fix ci issue for storage provisioner component.

Will see, Maybe we can help somehow with upstream minikube pr's

@AndrienkoAleksandr AndrienkoAleksandr removed the status/in-progress This issue has been taken by an engineer and is under active development. label Jan 10, 2020
@gattytto
Copy link
Author

gattytto commented Jan 10, 2020

@AndrienkoAleksandr this is an issue that affects and will affect many current and future cloud-native approaches around che. Even though your patch to chectl and operator give a workaround to the situation, I think there is still a chance to fix this witout doing double work for when this new features come to minikube (and other cloud solutions that emulate minikube's behauvoir).

In this case I think the best solution to both postgres data and workspaces data is to be able to choose a PersistentVolumeClaimName cr (and configmap) be it for postgress or single-pvc-strategy workspace data. And I will explain why I think this is the best way to do it:

first minikube will give an option to choose the "default" location for non-specified new persistent volumes. This is too generic and can be solved in a sanest way using the mentioned approach.

PersistentVolumeClaimName option for postgres data or workspaces:
workspacesPersistentVolumeClaimName
postgresPersistentVolumeClaimName
This options will allow the users to specify a pre-created PVC to invoke when any of che contianers request for storage. In this case it's the best approach because I can pre-create the PVC in my cluster and hard-code a PersistentVolumeName property to it.

PersistentVolume is the object that specifies the hostPath source property which is the main subject of this discussion.

instead of --PersistentVolumeName and --HostPathSource to CheCTL it can be workspacesPVCName and postgresPVCName, the rest can be hardcoded to the PVC itself (volume name, storage class, hostPathSource).

@AndrienkoAleksandr
Copy link
Contributor

@gattytto Hello, I think we can use storage class names instead of PVC names. User(administrator Che installation) could provision persisted volumes with storage class names. Then user(administrator) can start new che installation with class names and Che PVC(s) will be bind to created persisted volumes. I tested such Che configuration and it works without che-server and che-operator modification. I prepared new pr for chectl che-incubator/chectl#461
Also we can cover storage class names properties using helm installer #15677

Also I prepared doc:
eclipse-che/che-docs#1039

I believe these pr's it is technical minimum to resolve issue with host volume path without awaiting on minikube enhancements.

@sleshchenko thanks a lot for consultation about PVC configuration Eclipse Che.

@gattytto
Copy link
Author

@gattytto Hello, I think we can use storage class names instead of PVC names. User(administrator Che installation) could provision persisted volumes with storage class names.

that's even better, thank you. I didn't know it could be done this way and it sounds even better

@tolusha tolusha mentioned this issue Jan 27, 2020
35 tasks
@tolusha tolusha modified the milestones: Backlog - Deploy, 7.8.0 Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/chectl Issues related to chectl, the CLI of Che 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

4 participants