fix: Withdraw BGP routes gracefully on router shutdown - #414
Merged
Conversation
galactic-router had no graceful-shutdown story: on SIGTERM the process just exited while its embedded GoBGP server still held every peer session open. Peers only noticed via TCP RST or hold-timer expiry (default 90s minus keepalive slop) instead of an immediate withdrawal, so a rolling update or node drain could blackhole traffic for up to that long before routes cleared. The machinery to do this correctly already existed and was already correct: RuntimeManager.StopAll drives each RouterRuntime.Stop, which for GoBGP cancels its server context and lets the embedded BgpServer's own StopBgp run — which does send a Cease NOTIFICATION to every neighbor before closing the session, an immediate and explicit withdrawal rather than a timeout. The gap was that nothing ever called StopAll: the GoBGP server runs in a goroutine started independently of the manager's context and never registered as a Runnable, so controller-runtime's own graceful shutdown had no path to reach it, and StopAll had zero callers anywhere in the repo. runCmd now calls runtimeMgr.StopAll with a fresh, bounded-10s context right after mgr.Start returns (mgr.Start returning nil means the process's own ctx is already Done, from either a signal or the health server's fatal Serve error, so ctx itself can't be reused). No manifest change needed — the default 30s terminationGracePeriodSeconds already comfortably covers a 10s-bounded stop. Added internal/runtime/manager_test.go, covering StopAll for the first time (0% coverage before this on internal/runtime): confirms it stops every live runtime, returns the first error across all of them, and is a no-op with an empty manager. Verified on a live Kind cluster: applied a real BGPRouter CR to bring up an actual GoBGP runtime under the hardened container from the previous commit, confirmed Ready/RuntimeReady status, then deleted the pod and watched controller-runtime's shutdown sequence complete cleanly well inside the termination grace period. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ecv
approved these changes
Aug 16, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
galactic-router had no graceful shutdown story: on termination it exited immediately while its embedded BGP server still held every peer session open. Peers only noticed via connection reset or a hold-timer expiry, not an explicit withdrawal, so a rolling update or node drain could blackhole traffic until that timer fired. The routine to withdraw routes cleanly already existed but nothing ever called it. Shutdown now drives that routine directly, so peers get an immediate, explicit withdrawal instead of waiting on a timeout.
Test plan