Skip to content

run.md: --expose does NOT override Dockerfile EXPOSE#14625

Closed
SpencerBrown wants to merge 1 commit intomoby:masterfrom
SpencerBrown:patch-1
Closed

run.md: --expose does NOT override Dockerfile EXPOSE#14625
SpencerBrown wants to merge 1 commit intomoby:masterfrom
SpencerBrown:patch-1

Conversation

@SpencerBrown
Copy link

run.md states that the operator can override all defaults set in the Dockerfile, and explicitly says that --expose overrides the EXPOSE instruction. Neither of these are true. An EXPOSE instruction cannot be overridden, --expose can only add additional exposed ports.

This change fixes the instructions, and also takes the liberty of crisping up the grammar and phrasing in a place or two.

@GordonTheTurtle GordonTheTurtle added status/3-docs-review dco/no Automatically set by a bot when one of the commits lacks proper signature labels Jul 14, 2015
run.md states that the operator can override all defaults set in the Dockerfile, and explicitly says that `--expose` overrides the `EXPOSE` instruction. Neither of these are true. An `EXPOSE` instruction cannot be overridden, `--expose` can only add additional exposed ports.

This change fixes the instructions, and also takes the liberty of crisping up the grammar and phrasing in a place or two.

Signed-off-by: Spencer Brown <spencer@spencerbrown.org>
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jul 14, 2015
Copy link
Member

Choose a reason for hiding this comment

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

Note, developer making comments on grammar/sentance structure, but...

An image developer may set defaults for these same settings during `docker build`. 
Operaters can override nearly all the defaults set in the image.

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 23, 2015

@cpuguy83 @moxiegirl @SpencerBrown what's up here?

@SpencerBrown
Copy link
Author

As a new guy to Docker, I read the doc and it misled me. I raised an issue #14050 and @thaJeztah suggested I do a PR. So I did. @cpuguy83 does not agree that the doc is misleading and incorrect. I'm not sure how to proceed here. I figured I would just leave this PR for others to judge, I pretty much said my piece. But, here's a summary of how I got here.

From my POV, as a new guy reading the docs, I take them pretty literally. If it says "the operator can override" I assume it means I can override an EXPOSE instruction. I actually depended on this when building one of my first Dockerfiles and was unhappy to discover I could not actually override an EXPOSE, even though the docs said I could.

I see @cpuguy83 's point about "hints", now that I understand more, but for someone reading the docs, you should not assume they know the internals of how the system works. You need to be very explicit and clear. Calling it a "hint" is way too subtle for system documentation.

@moxiegirl
Copy link
Contributor

@SpencerBrown I agree we need to be more explicit. @cpuguy83 gave it a shot but admits to not having his best writing hat on. He's a sharp guy tho so let me see if I can come up with something that sorts it for everyone --- explicitly stated for you and technically accurate for him.

Copy link
Contributor

Choose a reason for hiding this comment

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

@SpencerBrown @cpuguy83 try this new intro --- WDYT?

--> https://gist.github.com/moxiegirl/3acf6eb35fbe777d1ffd

Docker run reference

Docker runs processes in isolated containers. A container is a process which
runs on a host. The host may be local or remote. When an operator executes
docker run, the container process that runs is isolated in that it has its
own file system, its own networking, and its own isolated process tree separate
from the host.

This page details how to use the docker run command to define the container's
resources at runtime.

General form

The basic docker run command takes this form:

$ docker run [OPTIONS] IMAGE[:TAG|@DIGEST] [COMMAND] [ARG...]

The docker run command must specify an IMAGE to derive
the container from. An image developer can define image defaults related to

  • detached or foreground running
  • container identification
  • network settings
  • runtime constraints on CPU and memory
  • privileges and LXC configuration

With the docker run [OPTIONS] an operator can add to or override the image
defaults set by a developer. And, additionally, operators can override nearly
all the defaults set by the Docker runtime itself. The operator's ability to
override image and Docker runtime defaults is why
run has more options than any other
docker command.

Note: Depending on your Docker system configuration, you may be required to
preface the docker run command with sudo. To avoid having to use sudo with
the docker command, your system administrator can create a Unix group called
docker and add users to it. For more information about this configuration,
refer to the Docker installation documentation for your operating system.

@SpencerBrown
Copy link
Author

first gist https://gist.github.com/moxiegirl/3acf6eb35fbe777d1ffd: lgtm, nice

second gist https://gist.github.com/moxiegirl/ee4dbfb64f1c0191df51: this is like 5x better than the previous. And it jibes completely with my now-improved understanding of Docker port mapping.

however, it "exposes" (pun intended) the lack of consistent terminology of the port mapping components.

example:
"An operator can use the --expose option to add to the exposed ports."
followed by:
"To expose an container's internal port, an operator can start the container with
the -P or -p flag or start the client container with --link."

Which is it? --expose exposes a port, or -P or -p exposes an internal port?

Then we see this formulation:
"The port number inside the container (where the service listens)"
which is another way of saying the same thing.

I would suggest sticking with "expose" and "publish" to describe the port mapping components.

I took a shot at FIFY, look at the comment to the gist.

@thaJeztah
Copy link
Member

Which is it? --expose exposes a port, or -P or -p exposes an internal port?

That caught my eye too. I think "publish" should be used (but I defer to @moxiegirl here, perhaps she had a special reason here).

@moxiegirl
Copy link
Contributor

I agree @SpencerBrown @thaJeztah the distinction makes for an easier read and goes with the p in the flag. Haven't heard from @cpuguy83 --- just want to make sure he is cool with it too. Thanks @SpencerBrown for pursuing the better language.

@cpuguy83
Copy link
Member

LGTM

@cpuguy83
Copy link
Member

Let me rephrase, looks really great @moxiegirl
Thanks @SpencerBrown

@thaJeztah
Copy link
Member

Thanks, @cpuguy83! (< -- @SpencerBrown don't be put off by his comments, he's a really nice guy in person, just likes his comments to be to-the-point 😄)

@SpencerBrown could you rebase, and update the PR with the suggestions @moxiegirl provided?

@thaJeztah
Copy link
Member

ping @SpencerBrown (see my previous comment: #14625 (comment)) thanks in advance!

@moxiegirl
Copy link
Contributor

@thaJeztah happy to carry for @SpencerBrown if necessary.

@thaJeztah
Copy link
Member

@moxiegirl that'd be great (no worries, @SpencerBrown, your sign-off stays intact :-)) thanks!

@SpencerBrown
Copy link
Author

hey all... I've been on vacation... @cpuguy83, it's all good... @moxiegirl, yes please take my suggestions on your gist and do what you will with them, I'm fine with whatever.

@thaJeztah
Copy link
Member

@SpencerBrown if you want to update this PR yourself, please do! ❤️ Just let us know if you want to 👍

@cpuguy83
Copy link
Member

ping, any update on this one? @SpencerBrown Would you like someone to carry this?

@thaJeztah
Copy link
Member

Actually I started on a carry, but was held up with other things, lol; master...thaJeztah:carry-14625

@thaJeztah
Copy link
Member

Updated PR here #15675, so I'm closing this one. Additional changes can be discussed on the new PR if needed.

Thanks @SpencerBrown for the initial work!

@thaJeztah thaJeztah closed this Aug 18, 2015
thaJeztah added a commit that referenced this pull request Aug 19, 2015
[Carry #14625] run.md: --expose does NOT override Dockerfile EXPOSE
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.

6 participants