Thanks for the contribution!

Since this is a relative large change (new subcommand, new feature) it may take a little while to get this reviewed and merged, but I think we should include some support for dealing with unmanaged containers.

Thanks for your review :)

I will fix the elements you pointed out.

I changed the behavior of ps --all sub command.

orphan is added in State collum as you suggested it, and orphaned containers can be filtered using service names. So using --all flag won't be raise an exception if an orphaned container with a missing service name exists.

The behavior ps without --all flag is untouched.

ps: I just realize that I forgot to add multiple service names in unit test for ps sub command. Sorry about that, I will amend my commit latter to add them.

Perhaps this should just be a flag on rm in stead of a subcommand, i.e. docker-compose rm --clean. I'm not sure I would expect the --clean flag to be on `docker-compose up

(just my thoughts)

@thaJeztah I think that's a good point. We may not need a whole new subcommand for this. Adding --include-orphaned to stop and rm would be enough.

For up I think the rational is that up is supposed to update the composition to the new state defined in the compose file. If services are removed, they are just lost. So I can see an argument for up having a flag to remove orphaned containers as well.

@cmehay what do you think about dropping the new subcommand and doing this with flags to stop, rm, and up (which you already have) ?

@dnephin @thaJeztah I think it would be much safer to add a sub command for this task. I mean, will rm --include-orphaned kill all my containers including orphaned one ? I don't think this is what user will expect.

By the way, if we add a flag to rm or stop for orphaned containers, a managed service could be killed or removed accidentally, using clean command is much safer IMHO.

Finally, for up command, as you said, I see up as idempotent command which sets services as they are described in yml file, so --clean flag for this command is useful I think

@cmehay rm doesn't remove running containers, so in that respect it's safe(-ish), unless data volumes are involved of course.

(Thinking out loud here) Perhaps we should have a similar approach to how docker handles "dangling" images and volumes; i.e., something like docker-compose rm $(docker-compose ps -a --filter orphaned=true)

Otoh, this may become too complicated for users, idk

For reference; there has been quite some discussion around implementing a docker clean command; moby/moby#928

I updated unit tests and ps method to fix behavior when results are filtered using service names.

I'm not very happy of what I've done in this method, I tricked with exceptions to list every containers, but the behavior of ps --all is now consistent.

If you have any suggestion to improve this, you're welcome :)

@thaJeztah regarding the discussion about docker clean, the difficulty with docker is that very hard to determine which containers should be removed or not using this sub command, this is probably why they didn't find a solution to this problem.

Regarding compose, we know exactly which containers will be removed, so this would be easier to find a solution 😄

I can't comment on the implementation, but I could really use this feature. When I'm testing a new docker-compose file, I often end up with dozens of orphan, stopped containers.

I would like to say something else to defend the idea of subcommand. I went to a docker meetup with Solomon Hykes few weeks ago, and he said that the important thing of docker is that docker lets people do the way they want to do.

I agree with that, people use containers many ways, so I think compose should provide a way to clean unmanaged containers the safer way as possible for the many ways docker is used.

Adding flag to existing subcommands if not very safe as these subcommands can alter managed containers, it would be harder to filter services, should display ambiguous informations about containers, and be result legit containers to be stopped or removed...

clean subcommand will never remove managed containers, it will remove only containers from services which has been removed from docker-compose.yml file, for instance it can be used safely if you're using exited containers.

I really hope people share my point of view, and please let me know if you want me to improve anything in the current implementation.

BTW, I'm really sorry about my English, I really hope this is not too painful to read 🙇

BTW, I'm really sorry about my English, I really hope this is not too painful to read 🙇

No worries there, @cmehay, had no problem whatsoever to read! Many (if not most) contributors don't have English as their primary language (including myself) 😄

The code has been rebased from master.

The unit test of clean subcommand seems to randomly fail due to the latency of exiting container. The test seems to be performed before the third container which run true cmd is actually exited.

Adding time.sleep(1) in the test should be enough but I wonder if this is acceptable...

I can also change the test to not test with exited container, as you prefer.

New paradigm on containers and services is driving this MR obsolete.