Merge branch 'service_restart_gnoi' of https://github.com/isabelmsft/…

…sonic-gnmi into service_restart_gnoi

Makefile

@@ -14,6 +14,8 @@ export GOBIN := $(abspath $(BUILD_DIR))
 export PATH := $(PATH):$(GOBIN):$(shell dirname $(GO))
 export CGO_LDFLAGS := -lswsscommon -lhiredis
 export CGO_CXXFLAGS := -I/usr/include/swss -w -Wall -fpermissive
+export MEMCHECK_CGO_LDFLAGS := $(CGO_LDFLAGS) -fsanitize=address
+export MEMCHECK_CGO_CXXFLAGS := $(CGO_CXXFLAGS) -fsanitize=leak
 
 ifeq ($(ENABLE_TRANSLIB_WRITE),y)
 BLD_TAGS := gnmi_translib_write
@@ -26,6 +28,11 @@ ifneq ($(BLD_TAGS),)
 BLD_FLAGS := -tags "$(strip $(BLD_TAGS))"
 endif
 
+MEMCHECK_TAGS := $(BLD_TAGS) gnmi_memcheck
+ifneq ($(MEMCHECK_TAGS),)
+MEMCHECK_FLAGS := -tags "$(strip $(MEMCHECK_TAGS))"
+endif
+
 ENABLE_DIALOUT_VALUE := 1
 ifeq ($(ENABLE_DIALOUT),n)
 ENABLE_DIALOUT_VALUE = 0
@@ -133,6 +140,13 @@ endif
 	gocov convert coverage-*.txt | gocov-xml -source $(shell pwd) > coverage.xml
 	rm -rf coverage-*.txt 
 
+check_memleak: $(DBCONFG) $(ENVFILE)
+	sudo CGO_LDFLAGS="$(MEMCHECK_CGO_LDFLAGS)" CGO_CXXFLAGS="$(MEMCHECK_CGO_CXXFLAGS)" $(GO) test -mod=vendor $(MEMCHECK_FLAGS) -v github.com/sonic-net/sonic-gnmi/telemetry
+	sudo CGO_LDFLAGS="$(MEMCHECK_CGO_LDFLAGS)" CGO_CXXFLAGS="$(MEMCHECK_CGO_CXXFLAGS)" $(GO) test $(MEMCHECK_FLAGS) -v github.com/sonic-net/sonic-gnmi/sonic_db_config
+	sudo CGO_LDFLAGS="$(MEMCHECK_CGO_LDFLAGS)" CGO_CXXFLAGS="$(MEMCHECK_CGO_CXXFLAGS)" $(GO) test -mod=vendor $(MEMCHECK_FLAGS) -v github.com/sonic-net/sonic-gnmi/gnmi_server -run TestGNMINative
+	sudo CGO_LDFLAGS="$(MEMCHECK_CGO_LDFLAGS)" CGO_CXXFLAGS="$(MEMCHECK_CGO_CXXFLAGS)" $(GO) test -mod=vendor $(MEMCHECK_FLAGS) -v github.com/sonic-net/sonic-gnmi/sonic_data_client
+
+
 clean:
 	$(RM) -r build
 	$(RM) -r vendor

azure-pipelines.yml

@@ -23,6 +23,8 @@ variables:
       value: $(System.PullRequest.TargetBranch)
     ${{ else }}:
       value: $(Build.SourceBranchName)
+  - name: UNIT_TEST_FLAG
+    value: 'ENABLE_TRANSLIB_WRITE=y'
 
 resources:
   repositories:
@@ -161,8 +163,13 @@ stages:
 
     - script: |
         pushd sonic-gnmi
-        make check_gotest ENABLE_TRANSLIB_WRITE=y
-      displayName: "Test"
+        make all && make check_memleak $(UNIT_TEST_FLAG)
+      displayName: "Check memory leak"
+
+    - script: |
+        pushd sonic-gnmi
+        make all && make check_gotest $(UNIT_TEST_FLAG)
+      displayName: "Run unit test"
 
     - publish: $(Build.ArtifactStagingDirectory)/
       artifact: sonic-gnmi

gnmi_server/server.go

@@ -4,12 +4,18 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"net"
+	"strings"
+	"sync"
+
 	"github.com/Azure/sonic-mgmt-common/translib"
 	"github.com/sonic-net/sonic-gnmi/common_utils"
 	spb "github.com/sonic-net/sonic-gnmi/proto"
 	spb_gnoi "github.com/sonic-net/sonic-gnmi/proto/gnoi"
 	spb_jwt_gnoi "github.com/sonic-net/sonic-gnmi/proto/gnoi/jwt"
 	sdc "github.com/sonic-net/sonic-gnmi/sonic_data_client"
+	ssc "github.com/sonic-net/sonic-gnmi/sonic_service_client"
+
 	log "github.com/golang/glog"
 	"github.com/golang/protobuf/proto"
 	gnmipb "github.com/openconfig/gnmi/proto/gnmi"
@@ -21,9 +27,6 @@ import (
 	"google.golang.org/grpc/peer"
 	"google.golang.org/grpc/reflection"
 	"google.golang.org/grpc/status"
-	"net"
-	"strings"
-	"sync"
 )
 
 var (
@@ -39,6 +42,10 @@ type Server struct {
 	config  *Config
 	cMu     sync.Mutex
 	clients map[string]*Client
+	// SaveStartupConfig points to a function that is called to save changes of
+	// configuration to a file. By default it points to an empty function -
+	// the configuration is not saved to a file.
+	SaveStartupConfig func() error
 	// ReqFromMaster point to a function that is called to verify if the request
 	// comes from a master controller.
 	ReqFromMaster func(req *gnmipb.SetRequest, masterEID *uint128) error
@@ -53,14 +60,14 @@ type AuthTypes map[string]bool
 type Config struct {
 	// Port for the Server to listen on. If 0 or unset the Server will pick a port
 	// for this Server.
-	Port     int64
-	LogLevel int
-	Threshold int
-	UserAuth AuthTypes
+	Port                int64
+	LogLevel            int
+	Threshold           int
+	UserAuth            AuthTypes
 	EnableTranslibWrite bool
-	EnableNativeWrite bool
-	ZmqPort string
-	IdleConnDuration int
+	EnableNativeWrite   bool
+	ZmqPort             string
+	IdleConnDuration    int
 }
 
 var AuthLock sync.Mutex
@@ -141,9 +148,10 @@ func NewServer(config *Config, opts []grpc.ServerOption) (*Server, error) {
 	reflection.Register(s)
 
 	srv := &Server{
-		s:       s,
-		config:  config,
-		clients: map[string]*Client{},
+		s:                 s,
+		config:            config,
+		clients:           map[string]*Client{},
+		SaveStartupConfig: saveOnSetDisabled,
 		// ReqFromMaster point to a function that is called to verify if
 		// the request comes from a master controller.
 		ReqFromMaster: ReqFromMasterDisabledMA,
@@ -162,7 +170,7 @@ func NewServer(config *Config, opts []grpc.ServerOption) (*Server, error) {
 	if srv.config.EnableTranslibWrite || srv.config.EnableNativeWrite {
 		gnoi_system_pb.RegisterSystemServer(srv.s, srv)
 	}
-	if srv.config.EnableTranslibWrite {		
+	if srv.config.EnableTranslibWrite {
 		spb_gnoi.RegisterSonicServiceServer(srv.s, srv)
 	}
 	spb_gnoi.RegisterDebugServer(srv.s, srv)
@@ -402,6 +410,25 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetRe
 	return &gnmipb.GetResponse{Notification: notifications}, nil
 }
 
+// saveOnSetEnabled saves configuration to a file
+func SaveOnSetEnabled() error {
+	sc, err := ssc.NewDbusClient()
+	if err != nil {
+		log.V(0).Infof("Saving startup config failed to create dbus client: %v", err)
+		return err
+	}
+	if err := sc.ConfigSave("/etc/sonic/config_db.json"); err != nil {
+		log.V(0).Infof("Saving startup config failed: %v", err)
+		return err
+	} else {
+		log.V(1).Infof("Success! Startup config has been saved!")
+	}
+	return nil
+}
+
+// SaveOnSetDisabeld does nothing.
+func saveOnSetDisabled() error { return nil }
+
 func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetResponse, error) {
 	e := s.ReqFromMaster(req, &s.masterEID)
 	if e != nil {
@@ -505,6 +532,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe
 		common_utils.IncCounter(common_utils.GNMI_SET_FAIL)
 	}
 
+	s.SaveStartupConfig()
 	return &gnmipb.SetResponse{
 		Prefix:   req.GetPrefix(),
 		Response: results,
@@ -602,7 +630,7 @@ func ReqFromMasterEnabledMA(req *gnmipb.SetRequest, masterEID *uint128) error {
 			// Role will be implemented later.
 			return status.Errorf(codes.Unimplemented, "MA: Role is not implemented")
 		}
-		
+
 		reqEID = uint128{High: ma.ElectionId.High, Low: ma.ElectionId.Low}
 		// Use the election ID that is in the last extension, so, no 'break' here.
 	}