Chore: Wrap provisioning in dskit service (#71598)

pkg/modules/dependencies.go

@@ -11,14 +11,16 @@ const (
 	GrafanaAPIServer string = "grafana-apiserver"
 	// HTTPServer is the HTTP server for Grafana
 	HTTPServer string = "http-server"
+	// Provisioning sets up Grafana with preconfigured datasources, dashboards, etc.
+	Provisioning string = "provisioning"
 )
 
 // dependencyMap defines Module Targets => Dependencies
 var dependencyMap = map[string][]string{
-	BackgroundServices: {},
+	BackgroundServices: {Provisioning, HTTPServer},
 
 	CertGenerator:    {},
 	GrafanaAPIServer: {CertGenerator},
 
-	All: {BackgroundServices, HTTPServer},
+	All: {Provisioning, HTTPServer, BackgroundServices},
 }

pkg/modules/registry/registry.go

@@ -9,6 +9,7 @@ import (
 	"github.com/grafana/grafana/pkg/modules"
 	"github.com/grafana/grafana/pkg/server/backgroundsvcs"
 	grafanaapiserver "github.com/grafana/grafana/pkg/services/grafana-apiserver"
+	"github.com/grafana/grafana/pkg/services/provisioning"
 )
 
 type Registry interface{}
@@ -24,6 +25,7 @@ func ProvideRegistry(
 	backgroundServiceRunner *backgroundsvcs.BackgroundServiceRunner,
 	certGenerator certgenerator.ServiceInterface,
 	httpServer *api.HTTPServer,
+	provisioningService *provisioning.ProvisioningServiceImpl,
 ) *registry {
 	return newRegistry(
 		log.New("modules.registry"),
@@ -32,6 +34,7 @@ func ProvideRegistry(
 		backgroundServiceRunner,
 		certGenerator,
 		httpServer,
+		provisioningService,
 	)
 }
 

pkg/server/server.go

@@ -19,7 +19,6 @@ import (
 	moduleRegistry "github.com/grafana/grafana/pkg/modules/registry"
 	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	"github.com/grafana/grafana/pkg/services/provisioning"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -35,13 +34,12 @@ type Options struct {
 
 // New returns a new instance of Server.
 func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry,
-	provisioningService provisioning.ProvisioningService,
 	usageStatsProvidersRegistry registry.UsageStatsProvidersRegistry, statsCollectorService *statscollector.Service,
 	moduleService modules.Engine,
 	_ moduleRegistry.Registry, // imported to invoke initialization via Wire
 ) (*Server, error) {
 	statsCollectorService.RegisterProviders(usageStatsProvidersRegistry.GetServices())
-	s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, moduleService)
+	s, err := newServer(opts, cfg, httpServer, roleRegistry, moduleService)
 	if err != nil {
 		return nil, err
 	}
@@ -54,20 +52,18 @@ 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,
 	moduleService modules.Engine) (*Server, error) {
 	return &Server{
-		HTTPServer:          httpServer,
+		HTTPServer:       httpServer,
-		provisioningService: provisioningService,
+		roleRegistry:     roleRegistry,
-		roleRegistry:        roleRegistry,
+		shutdownFinished: make(chan struct{}),
-		shutdownFinished:    make(chan struct{}),
+		log:              log.New("server"),
-		log:                 log.New("server"),
+		cfg:              cfg,
-		cfg:                 cfg,
+		pidFile:          opts.PidFile,
-		pidFile:             opts.PidFile,
+		version:          opts.Version,
-		version:             opts.Version,
+		commit:           opts.Commit,
-		commit:              opts.Commit,
+		buildBranch:      opts.BuildBranch,
-		buildBranch:         opts.BuildBranch,
+		moduleService:    moduleService,
-		moduleService:       moduleService,
 	}, nil
 }
 
@@ -85,10 +81,9 @@ type Server struct {
 	commit      string
 	buildBranch string
 
-	HTTPServer          *api.HTTPServer
+	HTTPServer    *api.HTTPServer
-	roleRegistry        accesscontrol.RoleRegistry
+	roleRegistry  accesscontrol.RoleRegistry
-	provisioningService provisioning.ProvisioningService
+	moduleService modules.Engine
-	moduleService       modules.Engine
 }
 
 // init initializes the server and its services.
@@ -114,11 +109,7 @@ func (s *Server) init(ctx context.Context) error {
 		return err
 	}
 
-	if err := s.roleRegistry.RegisterFixedRoles(ctx); err != nil {
+	return s.roleRegistry.RegisterFixedRoles(ctx)
-		return err
-	}
-
-	return s.provisioningService.RunInitProvisioners(ctx)
 }
 
 // Run initializes and starts services. This will block until all services have

pkg/server/server_test.go

@@ -15,7 +15,7 @@ import (
 
 func testServer(t *testing.T, m *modules.MockModuleEngine) *Server {
 	t.Helper()
-	s, err := newServer(Options{}, setting.NewCfg(), nil, &acimpl.Service{}, nil, m)
+	s, err := newServer(Options{}, setting.NewCfg(), nil, &acimpl.Service{}, m)
 	require.NoError(t, err)
 	// Required to skip configuration initialization that causes
 	// DI errors in this test.

pkg/services/provisioning/provisioning.go

@@ -6,10 +6,12 @@ import (
 	"path/filepath"
 	"sync"
 
+	"github.com/grafana/dskit/services"
+
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/modules"
 	plugifaces "github.com/grafana/grafana/pkg/plugins"
-	"github.com/grafana/grafana/pkg/registry"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/alerting"
 	"github.com/grafana/grafana/pkg/services/correlations"
@@ -52,7 +54,7 @@ func ProvideService(
 	secrectService secrets.Service,
 	orgService org.Service,
 ) (*ProvisioningServiceImpl, error) {
-	s := &ProvisioningServiceImpl{
+	ps := &ProvisioningServiceImpl{
 		Cfg:                          cfg,
 		SQLStore:                     sqlStore,
 		ac:                           ac,
@@ -76,12 +78,14 @@ func ProvideService(
 		log:                          log.New("provisioning"),
 		orgService:                   orgService,
 	}
-	return s, nil
+
+	ps.BasicService = services.NewBasicService(ps.RunInitProvisioners, ps.Run, nil).WithName(modules.Provisioning)
+
+	return ps, nil
 }
 
 type ProvisioningService interface {
-	registry.BackgroundService
+	services.NamedService
-	RunInitProvisioners(ctx context.Context) error
 	ProvisionDatasources(ctx context.Context) error
 	ProvisionPlugins(ctx context.Context) error
 	ProvisionNotifications(ctx context.Context) error
@@ -89,18 +93,21 @@ type ProvisioningService interface {
 	ProvisionAlerting(ctx context.Context) error
 	GetDashboardProvisionerResolvedPath(name string) string
 	GetAllowUIUpdatesFromConfig(name string) bool
+	RunInitProvisioners(ctx context.Context) error
 }
 
 // Add a public constructor for overriding service to be able to instantiate OSS as fallback
 func NewProvisioningServiceImpl() *ProvisioningServiceImpl {
 	logger := log.New("provisioning")
-	return &ProvisioningServiceImpl{
+	ps := &ProvisioningServiceImpl{
 		log:                     logger,
 		newDashboardProvisioner: dashboards.New,
 		provisionNotifiers:      notifiers.Provision,
 		provisionDatasources:    datasources.Provision,
 		provisionPlugins:        plugins.Provision,
 	}
+	ps.BasicService = services.NewBasicService(ps.RunInitProvisioners, ps.Run, nil).WithName(modules.Provisioning)
+	return ps
 }
 
 // Used for testing purposes
@@ -110,16 +117,20 @@ func newProvisioningServiceImpl(
 	provisionDatasources func(context.Context, string, datasources.Store, datasources.CorrelationsStore, org.Service) error,
 	provisionPlugins func(context.Context, string, plugifaces.Store, pluginsettings.Service, org.Service) error,
 ) *ProvisioningServiceImpl {
-	return &ProvisioningServiceImpl{
+	ps := &ProvisioningServiceImpl{
 		log:                     log.New("provisioning"),
 		newDashboardProvisioner: newDashboardProvisioner,
 		provisionNotifiers:      provisionNotifiers,
 		provisionDatasources:    provisionDatasources,
 		provisionPlugins:        provisionPlugins,
 	}
+	ps.BasicService = services.NewBasicService(ps.RunInitProvisioners, ps.Run, nil).WithName(modules.Provisioning)
+	return ps
 }
 
 type ProvisioningServiceImpl struct {
+	*services.BasicService
+
 	Cfg                          *setting.Cfg
 	SQLStore                     db.DB
 	orgService                   org.Service
@@ -197,8 +208,10 @@ func (ps *ProvisioningServiceImpl) Run(ctx context.Context) error {
 			continue
 		case <-ctx.Done():
 			// Root server context was cancelled so cancel polling and leave.
+			ps.mutex.Lock()
 			ps.cancelPolling()
-			return ctx.Err()
+			ps.mutex.Unlock()
+			return nil
 		}
 	}
 }

pkg/services/provisioning/provisioning_mock.go

@@ -1,6 +1,12 @@
 package provisioning
 
-import "context"
+import (
+	"context"
+
+	"github.com/grafana/dskit/services"
+
+	"github.com/grafana/grafana/pkg/modules"
+)
 
 type Calls struct {
 	RunInitProvisioners                 []interface{}
@@ -15,6 +21,7 @@ type Calls struct {
 }
 
 type ProvisioningServiceMock struct {
+	*services.BasicService
 	Calls                                   *Calls
 	RunInitProvisionersFunc                 func(ctx context.Context) error
 	ProvisionDatasourcesFunc                func(ctx context.Context) error
@@ -27,9 +34,11 @@ type ProvisioningServiceMock struct {
 }
 
 func NewProvisioningServiceMock(ctx context.Context) *ProvisioningServiceMock {
-	return &ProvisioningServiceMock{
+	s := &ProvisioningServiceMock{
 		Calls: &Calls{},
 	}
+	s.BasicService = services.NewBasicService(s.RunInitProvisioners, s.Run, nil).WithName(modules.Provisioning)
+	return s
 }
 
 func (mock *ProvisioningServiceMock) RunInitProvisioners(ctx context.Context) error {

pkg/services/provisioning/provisioning_test.go

@@ -40,7 +40,7 @@ func TestProvisioningServiceImpl(t *testing.T) {
 		serviceTest.waitForStop()
 
 		assert.False(t, serviceTest.serviceRunning, "Service should not be running")
-		assert.Equal(t, context.Canceled, serviceTest.serviceError, "Service should have returned canceled error")
+		assert.Nil(t, serviceTest.serviceError, "Service should not return canceled error")
 	})
 
 	t.Run("Failed reloading does not stop polling with old provisioned", func(t *testing.T) {