Repository: nats-io/nats-server
Pull request #1973 (2021-03-06 08:07:26)

Skip TERM signal during lame duck mode
In some platforms like K8S / Docker, the TERM signal is sent after calling `docker stop` / `kubectl delete pod`, letting the process terminate for some time before stopping the container.  

In order to terminate gracefully in NATS, it is needed to first send an USR2 signal to the server so that it enters lame duck mode before exiting. For example in k8s the `preStop` command can be used to send the signal:

```yaml
# Gracefully stop NATS Server on pod deletion or image upgrade.
#
lifecycle:
  preStop:
     exec:
        command: ["/bin/sh", "-c", "/nats-server -sl=ldm=/var/run/nats/nats.pid"]
```

But soon after sending the signal, K8S will send a `TERM` signal and abort the LDM graceful stop. In this PR, we add a check to the TERM signal handler so that LDM is able to wrap up and exit on its own before being forced to terminate (like `terminationGracePeriodSeconds` in K8S).

Signed-off-by: Waldemar Quevedo <wally@synadia.com>

/cc @nats-io/core



Patch:
diff --git a/server/const.go b/server/const.go
index 780e0a98..5cfbee10 100644
--- a/server/const.go
+++ b/server/const.go
@@ -29,6 +29,7 @@ const (
 
 	// private for now
 	commandLDMode = Command("ldm")
+	commandTerm   = Command("term")
 )
 
 var (
diff --git a/server/signal.go b/server/signal.go
index ee6bba52..cf35a6fa 100644
--- a/server/signal.go
+++ b/server/signal.go
@@ -48,9 +48,19 @@ func (s *Server) handleSignals() {
 			case sig := <-c:
 				s.Debugf("Trapped %q signal", sig)
 				switch sig {
-				case syscall.SIGINT, syscall.SIGTERM:
+				case syscall.SIGINT:
 					s.Shutdown()
 					os.Exit(0)
+				case syscall.SIGTERM:
+					// Shutdown unless graceful shutdown already in progress.
+					s.mu.Lock()
+					ldm := s.ldm
+					s.mu.Unlock()
+
+					if !ldm {
+						s.Shutdown()
+						os.Exit(0)
+					}
 				case syscall.SIGUSR1:
 					// File log re-open for rotating file logs.
 					s.ReOpenLogFile()
@@ -113,6 +123,8 @@ func ProcessSignal(command Command, pidStr string) error {
 		err = kill(pid, syscall.SIGHUP)
 	case commandLDMode:
 		err = kill(pid, syscall.SIGUSR2)
+	case commandTerm:
+		err = kill(pid, syscall.SIGTERM)
 	default:
 		err = fmt.Errorf("unknown signal %q", command)
 	}


Test patch:
diff --git a/server/signal_test.go b/server/signal_test.go
index 26075765..d0dc65f3 100644
--- a/server/signal_test.go
+++ b/server/signal_test.go
@@ -27,6 +27,7 @@ import (
 	"time"
 
 	"github.com/nats-io/nats-server/v2/logger"
+	"github.com/nats-io/nats.go"
 )
 
 func TestSignalToReOpenLogFile(t *testing.T) {
@@ -261,6 +262,32 @@ func TestProcessSignalQuitProcess(t *testing.T) {
 	}
 }
 
+func TestProcessSignalTermProcess(t *testing.T) {
+	killBefore := kill
+	called := false
+	kill = func(pid int, signal syscall.Signal) error {
+		called = true
+		if pid != 123 {
+			t.Fatalf("pid is incorrect.\nexpected: 123\ngot: %d", pid)
+		}
+		if signal != syscall.SIGTERM {
+			t.Fatalf("signal is incorrect.\nexpected: interrupt\ngot: %v", signal)
+		}
+		return nil
+	}
+	defer func() {
+		kill = killBefore
+	}()
+
+	if err := ProcessSignal(commandTerm, "123"); err != nil {
+		t.Fatalf("ProcessSignal failed: %v", err)
+	}
+
+	if !called {
+		t.Fatal("Expected kill to be called")
+	}
+}
+
 func TestProcessSignalReopenProcess(t *testing.T) {
 	killBefore := kill
 	called := false
@@ -338,3 +365,62 @@ func TestProcessSignalLameDuckMode(t *testing.T) {
 		t.Fatal("Expected kill to be called")
 	}
 }
+
+func TestProcessSignalTermDuringLameDuckMode(t *testing.T) {
+	opts := &Options{
+		Host:                "127.0.0.1",
+		Port:                -1,
+		NoSigs:              false,
+		NoLog:               true,
+		LameDuckDuration:    2 * time.Second,
+		LameDuckGracePeriod: 1 * time.Second,
+	}
+	s := RunServer(opts)
+	defer s.Shutdown()
+
+	// Create single NATS Connection which will cause the server
+	// to delay the shutdown.
+	doneCh := make(chan struct{})
+	nc, err := nats.Connect(fmt.Sprintf("nats://%s:%d", opts.Host, opts.Port),
+		nats.DisconnectHandler(func(*nats.Conn) {
+			close(doneCh)
+		}),
+	)
+	if err != nil {
+		t.Fatalf("Error on connect: %v", err)
+	}
+	defer nc.Close()
+
+	// Trigger lame duck based shutdown.
+	go s.lameDuckMode()
+
+	// Wait for client to be disconnected.
+	select {
+	case <-doneCh:
+		break
+	case <-time.After(3 * time.Second):
+		t.Fatalf("Timed out waiting for client to disconnect")
+	}
+
+	// Termination signal should not cause server to shutdown
+	// while in lame duck mode already.
+	syscall.Kill(syscall.Getpid(), syscall.SIGTERM)
+
+	// Wait for server shutdown due to lame duck shutdown.
+	timeoutCh := make(chan error)
+	timer := time.AfterFunc(3*time.Second, func() {
+		timeoutCh <- errors.New("Timed out waiting for server shutdown")
+	})
+	for range time.NewTicker(1 * time.Millisecond).C {
+		select {
+		case err := <-timeoutCh:
+			t.Fatal(err)
+		default:
+		}
+
+		if !s.isRunning() {
+			timer.Stop()
+			break
+		}
+	}
+}