From 35555b13ff4475cb470f61bad316c05128f1b4d7 Mon Sep 17 00:00:00 2001 From: Bisakh Mondal Date: Wed, 21 Apr 2021 16:43:15 +0530 Subject: [PATCH 1/4] single commit fix --- api/internal/core/store/store.go | 6 ++++-- api/internal/core/store/store_test.go | 3 ++- api/test/shell/cli_test.sh | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/api/internal/core/store/store.go b/api/internal/core/store/store.go index d529dd5eae..11f5082097 100644 --- a/api/internal/core/store/store.go +++ b/api/internal/core/store/store.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "fmt" + "os" "reflect" "sort" "sync" @@ -105,6 +106,7 @@ func (s *GenericStore) Init() error { key := ret[i].Key[len(s.opt.BasePath)+1:] objPtr, err := s.StringToObjPtr(ret[i].Value, key) if err != nil { + fmt.Fprintln(os.Stderr, "Error occurred while initializing logical store: ", s.opt.BasePath) return err } @@ -353,8 +355,8 @@ func (s *GenericStore) StringToObjPtr(str, key string) (interface{}, error) { ret := objPtr.Interface() err := json.Unmarshal([]byte(str), ret) if err != nil { - log.Errorf("json marshal failed: %s", err) - return nil, fmt.Errorf("json unmarshal failed: %s", err) + log.Errorf("json unmarshal failed: %s", err) + return nil, fmt.Errorf("json unmarshal failed\n\tRelated Key:\t\t%s\n\tError Description:\t%s", key, err) } if setter, ok := ret.(entity.BaseInfoSetter); ok { diff --git a/api/internal/core/store/store_test.go b/api/internal/core/store/store_test.go index c5574840f0..1cb8f8dd1c 100644 --- a/api/internal/core/store/store_test.go +++ b/api/internal/core/store/store_test.go @@ -205,7 +205,8 @@ func TestGenericStore_Init(t *testing.T) { Value: `{"Field1":"demo2-f1", "Field2":"demo2-f2"}`, }, }, - wantErr: fmt.Errorf("json unmarshal failed: invalid character ',' after object key"), + wantErr: fmt.Errorf("json unmarshal failed\n\tRelated Key:\t\tdemo1-f1\n\tError Description:\t" + + "invalid character ',' after object key"), wantListCalled: true, }, } diff --git a/api/test/shell/cli_test.sh b/api/test/shell/cli_test.sh index 876cd0e315..7b448ac157 100755 --- a/api/test/shell/cli_test.sh +++ b/api/test/shell/cli_test.sh @@ -520,6 +520,24 @@ if [[ `echo $(sudo ./manager-api remove) | grep -c "OK"` -ne "1" ]]; then echo "error while removing the service" exit 1 fi +# test manager-api output for bad data on etcd +# make a dummy entry +./etcd-v3.4.14-linux-amd64/etcdctl put /apisix/routes/unique1 "{\"id\":}" +sleep 2 + +./manager-api 2>man-api.err & +sleep 4 + +if [[ `cat man-api.err | grep -c "Error occurred while initializing logical store: /apisix/routes"` -ne '1' || +`cat man-api.err | grep -c "json unmarshal failed"` -ne '1' ]];then + echo "manager api failed to stream error on stderr for bad data" + exit 1 +fi +# delete dummy entry +./etcd-v3.4.14-linux-amd64/etcdctl del /apisix/routes/unique1 +# just to make sure +./manager-api stop +sleep 6 pkill -f etcd From f83925f4aec6decd6c0b1b925200127f2579dc89 Mon Sep 17 00:00:00 2001 From: Bisakh Mondal Date: Fri, 30 Apr 2021 15:31:11 +0530 Subject: [PATCH 2/4] Merge branch 'master' into fix-store-init-error --- api/cmd/managerapi.go | 21 ++++++++++++--------- api/test/shell/cli_test.sh | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/api/cmd/managerapi.go b/api/cmd/managerapi.go index 4f56b5fa7e..7c5bea489f 100644 --- a/api/cmd/managerapi.go +++ b/api/cmd/managerapi.go @@ -110,6 +110,14 @@ func manageAPI() error { } return nil }) + quit := make(chan os.Signal, 1) + signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) + + defer func() { + utils.CloseAll() + log.Infof("The Manager API server exited") + signal.Stop(quit) + }() droplet.Option.Orchestrator = func(mws []droplet.Middleware) []droplet.Middleware { var newMws []droplet.Middleware @@ -126,12 +134,14 @@ func manageAPI() error { } if err := store.InitStores(); err != nil { log.Errorf("init stores fail: %w", err) - panic(err) + fmt.Fprintf(os.Stderr, "%s\n", err) + utils.CloseAll() + os.Exit(1) } // routes r := internal.SetUpRouter() - addr := fmt.Sprintf("%s:%d", conf.ServerHost, conf.ServerPort) + addr := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.ServerPort)) s := &http.Server{ Addr: addr, Handler: r, @@ -141,10 +151,6 @@ func manageAPI() error { log.Infof("The Manager API is listening on %s", addr) - quit := make(chan os.Signal, 1) - signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) - defer signal.Stop(quit) - go func() { if err := s.ListenAndServe(); err != nil && err != http.ErrServerClosed { utils.CloseAll() @@ -187,9 +193,6 @@ func manageAPI() error { log.Errorf("Shutting down server error: %s", err) } - log.Infof("The Manager API server exited") - - utils.CloseAll() return nil } diff --git a/api/test/shell/cli_test.sh b/api/test/shell/cli_test.sh index 7b448ac157..2a6894ebba 100755 --- a/api/test/shell/cli_test.sh +++ b/api/test/shell/cli_test.sh @@ -538,6 +538,41 @@ fi # just to make sure ./manager-api stop sleep 6 +clean_up + +# run manager api as os service +# 2 times OK for installing and starting +if [[ `echo $(sudo ./manager-api start) | grep -o "OK" | wc -l` -ne "2" ]]; then + echo "error while initializing the service" + exit 1 +fi +# check running status +if [[ `echo $(sudo ./manager-api status) | grep -c "running..."` -ne "1" ]]; then + echo "error while starting the service" + exit 1 +fi +# stop the service +sudo ./manager-api stop +sleep 2 +# recheck running status +if [[ `echo $(sudo ./manager-api status) | grep -c "Service is stopped"` -ne "1" ]]; then + echo "error while stopping the service" + exit 1 +fi +# restart the service +# 1 time OK for just for starting +if [[ `echo $(sudo ./manager-api start) | grep -c "OK"` -ne "1" ]]; then + echo "error while restarting the service" + exit 1 +fi +# stop the service +sudo ./manager-api stop +sleep 2 +# remove the service +if [[ `echo $(sudo ./manager-api remove) | grep -c "OK"` -ne "1" ]]; then + echo "error while removing the service" + exit 1 +fi pkill -f etcd From c66c9144f89e2353465089ea8e36b3e145378f18 Mon Sep 17 00:00:00 2001 From: Bisakh Mondal Date: Tue, 4 May 2021 19:30:00 +0530 Subject: [PATCH 3/4] =?UTF-8?q?everything=20is=20gracefully=20handled?= =?UTF-8?q?=F0=9F=A5=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/cmd/managerapi.go | 112 ++++++++++++++++++++++---------------- api/internal/utils/pid.go | 3 + 2 files changed, 68 insertions(+), 47 deletions(-) diff --git a/api/cmd/managerapi.go b/api/cmd/managerapi.go index 7c5bea489f..fa5837f378 100644 --- a/api/cmd/managerapi.go +++ b/api/cmd/managerapi.go @@ -101,7 +101,7 @@ func manageAPI() error { if err := utils.WritePID(conf.PIDPath); err != nil { log.Errorf("failed to write pid: %s", err) - panic(err) + return err } utils.AppendToClosers(func() error { if err := os.Remove(conf.PIDPath); err != nil { @@ -110,12 +110,16 @@ func manageAPI() error { } return nil }) + + // Signal received to the process externally. quit := make(chan os.Signal, 1) signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) + // For internal error handling across multiple goroutines. + errsig := make(chan error, 1) defer func() { utils.CloseAll() - log.Infof("The Manager API server exited") + fmt.Println("closer") signal.Stop(quit) }() @@ -130,70 +134,84 @@ func manageAPI() error { if err := storage.InitETCDClient(conf.ETCDConfig); err != nil { log.Errorf("init etcd client fail: %w", err) - panic(err) + errsig <- err } if err := store.InitStores(); err != nil { log.Errorf("init stores fail: %w", err) - fmt.Fprintf(os.Stderr, "%s\n", err) - utils.CloseAll() - os.Exit(1) + errsig <- err } - // routes - r := internal.SetUpRouter() - addr := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.ServerPort)) - s := &http.Server{ - Addr: addr, - Handler: r, - ReadTimeout: time.Duration(1000) * time.Millisecond, - WriteTimeout: time.Duration(5000) * time.Millisecond, - } - - log.Infof("The Manager API is listening on %s", addr) - - go func() { - if err := s.ListenAndServe(); err != nil && err != http.ErrServerClosed { - utils.CloseAll() - log.Fatalf("listen and serv fail: %s", err) - } - }() - - // HTTPS - if conf.SSLCert != "" && conf.SSLKey != "" { - addrSSL := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.SSLPort)) - serverSSL := &http.Server{ - Addr: addrSSL, + var server, serverSSL *http.Server + if len(errsig) == 0 { // no error has occurred till now + // routes + r := internal.SetUpRouter() + addr := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.ServerPort)) + server = &http.Server{ + Addr: addr, Handler: r, ReadTimeout: time.Duration(1000) * time.Millisecond, WriteTimeout: time.Duration(5000) * time.Millisecond, - TLSConfig: &tls.Config{ - // Causes servers to use Go's default ciphersuite preferences, - // which are tuned to avoid attacks. Does nothing on clients. - PreferServerCipherSuites: true, - }, } + + log.Infof("The Manager API is listening on %s", addr) + go func() { - err := serverSSL.ListenAndServeTLS(conf.SSLCert, conf.SSLKey) - if err != nil && err != http.ErrServerClosed { - utils.CloseAll() - log.Fatalf("listen and serve for HTTPS failed: %s", err) + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Errorf("listen and serv fail: %s", err) + fmt.Println(err) + errsig <- err } }() - } + // HTTPS + if conf.SSLCert != "" && conf.SSLKey != "" { + addrSSL := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.SSLPort)) + serverSSL = &http.Server{ + Addr: addrSSL, + Handler: r, + ReadTimeout: time.Duration(1000) * time.Millisecond, + WriteTimeout: time.Duration(5000) * time.Millisecond, + TLSConfig: &tls.Config{ + // Causes servers to use Go's default ciphersuite preferences, + // which are tuned to avoid attacks. Does nothing on clients. + PreferServerCipherSuites: true, + }, + } + go func() { + err := serverSSL.ListenAndServeTLS(conf.SSLCert, conf.SSLKey) + if err != nil && err != http.ErrServerClosed { + //utils.CloseAll() + log.Errorf("listen and serve for HTTPS failed: %s", err) + errsig <- err + } + }() + } + } printInfo() - sig := <-quit - log.Infof("The Manager API server receive %s and start shutting down", sig.String()) + select { + case err := <-errsig: + return err - ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Second) - defer cancel() + case sig := <-quit: + log.Infof("The Manager API server receive %s and start shutting down", sig.String()) - if err := s.Shutdown(ctx); err != nil { - log.Errorf("Shutting down server error: %s", err) + shutdownServer(server) + shutdownServer(serverSSL) + log.Infof("The Manager API server exited") + return nil } +} + +func shutdownServer(server *http.Server) { + if server != nil { + ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Second) + defer cancel() - return nil + if err := server.Shutdown(ctx); err != nil { + log.Errorf("Shutting down server error: %s", err) + } + } } func newStartCommand() *cobra.Command { diff --git a/api/internal/utils/pid.go b/api/internal/utils/pid.go index 066a4c3e9e..4080a53ef4 100644 --- a/api/internal/utils/pid.go +++ b/api/internal/utils/pid.go @@ -26,6 +26,9 @@ import ( // WritePID write pid to the given file path. func WritePID(filepath string) error { + if _, err := os.Stat(filepath); err == nil { + return fmt.Errorf("instance of Manager API already running: a pid file exists in %s", filepath) + } pid := os.Getpid() f, err := os.OpenFile(filepath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC|os.O_CREATE, 0600) if err != nil { From 84e18e6da94fbd9aff6ddfa0ff6f46a52f6c0ef2 Mon Sep 17 00:00:00 2001 From: Bisakh Mondal Date: Tue, 4 May 2021 19:40:27 +0530 Subject: [PATCH 4/4] debugging lines removed --- api/cmd/managerapi.go | 79 ++++++++++++++++++-------------------- api/test/shell/cli_test.sh | 2 +- 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/api/cmd/managerapi.go b/api/cmd/managerapi.go index fa5837f378..a3543b1161 100644 --- a/api/cmd/managerapi.go +++ b/api/cmd/managerapi.go @@ -114,12 +114,9 @@ func manageAPI() error { // Signal received to the process externally. quit := make(chan os.Signal, 1) signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) - // For internal error handling across multiple goroutines. - errsig := make(chan error, 1) defer func() { utils.CloseAll() - fmt.Println("closer") signal.Stop(quit) }() @@ -134,59 +131,59 @@ func manageAPI() error { if err := storage.InitETCDClient(conf.ETCDConfig); err != nil { log.Errorf("init etcd client fail: %w", err) - errsig <- err + return err } if err := store.InitStores(); err != nil { log.Errorf("init stores fail: %w", err) - errsig <- err + return err } var server, serverSSL *http.Server - if len(errsig) == 0 { // no error has occurred till now - // routes - r := internal.SetUpRouter() - addr := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.ServerPort)) - server = &http.Server{ - Addr: addr, + // For internal error handling across multiple goroutines. + errsig := make(chan error, 1) + + // routes + r := internal.SetUpRouter() + addr := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.ServerPort)) + server = &http.Server{ + Addr: addr, + Handler: r, + ReadTimeout: time.Duration(1000) * time.Millisecond, + WriteTimeout: time.Duration(5000) * time.Millisecond, + } + + log.Infof("The Manager API is listening on %s", addr) + + go func() { + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Errorf("listen and serv fail: %s", err) + errsig <- err + } + }() + + // HTTPS + if conf.SSLCert != "" && conf.SSLKey != "" { + addrSSL := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.SSLPort)) + serverSSL = &http.Server{ + Addr: addrSSL, Handler: r, ReadTimeout: time.Duration(1000) * time.Millisecond, WriteTimeout: time.Duration(5000) * time.Millisecond, + TLSConfig: &tls.Config{ + // Causes servers to use Go's default ciphersuite preferences, + // which are tuned to avoid attacks. Does nothing on clients. + PreferServerCipherSuites: true, + }, } - - log.Infof("The Manager API is listening on %s", addr) - go func() { - if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { - log.Errorf("listen and serv fail: %s", err) - fmt.Println(err) + err := serverSSL.ListenAndServeTLS(conf.SSLCert, conf.SSLKey) + if err != nil && err != http.ErrServerClosed { + log.Errorf("listen and serve for HTTPS failed: %s", err) errsig <- err } }() - - // HTTPS - if conf.SSLCert != "" && conf.SSLKey != "" { - addrSSL := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.SSLPort)) - serverSSL = &http.Server{ - Addr: addrSSL, - Handler: r, - ReadTimeout: time.Duration(1000) * time.Millisecond, - WriteTimeout: time.Duration(5000) * time.Millisecond, - TLSConfig: &tls.Config{ - // Causes servers to use Go's default ciphersuite preferences, - // which are tuned to avoid attacks. Does nothing on clients. - PreferServerCipherSuites: true, - }, - } - go func() { - err := serverSSL.ListenAndServeTLS(conf.SSLCert, conf.SSLKey) - if err != nil && err != http.ErrServerClosed { - //utils.CloseAll() - log.Errorf("listen and serve for HTTPS failed: %s", err) - errsig <- err - } - }() - } } + printInfo() select { diff --git a/api/test/shell/cli_test.sh b/api/test/shell/cli_test.sh index 2a6894ebba..0a1da753e8 100755 --- a/api/test/shell/cli_test.sh +++ b/api/test/shell/cli_test.sh @@ -529,7 +529,7 @@ sleep 2 sleep 4 if [[ `cat man-api.err | grep -c "Error occurred while initializing logical store: /apisix/routes"` -ne '1' || -`cat man-api.err | grep -c "json unmarshal failed"` -ne '1' ]];then +`cat man-api.err | grep -c "Error: json unmarshal failed"` -ne '1' ]];then echo "manager api failed to stream error on stderr for bad data" exit 1 fi