runc
runc copied to clipboard
opencontainers
libct: update namespaces handling in the currentState
First determine if the namespace already exists, so it's clearer that it's handling namespaces that aren't included in the `c.config.Namespaces
This is kind of hard to review without any detailed description of what was done and why. @Iceber can you provide us with some more details about why you did this change, what this PR solves etc.
I might be wrong here, but from what I see, before this PR, if the namespace is set in config but not supported by the running kernel, this results in an error. With this PR, a non-supported namespace is ignored. I am not sure this is a correct behavior (and even if it is, it surely requires an explanation and a changelog entry).
@kolyshkin Thanks.
I looked at it again. I realized that I had overlooked the fact that the kernel was not configured with the CONFIG_*_NS options
Although the specconv.CreateLibcontainerConfig filters for namespace not supported by the runc.
https://github.com/opencontainers/runc/blob/497cd0c96ec49c7ad6959c4f1732c65a1f8070c1/libcontainer/specconv/spec_linux.go#L264-L272
And the ConfigValidator also checks if user namespace and cgroup namespace are supported by the system.
https://github.com/opencontainers/runc/blob/497cd0c96ec49c7ad6959c4f1732c65a1f8070c1/libcontainer/configs/validate/validator.go#L41-L46
However, given the kernel CONFIG_*_NS options, the config.Namespaces may still contain namespaces that are not supported by the system.
The current change filters out the unsupported namespaces in the config.Namespaces.
If the kernel is not configured with this CONFIG_NAMESPACE or CONFIG_UTS_NS or CONFIG_IPC_NS or CONFIG_PID_NS or CONFIG_NET_NS option, it will result in unsupported namespaces not being checked in (*linuxContainer).orderNamespacePaths and no error will be reported.
It doesn't feel right to do so. I'm going to modify this pr
LGTM overall. Can you please split the test improvement into a separate commit (as it has nothing to do with the rest of the changes), and describe both commits?