Skip to content

Add --uts=host to allow sharing the UTS namespace#12667

Merged
thaJeztah merged 1 commit intomoby:masterfrom
ibuildthecloud:host-uts
May 14, 2015
Merged

Add --uts=host to allow sharing the UTS namespace#12667
thaJeztah merged 1 commit intomoby:masterfrom
ibuildthecloud:host-uts

Conversation

@ibuildthecloud
Copy link
Copy Markdown
Contributor

Currently it is not possible to share the UTS namespace with the host. --net=host still creates a private UTS namespace. This PR adds the ability to share the UTS namespace. This means a container could set the hostname of the host, or as the hostname of the host changes, the hostname of the container will change too.

@icecrime
Copy link
Copy Markdown
Contributor

A bit of context in here #12185: I'm going to tentatively push this to 2-needs-code-review as I think design itself was proposed by @crosbymichael in #12185 (comment) (I haven't verified if the implementation fits though).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's inconsistent with Ipc and Pid above. Do we care?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibuildthecloud Preferably, we'd follow the Go style guidelines.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I will change the type to UTS

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented May 5, 2015

LGTM
ping @docker/core-maintainers

@jessfraz
Copy link
Copy Markdown
Contributor

jessfraz commented May 5, 2015

I think we can move to docs review LGTM

@stevvooe
Copy link
Copy Markdown
Contributor

stevvooe commented May 5, 2015

@ibuildthecloud Are we not fixing the initialisms? Uts should UTS.

@ibuildthecloud
Copy link
Copy Markdown
Contributor Author

I'll update that in the next hour.

@thaJeztah
Copy link
Copy Markdown
Member

@ibuildthecloud I see that docs aren't there yet, feel free to ping me once they are added. Apart from the "usual" places, perhaps this needs a mention in the "Advanced networking" article https://docs.docker.com/articles/networking/ as well.

@ibuildthecloud
Copy link
Copy Markdown
Contributor Author

Change Uts to UTS everywhere. Also added docs.

@thaJeztah I did look at that networking article and was a bit overwhelmed at where I should put the changes.

@thaJeztah
Copy link
Copy Markdown
Member

@ibuildthecloud yes, it's quite a piece. I'm not really an expert in this area, but I assumed some mentioning there could be useful. Perhaps someone can give a suggestion what kind of information / examples would be useful there? @LK4D4 @stevvooe

I'm okay with adding more information later, but a "in depth" article/reference that doesn't mention a particular option doesn't feel right :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should also be added to the "create" section.

(Unrelated, but it looks like --pid is missing there as well if create supports that, but that's better left for another PR)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should we add the possible value(s) here somehow?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only valid value is really host.

@thaJeztah
Copy link
Copy Markdown
Member

@ibuildthecloud thanks! Added some comments; #12667 (comment) and #12667 (comment) should still be looked at by the other docs-maintainers.

Also, before merging, the commits need to be squashed. If you can do so when addressing the docs reviews, that would be great (in case we overlook it before merging 😄)

@jessfraz jessfraz added this to the 1.7.0 milestone May 8, 2015
@tiborvass
Copy link
Copy Markdown
Contributor

@ibuildthecloud sorry needs rebase :S

@ibuildthecloud ibuildthecloud force-pushed the host-uts branch 2 times, most recently from 21c802f to dd0c1c6 Compare May 13, 2015 13:46
@ibuildthecloud
Copy link
Copy Markdown
Contributor Author

@tiborvass Another day, another rebase :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/is is/is/

@thaJeztah
Copy link
Copy Markdown
Member

Thanks! One small typo, other than that, docs LGTM

ping @fredlf @jamtur01 @moxiegirl @SvenDowideit I left #12667 (comment) for you 😄

@jamtur01
Copy link
Copy Markdown
Contributor

Docs LGTM

Signed-off-by: Darren Shepherd <darren@rancher.com>
@thaJeztah
Copy link
Copy Markdown
Member

Ok, I think this has all the required LGTMs. The Uts -> UTS change was done after the code LGTM, but I think that should be OK. (Famous last words)

Merging

thaJeztah added a commit that referenced this pull request May 14, 2015
Add --uts=host to allow sharing the UTS namespace
@thaJeztah thaJeztah merged commit ed25742 into moby:master May 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants