diff --git a/api/cmd/managerapi.go b/api/cmd/managerapi.go index 4f56b5fa7e..a3543b1161 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 { @@ -111,6 +111,15 @@ func manageAPI() error { return nil }) + // Signal received to the process externally. + quit := make(chan os.Signal, 1) + signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) + + defer func() { + utils.CloseAll() + signal.Stop(quit) + }() + droplet.Option.Orchestrator = func(mws []droplet.Middleware) []droplet.Middleware { var newMws []droplet.Middleware // default middleware order: resp_reshape, auto_input, traffic_log @@ -122,17 +131,21 @@ func manageAPI() error { if err := storage.InitETCDClient(conf.ETCDConfig); err != nil { log.Errorf("init etcd client fail: %w", err) - panic(err) + return err } if err := store.InitStores(); err != nil { log.Errorf("init stores fail: %w", err) - panic(err) + return err } + var server, serverSSL *http.Server + // For internal error handling across multiple goroutines. + errsig := make(chan error, 1) + // routes r := internal.SetUpRouter() - addr := fmt.Sprintf("%s:%d", conf.ServerHost, conf.ServerPort) - s := &http.Server{ + addr := net.JoinHostPort(conf.ServerHost, strconv.Itoa(conf.ServerPort)) + server = &http.Server{ Addr: addr, Handler: r, ReadTimeout: time.Duration(1000) * time.Millisecond, @@ -141,21 +154,17 @@ 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() - log.Fatalf("listen and serv fail: %s", err) + 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{ + serverSSL = &http.Server{ Addr: addrSSL, Handler: r, ReadTimeout: time.Duration(1000) * time.Millisecond, @@ -169,28 +178,37 @@ func manageAPI() error { 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) + 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 } +} - log.Infof("The Manager API server exited") +func shutdownServer(server *http.Server) { + if server != nil { + ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Second) + defer cancel() - utils.CloseAll() - 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/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/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 { diff --git a/api/test/shell/cli_test.sh b/api/test/shell/cli_test.sh index 876cd0e315..0a1da753e8 100755 --- a/api/test/shell/cli_test.sh +++ b/api/test/shell/cli_test.sh @@ -487,6 +487,59 @@ fi 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 +# 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 "Error: 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 +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