Fix shutdown problems in ping application. (#2141)

* Fix shutdown problems in ping application.

* Make sure HTTP server gracefully shuts down.
    * Since ListenAndServe() always returns a non-nil error, the application
      exited by logger.Fatal() before graceful shutdown process is completed.
* Remove unnecessary error log.
    * In shutdown process of udpServer, an error log was always emitted, but
      it's not an error in intentional shutdown triggered by signal.

* Fix inconsistency of log messages.

Co-authored-by: Mark Mandel <markmandel@google.com>
Co-authored-by: Alexander Apalikov <alexander.apalikov@globant.com>
Co-authored-by: Robert Bailey <robertbailey@google.com>

cmd/ping/main.go

@@ -43,11 +43,11 @@ var (
 func main() {
 	ctlConf := parseEnvFlags()
 	if err := ctlConf.validate(); err != nil {
-		logger.WithError(err).Fatal("could not create controller from environment or flags")
+		logger.WithError(err).Fatal("Could not create controller from environment or flags")
 	}
 
 	logger.WithField("version", pkg.Version).WithField("featureGates", runtime.EncodeFeatures()).
-		WithField("ctlConf", ctlConf).Info("starting ping...")
+		WithField("ctlConf", ctlConf).Info("Starting ping...")
 
 	ctx := signals.NewSigKillContext()
 
@@ -61,7 +61,7 @@ func main() {
 	defer cancel()
 
 	<-ctx.Done()
-	logger.Info("shutting down...")
+	logger.Info("Shutting down...")
 }
 
 func serveUDP(ctx context.Context, ctlConf config) *udpServer {
@@ -85,22 +85,25 @@ func serveHTTP(ctlConf config, h healthcheck.Handler) func() {
 	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
 		if _, err := w.Write([]byte(ctlConf.HTTPResponse)); err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
-			logger.WithError(err).Error("error responding to http request")
+			logger.WithError(err).Error("Error responding to http request")
 		}
 	})
 
 	go func() {
-		logger.Info("starting HTTP Server...")
-		logger.WithError(srv.ListenAndServe()).Fatal("could not start HTTP server")
+		logger.Info("Starting HTTP Server...")
+		if err := srv.ListenAndServe(); err != http.ErrServerClosed {
+			logger.WithError(err).Fatal("Could not start HTTP server")
+		}
 	}()
 
 	return func() {
 		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 		defer cancel()
 
 		if err := srv.Shutdown(ctx); err != nil {
-			logger.WithError(err).Fatal("could not shut down HTTP server")
+			logger.WithError(err).Fatal("Could not shut down HTTP server")
 		}
+		logger.Info("HTTP server was gracefully shut down")
 	}
 }
 

cmd/ping/udp.go

@@ -19,6 +19,7 @@ import (
 	"context"
 	"math"
 	"net"
+	"os"
 	"sync"
 	"time"
 
@@ -71,11 +72,11 @@ func newUDPServer(rateLimit rate.Limit) *udpServer {
 func (u *udpServer) run(ctx context.Context) {
 	u.healthy()
 
-	logger.Info("starting UDP server")
+	logger.Info("Starting UDP server")
 	var err error
 	u.conn, err = net.ListenPacket("udp", ":8080")
 	if err != nil {
-		logger.WithError(err).Fatal("could not start udp server")
+		logger.WithError(err).Fatal("Could not start udp server")
 	}
 
 	go func() {
@@ -111,7 +112,10 @@ func (u *udpServer) readWriteLoop(ctx context.Context) {
 				b := make([]byte, 1024)
 				_, sender, err := u.conn.ReadFrom(b)
 				if err != nil {
-					u.logger.WithError(err).Error("error reading udp packet")
+					if ctx.Err() != nil && err == os.ErrClosed {
+						return
+					}
+					u.logger.WithError(err).Error("Error reading udp packet")
 					continue
 				}
 				go u.rateLimitedEchoResponse(b, sender)
@@ -137,17 +141,17 @@ func (u *udpServer) rateLimitedEchoResponse(b []byte, sender net.Addr) {
 	if vis.limit.Allow() {
 		b = bytes.TrimRight(b, "\x00")
 		if _, err := u.conn.WriteTo(b, sender); err != nil {
-			u.logger.WithError(err).Error("error sending returning udp packet")
+			u.logger.WithError(err).Error("Error sending returning udp packet")
 		}
 	} else {
-		logger.WithField("addr", sender.String()).Warn("rate limited. No response sent.")
+		logger.WithField("addr", sender.String()).Warn("Rate limited. No response sent")
 	}
 }
 
 // close closes and shutdown the udp server
 func (u *udpServer) close() {
 	if err := u.conn.Close(); err != nil {
-		logger.WithError(err).Error("error closing udp connection")
+		logger.WithError(err).Error("Error closing udp connection")
 	}
 }