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

Client issue on command failure #4857

Closed
creack opened this issue Mar 25, 2014 · 4 comments · Fixed by #4882
Closed

Client issue on command failure #4857

creack opened this issue Mar 25, 2014 · 4 comments · Fixed by #4882
Milestone

Comments

@creack
Copy link
Contributor

creack commented Mar 25, 2014

When the command fails directly, the client gets lost:
2014/03/25 16:27:25 Unrecognized input header

FROM ubuntu
RUN apt-get update && apt-get install docker

Reproduce:
docker run creack/segv docker (pushed on public registry)

/cc @vieux

@shykes
Copy link
Contributor

shykes commented May 9, 2014

I agree with this as an immediate solution.. I prefer moving away from "magical merging"
based on the commands you ran before. I think it's cleaner to unify the build interface,
so that you can explicitly tell Docker "please modify the image metadata in this or that way".
That's consistent with my docker commit --change PR for example.

Longer term, it would be nice to have a TTY build instruction to specify "the default entrypoint
should have a tty allocated". There are 2 caveats there:

  1. Note that TTY should apply to the entrypoint, and not apply if the entrypoint is changed
    (either at run time or by an overlay image). It makes sense to say "by default run bash with a tty",
    but it doesn't make sense to say "whatever command you run in this container, always allocate a tty".

  2. TTY is complicated to implement because it requires cooperation by the client, unlike most
    other build instructions. I am not sure how that work, but I have a feeling it's complicated :)

I will leave this long-term discussion for later.

@shykes
Copy link
Contributor

shykes commented May 9, 2014

@vieux could you move the test from integration to integration-cli? It should make it
even simpler :)

After that I think we can merge.

Sorry for the slow review.

@vieux
Copy link
Contributor

vieux commented May 9, 2014

@shykes what test ?

@shykes
Copy link
Contributor

shykes commented May 16, 2014

@vieux sorry about that, those 2 comments were meant to be posted on #4882. I re-posted them in the right place.

@creack creack added the bug label May 16, 2014
@creack creack added this to the 1.0 milestone May 16, 2014
brownjohnf added a commit to balena-os/balena-yocto-scripts that referenced this issue Jun 7, 2017
Based on the threads listed below, this should resolve the issue, which
is related to building an image with the interactive terminal flag (-t)
but then running it without the flag.

moby/moby#4882
moby/moby#4857
moby/moby#5327

Change-Type: patch
floion pushed a commit to balena-os/balena-yocto-scripts that referenced this issue Jun 21, 2017
Based on the threads listed below, this should resolve the issue, which
is related to building an image with the interactive terminal flag (-t)
but then running it without the flag.

moby/moby#4882
moby/moby#4857
moby/moby#5327

Change-Type: patch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants