kubelet.makeMounts() is mixing the usage of the kubecontainer.Mount.Name

[yifan-gu]

kubelet.makeMounts is a help function that runs during each container creation.
It takes a pod spec, and a container spec to generate kubecontainer.Mount (name, container-path, host-path, readonly, selinux_relabel) by matching the container spec's volume mount to the pod spec's volumes.

But meanwhile, it's also generating arbitrary kubecontainer.Mount that has no corresponding volumes in the pod spec, such as mounting host machine's /etc/hosts.

Thus when runtime tries to look for the volumes by the volume name in the interface, it cannot find any volumes.

Another issue is if container spec defines a subPath in the volume mount, then we will end up with two kubecontainer.Mount that have same names, same container paths, but different host paths. This is not supported by rkt as one volume name should map to one host path.

So the action required here is:
Option 1: Generate correct volumes for these arbitrary mounts and subpath mounts, and pass them through PodSandboxConfig during the creation of the sandbox. Later it passes the runtime the mounts object (volume_name, container_path), and runtime is able to do the correct mounts by finding the volumes through names.

This implies the mounts for each apps within the sand box must have a pre-established volumes.

Option 2: Remove the mount name, or at least change that to not be the volume name. Require runtime to support mounting any container-path:host-path pairs, basically requires runtime to know nothing about volumes

cc @yujuhong @feiskyer @euank @alban @lucab @jonboulle

[yifan-gu]

cc @kubernetes/sig-node @kubernetes/sig-rktnetes

[jonboulle]

Volumes don't exist at all in the CRI API at the moment, which suggests that 2 is already the implicit path?

[yifan-gu]

@jonboulle Yes, the current docker implementation is the path 2. And according to @squeed rkt/rkt#3239 (comment) , it will will be doable by rkt soon. So I guess the action we need to do is to clean up the kubecontainer.Mount.Name, either remove it, or make it unique for each container-path:host-path pair.

[pmorie]

For the record, I would also like makeMounts to make the mountpoints for container termination logs.

[yifan-gu]

@pmorie Exactly!

[yujuhong]

For the record, I would also like makeMounts to make the mountpoints for container termination logs.

I have no objection to that.

[yifan-gu]

@pmorie @yujuhong Actually the makeMounts() in kuberuntime_container.go already makes the mount point for the termination log, so the mount points in CRI has the termination message log mount already. What we need to do is removing the old makeMounts() in kubelet_pods.go.

It should be in part of the whole removing pkg/kubelet/container/runtime.go work once docker is fully migrated to CRI.