Fix for issue preventing linux-headers from being properly exposed within non-ubuntu containers #8
Conversation
…thin non-ubuntu containers Sysbox-mgr was previously hard-coding the location where linux-headers (in the host system) would be bind-mounted into the sys-container. The previous logic was assuming that other linux-distros would also expect linux-headers in the same location (i.e. /usr/src/linux-headers-<*>-generic). During sys-container initialization now we identify the linux-distro within the container's rootfs, and based on that, we bind-mount the host's linux-headers into the path expected by that specific linux-distro. Notice that we are relying on the two most common patterns seen thus far: - centos / redhat / fedora: "/usr/src/kernels/<blah>" - ubuntu / debian / others: "/usr/src/linux-headers-<blah>-generics" This distro-to-path logic will need further refinement as we include support for more distros. Signed-off-by: Rodny Molina <rmolina@nestybox.com>
Signed-off-by: Rodny Molina <rmolina@nestybox.com>
Signed-off-by: Rodny Molina <rmolina@nestybox.com>
Signed-off-by: Rodny Molina <rmolina@nestybox.com>
Signed-off-by: Rodny Molina <rmolina@nestybox.com>
Signed-off-by: Rodny Molina <rmolina@nestybox.com>
utils.go
Outdated
| } | ||
|
|
||
| mounts := []specs.Mount{} | ||
| var src = kernelHdrPath |
There was a problem hiding this comment.
nit: no need for the "src" variable.
There was a problem hiding this comment.
I know. Just trying to avoid another 100+ column-wide line further below.
There was a problem hiding this comment.
Let's keep kernelHdrPath, it's more descriptive than src.
There was a problem hiding this comment.
i don't agree. "src" definition is 3 lines above so there's no room for confusion, and it enhances readability by keeping all code within decent column margins.
utils.go
Outdated
| // Follow symlinks as some distros (e.g., Ubuntu) heavily symlink the linux | ||
| // header directory. | ||
| mounts, err := createMountSpec( | ||
| src, |
There was a problem hiding this comment.
We need a comment here on why we use kernelHdrPath as both the source and destination of the mount: it's because we will in addition create a softlink from the expected kernel header dir in the sys container to the kernelHdrPath (as done in reqFsState()).
There was a problem hiding this comment.
A couple of more nits ...
Also: it would be good if we could have a unit test for reqFsState(). Is it easy to create one? I am thinking we can have the test fake the mgr.hostDistro and mgr.hostKernelHdrPath, as well as the rootfs header path, and then verify that reqFsState does what it needs to do.
Signed-off-by: Rodny Molina <rmolina@nestybox.com>
|
The logic that it would worth to unit-test is the one in getKernelHeaderSoftlink(), and for that to happen there's some mocking that we need to write in libutils. We should do it eventually, and the same applies to this entire mgr.go file. |
Sysbox-mgr was previously hard-coding the location where the host's linux-headers would be bind-mounted within the sys-container. The previous logic was assuming that other linux-distros would also expect linux-headers in the same location (i.e. /usr/src/linux-headers-<*>-generic).
Now, during sys-container initialization, we identify the linux-distro within the container's rootfs, and based on that, we bind-mount the host's linux-headers into the path expected by that specific linux-distro.
We are relying on this simple pattern to decide which path matches every linux-distro:
centos / redhat / fedora: "/usr/src/kernels/"
ubuntu / debian / others: "/usr/src/linux-headers--generics"
This distro-to-path logic will need further refinement as we include support for more distros.
Signed-off-by: Rodny Molina rmolina@nestybox.com