From e354950b94325ade3d8e1b58d8789ae2107f048b Mon Sep 17 00:00:00 2001 From: Leonard Goodell Date: Wed, 13 Jul 2022 17:38:59 -0700 Subject: [PATCH 1/4] fix: Ensure exit with non-zero code when error occurs fixes #318 Signed-off-by: Leonard Goodell --- bootstrap/bootstrap.go | 7 ++++++- bootstrap/handlers/httpserver.go | 18 ++++++++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index 5d76d705..4f073e18 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -181,7 +181,7 @@ func Run( useSecretProvider bool, handlers []interfaces.BootstrapHandler) { - wg, deferred, _ := RunAndReturnWaitGroup( + wg, deferred, success := RunAndReturnWaitGroup( ctx, cancel, commonFlags, @@ -195,6 +195,11 @@ func Run( handlers, ) + if !success { + cancel() + os.Exit(1) + } + defer deferred() // wait for go routines to stop executing. diff --git a/bootstrap/handlers/httpserver.go b/bootstrap/handlers/httpserver.go index 79f10850..50d52318 100644 --- a/bootstrap/handlers/httpserver.go +++ b/bootstrap/handlers/httpserver.go @@ -19,14 +19,16 @@ import ( "context" "encoding/json" "fmt" - "github.com/edgexfoundry/go-mod-core-contracts/v2/clients/logger" - "github.com/edgexfoundry/go-mod-core-contracts/v2/common" - commonDTO "github.com/edgexfoundry/go-mod-core-contracts/v2/dtos/common" "net/http" + "os" "strconv" "sync" "time" + "github.com/edgexfoundry/go-mod-core-contracts/v2/clients/logger" + "github.com/edgexfoundry/go-mod-core-contracts/v2/common" + commonDTO "github.com/edgexfoundry/go-mod-core-contracts/v2/dtos/common" + "github.com/edgexfoundry/go-mod-bootstrap/v2/bootstrap/container" "github.com/edgexfoundry/go-mod-bootstrap/v2/bootstrap/startup" "github.com/edgexfoundry/go-mod-bootstrap/v2/di" @@ -123,7 +125,6 @@ func (b *HttpServer) BootstrapHandler( defer wg.Done() <-ctx.Done() - lc.Info("Web server shutting down") _ = server.Shutdown(context.Background()) lc.Info("Web server shut down") }() @@ -139,10 +140,15 @@ func (b *HttpServer) BootstrapHandler( b.isRunning = true err := server.ListenAndServe() - if err != nil { + // "Server closed" error occurs when Shutdown above is called in the Done processing, so it can be ignored + if err != nil && err.Error() != "http: Server closed" { + // Other errors occur during bootstrapping, like port bind fails, are considered fatal lc.Errorf("Web server failed: %v", err) cancel := container.CancelFuncFrom(dic.Get) - cancel() // this will caused the service to stop + cancel() // this will clean up any long-running go functions that may have started + // Give time for clean up to occur before exiting. + time.Sleep(1 * time.Millisecond) + os.Exit(1) } else { lc.Info("Web server stopped") } From a98c0cbca18a32e55ed9faf6355b1bb5b7823ef3 Mon Sep 17 00:00:00 2001 From: Leonard Goodell Date: Wed, 20 Jul 2022 15:50:46 -0700 Subject: [PATCH 2/4] fix: Addressed PR feedeback on comparing error message Signed-off-by: Leonard Goodell --- bootstrap/handlers/httpserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/handlers/httpserver.go b/bootstrap/handlers/httpserver.go index 50d52318..3d2d7cae 100644 --- a/bootstrap/handlers/httpserver.go +++ b/bootstrap/handlers/httpserver.go @@ -141,7 +141,7 @@ func (b *HttpServer) BootstrapHandler( b.isRunning = true err := server.ListenAndServe() // "Server closed" error occurs when Shutdown above is called in the Done processing, so it can be ignored - if err != nil && err.Error() != "http: Server closed" { + if err != nil && err != http.ErrServerClosed { // Other errors occur during bootstrapping, like port bind fails, are considered fatal lc.Errorf("Web server failed: %v", err) cancel := container.CancelFuncFrom(dic.Get) From c4a0cb6151a3be339f7b26e2677149f8a29316b6 Mon Sep 17 00:00:00 2001 From: Leonard Goodell Date: Thu, 21 Jul 2022 10:55:30 -0700 Subject: [PATCH 3/4] refact: Reworked how wait for other go funcs to stop Also add comment why not logging error Signed-off-by: Leonard Goodell --- bootstrap/bootstrap.go | 2 ++ bootstrap/handlers/httpserver.go | 10 +++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index 4f073e18..8cdfadcd 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -196,6 +196,8 @@ func Run( ) if !success { + // This only occurs when a bootstrap handler has fail. + // The handler will have logged an error, so not need to log a message here. cancel() os.Exit(1) } diff --git a/bootstrap/handlers/httpserver.go b/bootstrap/handlers/httpserver.go index 3d2d7cae..7f80c8dd 100644 --- a/bootstrap/handlers/httpserver.go +++ b/bootstrap/handlers/httpserver.go @@ -144,10 +144,14 @@ func (b *HttpServer) BootstrapHandler( if err != nil && err != http.ErrServerClosed { // Other errors occur during bootstrapping, like port bind fails, are considered fatal lc.Errorf("Web server failed: %v", err) + + // Allow any long-running go functions that may have started to stop before exiting cancel := container.CancelFuncFrom(dic.Get) - cancel() // this will clean up any long-running go functions that may have started - // Give time for clean up to occur before exiting. - time.Sleep(1 * time.Millisecond) + cancel() + + // Wait for all long-running go functions to stop before exiting. + wg.Done() + wg.Wait() os.Exit(1) } else { lc.Info("Web server stopped") From 5d7a6977151e8ef56ec331c26bb9cffc4c5db620 Mon Sep 17 00:00:00 2001 From: Leonard Goodell Date: Thu, 21 Jul 2022 11:12:29 -0700 Subject: [PATCH 4/4] fix: Added requested comment Signed-off-by: Leonard Goodell --- bootstrap/handlers/httpserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/handlers/httpserver.go b/bootstrap/handlers/httpserver.go index 7f80c8dd..dd9fdfe1 100644 --- a/bootstrap/handlers/httpserver.go +++ b/bootstrap/handlers/httpserver.go @@ -150,7 +150,7 @@ func (b *HttpServer) BootstrapHandler( cancel() // Wait for all long-running go functions to stop before exiting. - wg.Done() + wg.Done() // Must do this to account for this go func's wg.Add above otherwise wait will block indefinitely wg.Wait() os.Exit(1) } else {