Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "CMD" #199

Merged
merged 1 commit into from
May 8, 2017
Merged

Add "CMD" #199

merged 1 commit into from
May 8, 2017

Conversation

tianon
Copy link
Collaborator

@tianon tianon commented Jul 19, 2016

This was rejected previously (#60) on the basis of keeping the layer count to an absolute minimum. Since Docker 1.10+, instructions like "CMD" simply modify metadata and no longer create an additional layer that needs to be transported at pull time. I'm hoping that means we can reconsider it? 😇

Closes #193

@bboreham
Copy link

What's the maintainers' view on this? It's my last hope for simpler demos now my 'shell' PR is closed.

(tests appear to have failed on a version check, @tianon)

@andyshinn
Copy link
Contributor

I'm not opposed to it. Though, I'd be curious if there are implications where people use bare ENTRYPOINTs and then suddenly get a CMD added to that.

If users have an ENTRYPOINT and this gets added and they don't explicitly override the CMD, will it be an issue?

@tianon
Copy link
Collaborator Author

tianon commented Sep 30, 2016

We actually ran into that enough early on that Docker resets any inherited CMD when it encounters ENTRYPOINT 👍

To illustrate:

$ docker build -

FROM alpine:3.4
CMD ["/bin/sh"]

<Ctrl+D>

Sending build context to Docker daemon 2.048 kB
Step 1 : FROM alpine:3.4
 ---> ee4603260daa
Step 2 : CMD /bin/sh
 ---> Running in c90072616302
 ---> 3da1cf08278a
Removing intermediate container c90072616302
Successfully built 3da1cf08278a
$ docker build -

FROM 3da1cf08278a
ENTRYPOINT ["echo", "hi"]

<Ctrl+D>

Sending build context to Docker daemon 2.048 kB
Step 1 : FROM 3da1cf08278a
 ---> 3da1cf08278a
Step 2 : ENTRYPOINT echo hi
 ---> Running in 59902a5804e8
 ---> f4fd19af8fff
Removing intermediate container 59902a5804e8
Successfully built f4fd19af8fff
$ docker run --rm f4fd19af8fff
hi

(simply hi as opposed to hi /bin/sh)

Copy link
Contributor

@andyshinn andyshinn left a comment

Choose a reason for hiding this comment

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

I'm not opposed seeing as how Docker has evolved in the way it handles layers. It also seems like something people want. Ping @gliderlabs/maintainers for someone else to also approve.

@ncopa
Copy link
Collaborator

ncopa commented Oct 18, 2016

I am all for this 👍

@andyshinn
Copy link
Contributor

Let's get this merged. @tianon can you update it for the 3.5 variants?

This was rejected previously on the basis of keeping the layer count to an absolute minimum.  Since Docker 1.10+, instructions like "CMD" simply modify metadata and no longer create an additional layer that needs to be transported at pull time.
@tianon
Copy link
Collaborator Author

tianon commented May 8, 2017

Updated! 👍

@andyshinn andyshinn merged commit 5e37905 into gliderlabs:master May 8, 2017
@tianon tianon deleted the cmd branch May 8, 2017 16:38
tao12345666333 pushed a commit to tao12345666333/docker-alpine that referenced this pull request Jul 12, 2018
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.

None yet

4 participants