Chore: refactor grafana-apiserver a bit (grafana#74177)

Author: mildwonkey

.github/CODEOWNERS

@@ -98,7 +98,6 @@
 /pkg/server/ @grafana/backend-platform
 /pkg/services/annotations/ @grafana/backend-platform
 /pkg/services/apikey/ @grafana/grafana-authnz-team
-/pkg/services/certgenerator @grafana/grafana-app-platform-squad
 /pkg/services/cleanup/ @grafana/backend-platform
 /pkg/services/contexthandler/ @grafana/backend-platform
 /pkg/services/correlations/ @grafana/explore-squad

pkg/modules/dependencies.go

@@ -0,0 +1,15 @@
+package modules
+
+const (
+	// All includes all modules necessary for Grafana to run as a standalone server
+	All string = "all"
+
+	Core             string = "core"
+	GrafanaAPIServer string = "grafana-apiserver"
+)
+
+var dependencyMap = map[string][]string{
+	GrafanaAPIServer: {},
+	Core:             {},
+	All:              {Core},
+}

pkg/modules/listener.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/grafana/dskit/modules"
 	"github.com/grafana/dskit/services"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 )
 
@@ -30,12 +31,12 @@ func (l *serviceListener) Stopped() {
 
 func (l *serviceListener) Failure(service services.Service) {
 	// if any service fails, stop all services
-	if err := l.service.Shutdown(context.Background()); err != nil {
+	if err := l.service.Shutdown(context.Background(), service.FailureCase().Error()); err != nil {
 		l.log.Error("Failed to stop all modules", "err", err)
 	}
 
 	// log which module failed
-	for module, s := range l.service.ServiceMap {
+	for module, s := range l.service.serviceMap {
 		if s == service {
 			if errors.Is(service.FailureCase(), modules.ErrStopProcess) {
 				l.log.Info("Received stop signal via return error", "module", module, "err", service.FailureCase())

pkg/modules/modules.go

@@ -8,109 +8,103 @@ import (
 	"github.com/grafana/dskit/services"
 
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
-	"github.com/grafana/grafana/pkg/setting"
 )
 
 type Engine interface {
-	Init(context.Context) error
 	Run(context.Context) error
-	Shutdown(context.Context) error
+	Shutdown(context.Context, string) error
 }
 
 type Manager interface {
-	RegisterModule(name string, initFn func() (services.Service, error), deps ...string)
-	RegisterInvisibleModule(name string, initFn func() (services.Service, error), deps ...string)
+	RegisterModule(name string, fn initFn)
+	RegisterInvisibleModule(name string, fn initFn)
 }
 
 var _ Engine = (*service)(nil)
 var _ Manager = (*service)(nil)
 
 // service manages the registration and lifecycle of modules.
 type service struct {
-	cfg           *setting.Cfg
-	log           log.Logger
-	targets       []string
-	dependencyMap map[string][]string
-
-	ModuleManager  *modules.Manager
-	ServiceManager *services.Manager
-	ServiceMap     map[string]services.Service
+	log     log.Logger
+	targets []string
+
+	moduleManager  *modules.Manager
+	serviceManager *services.Manager
+	serviceMap     map[string]services.Service
 }
 
-func ProvideService(
-	cfg *setting.Cfg,
-	features *featuremgmt.FeatureManager,
+func New(
+	targets []string,
 ) *service {
 	logger := log.New("modules")
 
 	return &service{
-		cfg:           cfg,
-		log:           logger,
-		targets:       cfg.Target,
-		dependencyMap: map[string][]string{},
+		log:     logger,
+		targets: targets,
 
-		ModuleManager: modules.NewManager(logger),
-		ServiceMap:    map[string]services.Service{},
+		moduleManager: modules.NewManager(logger),
+		serviceMap:    map[string]services.Service{},
 	}
 }
 
-// Init initializes all registered modules.
-func (m *service) Init(_ context.Context) error {
+// Run starts all registered modules.
+func (m *service) Run(ctx context.Context) error {
 	var err error
 
-	for mod, targets := range m.dependencyMap {
-		if err := m.ModuleManager.AddDependency(mod, targets...); err != nil {
+	for mod, targets := range dependencyMap {
+		if !m.moduleManager.IsModuleRegistered(mod) {
+			continue
+		}
+		if err := m.moduleManager.AddDependency(mod, targets...); err != nil {
 			return err
 		}
 	}
 
-	m.ServiceMap, err = m.ModuleManager.InitModuleServices(m.targets...)
+	m.serviceMap, err = m.moduleManager.InitModuleServices(m.targets...)
 	if err != nil {
 		return err
 	}
 
 	// if no modules are registered, we don't need to start the service manager
-	if len(m.ServiceMap) == 0 {
+	if len(m.serviceMap) == 0 {
 		return nil
 	}
 
 	var svcs []services.Service
-	for _, s := range m.ServiceMap {
+	for _, s := range m.serviceMap {
 		svcs = append(svcs, s)
 	}
-	m.ServiceManager, err = services.NewManager(svcs...)
 
-	return err
-}
+	m.serviceManager, err = services.NewManager(svcs...)
+	if err != nil {
+		return err
+	}
 
-// Run starts all registered modules.
-func (m *service) Run(ctx context.Context) error {
 	// we don't need to continue if no modules are registered.
 	// this behavior may need to change if dskit services replace the
 	// current background service registry.
-	if len(m.ServiceMap) == 0 {
+	if len(m.serviceMap) == 0 {
 		m.log.Warn("No modules registered...")
 		<-ctx.Done()
 		return nil
 	}
 
 	listener := newServiceListener(m.log, m)
-	m.ServiceManager.AddListener(listener)
+	m.serviceManager.AddListener(listener)
 
 	m.log.Debug("Starting module service manager")
 	// wait until a service fails or stop signal was received
-	err := m.ServiceManager.StartAsync(ctx)
+	err = m.serviceManager.StartAsync(ctx)
 	if err != nil {
 		return err
 	}
 
-	err = m.ServiceManager.AwaitStopped(ctx)
+	err = m.serviceManager.AwaitStopped(ctx)
 	if err != nil {
 		return err
 	}
 
-	failed := m.ServiceManager.ServicesByState()[services.Failed]
+	failed := m.serviceManager.ServicesByState()[services.Failed]
 	for _, f := range failed {
 		// the service listener will log error details for all modules that failed,
 		// so here we return the first error that is not ErrStopProcess
@@ -123,27 +117,27 @@ func (m *service) Run(ctx context.Context) error {
 }
 
 // Shutdown stops all modules and waits for them to stop.
-func (m *service) Shutdown(ctx context.Context) error {
-	if m.ServiceManager == nil {
+func (m *service) Shutdown(ctx context.Context, reason string) error {
+	if m.serviceManager == nil {
 		m.log.Debug("No modules registered, nothing to stop...")
 		return nil
 	}
-	m.ServiceManager.StopAsync()
-	m.log.Info("Awaiting services to be stopped...")
-	return m.ServiceManager.AwaitStopped(ctx)
+	m.serviceManager.StopAsync()
+	m.log.Info("Awaiting services to be stopped...", "reason", reason)
+	return m.serviceManager.AwaitStopped(ctx)
 }
 
+type initFn func() (services.Service, error)
+
 // RegisterModule registers a module with the dskit module manager.
-func (m *service) RegisterModule(name string, initFn func() (services.Service, error), deps ...string) {
-	m.ModuleManager.RegisterModule(name, initFn)
-	m.dependencyMap[name] = deps
+func (m *service) RegisterModule(name string, fn initFn) {
+	m.moduleManager.RegisterModule(name, fn)
 }
 
 // RegisterInvisibleModule registers an invisible module with the dskit module manager.
 // Invisible modules are not visible to the user, and are intendent to be used as dependencies.
-func (m *service) RegisterInvisibleModule(name string, initFn func() (services.Service, error), deps ...string) {
-	m.ModuleManager.RegisterModule(name, initFn, modules.UserInvisibleModule)
-	m.dependencyMap[name] = deps
+func (m *service) RegisterInvisibleModule(name string, fn initFn) {
+	m.moduleManager.RegisterModule(name, fn, modules.UserInvisibleModule)
 }
 
 // IsModuleEnabled returns true if the module is enabled.

pkg/modules/wire.go

@@ -18,7 +18,6 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/infra/usagestats/statscollector"
-	"github.com/grafana/grafana/pkg/modules"
 	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/provisioning"
@@ -39,15 +38,14 @@ type Options struct {
 func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry,
 	provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry,
 	usageStatsProvidersRegistry registry.UsageStatsProvidersRegistry, statsCollectorService *statscollector.Service,
-	moduleService modules.Engine,
 ) (*Server, error) {
 	statsCollectorService.RegisterProviders(usageStatsProvidersRegistry.GetServices())
-	s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider, moduleService)
+	s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := s.init(); err != nil {
+	if err := s.Init(); err != nil {
 		return nil, err
 	}
 
@@ -56,7 +54,6 @@ func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistr
 
 func newServer(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry,
 	provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry,
-	moduleService modules.Engine,
 ) (*Server, error) {
 	rootCtx, shutdownFn := context.WithCancel(context.Background())
 	childRoutines, childCtx := errgroup.WithContext(rootCtx)
@@ -76,7 +73,6 @@ func newServer(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleR
 		commit:              opts.Commit,
 		buildBranch:         opts.BuildBranch,
 		backgroundServices:  backgroundServiceProvider.GetServices(),
-		moduleService:       moduleService,
 	}
 
 	return s, nil
@@ -103,11 +99,10 @@ type Server struct {
 	HTTPServer          *api.HTTPServer
 	roleRegistry        accesscontrol.RoleRegistry
 	provisioningService provisioning.ProvisioningService
-	moduleService       modules.Engine
 }
 
-// init initializes the server and its services.
-func (s *Server) init() error {
+// Init initializes the server and its services.
+func (s *Server) Init() error {
 	s.mtx.Lock()
 	defer s.mtx.Unlock()
 
@@ -120,11 +115,6 @@ func (s *Server) init() error {
 		return err
 	}
 
-	// Initialize dskit modules.
-	if err := s.moduleService.Init(s.context); err != nil {
-		return err
-	}
-
 	if err := metrics.SetEnvironmentInformation(s.cfg.MetricsGrafanaEnvironmentInfo); err != nil {
 		return err
 	}
@@ -141,19 +131,10 @@ func (s *Server) init() error {
 func (s *Server) Run() error {
 	defer close(s.shutdownFinished)
 
-	if err := s.init(); err != nil {
+	if err := s.Init(); err != nil {
 		return err
 	}
 
-	// Start dskit modules.
-	s.childRoutines.Go(func() error {
-		err := s.moduleService.Run(s.context)
-		if err != nil && !errors.Is(err, context.Canceled) {
-			return err
-		}
-		return nil
-	})
-
 	services := s.backgroundServices
 
 	// Start background services.
@@ -197,9 +178,6 @@ func (s *Server) Shutdown(ctx context.Context, reason string) error {
 	var err error
 	s.shutdownOnce.Do(func() {
 		s.log.Info("Shutdown started", "reason", reason)
-		if err := s.moduleService.Shutdown(ctx); err != nil {
-			s.log.Error("Failed to shutdown modules", "error", err)
-		}
 		// Call cancel func to stop background services.
 		s.shutdownFn()
 		// Wait for server to shut down

pkg/server/server.go

@@ -48,7 +48,7 @@ func (s *testService) IsDisabled() bool {
 
 func testServer(t *testing.T, services ...registry.BackgroundService) *Server {
 	t.Helper()
-	s, err := newServer(Options{}, setting.NewCfg(), nil, &acimpl.Service{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...), &MockModuleService{})
+	s, err := newServer(Options{}, setting.NewCfg(), nil, &acimpl.Service{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...))
 	require.NoError(t, err)
 	// Required to skip configuration initialization that causes
 	// DI errors in this test.

pkg/server/server_test.go

@@ -0,0 +1,44 @@
+package server
+
+import (
+	"context"
+
+	"github.com/grafana/dskit/services"
+
+	"github.com/grafana/grafana/pkg/api"
+	"github.com/grafana/grafana/pkg/setting"
+)
+
+type coreService struct {
+	*services.BasicService
+	cla     setting.CommandLineArgs
+	opts    Options
+	apiOpts api.ServerOptions
+	server  *Server
+}
+
+func NewService(opts Options, apiOpts api.ServerOptions) (*coreService, error) {
+	s := &coreService{
+		opts:    opts,
+		apiOpts: apiOpts,
+	}
+	s.BasicService = services.NewBasicService(s.start, s.running, s.stop)
+	return s, nil
+}
+
+func (s *coreService) start(_ context.Context) error {
+	serv, err := Initialize(s.cla, s.opts, s.apiOpts)
+	if err != nil {
+		return err
+	}
+	s.server = serv
+	return s.server.Init()
+}
+
+func (s *coreService) running(_ context.Context) error {
+	return s.server.Run()
+}
+
+func (s *coreService) stop(failureReason error) error {
+	return s.server.Shutdown(context.Background(), failureReason.Error())
+}