fix: be more idiomatic & address comments

 * Use a Context to manage the background server
 * Use a Context to manage the polling function
 * Replaced metrics.Server with bg_server.httpBgServer

 * Made private and moved the polling functionnality into server.go as
   after using a Context it was nothing more than a single function

 * Use a signal.NotifyContext to listen to signals instead of manually
   setting up a waiter and registering it as an extra listener.

Author: sambuc

cmd/csi-rclone-plugin/main.go

@@ -1,9 +1,12 @@
 package main
 
 import (
+	"context"
 	"flag"
 	"fmt"
 	"os"
+	"os/signal"
+	"syscall"
 	"time"
 
 	"github.com/SwissDataScienceCenter/csi-rclone/pkg/metrics"
@@ -78,10 +81,13 @@ func main() {
 
 	root.ParseFlags(os.Args[1:])
 
-	if metricsServerConfig.IsEnabled() {
-		metricsServer := metricsServerConfig.NewServer(&meters, 1*time.Second, 5*time.Second)
+	if metricsServerConfig.Enable {
+		// Gracefully exit the metrics background servers
+		ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT)
+		defer stop()
+
+		metricsServer := metricsServerConfig.NewServer(ctx, 5*time.Second, 30*time.Second, &meters)
 		go metricsServer.ListenAndServe()
-		defer metricsServer.Shutdown()
 	}
 
 	if err := root.Execute(); err != nil {

pkg/metrics/bg_server.go

@@ -1,67 +1,99 @@
 package metrics
 
 import (
+	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/spf13/cobra"
+	"k8s.io/klog"
 )
 
 type Server struct {
-	server  httpBgServer
-	Watcher Watcher
+	http             http.Server
+	shutdownFinished chan struct{}
 }
 
 func (server *Server) ListenAndServe() {
-	server.server.ListenAndServe()
+	err := server.http.ListenAndServe()
+
+	if errors.Is(err, http.ErrServerClosed) {
+		// Expected in case of shutdown.
+		err = nil
+	} else {
+		klog.Warning("HTTP server failed: " + err.Error())
+	}
+
+	<-server.shutdownFinished
 }
 
-func (server *Server) Shutdown() {
-	if server.Watcher.tickerStop != nil {
-		server.Watcher.Stop()
+func (server *Server) shutdown(waitForExit context.Context, exitTimeout time.Duration) {
+	<-waitForExit.Done()
+
+	ctx, cancel := context.WithTimeout(context.Background(), exitTimeout)
+	defer cancel()
+
+	err := server.http.Shutdown(ctx)
+	if err != nil {
+		klog.Warning("Error while shutting down HTTP server: " + err.Error())
+	} else {
+		klog.Info("HTTP server successfully shut down.")
+	}
+	close(server.shutdownFinished)
+}
+
+type Observable func()
+
+func poll(ctx context.Context, period time.Duration, observe Observable) {
+	ticker := time.NewTicker(period)
+	defer ticker.Stop()
+
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-ticker.C:
+			observe()
+		}
 	}
-	server.server.Shutdown()
 }
 
 type ServerConfig struct {
 	host       string
 	port       int16
 	pathPrefix string
-	enable     bool
+	Enable     bool
 }
 
 func (config *ServerConfig) CommandLineParameters(root *cobra.Command) {
 	root.PersistentFlags().StringVar(&config.host, "metrics-host", "localhost", "Host name or ip address for Prometheus metrics")
 	root.PersistentFlags().Int16Var(&config.port, "metrics-port", 80, "Port for Prometheus metrics")
-	root.PersistentFlags().BoolVar(&config.enable, "metrics-enable", false, "Enable Prometheus metrics")
+	root.PersistentFlags().BoolVar(&config.Enable, "metrics-enable", false, "Enable Prometheus metrics")
 
 	config.pathPrefix = "/metrics"
 }
 
-func (config *ServerConfig) NewServer(meters *[]Observable, pollDelay, shutdownTimeout time.Duration) *Server {
+func (config *ServerConfig) NewServer(ctx context.Context, shutdownTimeout, pollPeriod time.Duration, meters *[]Observable) *Server {
 	mux := http.NewServeMux()
 	mux.Handle(config.pathPrefix, promhttp.Handler())
 
 	server := Server{
-		server: httpBgServer{
-			server:           http.Server{Addr: fmt.Sprintf("%s:%d", config.host, config.port), Handler: mux},
-			shutdownFinished: make(chan struct{}),
-			shutdownTimeout:  shutdownTimeout,
-		},
+		http:             http.Server{Addr: fmt.Sprintf("%s:%d", config.host, config.port), Handler: mux},
+		shutdownFinished: make(chan struct{}),
 	}
 
-	server.Watcher.Poll(
+	go poll(ctx, pollPeriod,
 		func() {
 			for _, observer := range *meters {
 				observer()
 			}
 		},
-		pollDelay,
 	)
 
+	go server.shutdown(ctx, shutdownTimeout)
+
 	return &server
 }
-
-func (config *ServerConfig) IsEnabled() bool { return config.enable }