include stopped containers #130
Conversation
| @@ -97,7 +97,7 @@ func splitDockerImage(img string) (string, string, string) { | |||
| func getContainers(client *docker.Client) ([]*RuntimeContainer, error) { | |||
|
|
|||
| apiContainers, err := client.ListContainers(docker.ListContainersOptions{ | |||
| All: false, | |||
| All: true, | |||
There was a problem hiding this comment.
Wouldn't it make sense to let the server do the filtering and make the All flag dependent on the value of includeStopped?
There was a problem hiding this comment.
There's a case where a config file contains 2 configs, one needing the stopped containers, but not the other, and this method is called once for both configs. I agree it's not optimized in the case where nobody needs it, which is the most common use case, but I was not sure if it was worth optimizing it.
I can check if at least one config use it if you want. I don't know if there's a better solution.
There was a problem hiding this comment.
That seems like a fine solution to me.
I'd consider introducing a Generator struct that contains the docker.Client and the Config and moving the generate* functions to hang off of that.
@jwilder What do you think?
There was a problem hiding this comment.
I think this approach is fine as is for now. I'm not sure I understand how the Generator concept would work, but not opposed to that either.
There was a problem hiding this comment.
It seems like a Generator type would pretty naturally fall out of the effort of turning docker-gen into a library that I mentioned over in nginx-proxy/nginx-proxy#254 (comment)
There was a problem hiding this comment.
I just opened #131 as a place to discuss the issue.
|
👍 I agree, this would be a really useful feature. Our use case is generating vhost definitions for stopped containers that point to an internal web service that will start those containers back up again when visited. I'm sure this is already known, but a change in the returned structs will require an update to the "Templating" section of the documentation and the addition of the I wasn't sure whether or not this was done as part of PRs on this project, or as a pre-release task. |
|
Can you rebase? |
|
Yes, rebase is done. |
|
Thanks @minhdoboi! |
It may be interesting to see stopped containers.
For example, my use case was to expose volumes mount points to nginx, that contains the results of a dockerized script.