diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 662ed9c8ad5..8733d9a9a30 100644 --- a/pkg/api/http_server.go +++ b/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 } diff --git a/pkg/modules/dependencies.go b/pkg/modules/dependencies.go index 4343a2e1679..5f4a0734eb7 100644 --- a/pkg/modules/dependencies.go +++ b/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}, } diff --git a/pkg/modules/registry/registry.go b/pkg/modules/registry/registry.go index 4ad999feb34..9dbe3f95c3d 100644 --- a/pkg/modules/registry/registry.go +++ b/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, ) } diff --git a/pkg/modules/registry/registry_test.go b/pkg/modules/registry/registry_test.go index f5464883eda..d05ca584116 100644 --- a/pkg/modules/registry/registry_test.go +++ b/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" diff --git a/pkg/server/backgroundsvcs/background_services.go b/pkg/server/backgroundsvcs/background_services.go index d7254e22be2..83c9d820cd3 100644 --- a/pkg/server/backgroundsvcs/background_services.go +++ b/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,