Add reload support

[klausenbusk]

I have only tested 1.6 and only slightly.

Reload like docker kill --signal=USR2 <container> or HUP

Full path and pid is required for haporxy-systemd-wrapper to work.
Fix #5

[yosifkit]

Wow, I didn't realize that this was here (though only available in 1.5 and 1.6). I was sad about the full path to the upstream-included haproxy-systemd-wrapper and spent much longer than I should have in trying to debug why it didn't work properly when used without the full path. It seems the problem lies on line 134; they use execv rather than execvp, so it fails to re-exec itself since argv[0] is just haproxy-systemd-wrapper. I am not sure if this is a "bug" according to upstream or a usecase outside of their scope.

Either way, I like using the solution provided, but would love to make it easier for users. Would it be helpful for us to provide a small wrapper script so that a user could directly pass arguments (and we add -p if they don't provide one)?

$ docker run -v /my/config:/... haproxy -m 100 -f /... -N 1000

[klausenbusk]

and we add -p if they don't provide one

haproxy-systemd-wrapper is hardcoded to /run/haproxy.pid, so I don't think it make sense to let the user specific it.

Either way, I like using the solution provided, but would love to make it easier for users. Would it be helpful for us to provide a small wrapper script so that a user could directly pass arguments

Shouldn't changing /usr/local/sbin/haproxy-systemd-wrapper to a ENTRYPOINT be enough?

(though only available in 1.5 and 1.6).

Do we need to write some sort of update.sh (like some of the other docker repo), or is it okay if one of the dockerfile differ from the rest?

[tianon]

I think something as simple as this is what @yosifkit was referring to:

diff --git a/1.6/Dockerfile b/1.6/Dockerfile
index fbcae43..f01fc30 100644
--- a/1.6/Dockerfile
+++ b/1.6/Dockerfile
@@ -27,4 +27,6 @@ RUN buildDeps='curl gcc libc6-dev libpcre3-dev libssl-dev make' \
    && rm -rf /usr/src/haproxy \
    && apt-get purge -y --auto-remove $buildDeps

+COPY docker-entrypoint.sh /
+ENTRYPOINT ["/docker-entrypoint.sh"]
 CMD ["haproxy", "-f", "/usr/local/etc/haproxy/haproxy.cfg"]
diff --git a/1.6/docker-entrypoint.sh b/1.6/docker-entrypoint.sh
new file mode 100755
index 0000000..bdb02c3
--- /dev/null
+++ b/1.6/docker-entrypoint.sh
@@ -0,0 +1,15 @@
+#!/bin/bash
+set -e
+
+# first arg is `-f` or `--some-option`
+if [ "${1:0:1}" = '-' ]; then
+   set -- haproxy "$@"
+fi
+
+if [ "$1" = 'haproxy' ]; then
+   # if the user wants "haproxy", let's use "haproxy-systemd-wrapper" instead so we can have proper reloadability implemented by upstream
+   shift # "haproxy"
+   set -- "$(which haproxy-systemd-wrapper)" -p /run/haproxy.pid "$@"
+fi
+
+exec "$@"

(allows the user to still specify any arbitrary args, and allows the image to still work as-is without any modification in the same way it does currently)

[tianon]

(and this repo doesn't use a Dockerfile.template, so update.sh should be fine as-is)

[tianon]

https://github.com/horms/haproxy/blob/83104804996f354f167d8bdfcd27fb1f6148a252/src/haproxy-systemd-wrapper.c#L153-L161 is the money shot for where haproxy-systemd-wrapper reads -p from the commandline to know where haproxy is going to write the PID file out to (so it should be fine for the user to specify one, and we'll automatically honor it appropriately if they really want to go there without any extra work)

[tianon]

This will also enable -Ds, which should force foregrounding and logging to stdout:

       -Ds    Start in systemd daemon mode, keeping a process in foreground.

[klausenbusk]

logging to stdout:

I don't think you correct, at least I couldn't get that behavior a few days ago. For that we need something like #19

[tianon]

Arg, you are correct -- this doesn't solve that problem. 😢

[tianon]

https://www.mail-archive.com/haproxy@formilux.org/msg17436.html is an interesting (and depressing) post on that front

[klausenbusk]

How does this look? @tianon

Maybe #19 could be a solution for the logging issue.

[tianon]

This looks good, but the changes to 1.4 need to be reverted (since it doesn't have the haproxy-systemd-wrapper binary). 👍

[klausenbusk]

This looks good, but the changes to 1.4 need to be reverted (since it doesn't have the haproxy-systemd-wrapper binary). 👍

The code hase a [ "$HAPROXY_MAJOR" != "1.4" ] (it was 1.3 before, I just fixed it) if, shouldn't that be okay?

[tianon]

Technically, but what's the point of doing so when we don't use a template? (and can thus just have 1.4 diverge in this instance, keeping the script slightly simpler)

[klausenbusk]

Technically, but what's the point of doing so when we don't use a template? (and can thus just have 1.4 diverge in this instance, keeping the script slightly simpler)

You the boss :) Done.

[tianon]

Thanks ❤️

This will probably need some minor updates for Alpine support (POSIX vs Bash), but I'm happy to take on those changes if you'd like!

LGTM

[klausenbusk]

but I'm happy to take on those changes if you'd like!

Fine with me.

[yosifkit]

LGTM