chore: wrap HTTP server in a dskit module (#71601)

* chore: wrap HTTP server in a dskit module

Much of the logic from this comes from the POC branch, so:
- credit for this work goes to everyone else
- mistakes are my own
This is needed to support microservice deployment modes.
* added an arbitrarily-chosen 30second timeout

pkg/api/http_server.go

@@ -11,8 +11,9 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
-	"sync"
+	"time"
 
+	"github.com/grafana/dskit/services"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 
@@ -32,6 +33,7 @@ import (
 	"github.com/grafana/grafana/pkg/middleware"
 	"github.com/grafana/grafana/pkg/middleware/csrf"
 	"github.com/grafana/grafana/pkg/middleware/loggermw"
+	"github.com/grafana/grafana/pkg/modules"
 	"github.com/grafana/grafana/pkg/plugins"
 	"github.com/grafana/grafana/pkg/plugins/pluginscdn"
 	"github.com/grafana/grafana/pkg/registry/corekind"
@@ -103,6 +105,10 @@ import (
 )
 
 type HTTPServer struct {
+	// services.NamedService is embedded so we can inherit AddListener, without
+	// implementing the service interface.
+	services.NamedService
+
 	log              log.Logger
 	web              *web.Mux
 	context          context.Context
@@ -110,6 +116,7 @@ type HTTPServer struct {
 	middlewares      []web.Handler
 	namedMiddlewares []routing.RegisterNamedMiddleware
 	bus              bus.Bus
+	errs             chan error
 
 	pluginContextProvider        *plugincontext.Provider
 	RouteRegister                routing.RouteRegister
@@ -351,6 +358,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 		authnService:                 authnService,
 		pluginsCDNService:            pluginsCDNService,
 		starApi:                      starApi,
+		errs:                         make(chan error),
 	}
 	if hs.Listener != nil {
 		hs.log.Debug("Using provided listener")
@@ -363,6 +371,8 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 	if err := hs.declareFixedRoles(); err != nil {
 		return nil, err
 	}
+
+	hs.NamedService = services.NewBasicService(hs.start, hs.running, hs.stop).WithName(modules.HTTPServer)
 	return hs, nil
 }
 
@@ -374,7 +384,7 @@ func (hs *HTTPServer) AddNamedMiddleware(middleware routing.RegisterNamedMiddlew
 	hs.namedMiddlewares = append(hs.namedMiddlewares, middleware)
 }
 
-func (hs *HTTPServer) Run(ctx context.Context) error {
+func (hs *HTTPServer) start(ctx context.Context) error {
 	hs.context = ctx
 
 	hs.applyRoutes()
@@ -406,42 +416,57 @@ func (hs *HTTPServer) Run(ctx context.Context) error {
 	hs.log.Info("HTTP Server Listen", "address", listener.Addr().String(), "protocol",
 		hs.Cfg.Protocol, "subUrl", hs.Cfg.AppSubURL, "socket", hs.Cfg.SocketPath)
 
-	var wg sync.WaitGroup
-	wg.Add(1)
-
-	// handle http shutdown on server context done
-	go func() {
-		defer wg.Done()
-
-		<-ctx.Done()
-		if err := hs.httpSrv.Shutdown(context.Background()); err != nil {
-			hs.log.Error("Failed to shutdown server", "error", err)
-		}
-	}()
-
 	switch hs.Cfg.Protocol {
 	case setting.HTTPScheme, setting.SocketScheme:
-		if err := hs.httpSrv.Serve(listener); err != nil {
-			if errors.Is(err, http.ErrServerClosed) {
-				hs.log.Debug("server was shutdown gracefully")
-				return nil
+		go func() {
+			if err := hs.httpSrv.Serve(listener); err != nil {
+				if errors.Is(err, http.ErrServerClosed) {
+					hs.log.Debug("server was shutdown gracefully")
+					close(hs.errs)
+					return
+				}
+				hs.errs <- err
 			}
-			return err
-		}
+		}()
 	case setting.HTTP2Scheme, setting.HTTPSScheme:
-		if err := hs.httpSrv.ServeTLS(listener, hs.Cfg.CertFile, hs.Cfg.KeyFile); err != nil {
-			if errors.Is(err, http.ErrServerClosed) {
-				hs.log.Debug("server was shutdown gracefully")
-				return nil
+		go func() {
+			if err := hs.httpSrv.ServeTLS(listener, hs.Cfg.CertFile, hs.Cfg.KeyFile); err != nil {
+				if errors.Is(err, http.ErrServerClosed) {
+					hs.log.Debug("server was shutdown gracefully")
+					close(hs.errs)
+					return
+				}
+				hs.errs <- err
 			}
-			return err
-		}
+		}()
 	default:
 		panic(fmt.Sprintf("Unhandled protocol %q", hs.Cfg.Protocol))
 	}
 
-	wg.Wait()
+	return nil
+}
 
+func (hs *HTTPServer) running(ctx context.Context) error {
+	select {
+	case err, ok := <-hs.errs:
+		if !ok {
+			return nil
+		}
+		return err
+	case <-ctx.Done():
+	}
+
+	return nil
+}
+
+func (hs *HTTPServer) stop(_ error) error {
+	// Create a context with a timeout
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+
+	if err := hs.httpSrv.Shutdown(ctx); err != nil {
+		return fmt.Errorf("failed to shutdown server: %w", err)
+	}
 	return nil
 }
 

pkg/modules/dependencies.go

@@ -9,6 +9,8 @@ const (
 	CertGenerator string = "cert-generator"
 	// GrafanaAPIServer is the Kubertenes API server for Grafana Resources
 	GrafanaAPIServer string = "grafana-apiserver"
+	// HTTPServer is the HTTP server for Grafana
+	HTTPServer string = "http-server"
 )
 
 // dependencyMap defines Module Targets => Dependencies
@@ -18,5 +20,5 @@ var dependencyMap = map[string][]string{
 	CertGenerator:    {},
 	GrafanaAPIServer: {CertGenerator},
 
-	All: {BackgroundServices},
+	All: {BackgroundServices, HTTPServer},
 }

pkg/modules/registry/registry.go

@@ -2,8 +2,9 @@ package registry
 
 import (
 	"github.com/grafana/dskit/services"
-
 	"github.com/grafana/grafana-apiserver/pkg/certgenerator"
+
+	"github.com/grafana/grafana/pkg/api"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/modules"
 	"github.com/grafana/grafana/pkg/server/backgroundsvcs"
@@ -22,6 +23,7 @@ func ProvideRegistry(
 	apiServer grafanaapiserver.Service,
 	backgroundServiceRunner *backgroundsvcs.BackgroundServiceRunner,
 	certGenerator certgenerator.ServiceInterface,
+	httpServer *api.HTTPServer,
 ) *registry {
 	return newRegistry(
 		log.New("modules.registry"),
@@ -29,6 +31,7 @@ func ProvideRegistry(
 		apiServer,
 		backgroundServiceRunner,
 		certGenerator,
+		httpServer,
 	)
 }
 

pkg/modules/registry/registry_test.go

@@ -3,9 +3,8 @@ package registry
 import (
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
 	"github.com/grafana/dskit/services"
+	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/modules"

pkg/server/backgroundsvcs/background_services.go

@@ -1,7 +1,6 @@
 package backgroundsvcs
 
 import (
-	"github.com/grafana/grafana/pkg/api"
 	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/infra/remotecache"
 	"github.com/grafana/grafana/pkg/infra/tracing"
@@ -41,7 +40,7 @@ import (
 )
 
 func ProvideBackgroundServiceRegistry(
-	httpServer *api.HTTPServer, ng *ngalert.AlertNG, cleanup *cleanup.CleanUpService, live *live.GrafanaLive,
+	ng *ngalert.AlertNG, cleanup *cleanup.CleanUpService, live *live.GrafanaLive,
 	pushGateway *pushhttp.Gateway, notifications *notifications.NotificationService, processManager *process.Manager,
 	rendering *rendering.RenderingService, tokenService auth.UserTokenBackgroundService, tracing tracing.Tracer,
 	provisioning *provisioning.ProvisioningServiceImpl, alerting *alerting.AlertEngine, usageStats *uss.UsageStats,
@@ -61,7 +60,6 @@ func ProvideBackgroundServiceRegistry(
 	_ *grpcserver.HealthService, _ entity.EntityStoreServer, _ *grpcserver.ReflectionService, _ *ldapapi.Service,
 ) *BackgroundServiceRegistry {
 	return NewBackgroundServiceRegistry(
-		httpServer,
 		ng,
 		cleanup,
 		live,