feat: gracefully drain the webhook server on shutdown (opt-in)#67
feat: gracefully drain the webhook server on shutdown (opt-in)#67BarthV wants to merge 1 commit into
Conversation
BarthV
requested review from
Buzzglo,
TebogoTS,
ampie,
januarios,
k0da and
kuritka
as code owners
|
Hi ! This PR is 1 of the 2 (the other is #66 ) required to mitigate/lower the risk of leaving orphaned A / TXT records on infoblox. The code is heavily claude reviewed. I've benchtested / stressed both of them in lab and I'm now running them in production. It works as expected, only a benign risk of lone TXT still persist, but cannot be solved because of the atomic WAPI call design (non transactional). Feel free to ask questions or raise any remark. |
|
Hi, I'll take a look asap |
|
Hello @BarthV , could you please update the PR to address lint issues and make sure the pipeline passes? Thank you |
The webhook API server (main -> server.Start -> external-dns api.StartHTTPApi) had no signal handling, so SIGTERM from a normal pod termination exited the process immediately and could abort an in-flight /records apply. external-dns applies a record and its ownership TXT as separate sequential WAPI calls, so a mid-apply exit can strand a half-written or half-deleted record in the grid. Run the API server directly (reusing the external-dns api handlers) with a signal-triggered shutdown. On SIGINT/SIGTERM the server stops accepting new connections and, when SERVER_SHUTDOWN_TIMEOUT is positive, drains in-flight requests for up to that long before exiting; the first signal is caught and the handler is then deregistered so a second signal still terminates immediately. The drain is OPT-IN: SERVER_SHUTDOWN_TIMEOUT defaults to 0, which closes the server immediately and preserves the previous immediate-exit behaviour; operators enable draining by setting a positive timeout and a matching terminationGracePeriodSeconds. SERVER_READ_TIMEOUT/SERVER_WRITE_TIMEOUT are deliberately left unwired (a write timeout shorter than a slow apply would cut it mid-flight) — see the comments in serve. Start now returns a bind error instead of calling log.Fatal, so it is testable and main owns the exit.
BarthV
force-pushed
the
feat/graceful-shutdown
branch
from
37aac37 to
4d6990e
Compare
Lint fixes has been pushed (commit squashed into the same single commit) |
3 participants
The webhook API server (main -> server.Start -> external-dns api.StartHTTPApi) had no signal handling, so SIGTERM from a normal pod termination exited the process immediately and could abort an in-flight /records apply. external-dns applies a record and its ownership TXT as separate sequential WAPI calls, so a mid-apply exit can strand a half-written or half-deleted record in the grid.
Run the API server directly (reusing the external-dns api handlers) with a signal-triggered shutdown. On SIGINT/SIGTERM the server stops accepting new connections and, when SERVER_SHUTDOWN_TIMEOUT is positive, drains in-flight requests for up to that long before exiting; the first signal is caught and the handler is then deregistered so a second signal still terminates immediately.
The drain is OPT-IN: SERVER_SHUTDOWN_TIMEOUT defaults to 0, which closes the server immediately and preserves the previous immediate-exit behaviour; operators enable draining by setting a positive timeout and a matching terminationGracePeriodSeconds. SERVER_READ_TIMEOUT/SERVER_WRITE_TIMEOUT are deliberately left unwired (a write timeout shorter than a slow apply would cut it mid-flight) — see the comments in serve.
Start now returns a bind error instead of calling log.Fatal, so it is testable and main owns the exit.