diff --git a/cmd/lnd/main.go b/cmd/lnd/main.go index 3f30567c14f..9a2943a3509 100644 --- a/cmd/lnd/main.go +++ b/cmd/lnd/main.go @@ -41,4 +41,13 @@ func main() { _, _ = fmt.Fprintln(os.Stderr, err) os.Exit(1) } + + // If the shutdown was requested programmatically (e.g. a health check + // detected that the chain backend is unavailable) rather than via an + // OS signal, exit with a non-zero status code. This allows process + // managers such as systemd with Restart=on-failure to automatically + // restart lnd rather than treating the exit as intentional. + if shutdownInterceptor.RequestedShutdown() { + os.Exit(1) + } } diff --git a/docs/release-notes/release-notes-0.22.0.md b/docs/release-notes/release-notes-0.22.0.md index 260c099f1f2..a34544c7b6c 100644 --- a/docs/release-notes/release-notes-0.22.0.md +++ b/docs/release-notes/release-notes-0.22.0.md @@ -22,6 +22,13 @@ # Bug Fixes +* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/10944) where + lnd exited with status code `0` after a health check triggered a + programmatic shutdown (e.g. the chain backend becoming unreachable). lnd + now exits with code `1` in that case, so process managers such as systemd + with `Restart=on-failure` automatically restart the daemon instead of + treating the exit as intentional. + * Bitcoind outbound peer health checks [now use](https://github.com/lightningnetwork/lnd/pull/10686) `getnetworkinfo.connections_out` instead of `getpeerinfo`. The same PR also [clarifies](https://github.com/lightningnetwork/lnd/issues/10568) the ZMQ diff --git a/signal/signal.go b/signal/signal.go index 8b4a01485ce..615baebf492 100644 --- a/signal/signal.go +++ b/signal/signal.go @@ -113,6 +113,12 @@ type Interceptor struct { // Notifier handles sending shutdown notifications. Notifier Notifier + + // requestedShutdown is set to 1 atomically when the shutdown was + // triggered programmatically via RequestShutdown, as opposed to an OS + // signal. This allows callers to distinguish abnormal shutdowns (e.g. + // health check failure) from user-initiated ones and exit accordingly. + requestedShutdown uint32 } // Intercept starts the interception of interrupt signals and returns an `Interceptor` instance. @@ -179,6 +185,7 @@ func (c *Interceptor) mainInterruptHandler() { case <-c.shutdownRequestChannel: log.Infof("Received shutdown request.") + atomic.StoreUint32(&c.requestedShutdown, 1) shutdown() case <-c.quit: @@ -222,6 +229,15 @@ func (c *Interceptor) RequestShutdown() { } } +// RequestedShutdown returns true if the shutdown was triggered +// programmatically via RequestShutdown (e.g. a health check failure), rather +// than by an OS signal such as SIGINT or SIGTERM. Callers can use this to exit +// with a non-zero status code so that process managers like systemd with +// Restart=on-failure will automatically restart the daemon. +func (c *Interceptor) RequestedShutdown() bool { + return atomic.LoadUint32(&c.requestedShutdown) != 0 +} + // ShutdownChannel returns the channel that will be closed once the main // interrupt handler has exited. func (c *Interceptor) ShutdownChannel() <-chan struct{} { diff --git a/signal/signal_test.go b/signal/signal_test.go new file mode 100644 index 00000000000..3ea52fe007f --- /dev/null +++ b/signal/signal_test.go @@ -0,0 +1,66 @@ +package signal + +import ( + "os" + "syscall" + "testing" + + "github.com/stretchr/testify/require" +) + +// newTestInterceptor builds an Interceptor and starts its handler goroutine +// without going through Intercept(), which holds a package-level lock that +// prevents more than one active interceptor at a time. This lets each subtest +// own an independent interceptor. +func newTestInterceptor() *Interceptor { + c := &Interceptor{ + interruptChannel: make(chan os.Signal, 1), + shutdownChannel: make(chan struct{}), + shutdownRequestChannel: make(chan struct{}), + quit: make(chan struct{}), + } + go c.mainInterruptHandler() + return c +} + +// TestRequestedShutdown checks that RequestedShutdown returns false when lnd +// exits due to an OS signal and true when shutdown is triggered +// programmatically via RequestShutdown (e.g. a health check failure). +func TestRequestedShutdown(t *testing.T) { + t.Parallel() + + t.Run("os signal does not set requested shutdown", func(t *testing.T) { + t.Parallel() + + interceptor := newTestInterceptor() + + // Simulate an OS signal (SIGINT) the way the OS would deliver + // it, bypassing RequestShutdown. + interceptor.interruptChannel <- syscall.SIGINT + + // Wait for the shutdown goroutine to finish. + <-interceptor.ShutdownChannel() + + // A signal-driven shutdown must not set the flag; exiting with + // code 0 is the correct behaviour in that case. + require.False(t, interceptor.RequestedShutdown()) + }) + + t.Run("programmatic shutdown sets requested shutdown", func(t *testing.T) { + t.Parallel() + + interceptor := newTestInterceptor() + + // Trigger a programmatic shutdown, as the health check does + // when bitcoind becomes unreachable. + interceptor.RequestShutdown() + + // Wait for the shutdown goroutine to finish. + <-interceptor.ShutdownChannel() + + // A programmatic shutdown must set the flag so that main exits + // with code 1, allowing systemd (Restart=on-failure) to + // restart lnd automatically. + require.True(t, interceptor.RequestedShutdown()) + }) +}