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

Proposal: allow docker attach for non-running containers #26705

Closed
aanand opened this issue Sep 19, 2016 · 8 comments
Closed

Proposal: allow docker attach for non-running containers #26705

aanand opened this issue Sep 19, 2016 · 8 comments
Labels
area/cli kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny

Comments

@aanand
Copy link
Contributor

aanand commented Sep 19, 2016

In Compose (specifically docker-compose run), instead of duplicating all of Docker's considerable client-side attach logic, we'd like to start shelling out to docker attach instead, so we have better parity across all platforms (especially Windows).

However, there's currently client-side code which prohibits a user from docker attach-ing to a non-running container. This is a showstopper: to avoid race conditions, the order of operations must be create, attach, start.

I'd like to either remove this logic or add a flag to skip it.

@thaJeztah thaJeztah added area/runtime kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny labels Sep 19, 2016
@thaJeztah
Copy link
Member

ping @crosbymichael @mlaventure PTAL

@tonistiigi
Copy link
Member

Switched labels as this is allowed in API already. Does this have to be cli flag or can't you use the client API? Public client.ContainerAttach() method doesn't have this limitation.

@aanand
Copy link
Contributor Author

aanand commented Sep 20, 2016

@tonistiigi We can't use the Go API, because Compose is written in Python! Shelling out to docker attach is the only way I can see to re-use Engine's client-side attach code from Compose.

@thaJeztah
Copy link
Member

Perhaps @tonistiigi meant the /container/(id or name)/attach/ endpoint, which is used by client.ContainerAttach()? https://github.com/docker/docker/blob/3ea762b9f6ba256cf51bd2c35988f0c48bccf0b0/client/container_attach.go#L14-L34, there's also a web socket endpoint; https://docs.docker.com/engine/reference/api/docker_remote_api_v1.24/#/attach-to-a-container-websocket

@aanand
Copy link
Contributor Author

aanand commented Sep 20, 2016

Well, those API endpoints are what we're using right now. The problem is that replicating all of the interactive functionality of docker run/attach/exec still involves a great deal of client-side logic (some of which is detailed here).

All of that logic has been reimplemented in Python, which has inevitably become a huge time sink and a source of many bugs over the years. It's also currently POSIX-only, because attaching interactively on Windows involves a completely different set of system calls.

Accordingly, over time, we'd like to remove it and rely on the Docker client binary instead.

@thaJeztah
Copy link
Member

Ah, clear. (I think ideally, we should have an improved API to make it easier to use)

@tonistiigi
Copy link
Member

@aanand How about docker start -a, wouldn't that work for you?

@aanand
Copy link
Contributor Author

aanand commented Sep 22, 2016

@tonistiigi I didn't know about docker start -a! That will probably work, yes. I'll close this and reopen if it turns out it doesn't work for any reason. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny
Projects
None yet
Development

No branches or pull requests

3 participants