Workspace files are not removed when Common PVC strategy if configured

[sleshchenko]

Description

Workspace files are not removed when Common PVC strategy is configured.

P.S. Logged error message is not useful at all. It is needed to consider fething Job Pod logs if possible in case of error in this issue or a separate one.

Reproduction Steps

1. Deploy Che Server on OpenShift (minishift on my test case) with common PVC strategy configured.
2. Create a workspace from Che 7 stack or any other.
3. Start the workspace.
4. Stop the workspace.
5. Delete the workspace.
6. Che PV folder for common PVC and check Che Server logs.
   Expected: PV folder does not contain filed of removed workspace anymore. Che Server log does not container error about failing of files removing job.
   Actual: PV folder still contains files of removed workspaces. Che Server log contains error that occurred during files cleaning up. See Diagnostics section.

OS and version:
Che Server 6.18.0-SNAPSHOT
Docker image docker.io/eclipse/che-server:nightly (987c479083cd 16.01.2018 14:07)

Single-User Che was deployed on minishift

serg@sergpad:~/projects/che$ minishift version
minishift v1.26.1+1e20f27
serg@sergpad:~/projects/che$ oc version
oc v3.10.0-rc.0+c20e215
kubernetes v1.10.0+b81c8f8
features: Basic-Auth GSSAPI Kerberos SPNEGO

Diagnostics:

Che Server Logs

2019-01-16 12:00:12,536[er-ThreadPool-0]  [ERROR] [e.c.w.i.k.n.p.PVCSubPathHelper 148]  - Job 
command '[rm, -rf, /tmp/job_mount/workspace41yc8hv9o2pacs9i]' execution is failed. Status 
'PodStatus(conditions=[PodCondition(lastProbeTime=null, lastTransitionTime=2019-01-16T12:00:10Z, 
message=null, reason=null, status=True, type=Initialized, additionalProperties={}), 
PodCondition(lastProbeTime=null, lastTransitionTime=2019-01-16T12:00:10Z, message=containers with 
unready status: [rm-workspace41yc8hv9o2pacs9i], reason=ContainersNotReady, status=False, 
type=Ready, additionalProperties={}), PodCondition(lastProbeTime=null, lastTransitionTime=null, 
message=containers with unready status: [rm-workspace41yc8hv9o2pacs9i], 
reason=ContainersNotReady, status=False, type=ContainersReady, additionalProperties={}), 
PodCondition(lastProbeTime=null, lastTransitionTime=2019-01-16T12:00:10Z, message=null, 
reason=null, status=True, type=PodScheduled, additionalProperties={})], containerStatuses=
[ContainerStatus(containerID=docker://e4a949a63f8d4b1f087a9d3510cdbb0b3d3bc87793ece82e9f15bd
b337d54c35, image=docker.io/centos:centos7, imageID=docker-
pullable://docker.io/centos@sha256:184e5f35598e333bfa7de10d8fb1cebb5ee4df5bc0f970bf2b1e7c734
5136426, lastState=ContainerState(running=null, terminated=null, waiting=null, additionalProperties={}), 
name=rm-workspace41yc8hv9o2pacs9i, ready=false, restartCount=0, 
state=ContainerState(running=null, 
terminated=ContainerStateTerminated(containerID=docker://e4a949a63f8d4b1f087a9d3510cdbb0b3d3b
c87793ece82e9f15bdb337d54c35, exitCode=1, finishedAt=2019-01-16T12:00:11Z, message=null, 
reason=Error, signal=null, startedAt=2019-01-16T12:00:11Z, additionalProperties={}), waiting=null, 
additionalProperties={}), additionalProperties={})], hostIP=10.0.2.15, initContainerStatuses=[], 
message=null, phase=Failed, podIP=172.17.0.8, qosClass=Burstable, reason=null, startTime=2019-01-
16T12:00:10Z, additionalProperties={})'.

OpenShift Events

PV folder after workspace removing

[root@minishift openshift.local.pv]# tree pv0045
pv0045
|-- workspace41yc8hv9o2pacs9i
| |-- che-logs
| | |-- che-plugin-broker
| | | |-- broker5ib5vi
| | | -- brokerjglup9 | | -- ws
| | |-- che-machine-exec
| | |-- dev
| | -- theia-ide | |-- plugins | -- projects

[sleshchenko]

@eclipse/eclipse-che-qa During manual testing of my PR I discovered two bugs with removing workspaces data after workspaces removing (for common and per-workspace PVCs strategies). Is there any chance to add selenium tests for these use-cases? Should I create a separate issue?

[rhopp]

@sleshchenko That's interesting... We've already found this issue while testing Codeready Workspaces, but from latest comment it seems it's no longer reproducible: https://issues.jboss.org/browse/CRW-69
I guess we have to take a look at this issue once again.

[amisevsk]

After looking into this issue, it looks like this is expected behaviour on the Kubernetes side when CHE_INFRA_KUBERNETES_PVC_PRECREATE__SUBPATHS is false.

In the common strategy, we create a pod with subpaths like:

Mount: claim-che-workspace, subpath workspacex/projects → /projects
Mount: claim-che-workspace, subpath workspacex/m2 → /home/user/.m2
Mount: claim-che-workspace, subpath workspacex/javadata → /home/user/jdtls/data 
Mount: claim-che-workspace, subpath workspacex/che-logs/dev-machine → /workspace_logs

Kubernetes will create the subpath directories in the PVC as needed. However, only the leaf directory has write permissions:

sh-4.2$ find . -maxdepth 3 -exec stat --format='%A %U %G %n' '{}' \;
drwxrwx--- root root .
drwxr-x--- root root ./workspacefpb6ksbn4z8fiet6
drwxrwx--- root root ./workspacefpb6ksbn4z8fiet6/projects
drwxrwx--- root root ./workspacefpb6ksbn4z8fiet6/m2
drwxrwx--- root root ./workspacefpb6ksbn4z8fiet6/javadata
drwxr-x--- root root ./workspacefpb6ksbn4z8fiet6/che-logs
drwxrwx--- root root ./workspacefpb6ksbn4z8fiet6/che-logs/dev-machine

sh-4.2$ id
uid=1000130000 gid=0(root) groups=0(root),1000130000

This means that the cleanup job that is created when a workspace is deleted cannot remove folders in the workspace directory.

As a workaround, the setting CHE_INFRA_KUBERNETES_PVC_PRECREATE__SUBPATHS needs to be set to true, since this will force Che to manually create the directories before mounting.

drwxr-xr-x 1000130000 root ./workspacewrenyfxrresivmu1
drwxr-xr-x 1000130000 root ./workspacewrenyfxrresivmu1/m2
drwxr-xr-x 1000130000 root ./workspacewrenyfxrresivmu1/projects
drwxr-xr-x 1000130000 root ./workspacewrenyfxrresivmu1/che-logs
drwxr-xr-x 1000130000 root ./workspacewrenyfxrresivmu1/javadata
drwxr-xr-x 1000130000 root ./workspacewrenyfxrresivmu1/che-logs/dev-machine

As further background (thanks @wongma7 for the help), AFAICT this change came about in fixing CVE-2017-1002101, as chmoding the subpaths is labelled TODO since that PR.

The only detail I haven't been able to figure out is why we only noticed this now -- it seems like this should have been a problem for quite some time (the commit fixing the CVE was 10 months ago).

[amisevsk]

To be clear however, the files inside the folders are deleted successfully, so minimal space is wasted.

[amisevsk]

PR #12513 sets precreate property to true by default.

[amisevsk]

Created PR #12514 to grab logs from failed pods.

[amisevsk]

Closing issue as there's nothing more to be done on our end.