oci: include the domainname in "kernel.domainname"#37302
oci: include the domainname in "kernel.domainname"#37302cpuguy83 merged 2 commits intomoby:masterfrom
Conversation
|
We need opencontainers/runc#1827 in order for this to be handled properly on the runtime side. |
Codecov Report
@@ Coverage Diff @@
## master #37302 +/- ##
=========================================
Coverage ? 36.11%
=========================================
Files ? 610
Lines ? 45275
Branches ? 0
=========================================
Hits ? 16349
Misses ? 26685
Partials ? 2241 |
There was a problem hiding this comment.
I should point out that the original code (using FullHostname was not really correct before, since the Domainname was not properly exposed or used). So this part is not an accidental change.
|
And now we need to require opencontainers/runc@3ccfa2f. |
There was a problem hiding this comment.
Hmm, so, if a user have set kernel.domainname in HostConfig.Sysctls it will stop working, right? It seems a backward-incompatible change. Also, since setting sysctls directly is lower level than parameters in Config, it probably should take precedence.
There was a problem hiding this comment.
Hmm, so, if a user have set kernel.domainname in HostConfig.Sysctls it will stop working, right?
I couldn't find anywhere that actually set HostConfig.Sysctls (though since we expose it through the API maybe somebody has used it in the past). But you're right that the precedence is the wrong way around (I forgot that HostConfig was actually accessible through the API -- if it was internal-only this change wouldn't break because I don't think we've ever used Sysctls internally).
8c2f8fc to
aeea5dc
Compare
13e1b36 to
d663653
Compare
d663653 to
6205a5c
Compare
|
Rebased. /ping @kolyshkin |
|
@cyphar Thanks! Can you add a test for this? Namely:
|
6205a5c to
9905cde
Compare
The OCI doesn't have a specific field for an NIS domainname[1] (mainly because FreeBSD and Solaris appear to have a similar concept but it is configured entirely differently). However, on Linux, the NIS domainname can be configured through both the setdomainname(2) syscall but also through the "kernel.domainname" sysctl. Since the OCI has a way of injecting sysctls this means we don't need to have any OCI changes to support NIS domainnames (and we can always switch if the OCI picks up such support in the future). It should be noted that because we have to generate this each spec creation we also have to make sure that it's not clobbered by the HostConfig. I'm pretty sure making this change generic (so that HostConfig will not clobber any pre-set sysctls) will not cause other issues to crop up. [1]: opencontainers/runtime-spec#592 Signed-off-by: Aleksa Sarai <asarai@suse.de>
9905cde to
7e09671
Compare
This also includes a few refactors of oci_linux_test.go. Signed-off-by: Aleksa Sarai <asarai@suse.de>
7e09671 to
f38ac72
Compare
|
@cpuguy83 Tests added. |
|
So, it looks like the last commit removes the very essence of the shm test cases I wrote earlier... |
|
...or maybe not; need to check it while it's not 3am and I'm using a regular PC not a phone. |
|
@kolyshkin The second commit consolidates the "fake daemon" pattern used, and allows us to use |
|
LGTM, thanks @cyphar |
The OCI doesn't have a specific field for an NIS domainname (mainly
because FreeBSD and Solaris appear to have a similar concept but it is
configured entirely differently).
However, on Linux, the NIS domainname can be configured through both the
setdomainname(2) syscall but also through the "kernel.domainname"
sysctl. Since the OCI has a way of injecting sysctls this means we don't
need to have any OCI changes to support NIS domainnames (and we can
always switch if the OCI picks up such support in the future).
My ear tufts let me show you them by Bill Stilwell
Fixes: #27067
Signed-off-by: Aleksa Sarai asarai@suse.de