From d280fedb3f537581d14f6e6227c7c2aa667ea106 Mon Sep 17 00:00:00 2001 From: Todd Treece <360020+toddtreece@users.noreply.github.com> Date: Tue, 18 Jul 2023 15:37:25 -0400 Subject: [PATCH] Chore: Wrap provisioning in dskit service (#71598) --- pkg/modules/dependencies.go | 6 ++- pkg/modules/registry/registry.go | 3 ++ pkg/server/server.go | 39 +++++++------------ pkg/server/server_test.go | 2 +- pkg/services/provisioning/provisioning.go | 29 ++++++++++---- .../provisioning/provisioning_mock.go | 13 ++++++- .../provisioning/provisioning_test.go | 2 +- 7 files changed, 56 insertions(+), 38 deletions(-) diff --git a/pkg/modules/dependencies.go b/pkg/modules/dependencies.go index 5f4a0734eb7..4983db05148 100644 --- a/pkg/modules/dependencies.go +++ b/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}, } diff --git a/pkg/modules/registry/registry.go b/pkg/modules/registry/registry.go index 9dbe3f95c3d..1963abca82a 100644 --- a/pkg/modules/registry/registry.go +++ b/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, ) } diff --git a/pkg/server/server.go b/pkg/server/server.go index 22f4f96e1df..531d2467b5d 100644 --- a/pkg/server/server.go +++ b/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, - provisioningService: provisioningService, - roleRegistry: roleRegistry, - shutdownFinished: make(chan struct{}), - log: log.New("server"), - cfg: cfg, - pidFile: opts.PidFile, - version: opts.Version, - commit: opts.Commit, - buildBranch: opts.BuildBranch, - moduleService: moduleService, + HTTPServer: httpServer, + roleRegistry: roleRegistry, + shutdownFinished: make(chan struct{}), + log: log.New("server"), + cfg: cfg, + pidFile: opts.PidFile, + version: opts.Version, + commit: opts.Commit, + buildBranch: opts.BuildBranch, + moduleService: moduleService, }, nil } @@ -85,10 +81,9 @@ type Server struct { commit string buildBranch string - HTTPServer *api.HTTPServer - roleRegistry accesscontrol.RoleRegistry - provisioningService provisioning.ProvisioningService - moduleService modules.Engine + HTTPServer *api.HTTPServer + roleRegistry accesscontrol.RoleRegistry + 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 err - } - - return s.provisioningService.RunInitProvisioners(ctx) + return s.roleRegistry.RegisterFixedRoles(ctx) } // Run initializes and starts services. This will block until all services have diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index d27096b256d..05b7ba1d175 100644 --- a/pkg/server/server_test.go +++ b/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. diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index 6496c9bbb90..940755259af 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/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 - RunInitProvisioners(ctx context.Context) error + services.NamedService 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 } } } diff --git a/pkg/services/provisioning/provisioning_mock.go b/pkg/services/provisioning/provisioning_mock.go index 37556aefe28..97935ea7c15 100644 --- a/pkg/services/provisioning/provisioning_mock.go +++ b/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 { diff --git a/pkg/services/provisioning/provisioning_test.go b/pkg/services/provisioning/provisioning_test.go index e7626af9d37..b4c6b89d191 100644 --- a/pkg/services/provisioning/provisioning_test.go +++ b/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) {