-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
add note about daemon mode #248
Conversation
👍 |
Can you put that in |
|
moved to the right file. if moby/moby#13595 leads to a linkable explanation, we should link this note to that section. i think even then, this fact should be mentioned in each library section where the user is supposed to overwrite the base configuration like with haproxy. its also specific what configuration you need to put or not put to make the service stay in the foreground. |
Making note of every possible way an image's configuration might interact with the fundamentals of how docker works is untenable, and it doesn't make sense to me that this particular behavior should be any sort of special case worth mentioning. The container exits when PID 1 exits. It's not some exceptional case to some arbitrary sub-category of images, but is the standard behavior of docker. |
well yes and no. services like nginx decide by the command line arguments whether they go into background or stay the active process. there is no need for a note there. but with haproxy, i just cobbled together a configuration from various sources on the internet and most of them made it run as daemon. as this is controlled by a field in the config file, and the instructions tell to overwrite the default config file, its much more likely to happen. if this reasoning makes sense to you, we can try to rephrase the note to better explain this. if you want to close it go ahead, i spent my hours on it and now know how to not configure haproxy in docker. |
In this specific case, IMO it does make sense to at least mention this since it is pretty common practice to include "daemonization" in the (although |
i tried to improve the note a bit, and fixed Docker to be correctly spelled.
|
I see the justification, but we could make a similar argument regarding the port directive here, yet we don't feel the need to point out that if you change the port via Should we also include similar notes for port behavior in all images where it's applicable? How is this different or worthy of special note? If it is, where do we draw the line in terms of what we include in image documentation? I'd be completely sold on including this if there was some reason this behavior was a distinct and special case to haproxy. I understand that the container exits when PID 1 exits is a less intuitive behavior of docker if you don't understand what's going on behind the scenes, but I don't think that's sufficient justification for including it in image documentation. |
ports is pretty obvious i hope, and something people know from firewalls
and networking.
whether a process goes into the background is less obvious i think.
particularly as many Docker images will not have the issue because the
startup scripts usually choose whether to go into daemon mode or not.
but haproxy does this with the configuration file, creating an extra
pitfall.
but i see the point of not cluttering documentation with redundant
things. i won't be angry if this is not merged.
|
Right, I chose to bring up the ports because it is more obvious. To me, the argument you're making makes exactly as much sense as including a note about the ports. I can even take the exact argument and switch out the contextual piece, and it's equivalent. "particularly as many Docker images will not have the issue because the startup scripts usually choose whether to go [to the default port or not]. but haproxy does this with the configuration file, creating an extra pitfall." I'm strongly against this being included here, but if @tianon feels strongly that it should be included, I'll defer. |
being new to docker and haproxy, it took me quite a while to figure out why things just did not work. the issue was also reported in the haproxy repository: docker-library/haproxy#1