From f750373924a752d27bb8d4f3c689bb03cddf5ac4 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Mon, 14 Nov 2022 15:42:03 +0800 Subject: [PATCH 1/6] Add gnmi_dump tool for debug and unit test. --- Makefile | 4 +++ common_utils/context.go | 33 +++++++++++++++++++-- common_utils/shareMem.go | 64 ++++++++++++++++++++++++++++++++++++++++ gnmi_dump/gnmi_dump.go | 29 ++++++++++++++++++ gnmi_server/server.go | 24 +++++++++++---- 5 files changed, 146 insertions(+), 8 deletions(-) create mode 100644 common_utils/shareMem.go create mode 100644 gnmi_dump/gnmi_dump.go diff --git a/Makefile b/Makefile index 2813f3f6..7d007ed5 100644 --- a/Makefile +++ b/Makefile @@ -57,6 +57,7 @@ ifeq ($(CROSS_BUILD_ENVIRON),y) $(GO) build -o ${GOBIN}/gnmi_set -mod=vendor github.com/jipanyang/gnxi/gnmi_set $(GO) build -o ${GOBIN}/gnmi_cli -mod=vendor github.com/openconfig/gnmi/cmd/gnmi_cli $(GO) build -o ${GOBIN}/gnoi_client -mod=vendor github.com/sonic-net/sonic-gnmi/gnoi_client + $(GO) build -o ${GOBIN}/gnmi_dump -mod=vendor github.com/sonic-net/sonic-gnmi/gnmi_dump else $(GO) install -mod=vendor $(BLD_FLAGS) github.com/sonic-net/sonic-gnmi/telemetry $(GO) install -mod=vendor $(BLD_FLAGS) github.com/sonic-net/sonic-gnmi/dialout/dialout_client_cli @@ -64,6 +65,7 @@ else $(GO) install -mod=vendor github.com/jipanyang/gnxi/gnmi_set $(GO) install -mod=vendor github.com/openconfig/gnmi/cmd/gnmi_cli $(GO) install -mod=vendor github.com/sonic-net/sonic-gnmi/gnoi_client + $(GO) install -mod=vendor github.com/sonic-net/sonic-gnmi/gnmi_dump endif check_gotest: @@ -99,6 +101,7 @@ install: $(INSTALL) -D $(BUILD_DIR)/gnmi_set $(DESTDIR)/usr/sbin/gnmi_set $(INSTALL) -D $(BUILD_DIR)/gnmi_cli $(DESTDIR)/usr/sbin/gnmi_cli $(INSTALL) -D $(BUILD_DIR)/gnoi_client $(DESTDIR)/usr/sbin/gnoi_client + $(INSTALL) -D $(BUILD_DIR)/gnmi_dump $(DESTDIR)/usr/sbin/gnmi_dump deinstall: @@ -107,5 +110,6 @@ deinstall: rm $(DESTDIR)/usr/sbin/gnmi_get rm $(DESTDIR)/usr/sbin/gnmi_set rm $(DESTDIR)/usr/sbin/gnoi_client + rm $(DESTDIR)/usr/sbin/gnmi_dump diff --git a/common_utils/context.go b/common_utils/context.go index b7c257ef..ad1b7945 100644 --- a/common_utils/context.go +++ b/common_utils/context.go @@ -36,6 +36,16 @@ const requestContextKey contextkey = 0 // Request Id generator var requestCounter uint64 +var CountersName = [...]string { + "GNMI get", + "GNMI get fail", + "GNMI set", + "GNMI set fail", +} + +var globalCounters [len(CountersName)]uint64 + + // GetContext function returns the RequestContext object for a // gRPC request. RequestContext is maintained as a context value of // the request. Creates a new RequestContext object is not already @@ -55,8 +65,25 @@ func GetContext(ctx context.Context) (*RequestContext, context.Context) { func GetUsername(ctx context.Context, username *string) { rc, _ := GetContext(ctx) - if rc != nil { - *username = rc.Auth.User - } + if rc != nil { + *username = rc.Auth.User + } +} + +func InitCounters() { + for i := 0; i < len(CountersName); i++ { + globalCounters[i] = 0 + } + SetMemCounters(&globalCounters) +} + +func IncCounter(name string) { + for i := 0; i < len(CountersName); i++ { + if CountersName[i] == name { + atomic.AddUint64(&globalCounters[i], 1) + break + } + } + SetMemCounters(&globalCounters) } diff --git a/common_utils/shareMem.go b/common_utils/shareMem.go new file mode 100644 index 00000000..168865e3 --- /dev/null +++ b/common_utils/shareMem.go @@ -0,0 +1,64 @@ +package common_utils + +import ( + "fmt" + "syscall" + "unsafe" +) + +// Use share memory to dump GNMI internal counters, +// GNMI server and gnmi_dump should use memKey to access the share memory, +// memSize is 1024 bytes, so we can support 128 counters +// memMode is 0x380, this value is O_RDWR|IPC_CREAT, +// O_RDWR means: Owner can write and read the file, everyone else can't. +// IPC_CREAT means: Create a shared memory segment if a shared memory identifier does not exist for memKey. +var ( + memKey = 7749 + memSize = 1024 + memMode = 0x380 +) + +func SetMemCounters(counters *[len(CountersName)]uint64) error { + shmid, _, err := syscall.Syscall(syscall.SYS_SHMGET, uintptr(memKey), uintptr(memSize), uintptr(memMode)) + if int(shmid) == -1 { + return fmt.Errorf("syscall error, err: %v\n", err) + } + + shmaddr, _, err := syscall.Syscall(syscall.SYS_SHMAT, shmid, 0, 0) + if int(shmaddr) == -1 { + return fmt.Errorf("syscall error, err: %v\n", err) + } + defer syscall.Syscall(syscall.SYS_SHMDT, shmaddr, 0, 0) + + const size = unsafe.Sizeof(uint64(0)) + addr := uintptr(unsafe.Pointer(shmaddr)) + + for i := 0; i < len(counters); i++ { + *(*uint64)(unsafe.Pointer(addr)) = counters[i] + addr += size + } + return nil +} + +func GetMemCounters(counters *[len(CountersName)]uint64) error { + shmid, _, err := syscall.Syscall(syscall.SYS_SHMGET, uintptr(memKey), uintptr(memSize), uintptr(memMode)) + if int(shmid) == -1 { + return fmt.Errorf("syscall error, err: %v\n", err) + } + + shmaddr, _, err := syscall.Syscall(syscall.SYS_SHMAT, shmid, 0, 0) + if int(shmaddr) == -1 { + return fmt.Errorf("syscall error, err: %v\n", err) + } + defer syscall.Syscall(syscall.SYS_SHMDT, shmaddr, 0, 0) + + const size = unsafe.Sizeof(uint64(0)) + addr := uintptr(unsafe.Pointer(shmaddr)) + + for i := 0; i < len(counters); i++ { + counters[i] = *(*uint64)(unsafe.Pointer(addr)) + addr += size + } + return nil +} + diff --git a/gnmi_dump/gnmi_dump.go b/gnmi_dump/gnmi_dump.go new file mode 100644 index 00000000..04ba47a0 --- /dev/null +++ b/gnmi_dump/gnmi_dump.go @@ -0,0 +1,29 @@ +package main + +import ( + "flag" + "fmt" + "github.com/sonic-net/sonic-gnmi/common_utils" +) + +const help = ` +gnmi_dump is used to dump internal counters for debugging purpose, +including GNMI request counter, GNOI request counter and DBUS request counter. +` + +func main() { + flag.Usage = func() { + fmt.Print(help) + } + flag.Parse() + var counters [len(common_utils.CountersName)]uint64 + err := common_utils.GetMemCounters(&counters) + if err != nil { + fmt.Printf("Error: Fail to read counters, %v", err) + return + } + fmt.Printf("Dump GNMI counters\n") + for i := 0; i < len(common_utils.CountersName); i++ { + fmt.Printf("%v---%v\n", common_utils.CountersName[i], counters[i]) + } +} diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 7717e1d9..74635560 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -124,6 +124,8 @@ func NewServer(config *Config, opts []grpc.ServerOption) (*Server, error) { return nil, errors.New("config not provided") } + common_utils.InitCounters() + s := grpc.NewServer(opts...) reflection.Register(s) @@ -274,26 +276,32 @@ func (s *Server) checkEncodingAndModel(encoding gnmipb.Encoding, models []*gnmip // Get implements the Get RPC in gNMI spec. func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetResponse, error) { + common_utils.IncCounter("GNMI get") ctx, err := authenticate(s.config.UserAuth, ctx) if err != nil { + common_utils.IncCounter("GNMI get fail") return nil, err } if req.GetType() != gnmipb.GetRequest_ALL { + common_utils.IncCounter("GNMI get fail") return nil, status.Errorf(codes.Unimplemented, "unsupported request type: %s", gnmipb.GetRequest_DataType_name[int32(req.GetType())]) } if err = s.checkEncodingAndModel(req.GetEncoding(), req.GetUseModels()); err != nil { + common_utils.IncCounter("GNMI get fail") return nil, status.Error(codes.Unimplemented, err.Error()) } var target string prefix := req.GetPrefix() if prefix == nil { + common_utils.IncCounter("GNMI get fail") return nil, status.Error(codes.Unimplemented, "No target specified in prefix") } else { target = prefix.GetTarget() if target == "" { + common_utils.IncCounter("GNMI get fail") return nil, status.Error(codes.Unimplemented, "Empty target data not supported yet") } } @@ -315,11 +323,13 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetRe } if err != nil { + common_utils.IncCounter("GNMI get fail") return nil, status.Error(codes.NotFound, err.Error()) } notifications := make([]*gnmipb.Notification, len(paths)) spbValues, err := dc.Get(nil) if err != nil { + common_utils.IncCounter("GNMI get fail") return nil, status.Error(codes.NotFound, err.Error()) } @@ -339,8 +349,14 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetRe } func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetResponse, error) { + common_utils.IncCounter("GNMI set") + if s.config.EnableTranslibWrite == false { + common_utils.IncCounter("GNMI set fail") + return nil, grpc.Errorf(codes.Unimplemented, "GNMI is in read-only mode") + } ctx, err := authenticate(s.config.UserAuth, ctx) if err != nil { + common_utils.IncCounter("GNMI set fail") return nil, err } var results []*gnmipb.UpdateResult @@ -388,13 +404,11 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe /* Add to Set response results. */ results = append(results, &res) } - if s.config.EnableTranslibWrite { - err = dc.Set(req.GetDelete(), req.GetReplace(), req.GetUpdate()) - } else { - return nil, grpc.Errorf(codes.Unimplemented, "Telemetry is in read-only mode") + err = dc.Set(req.GetDelete(), req.GetReplace(), req.GetUpdate()) + if err != nil { + common_utils.IncCounter("GNMI set fail") } - return &gnmipb.SetResponse{ Prefix: req.GetPrefix(), Response: results, From fd01aa90df76d5e2cb36521acc279bede1de056e Mon Sep 17 00:00:00 2001 From: ganglyu Date: Mon, 14 Nov 2022 16:35:28 +0800 Subject: [PATCH 2/6] Add test case to improve coverage. --- gnmi_server/server_test.go | 116 ++++++++++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 1 deletion(-) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 3a0c0da8..57f0ce7b 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -107,6 +107,25 @@ func createServer(t *testing.T, port int64) *Server { return s } +func createReadServer(t *testing.T, port int64) *Server { + certificate, err := testcert.NewCert() + if err != nil { + t.Errorf("could not load server key pair: %s", err) + } + tlsCfg := &tls.Config{ + ClientAuth: tls.RequestClientCert, + Certificates: []tls.Certificate{certificate}, + } + + opts := []grpc.ServerOption{grpc.Creds(credentials.NewTLS(tlsCfg))} + cfg := &Config{Port: port, EnableTranslibWrite: false} + s, err := NewServer(cfg, opts) + if err != nil { + t.Errorf("Failed to create gNMI server: %v", err) + } + return s +} + func createAuthServer(t *testing.T, port int64) *Server { certificate, err := testcert.NewCert() if err != nil { @@ -118,7 +137,7 @@ func createAuthServer(t *testing.T, port int64) *Server { } opts := []grpc.ServerOption{grpc.Creds(credentials.NewTLS(tlsCfg))} - cfg := &Config{Port: port, UserAuth: AuthTypes{"password": true, "cert": true, "jwt": true}} + cfg := &Config{Port: port, EnableTranslibWrite: true, UserAuth: AuthTypes{"password": true, "cert": true, "jwt": true}} s, err := NewServer(cfg, opts) if err != nil { t.Errorf("Failed to create gNMI server: %v", err) @@ -732,6 +751,101 @@ func TestGnmiSet(t *testing.T) { s.s.Stop() } +func TestGnmiSetReadOnly(t *testing.T) { + s := createReadServer(t, 8081) + go runServer(t, s) + defer s.s.Stop() + + tlsConfig := &tls.Config{InsecureSkipVerify: true} + opts := []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))} + + targetAddr := "127.0.0.1:8081" + conn, err := grpc.Dial(targetAddr, opts...) + if err != nil { + t.Fatalf("Dialing to %q failed: %v", targetAddr, err) + } + defer conn.Close() + + gClient := pb.NewGNMIClient(conn) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + req := &pb.SetRequest{} + _, err = gClient.Set(ctx, req) + gotRetStatus, ok := status.FromError(err) + if !ok { + t.Fatal("got a non-grpc error from grpc call") + } + wantRetCode := codes.Unimplemented + if gotRetStatus.Code() != wantRetCode { + t.Log("err: ", err) + t.Fatalf("got return code %v, want %v", gotRetStatus.Code(), wantRetCode) + } +} + +func TestGnmiSetAuthFail(t *testing.T) { + s := createAuthServer(t, 8081) + go runServer(t, s) + defer s.s.Stop() + + tlsConfig := &tls.Config{InsecureSkipVerify: true} + opts := []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))} + + targetAddr := "127.0.0.1:8081" + conn, err := grpc.Dial(targetAddr, opts...) + if err != nil { + t.Fatalf("Dialing to %q failed: %v", targetAddr, err) + } + defer conn.Close() + + gClient := pb.NewGNMIClient(conn) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + req := &pb.SetRequest{} + _, err = gClient.Set(ctx, req) + gotRetStatus, ok := status.FromError(err) + if !ok { + t.Fatal("got a non-grpc error from grpc call") + } + wantRetCode := codes.Unimplemented + if gotRetStatus.Code() != wantRetCode { + t.Log("err: ", err) + t.Fatalf("got return code %v, want %v", gotRetStatus.Code(), wantRetCode) + } +} + +func TestGnmiGetAuthFail(t *testing.T) { + s := createAuthServer(t, 8081) + go runServer(t, s) + + tlsConfig := &tls.Config{InsecureSkipVerify: true} + opts := []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))} + + targetAddr := "127.0.0.1:8081" + conn, err := grpc.Dial(targetAddr, opts...) + if err != nil { + t.Fatalf("Dialing to %q failed: %v", targetAddr, err) + } + defer conn.Close() + + gClient := pb.NewGNMIClient(conn) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + req := &pb.GetRequest{} + _, err = gClient.Get(ctx, req) + gotRetStatus, ok := status.FromError(err) + if !ok { + t.Fatal("got a non-grpc error from grpc call") + } + wantRetCode := codes.Unauthenticated + if gotRetStatus.Code() != wantRetCode { + t.Log("err: ", err) + t.Fatalf("got return code %v, want %v", gotRetStatus.Code(), wantRetCode) + } +} + func runGnmiTestGet(t *testing.T, namespace string) { //t.Log("Start gNMI client") tlsConfig := &tls.Config{InsecureSkipVerify: true} From 5ffcc3af2ba9d8260beceaa585065552f746a4f7 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Mon, 14 Nov 2022 16:39:09 +0800 Subject: [PATCH 3/6] Fix test bug --- gnmi_server/server_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 57f0ce7b..04302c17 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -808,7 +808,7 @@ func TestGnmiSetAuthFail(t *testing.T) { if !ok { t.Fatal("got a non-grpc error from grpc call") } - wantRetCode := codes.Unimplemented + wantRetCode := codes.Unauthenticated if gotRetStatus.Code() != wantRetCode { t.Log("err: ", err) t.Fatalf("got return code %v, want %v", gotRetStatus.Code(), wantRetCode) @@ -818,6 +818,7 @@ func TestGnmiSetAuthFail(t *testing.T) { func TestGnmiGetAuthFail(t *testing.T) { s := createAuthServer(t, 8081) go runServer(t, s) + defer s.s.Stop() tlsConfig := &tls.Config{InsecureSkipVerify: true} opts := []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))} From 52e5d89a902bc2b699c3b2dcb0587bf2c061eecd Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 16 Nov 2022 03:20:41 +0000 Subject: [PATCH 4/6] Update test case. --- gnmi_server/server_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 04302c17..0f174993 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -89,9 +89,10 @@ func loadDBNotStrict(t *testing.T, rclient *redis.Client, mpi map[string]interfa } func createServer(t *testing.T, port int64) *Server { + t.Helper() certificate, err := testcert.NewCert() if err != nil { - t.Errorf("could not load server key pair: %s", err) + t.Fatalf("could not load server key pair: %s", err) } tlsCfg := &tls.Config{ ClientAuth: tls.RequestClientCert, @@ -108,9 +109,10 @@ func createServer(t *testing.T, port int64) *Server { } func createReadServer(t *testing.T, port int64) *Server { + t.Helper() certificate, err := testcert.NewCert() if err != nil { - t.Errorf("could not load server key pair: %s", err) + t.Fatalf("could not load server key pair: %s", err) } tlsCfg := &tls.Config{ ClientAuth: tls.RequestClientCert, @@ -127,9 +129,10 @@ func createReadServer(t *testing.T, port int64) *Server { } func createAuthServer(t *testing.T, port int64) *Server { + t.Helper() certificate, err := testcert.NewCert() if err != nil { - t.Errorf("could not load server key pair: %s", err) + t.Fatalf("could not load server key pair: %s", err) } tlsCfg := &tls.Config{ ClientAuth: tls.RequestClientCert, From ce50a22deb47844fe908247eacf151ef5d800ee0 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 16 Nov 2022 05:15:54 +0000 Subject: [PATCH 5/6] Use enum to replace string. --- common_utils/context.go | 40 ++++++++++++++++++++++++++-------------- common_utils/shareMem.go | 4 ++-- gnmi_dump/gnmi_dump.go | 7 ++++--- gnmi_server/server.go | 24 ++++++++++++------------ 4 files changed, 44 insertions(+), 31 deletions(-) diff --git a/common_utils/context.go b/common_utils/context.go index ad1b7945..d0220054 100644 --- a/common_utils/context.go +++ b/common_utils/context.go @@ -36,14 +36,31 @@ const requestContextKey contextkey = 0 // Request Id generator var requestCounter uint64 -var CountersName = [...]string { - "GNMI get", - "GNMI get fail", - "GNMI set", - "GNMI set fail", +type CounterType int +const ( + GNMI_GET CounterType = iota + GNMI_GET_FAIL + GNMI_SET + GNMI_SET_FAIL + COUNTER_SIZE +) + +func (c CounterType) String() string { + switch c { + case GNMI_GET: + return "GNMI get" + case GNMI_GET_FAIL: + return "GNMI get fail" + case GNMI_SET: + return "GNMI set" + case GNMI_SET_FAIL: + return "GNMI set fail" + default: + return "" + } } -var globalCounters [len(CountersName)]uint64 +var globalCounters [COUNTER_SIZE]uint64 // GetContext function returns the RequestContext object for a @@ -71,19 +88,14 @@ func GetUsername(ctx context.Context, username *string) { } func InitCounters() { - for i := 0; i < len(CountersName); i++ { + for i := 0; i < int(COUNTER_SIZE); i++ { globalCounters[i] = 0 } SetMemCounters(&globalCounters) } -func IncCounter(name string) { - for i := 0; i < len(CountersName); i++ { - if CountersName[i] == name { - atomic.AddUint64(&globalCounters[i], 1) - break - } - } +func IncCounter(cnt CounterType) { + atomic.AddUint64(&globalCounters[cnt], 1) SetMemCounters(&globalCounters) } diff --git a/common_utils/shareMem.go b/common_utils/shareMem.go index 168865e3..ad0caeb0 100644 --- a/common_utils/shareMem.go +++ b/common_utils/shareMem.go @@ -18,7 +18,7 @@ var ( memMode = 0x380 ) -func SetMemCounters(counters *[len(CountersName)]uint64) error { +func SetMemCounters(counters *[int(COUNTER_SIZE)]uint64) error { shmid, _, err := syscall.Syscall(syscall.SYS_SHMGET, uintptr(memKey), uintptr(memSize), uintptr(memMode)) if int(shmid) == -1 { return fmt.Errorf("syscall error, err: %v\n", err) @@ -40,7 +40,7 @@ func SetMemCounters(counters *[len(CountersName)]uint64) error { return nil } -func GetMemCounters(counters *[len(CountersName)]uint64) error { +func GetMemCounters(counters *[int(COUNTER_SIZE)]uint64) error { shmid, _, err := syscall.Syscall(syscall.SYS_SHMGET, uintptr(memKey), uintptr(memSize), uintptr(memMode)) if int(shmid) == -1 { return fmt.Errorf("syscall error, err: %v\n", err) diff --git a/gnmi_dump/gnmi_dump.go b/gnmi_dump/gnmi_dump.go index 04ba47a0..5e02e62f 100644 --- a/gnmi_dump/gnmi_dump.go +++ b/gnmi_dump/gnmi_dump.go @@ -16,14 +16,15 @@ func main() { fmt.Print(help) } flag.Parse() - var counters [len(common_utils.CountersName)]uint64 + var counters [common_utils.COUNTER_SIZE]uint64 err := common_utils.GetMemCounters(&counters) if err != nil { fmt.Printf("Error: Fail to read counters, %v", err) return } fmt.Printf("Dump GNMI counters\n") - for i := 0; i < len(common_utils.CountersName); i++ { - fmt.Printf("%v---%v\n", common_utils.CountersName[i], counters[i]) + for i := 0; i < int(common_utils.COUNTER_SIZE); i++ { + cnt := common_utils.CounterType(i) + fmt.Printf("%v---%v\n", cnt.String(), counters[i]) } } diff --git a/gnmi_server/server.go b/gnmi_server/server.go index 74635560..d97c0070 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -276,32 +276,32 @@ func (s *Server) checkEncodingAndModel(encoding gnmipb.Encoding, models []*gnmip // Get implements the Get RPC in gNMI spec. func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetResponse, error) { - common_utils.IncCounter("GNMI get") + common_utils.IncCounter(common_utils.GNMI_GET) ctx, err := authenticate(s.config.UserAuth, ctx) if err != nil { - common_utils.IncCounter("GNMI get fail") + common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, err } if req.GetType() != gnmipb.GetRequest_ALL { - common_utils.IncCounter("GNMI get fail") + common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, status.Errorf(codes.Unimplemented, "unsupported request type: %s", gnmipb.GetRequest_DataType_name[int32(req.GetType())]) } if err = s.checkEncodingAndModel(req.GetEncoding(), req.GetUseModels()); err != nil { - common_utils.IncCounter("GNMI get fail") + common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, status.Error(codes.Unimplemented, err.Error()) } var target string prefix := req.GetPrefix() if prefix == nil { - common_utils.IncCounter("GNMI get fail") + common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, status.Error(codes.Unimplemented, "No target specified in prefix") } else { target = prefix.GetTarget() if target == "" { - common_utils.IncCounter("GNMI get fail") + common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, status.Error(codes.Unimplemented, "Empty target data not supported yet") } } @@ -323,13 +323,13 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetRe } if err != nil { - common_utils.IncCounter("GNMI get fail") + common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, status.Error(codes.NotFound, err.Error()) } notifications := make([]*gnmipb.Notification, len(paths)) spbValues, err := dc.Get(nil) if err != nil { - common_utils.IncCounter("GNMI get fail") + common_utils.IncCounter(common_utils.GNMI_GET_FAIL) return nil, status.Error(codes.NotFound, err.Error()) } @@ -349,14 +349,14 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetRe } func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetResponse, error) { - common_utils.IncCounter("GNMI set") + common_utils.IncCounter(common_utils.GNMI_SET) if s.config.EnableTranslibWrite == false { - common_utils.IncCounter("GNMI set fail") + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) return nil, grpc.Errorf(codes.Unimplemented, "GNMI is in read-only mode") } ctx, err := authenticate(s.config.UserAuth, ctx) if err != nil { - common_utils.IncCounter("GNMI set fail") + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) return nil, err } var results []*gnmipb.UpdateResult @@ -406,7 +406,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe } err = dc.Set(req.GetDelete(), req.GetReplace(), req.GetUpdate()) if err != nil { - common_utils.IncCounter("GNMI set fail") + common_utils.IncCounter(common_utils.GNMI_SET_FAIL) } return &gnmipb.SetResponse{ From 7ce8590da3cbad2b6fee89467eb0032fc2b268fb Mon Sep 17 00:00:00 2001 From: ganglyu Date: Thu, 17 Nov 2022 01:25:51 +0000 Subject: [PATCH 6/6] Use t.Fatalf --- gnmi_server/server_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go index 0f174993..b3d02dfd 100644 --- a/gnmi_server/server_test.go +++ b/gnmi_server/server_test.go @@ -103,7 +103,7 @@ func createServer(t *testing.T, port int64) *Server { cfg := &Config{Port: port, EnableTranslibWrite: true} s, err := NewServer(cfg, opts) if err != nil { - t.Errorf("Failed to create gNMI server: %v", err) + t.Fatalf("Failed to create gNMI server: %v", err) } return s } @@ -123,7 +123,7 @@ func createReadServer(t *testing.T, port int64) *Server { cfg := &Config{Port: port, EnableTranslibWrite: false} s, err := NewServer(cfg, opts) if err != nil { - t.Errorf("Failed to create gNMI server: %v", err) + t.Fatalf("Failed to create gNMI server: %v", err) } return s } @@ -143,7 +143,7 @@ func createAuthServer(t *testing.T, port int64) *Server { cfg := &Config{Port: port, EnableTranslibWrite: true, UserAuth: AuthTypes{"password": true, "cert": true, "jwt": true}} s, err := NewServer(cfg, opts) if err != nil { - t.Errorf("Failed to create gNMI server: %v", err) + t.Fatalf("Failed to create gNMI server: %v", err) } return s }