Skip to content

Added option to restart a docker container in -notify #24

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

Closed
wants to merge 0 commits into from

Conversation

JustAdam
Copy link
Contributor

This solves issue #22 (not completely, but provides the requested feature). I haven't added full access to the Docker API here as I think the only reason to access it, is for restarting containers.

So to restart a container, specify -notify "restart-container CONTAINER-ID". This syntax feels a bit clunky, but adding an additional flag does as well.

@JustAdam JustAdam changed the title Add option to restart a docker container in -notify Added option to restart a docker container in -notify Sep 18, 2014
@jwilder
Copy link
Collaborator

jwilder commented Sep 19, 2014

This is a little funky from a CLI standpoint. I have a suggested workaround that I commented on in #22.

@JustAdam
Copy link
Contributor Author

The main purpose of this patch is too be able to run docker-gen by itself in a container. Therefore I think it is vital that this functionality is built into docker-gen and not provided by an external dependency (script/program). The external dependency provides an additional tool to maintain (ensuring the docker end point is available, ensure api calls are upto date etc.) and package.

I would really like to see this patch accepted, as it would make docker-gen much more usable (certainly for me and cnf!).

I agree with the syntax being funky, and have thought a bit more about the additional flag, and perhaps it is the only way to go. If we can agree, I will rewrite the patch to include a new flag: -restart-container=CONTAINER_ID.
So either the flag or old flag, both or none can be specified and all behaves as it did before. If both are specified then it issues the restart and the notify.

@JustAdam
Copy link
Contributor Author

Continued in #27

@JustAdam JustAdam closed this Sep 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants