Skip to content

Conversation

@TomasTomecek
Copy link

Resolves #593

Still wip

  • basic functionality
  • tests
  • docs
  • interactive exec (docker exec -ti equiv)

@TomasTomecek
Copy link
Author

@aanand @dnephin could I get some feedback here?

Copy link

Choose a reason for hiding this comment

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

Instead of the filter use stopped=False (which is the default).

@dnephin
Copy link

dnephin commented Sep 28, 2015

I think there is still an issue with exec (similar to attach) in that it's fundamentally a container oriented command, not a service oriented one. The primary reason to use it is to get interactive access to a running container, but compose is concerned with the service abstraction, not individual containers.

docker exec container_name works just as well for interacting with a compose container.

So I'm not exactly sure how docker-compose exec should work.

@TomasTomecek
Copy link
Author

The reason I started working on this is

  1. I wanted this feature, for me it's easier to type

    docker-compose exec web bash

    rather than

    docker exec -ti i-have-no-idea-about-the container-name bash

  2. number of +1s in Support docker exec command #593

I agree that exec might not make sense if a service is composed of multiple containers. On the other hand, user may be interested in running the command in any of the containers (so this could be configurable).

@TomasTomecek
Copy link
Author

@dnephin any update?

@TomasTomecek
Copy link
Author

@aanand @bfirsh @dnephin once again, any update?

Just FYI, k8s support exec: https://cloud.google.com/container-engine/docs/kubectl/exec

@aanand
Copy link

aanand commented Nov 5, 2015

docker-compose port works on a single container (it has an optional --index parameter to specify which one, but that will probably become useless when we ditch sequential numbers).

I agree that the UX for doing an exec is not good, so a docker-compose exec which attaches to an arbitrary container sounds OK to me.

There was another PR/proposal for an exec which executed the specified command on all containers for the service, but I'm not convinced that's a good enough idea to warrant baking into Compose, and it's useless for interactive programs like bash.

This particular implementation looks like a good start, but I think it absolutely needs to support interactivity (via dockerpty) to be useful.

Thoughts?

@dnephin
Copy link

dnephin commented Nov 5, 2015

The equivalent of that k8s exec command is the standard docker exec, you still need to specify the pod and the container.

@TomasTomecek
Copy link
Author

This particular implementation looks like a good start, but I think it absolutely needs to support interactivity (via dockerpty) to be useful.

Absolutely. I was just waiting for you guys to get some feedback that this is the feature you would be interested in.

I'm planning to propose a patch to dockerpty. Without that support, I feel like that this feature is worthless. I'll get back to you once I have something to share.

you still need to specify the pod and the container.

Looks like that container argument is optional, snippet from docs:

// get output from running 'date' from pod 123456-7890, using the first container by default
$ kubectl exec 123456-7890 date

@TomasTomecek
Copy link
Author

rebased and rewritten (added interactive support with updated dockerpty)

@thaJeztah
Copy link
Member

Looks like a test is failing, not sure if it's related

@TomasTomecek
Copy link
Author

@thaJeztah tests are untouched, am waiting for design review first

@TomasTomecek
Copy link
Author

@aanand @dnephin can we get this moving so it lands into next compose release?

-d Detached mode: Run command in the background.
--privileged Give extended privileges to the process.
--user USER Run the command as this user.
-T Disable pseudo-tty allocation. By default `docker-compose run`
Copy link

Choose a reason for hiding this comment

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

s/run/exec/

Copy link
Author

Choose a reason for hiding this comment

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

fixed (well spotted)

@TomasTomecek
Copy link
Author

rebased, updated

@TomasTomecek
Copy link
Author

those test failures seem unrelated to this PR

@TomasTomecek
Copy link
Author

I'm really bad at copy-pasting. I'm sorry about that.

Should be fixed now.

To be honest, I'm unsure how that signal handing should suppose to work. When I ran compose like this

docker-compose exec asd sh

and then hit ^d I expected the exception to be raised. That didn't happen.

@TomasTomecek
Copy link
Author

rebased b/c of the ^ unrelated test failure

@dnephin
Copy link

dnephin commented Feb 26, 2016

LGTM

Resolves docker#593

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>
@TomasTomecek
Copy link
Author

rebased

@aanand
Copy link

aanand commented Feb 29, 2016

logo_exec_menor

LGTM

aanand added a commit that referenced this pull request Feb 29, 2016
@aanand aanand merged commit 97c9c12 into docker:master Feb 29, 2016
@TomasTomecek TomasTomecek deleted the 593-implement-exec branch March 1, 2016 07:28
@TomasTomecek
Copy link
Author

\o/

This was a ride!

@willejs
Copy link

willejs commented Mar 2, 2016

👌

@darkn3rd
Copy link

Will this also have exec:? for docker-compose.yml? I need this for a wrapper script use case, in order to do setup before the process starts.

@TomasTomecek
Copy link
Author

@darkn3rd no, it's a cli command; feel free to open an issue with the feature request

@thaJeztah
Copy link
Member

@darkn3rd perhaps having an entrypoint script would be a solution for that? I think the "exec" would be executed after the image's main CMD is started, so probably not the best solution to perform these kind of actions.

@TomasTomecek
Copy link
Author

@thaJeztah to be honest I totally understand @darkn3rd's proposal: I've run into a similar issue: I have a web app with database started by compose. As soon as it's up, I want to populate database with some initial data and am not very keen on doing:

$ docker-compose up -d
$ docker-compose exec populate-database.sh

Would be awesome if compose had support for hooks.

@thaJeztah
Copy link
Member

@TomasTomecek well, the regular approach is to have the entrypoint script take care of that, for example, see the Initializing a fresh instance section for the MySQL image (and the script itself)

Overall, docker exec is really designed for debugging purposes only. Also, initializing a database can be done through a "utility" container, that connects to the database-container to populate the database if needed

@dnephin
Copy link

dnephin commented Apr 14, 2016

It would be even better to do it as part of build instead of in the entrypoint script. That way it is cached and the DB is ready to handle requests immediately, instead of waiting for initialization when it starts.

@thaJeztah
Copy link
Member

@dnephin yeah, problem is that the official images declare a VOLUME, so it's not possible to pre-populate databases if you're using the official images (unless you alter the configuration and use a different location I guess)

@TomasTomecek
Copy link
Author

@thaJeztah to me, custom entrypoint scripts are pure hack: initializing a service and running a service are two completely different operations which shouldn't be mixed in a single script -- when there is something wrong with such script, you need to debug it in order to have the service running in some form; on top of that, it's not clear what such script does (since usually it's several hundreds of LoC of cryptic shell code).

I like the idea with utility container, since it's clearly separates spinning database from its initialization.

@dnephin my issue with baking data inside database during build is that you need to run the database during build -- usually that's not a trivial thing. Not even saying that you have an image which can be only used within only one application in a single type of environment (dev/stage/prod).

@mccrodp
Copy link

mccrodp commented Apr 24, 2016

Hey all, great work getting this merged, hope I'm not hijacking...was looking to use this exact functionality using docker-compose, rather than spawning a new container with run.

I see it in the docs now here: https://docs.docker.com/compose/reference/exec/

However, I get an error after running and don't see exec in the command list on the cli.

No such command: exec

I did an upgrade using docker-machine to v1.11.0 of boot2docker, so I presume I should be able to test this PR? Many thanks!

@thaJeztah
Copy link
Member

@mccrodp did you download the latest version of docker toolbox, or performed docker-machine upgrade? docker-machine upgrade only updates the VM to the latest version of boot2docker, but doesn't upgrade docker machine itself, not the other tools (such as docker compose). Does docker-compose --version show that you're running 1.7.0?

@mccrodp
Copy link

mccrodp commented Apr 24, 2016

@thaJeztah ooops, you were dead right! I had only ran the upgrade from the command line rather than the whole toolbox app (I was still on 1.6.2). Thanks for your help.

All working now with docker-compose exec. Awesome PR, thanks all involved!

ndeloof pushed a commit to ndeloof/compose that referenced this pull request Jan 30, 2023
ndeloof pushed a commit to ndeloof/compose that referenced this pull request Jan 30, 2023
ndeloof pushed a commit to ndeloof/compose that referenced this pull request Jan 31, 2023
ndeloof pushed a commit to ndeloof/compose that referenced this pull request Feb 1, 2023
ndeloof pushed a commit that referenced this pull request Feb 2, 2023
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