Remove no-op mkdir init container

[sleshchenko]

What does this PR do?

Since we mount subpaths to other init containers, like https://github.com/devfile/devworkspace-operator/blob/master/samples/all-in-one-theia-nodejs.devworkspace.yaml#L112, this mkdir init containers is no-op and does nothing. See example from real Theia-Next pod

{
  "image": "quay.io/eclipse/che-theia-endpoint-runtime-binary:next",
  "imagePullPolicy": "Always",
  "name": "remote-runtime-injector",
  "volumeMounts": [
    {
      "mountPath": "/remote-endpoint",
      "name": "claim-devworkspace",
      "subPath": "workspace0dbcaccab5834328/remote-endpoint/"
    }
  ]
}
{
  "args": [
    "-p",
    "-v",
    "-m",
    "777",
    "/tmp/devworkspaces/workspace0dbcaccab5834328" !!! <- it's no op because kubelet already should init this for remote-endpoint
  ],
  "command": [
    "/usr/bin/mkdir"
  ],
  "image": "registry.access.redhat.com/ubi8/ubi-minimal",
  "imagePullPolicy": "Always",
  "name": "precreate-subpaths",
  "volumeMounts": [
    {
      "mountPath": "/tmp/devworkspaces",
      "name": "claim-devworkspace"
    },
  ]
}

So, this PR just simply removes that init container and if we really need precreate subpaths on some of K8s clusters, we need to implement it with a dedicated pod that will be run before main workspace Deployment.

What issues does this PR fix or reference?

#198

Is it tested? How?

It's not tested yet.

[openshift-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[amisevsk]

I'm generally for removing unnecessary code, but it's worth noting that the original issue the precreate-subpaths init container was made for still exists. I had to poke around old Che issues/PRs, but one of the original issues is in cleaning up workspace storage when using something like the common PVC strategy (sharing one PVC between workspaces). Using the yaml spec

apiVersion: v1
kind: Pod
metadata:
  annotations:
    openshift.io/scc: restricted
  name: test-pod
spec:
  containers:
  - image: quay.io/eclipse/che-theia:7.22.1
    imagePullPolicy: Always
    name: theia-ide
    securityContext:
      runAsUser: 1000710000
    volumeMounts:
    - mountPath: /dir1/
      name: claim-devworkspace
      subPath: test/path1
    - mountPath: /dir2/
      name: claim-devworkspace
      subPath: test/path1/path2/path3/path4
  restartPolicy: Always
  terminationGracePeriodSeconds: 1
  volumes:
  - name: claim-devworkspace
    persistentVolumeClaim:
      claimName: claim-devworkspace

you'll end up with directories path2 and path3 that can only be deleted by the root user:

ls -lR /dir1
/dir1:
total 0
drwxr-x---    3 0        root            19 Dec  2 17:41 path2

/dir1/path2:
total 0
drwxr-x---    3 0        root            19 Dec  2 17:41 path3

/dir1/path2/path3:
total 0
drwxrwx---    2 0        root             6 Dec  2 17:41 path4

On the Che side, we have to run jobs to clean up PVC paths when a workspace is deleted, otherwise workspace PVCs gradually fill up over time. Without the pre-create pod, it's not possible to fully clean a workspace's PVC subpath.

[sleshchenko]

@amisevsk Thanks for figuring out and reminding stuff with cleaning up files from PVC.

So, I've reproduced what you reference with Pods:
Two workspaces created:

• workspacecfcf8477f79945d7 one without mkdir
• workspace256986cb19144cc2 one workspace06843da3e20b4ada (master branch)
  Trying to remove workspace files on PVC, with precreate and without does not work in the both cases, despite of a bit different permissions:
  
  
  
  P.S. In both cases Theia works the same and no issues to write projects or plugins.

Pod I used for testing. No securityContext

apiVersion: v1
kind: Pod
metadata:
  name: mount-root
spec:
  containers:
  - image: quay.io/eclipse/che-theia:7.22.1
    imagePullPolicy: Always
    name: theia-ide
    volumeMounts: 
        -
          "mountPath": "/tmp/devworkspaces"
          "name": "claim-devworkspace"
  restartPolicy: Always
  terminationGracePeriodSeconds: 1
  volumes:
  - name: claim-devworkspace
    persistentVolumeClaim:
      claimName: claim-devworkspace

BUT when I mount claim-devworkspace to deployment, everything works just fine:
workspaceaeb6c2de61904050 with mkdir
workspace06843da3e20b4ada without mkdir

Deployment YAML

apiVersion: apps/v1
kind: Deployment
metadata:
  name: devworkspaces-pvc
spec:
  replicas: 1
  selector:
      matchLabels:
        app: devworkspaces-pvc
  template:
    metadata:
      labels:
        app: devworkspaces-pvc
    spec:
      containers:
      - image: quay.io/eclipse/che-theia:7.22.1
        imagePullPolicy: Always
        name: theia-ide
        volumeMounts: 
            -
              "mountPath": "/devworkspaces"
              "name": "claim-devworkspace"
      restartPolicy: Always
      terminationGracePeriodSeconds: 1
      volumes:
      - name: claim-devworkspace
        persistentVolumeClaim:
          claimName: claim-devworkspace

So, Pod and Deployment work in a different way with PVC. And it may even be different for K8s.

[sleshchenko]

I've created a separate issue to implement a proper mechanism to precreate and clean up subpaths once the workspace is removed #214

[sleshchenko]

The research above did not give me a clear understanding if mkdir as initContainer is really redundant or not 😕

[amisevsk]

Yeah I'm not entirely sure -- as far as I know our usage of PVCs shouldn't be impacted by the original issue, since I think the original problem was due to mounting something like /workspaceId/workspace-logs/log-for-theia, which would result in workspace-logs not having permissions to properly remove files.

Also I've noticed when running in crc and minikube that the UID behaviour is slightly different from production clusters I've used -- I'm able to set UID=0 on crc if I manually create a pod, and in that case I can delete anything I want.

[amisevsk]

I think we should merge this PR as-is. We don't need this functionality now, and whether we need it or not will be decided by how we implement persistent storage. It would not be too difficult to re-add it later if it turns out to be crucial.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, sleshchenko

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [amisevsk,sleshchenko]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci-robot]

New changes are detected. LGTM label has been removed.

[sleshchenko]

On the minikube we have the issue with permissions but that preCreate init container does not help at all:

!!!Without Precreate

> kc exec -ti devworkspaces-pvc-5c57d86ff6-h7ls8 -- /bin/bash
bash-5.0$ cd /devworkspaces/
bash-5.0$ ls -la
total 32
drwxrwxrwx    8 0        root          4096 Dec  4 11:21 .
drwxr-xr-x    1 0        root          4096 Dec  4 11:28 ..
drwxr-xr-x    6 0        root          4096 Dec  4 11:21 workspace217a195d4747498d
bash-5.0$ rm -rf workspace217a195d4747498d
rm: can't remove 'workspace217a195d4747498d/remote-endpoint': Permission denied
rm: can't remove 'workspace217a195d4747498d/plugins': Permission denied
rm: can't remove 'workspace217a195d4747498d/plugins-node-debug-plugin': Permission denied
rm: can't remove 'workspace217a195d4747498d/projects': Permission denied

!!!With Precreate

> kc exec -ti devworkspaces-pvc-5c57d86ff6-mswp8 -- /bin/bash
bash-5.0$ rm -rf /devworkspaces/workspace8d2e5cfbf78b4b91
rm: can't remove '/devworkspaces/workspace8d2e5cfbf78b4b91/remote-endpoint': Permission denied
rm: can't remove '/devworkspaces/workspace8d2e5cfbf78b4b91/plugins': Permission denied
rm: can't remove '/devworkspaces/workspace8d2e5cfbf78b4b91/plugins-node-debug-plugin': Permission denied
rm: can't remove '/devworkspaces/workspace8d2e5cfbf78b4b91/projects': Permission denied

bash-5.0$ cd /devworkspaces
bash-5.0$ ls -la
total 36
drwxrwxrwx    9 0        root          4096 Dec  4 11:30 .
drwxr-xr-x    1 0        root          4096 Dec  4 11:32 ..
drwxr-xr-x    6 0        root          4096 Dec  4 11:21 workspace217a195d4747498d
drwxr-xr-x    6 0        root          4096 Dec  4 11:31 workspace8d2e5cfbf78b4b91

After playing with mkdir pod command, I think it's clear that initContainer can not handle the permissions issue as I initially supposed because subfolders are already created with the wrong permissions and init container is not permitted to change it (without root access):

		Args: []string{
			"-c",
			"ls -la /tmp/devworkspaces/ | grep " + workspaceId +
				"&& ls -la /tmp/devworkspaces/" + workspaceId +
				"&& mkdir -p -v -m 777 /tmp/devworkspaces/" + workspaceId +
				"&& chmod -R 777 /tmp/devworkspaces/" + workspaceId +
				"&& ls -la /tmp/devworkspaces/" + workspaceId,
		},

> kc logs workspace89407d97442642bd-7c6b94bb86-pwxkg -c precreate-subpaths
drwxr-xr-x  4 root root 4096 Dec  4 11:54 workspace89407d97442642bd
total 16
drwxr-xr-x  4 root root 4096 Dec  4 11:54 .
drwxrwxrwx 15 root root 4096 Dec  4 11:54 ..
drwxrwxrwx  3 root root 4096 Dec  4 11:54 plugins
drwxrwxrwx  2 root root 4096 Dec  4 11:54 remote-endpoint
chmod: changing permissions of '/tmp/devworkspaces/workspace89407d97442642bd': Operation not permitted
chmod: changing permissions of '/tmp/devworkspaces/workspace89407d97442642bd/remote-endpoint': Operation not permitted
chmod: changing permissions of '/tmp/devworkspaces/workspace89407d97442642bd/plugins': Operation not permitte