-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Wait for all containers to exit #1754
Conversation
Perfect. I love this! |
Cool! Now I can get rid of the ugly |
LGTM |
549e707
to
76e9de7
Compare
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com> Conflicts: compose/cli/multiplexer.py
76e9de7
to
80d90a7
Compare
Wait for all containers to exit
I'm currently using docker-compose for tests, with a main container that runs the application (with the tests) and a few other containers with databases/elasticsearch/... In compose 1.4 I could just do:
When the application tests end, compose would exit and the tests finish. Are there best practices to change this to docker-compose 1.5? Thank you |
Option 1 I would recommend changing your test container to have a default no-op command (something like
Using Option 2 If you wanted to continue to use
|
Thanks @dnephin option 1 worked like a charm :) |
What was the logic for merging this as the default behaviour into the main branch without adding a flag to bypass it ?. The "stop when any container exits" behaviour was the default until 1.5.0 . Which means it is a major change without any proper way to opt out. Now, I got weird deployment issues were a system continue to run with halted containers. The only solutions left are to:
|
@cgeoffroy can you tell us more about your use case? Could the issue be addressed by setting a restart policy? Or by either of the examples I included above? |
I agree with @cgeoffroy. It's unfortunate not to have the possibility to have the pre 1.5 behavior. Our use case is that we use this in testing, and when a container goes down, it's probably because of a bug. So, automatic restarts are not wanted. Rather, a clear indication that there has been a problem is of high value. The compose session ending is much clearer than that the other services will have to time out trying to reach the service that has gone down. It's not the same case as @VAS had, since the tests are executing externally to the compose environment. We don't know which service could go down, they're all equally likely. #2130 seems like it would be what we need. |
@dnephin I'd like to add our use case. The workarounds mentioned introduce redundancy: I have to do More generally speaking I think the use case of treating the combination of services defined with compose as 1 unit and thus exiting when one of the units fails is just as valid as the use case treating the services as loosely coupled and not exiting when one of the units fails. TL;DR I'd like something like #2130 as well :) |
I've also filed #2474 because our integration testing workflow is no longer compatible with Compose as of 1.5.x. This is the second consecutive "breakage" to our app introduced by Compose in subsequent minor versions (1.4.x stopped recreating containers by default (but provided a flag to re-enable the previous default behavior)). Is there, or could there be, a process for gathering user feedback/requirements that could be used to evaluate the cost/benefits of these breaking changes (and better prepare users for them)? |
User feedback is really import to us. Both of these changes you mentioned were a result of lots of user feedback, but everyone uses Compose in a slightly different way, so it's hard to get it just right for everyone. The process we use is generally github issues and release candidates. We use milestones to indicate which issues are going to be in an upcoming release (we've started to put issues into a milestone at the beginning of the cycle). Releases candidates are also created many weeks in advance so users can try out the new changes. We also try to make sure that there is always a way to get the old behaviour. With the first change it was using I think it's also important to try and document recommended uses of compose. For example http://docs.docker.com/compose/#automated-testing-environments uses |
Thanks for the detailed response @dnephin. I appreciate the thought and effort that's gone into the process. |
It was recently added here: https://github.com/docker/compose/blob/master/docs/faq.md#whats-the-difference-between-up-run-and-start Which will be published to the official docs pretty soon |
Great. Thanks @dnephin! |
@dnephin, we had I think the same use case as a number of other people here - a number of interacting apps which communicate with each other and a test runner. The specific thing that was nice before was that all our logs got interleaved, so we could broadly see what was going on from just a jenkins build log - the test failure generally came shortly after a stack trace. Do you have any recommendations as to how we can reobtain the previous behaviour? Our script currently looks like:
|
I think maybe this:
Having to background The other change you'd need to make would be to set a
I personally like that approach because it lets you use your We can also continue the discussion in #2474 about adding a flag to restore the "shutdown on first container exit" behaviour. |
@dnephin What's the point of having an abstraction layer like Compose when I have to add another abstraction layer like a shell script to manage Compose to do simple stuff like this? The simplicity of Compose is totally lost this way. If we say we have to move to |
@simonvanderveldt I don't really see compose or shell scripts as abstractions, they are just tools to accomplish a specific task. They don't change or hide the underlying concepts or interfaces. Compose is, as it states in the description, "a tool for defining and running multi-container Docker applications". It is not a scripting language (like bash). Could you accomplish what Compose does in any programming languages? Of course.
The behavior before 1.5 was effectively a bug. It never worked with data volume containers, and it didn't give you an appropriate exit status code, so it was never intended to work that way.
A single command is never going to encapsulate every use case. Compose gets used in many different ways. I don't see having to write multiple commands as a problem. However trying to reduce the number of commands necessary for common use-cases is a goal. The example above does require a few extra commands. We'll be reducing this soon with: #12, #2227, #2277. We can also discuss #2474 and #2490 as options. |
For anyone banging their head against the desk trying to get this to work four years later, the semantics appear to have changed. Use a compose file like this:
Then invoke the tests using compose like this:
That will:
|
Behold: the rainbow.
I've also made it so that if an exception gets raised when attaching to a container, it gets raised in the main thread and
docker-compose up
exits immediately. This should fix the problem @icy was trying to solve in #1723.This is a hard one to integration test without resorting to threads or multiprocessing, but I've written some unit tests for Multiplexer that assert the basic logic.
Would like to get some feedback on the approach and the change in functionality (which may surprise some users and qualify as breaking backwards-compatibility) before this is merged.