Properly share UTS namespaces in a pod #3741
Conversation
|
@matpen PTAL |
|
/approve |
openshift-ci-robot
added
the
approved
|
LGTM |
pkg/spec/spec.go
Outdated
| } else if config.NetMode.IsHost() || config.UtsMode.IsHost() { | ||
| hostname, err = os.Hostname() | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "unable to retrieve hostname") |
There was a problem hiding this comment.
Logging the attempted hostname would help with debugging.
There was a problem hiding this comment.
I would expect hostname to be empty here, because we errored on a call to os.Hostname(). I certainly don't expect it to have anything of value, but I can note that it failed to get the host's hostname
pkg/spec/spec.go
Outdated
| @@ -606,6 +616,9 @@ func addUTSNS(config *CreateConfig, g *generate.Generator) error { | |||
| if utsMode.IsHost() { | |||
| return g.RemoveLinuxNamespace(string(spec.UTSNamespace)) | |||
| } | |||
| if utsMode.IsContainer() { | |||
| logrus.Debug("using container utsmode") | |||
There was a problem hiding this comment.
Which container is using utsmode?
There was a problem hiding this comment.
this is following the format of the other namespace parsing functions. None have access to the container ID, so it's not printed for any of them
There was a problem hiding this comment.
You do see where in a N-user/container environment this is just noise and zero value?
There was a problem hiding this comment.
I certainly do, I'll remove all of them then
There was a problem hiding this comment.
EDIT: I can fix this actually
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, jwhonce, mheon 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:
Approvers can indicate their approval by writing |
haircommander
force-pushed
the
uts-fix
branch
2 times, most recently
from
0a6af4e to
81e9665
Compare
|
/hold |
openshift-ci-robot
added
do-not-merge/hold
Sharing a UTS namespace means sharing the hostname. Fix situations where a container in a pod didn't properly share the hostname of the pod. Signed-off-by: Peter Hunt <[email protected]>
Signed-off-by: Peter Hunt <[email protected]>
|
Test failures look legitimate? |
Signed-off-by: Peter Hunt <[email protected]>
it sure was haha |
If we call Container(), we expect the namespace to be prefixed with "container:". Add this check, and refactor to use named const strings instead of string literals Signed-off-by: Peter Hunt <[email protected]>
|
finally green @mheon @baude @jwhonce @rhatdan @TomSweeneyRedHat PTAL |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
github-actions
bot
added
the
locked - please file new issue/PR
Hostname wasn't previously shared when a container shared a UTS namespace. Fix this, and add a test
fixes: #3547