-
Notifications
You must be signed in to change notification settings - Fork 188
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #262 from buzzfeed/graceful-shutdown
cmd: ensure http servers shut down gracefully
- Loading branch information
Showing
8 changed files
with
263 additions
and
21 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package httpserver | ||
|
|
||
| import ( | ||
| "context" | ||
| "net" | ||
| "net/http" | ||
| "os" | ||
| "os/signal" | ||
| "syscall" | ||
| "time" | ||
|
|
||
| "github.com/buzzfeed/sso/internal/pkg/logging" | ||
| ) | ||
|
|
||
| // OS signals that will initiate graceful shutdown of the http server. | ||
| // | ||
| // NOTE: defined in a variable so that they may be overridden by tests. | ||
| var shutdownSignals = []os.Signal{ | ||
| syscall.SIGINT, | ||
| syscall.SIGTERM, | ||
| } | ||
|
|
||
| // Run runs an http server and ensures that it is shut down gracefully within | ||
| // the given shutdown timeout, allowing all in-flight requests to complete. | ||
| // | ||
| // Returns an error if a) the server fails to listen on its port or b) the | ||
| // shutdown timeout elapses before all in-flight requests are finished. | ||
| func Run(srv *http.Server, shutdownTimeout time.Duration, logger *logging.LogEntry) error { | ||
| // Logic below copied from the stdlib http.Server ListenAndServe() method: | ||
| // https://github.com/golang/go/blob/release-branch.go1.13/src/net/http/server.go#L2805-L2826 | ||
| addr := srv.Addr | ||
| if addr == "" { | ||
| addr = ":http" | ||
| } | ||
| ln, err := net.Listen("tcp", addr) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return runWithListener(ln, srv, shutdownTimeout, logger) | ||
| } | ||
|
|
||
| // runWithListener does the heavy lifting for Run() above, and is decoupled | ||
| // only for testing purposes | ||
| func runWithListener(ln net.Listener, srv *http.Server, shutdownTimeout time.Duration, logger *logging.LogEntry) error { | ||
| var ( | ||
| // shutdownCh triggers graceful shutdown on SIGINT or SIGTERM | ||
| shutdownCh = make(chan os.Signal, 1) | ||
|
|
||
| // exitCh will be closed when it is safe to exit, after graceful shutdown | ||
| exitCh = make(chan struct{}) | ||
|
|
||
| // shutdownErr allows any error from srv.Shutdown to propagate out up | ||
| // from the goroutine | ||
| shutdownErr error | ||
| ) | ||
|
|
||
| signal.Notify(shutdownCh, shutdownSignals...) | ||
|
|
||
| go func() { | ||
| sig := <-shutdownCh | ||
| logger.Info("shutdown started by signal: ", sig) | ||
| signal.Stop(shutdownCh) | ||
|
|
||
| logger.Info("waiting for server to shut down in ", shutdownTimeout) | ||
| ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) | ||
| defer cancel() | ||
|
|
||
| shutdownErr = srv.Shutdown(ctx) | ||
| close(exitCh) | ||
| }() | ||
|
|
||
| if serveErr := srv.Serve(ln); serveErr != nil && serveErr != http.ErrServerClosed { | ||
| return serveErr | ||
| } | ||
|
|
||
| <-exitCh | ||
| logger.Info("shutdown finished") | ||
|
|
||
| return shutdownErr | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| package httpserver | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net" | ||
| "net/http" | ||
| "os" | ||
| "sync" | ||
| "syscall" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/buzzfeed/sso/internal/pkg/logging" | ||
| ) | ||
|
|
||
| func newLocalListener(t *testing.T) net.Listener { | ||
| t.Helper() | ||
|
|
||
| l, err := net.Listen("tcp", "127.0.0.1:0") | ||
| if err != nil { | ||
| t.Fatalf("failed to listen on a port: %v", err) | ||
| } | ||
| return l | ||
| } | ||
|
|
||
| func TestGracefulShutdown(t *testing.T) { | ||
| proc, err := os.FindProcess(os.Getpid()) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| // override shutdown signals used by Run for testing purposes | ||
| shutdownSignals = []os.Signal{syscall.SIGUSR1} | ||
|
|
||
| logger := logging.NewLogEntry() | ||
|
|
||
| testCases := map[string]struct { | ||
| shutdownTimeout time.Duration | ||
| requestDelay time.Duration | ||
| expectShutdownErr bool | ||
| expectRequestErr bool | ||
| }{ | ||
| "clean shutdown": { | ||
| shutdownTimeout: 1 * time.Second, | ||
| requestDelay: 250 * time.Millisecond, | ||
| expectShutdownErr: false, | ||
| expectRequestErr: false, | ||
| }, | ||
| "timeout elapsed": { | ||
| shutdownTimeout: 50 * time.Millisecond, | ||
| requestDelay: 250 * time.Millisecond, | ||
| expectShutdownErr: true, | ||
|
|
||
| // In real usage, we would expect the request to be aborted when | ||
| // the server is shut down and its process exits before it can | ||
| // finish responding. | ||
| // | ||
| // But because we're running the server within the test process, | ||
| // which does not exit after shutdown, the goroutine handling the | ||
| // long-running request does not seem to get canceled and the | ||
| // request ends up completing successfully even after the server | ||
| // has shut down. | ||
| // | ||
| // Properly testing this would require something like re-running | ||
| // the test binary as a subprocess to which we can send SIGTERM, | ||
| // but doing that would add a lot more complexity (e.g. having it | ||
| // bind to a random available port and then running a separate | ||
| // subprocess to figure out the port to which it is bound, all in a | ||
| // cross-platform way). | ||
| // | ||
| // If we wanted to go that route, some examples of the general | ||
| // approach can be seen here: | ||
| // | ||
| // - http://cs-guy.com/blog/2015/01/test-main/#toc_3 | ||
| // - https://talks.golang.org/2014/testing.slide#23 | ||
| expectRequestErr: false, | ||
| }, | ||
| } | ||
|
|
||
| for name, tc := range testCases { | ||
| t.Run(name, func(t *testing.T) { | ||
| var ( | ||
| ln = newLocalListener(t) | ||
| addr = ln.Addr().String() | ||
| url = fmt.Sprintf("http://%s", addr) | ||
| ) | ||
|
|
||
| srv := &http.Server{ | ||
| Addr: addr, | ||
| Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| time.Sleep(tc.requestDelay) | ||
| }), | ||
| } | ||
|
|
||
| var ( | ||
| wg sync.WaitGroup | ||
| shutdownErr error | ||
| requestErr error | ||
| ) | ||
|
|
||
| // Run our server and wait for a stop signal | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| shutdownErr = runWithListener(ln, srv, tc.shutdownTimeout, logger) | ||
| }() | ||
|
|
||
| // give the server time to start listening | ||
| <-time.After(50 * time.Millisecond) | ||
|
|
||
| // make a request | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| _, requestErr = http.Get(url) | ||
| }() | ||
|
|
||
| // give the request some time to connect | ||
| <-time.After(1 * time.Millisecond) | ||
|
|
||
| // tell server to shut down gracefully | ||
| proc.Signal(syscall.SIGUSR1) | ||
|
|
||
| // wait for server to shut down and requests to complete | ||
| wg.Wait() | ||
|
|
||
| if tc.expectShutdownErr { | ||
| if shutdownErr == nil { | ||
| t.Fatalf("did not get expected shutdown error") | ||
| } | ||
| } else { | ||
| if shutdownErr != nil { | ||
| t.Fatalf("got unexpected shutdown error: %s", shutdownErr) | ||
| } | ||
| } | ||
|
|
||
| if tc.expectRequestErr && requestErr == nil { | ||
| if requestErr == nil { | ||
| t.Fatalf("did not get expected request error") | ||
| } | ||
| } else { | ||
| if requestErr != nil { | ||
| t.Fatalf("got unexpected request error: %s", requestErr) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters