Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

Emulate behaviour of haproxy-systemd-wrapper. #390

Merged
merged 1 commit into from
Jan 18, 2017

Conversation

@brndnmtthws brndnmtthws force-pushed the use-pipe-for-safe-reloads branch 15 times, most recently from aead97d to 96023d4 Compare December 24, 2016 08:33
@brndnmtthws brndnmtthws force-pushed the use-pipe-for-safe-reloads branch 2 times, most recently from ab5ac4a to 16c9ab6 Compare December 24, 2016 09:09
@vixns
Copy link
Contributor

vixns commented Dec 24, 2016

You may also care about signals propagation, see http://git.haproxy.org/?p=haproxy-1.7.git;a=commit;h=4351ea61fbddf88c960179d60b0e0f1b090f0b70

@brndnmtthws
Copy link
Contributor Author

Correct me if I'm wrong, but I think that only matters if you're using signals to reload HAProxy, which we aren't.

@@ -33,7 +33,7 @@ reload() {

# Trigger reload
LATEST_HAPROXY_PID=$(cat $PIDFILE)
haproxy -p $PIDFILE -f /marathon-lb/haproxy.cfg -D -sf $LATEST_HAPROXY_PID 200>&-
/marathon-lb/haproxy_wrapper.py `which haproxy` -p $PIDFILE -f /marathon-lb/haproxy.cfg -D -sf $LATEST_HAPROXY_PID 200>&-
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not fork this, must block the execution until reload is over.
test the return code, should log to stderr in case of error.

@@ -33,7 +33,7 @@ reload() {

# Trigger reload
LATEST_HAPROXY_PID=$(cat $PIDFILE)
haproxy -p $PIDFILE -f /marathon-lb/haproxy.cfg -D -sf $LATEST_HAPROXY_PID 200>&-
/marathon-lb/haproxy_wrapper.py `which haproxy` -p $PIDFILE -f /marathon-lb/haproxy.cfg -D -sf $LATEST_HAPROXY_PID 200>&-
if [ -n "${HAPROXY_RELOAD_SIGTERM_DELAY-}" ]; then
sleep $HAPROXY_RELOAD_SIGTERM_DELAY && kill $LATEST_HAPROXY_PID 200>&- 2>/dev/null &
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HAPROXY_RELOAD_SIGTERM_DELAY is no longer needed

if len(ret) == 0:
close_and_swallow(pipefd[0])
close_and_swallow(pipefd[1])
return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is empty ret an error case ? if not should return True here

# Close the write side
os.close(pipefd[1])
while wait_on_haproxy_pipe(pipefd):
time.sleep(0.01)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add an explicit exit code / error message

@brndnmtthws brndnmtthws force-pushed the use-pipe-for-safe-reloads branch 9 times, most recently from 3eb9900 to 42a70ae Compare January 18, 2017 19:07
@brndnmtthws brndnmtthws force-pushed the use-pipe-for-safe-reloads branch 10 times, most recently from 0173b24 to 0cd1916 Compare January 18, 2017 20:05
@brndnmtthws
Copy link
Contributor Author

This seems to be working now. I'm going to go ahead and merge it soon.

@brndnmtthws brndnmtthws merged commit e493bad into master Jan 18, 2017
@brndnmtthws brndnmtthws deleted the use-pipe-for-safe-reloads branch January 18, 2017 20:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants