Skip to content

Commit

Permalink
Add signal handler to stop gnmi server for when sigterm or sigquit is…
Browse files Browse the repository at this point in the history
… called (sonic-net#189)
  • Loading branch information
zbud-msft authored Feb 24, 2024
1 parent 5b59c57 commit 736e3b4
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 7 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ $(ENVFILE):
tools/test/env.sh | grep -v DB_CONFIG_PATH | tee $@

check_gotest: $(DBCONFG) $(ENVFILE)
sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-data.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/telemetry
sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-telemetry.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/telemetry
sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-config.txt -covermode=atomic -v github.com/sonic-net/sonic-gnmi/sonic_db_config
sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -timeout 20m -coverprofile=coverage-gnmi.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/gnmi_server -coverpkg ../...
ifneq ($(ENABLE_DIALOUT_VALUE),0)
Expand Down
32 changes: 29 additions & 3 deletions telemetry/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"io/ioutil"
"time"
"os"
"os/signal"
"syscall"
"sync"
log "github.com/golang/glog"
"google.golang.org/grpc"
Expand Down Expand Up @@ -55,9 +57,17 @@ func runTelemetry(args []string) error {
}

var wg sync.WaitGroup
// serverControlSignal channel is a channel that will be used to notify gnmi server of sigcall which should stop server
var serverControlSignal = make(chan int, 1)
sigchannel := make(chan os.Signal, 1)
signal.Notify(sigchannel, syscall.SIGTERM, syscall.SIGQUIT)

wg.Add(1)
go startGNMIServer(telemetryCfg, cfg, &wg)
go startGNMIServer(telemetryCfg, cfg, serverControlSignal, &wg)

wg.Add(1)
go signalHandler(serverControlSignal, &wg, sigchannel)

wg.Wait()
return nil
}
Expand Down Expand Up @@ -149,7 +159,15 @@ func isFlagPassed(fs *flag.FlagSet, name string) bool {
return found
}

func startGNMIServer(telemetryCfg *TelemetryConfig, cfg *gnmi.Config, wg *sync.WaitGroup) {
func signalHandler(serverControlSignal chan<- int, wg *sync.WaitGroup, sigchannel <-chan os.Signal) {
defer wg.Done()
select {
case <-sigchannel:
serverControlSignal <- 0
}
}

func startGNMIServer(telemetryCfg *TelemetryConfig, cfg *gnmi.Config, serverControlSignal <-chan int, wg *sync.WaitGroup) {
defer wg.Done()
var opts []grpc.ServerOption
if !*telemetryCfg.NoTLS {
Expand Down Expand Up @@ -245,7 +263,15 @@ func startGNMIServer(telemetryCfg *TelemetryConfig, cfg *gnmi.Config, wg *sync.W

log.V(1).Infof("Auth Modes: %v", telemetryCfg.UserAuth)
log.V(1).Infof("Starting RPC server on address: %s", s.Address())
s.Serve() // blocks until closed

go func() {
if err := s.Serve(); err != nil {
log.Errorf("Serve returned with err: %v", err)
}
}()

<-serverControlSignal
log.V(1).Infof("Received signal for gnmi server to close")
s.Stop()
log.Flush()
}
Expand Down
69 changes: 66 additions & 3 deletions telemetry/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import (
gnmi "github.com/sonic-net/sonic-gnmi/gnmi_server"
"github.com/agiledragon/gomonkey/v2"
"os"
"syscall"
"time"
"context"
"encoding/pem"
"fmt"
log "github.com/golang/glog"
Expand All @@ -26,7 +29,10 @@ func TestRunTelemetry(t *testing.T) {
patches := gomonkey.ApplyFunc(setupFlags, func(*flag.FlagSet) (*TelemetryConfig, *gnmi.Config, error) {
return telemetryCfg, cfg, nil
})
patches.ApplyFunc(startGNMIServer, func(_ *TelemetryConfig, _ *gnmi.Config, wg *sync.WaitGroup) {
patches.ApplyFunc(startGNMIServer, func(_ *TelemetryConfig, _ *gnmi.Config, serverControlSignal <-chan int, wg *sync.WaitGroup) {
defer wg.Done()
})
patches.ApplyFunc(signalHandler, func(serverControlSignal chan<- int, wg *sync.WaitGroup, sigchannel <-chan os.Signal) {
defer wg.Done()
})
defer patches.Reset()
Expand Down Expand Up @@ -113,6 +119,12 @@ func TestFlags(t *testing.T) {
func TestStartGNMIServer(t *testing.T) {
testServerCert := "../testdata/testserver.cer"
testServerKey := "../testdata/testserver.key"
timeoutInterval := 3
tick := time.NewTicker(100 * time.Millisecond)
defer tick.Stop()

ctx, cancel := context.WithTimeout(context.Background(), time.Duration(timeoutInterval) * time.Second)
defer cancel()

originalArgs := os.Args
defer func() {
Expand All @@ -122,6 +134,7 @@ func TestStartGNMIServer(t *testing.T) {
fs := flag.NewFlagSet("testStartGNMIServer", flag.ContinueOnError)
os.Args = []string{"cmd", "-port", "8080", "-server_crt", testServerCert, "-server_key", testServerKey}
telemetryCfg, cfg, err := setupFlags(fs)

if err != nil {
t.Errorf("Expected err to be nil, got err %v", err)
}
Expand All @@ -142,6 +155,7 @@ func TestStartGNMIServer(t *testing.T) {
return ""
})

serverControlSignal := make(chan int, 1)
wg := &sync.WaitGroup{}

exitCalled := false
Expand All @@ -153,7 +167,15 @@ func TestStartGNMIServer(t *testing.T) {

wg.Add(1)

go startGNMIServer(telemetryCfg, cfg, wg)
go startGNMIServer(telemetryCfg, cfg, serverControlSignal, wg)

select {
case <-tick.C: // Simulate shutdown
sendShutdownSignal(serverControlSignal)
case <-ctx.Done():
t.Errorf("Failed to send shutdown signal")
return
}

wg.Wait()

Expand Down Expand Up @@ -214,6 +236,7 @@ func TestSHA512Checksum(t *testing.T) {
}

err = saveCertKeyPair(testServerCert, testServerKey)

if err != nil {
t.Errorf("Expected err to be nil, got err %v", err)
}
Expand All @@ -237,6 +260,7 @@ func TestSHA512Checksum(t *testing.T) {
return ""
})

serverControlSignal := make(chan int, 1)
wg := &sync.WaitGroup{}

patches.ApplyMethod(reflect.TypeOf(&gnmi.Server{}), "Stop", func(_ *gnmi.Server) {
Expand All @@ -246,7 +270,46 @@ func TestSHA512Checksum(t *testing.T) {

wg.Add(1)

go startGNMIServer(telemetryCfg, cfg, wg)
go startGNMIServer(telemetryCfg, cfg, serverControlSignal, wg)

sendShutdownSignal(serverControlSignal)

wg.Wait()
}

func TestSignalHandler(t *testing.T) {
testHandlerSyscall(t, syscall.SIGTERM)
testHandlerSyscall(t, syscall.SIGQUIT)
}

func testHandlerSyscall(t *testing.T, signal os.Signal) {
timeoutInterval := 1
ctx, cancel := context.WithTimeout(context.Background(), time.Duration(timeoutInterval) * time.Second)
defer cancel()

serverControlSignal := make(chan int, 1)
testSigChan := make(chan os.Signal, 1)
wg := &sync.WaitGroup{}

wg.Add(1)

go signalHandler(serverControlSignal, wg, testSigChan)

testSigChan <- signal

select {
case val := <-serverControlSignal:
if val != 0 {
t.Errorf("Expected 0 from serverControlSignal, got %d", val)
}
case <-ctx.Done():
t.Errorf("Expected a value from serverControlSignal, but got none")
return
}

wg.Wait()
}

func sendShutdownSignal(serverControlSignal chan<- int) {
serverControlSignal <- 0
}

0 comments on commit 736e3b4

Please sign in to comment.