diff --git a/pkg/services/provisioning/dashboards/file_reader.go b/pkg/services/provisioning/dashboards/file_reader.go index 171a46bbd14..a061558e82a 100644 --- a/pkg/services/provisioning/dashboards/file_reader.go +++ b/pkg/services/provisioning/dashboards/file_reader.go @@ -24,6 +24,8 @@ import ( var ( // ErrFolderNameMissing is returned when folder name is missing. ErrFolderNameMissing = errors.New("folder name missing") + // ErrGetOrCreateFolder is returned when there is a failure to fetch or create a provisioning folder. + ErrGetOrCreateFolder = errors.New("failed to get or create provisioning folder") ) // FileReader is responsible for reading dashboards from disk and @@ -147,7 +149,7 @@ func (fr *FileReader) storeDashboardsInFolder(ctx context.Context, filesFoundOnD dashboardRefs map[string]*dashboards.DashboardProvisioning, usageTracker *usageTracker) error { folderID, folderUID, err := fr.getOrCreateFolder(ctx, fr.Cfg, fr.dashboardProvisioningService, fr.Cfg.Folder) if err != nil && !errors.Is(err, ErrFolderNameMissing) { - return err + return fmt.Errorf("%w with name %q: %w", ErrGetOrCreateFolder, fr.Cfg.Folder, err) } // save dashboards based on json files @@ -177,7 +179,7 @@ func (fr *FileReader) storeDashboardsInFoldersFromFileStructure(ctx context.Cont folderID, folderUID, err := fr.getOrCreateFolder(ctx, fr.Cfg, fr.dashboardProvisioningService, folderName) if err != nil && !errors.Is(err, ErrFolderNameMissing) { - return fmt.Errorf("can't provision folder %q from file system structure: %w", folderName, err) + return fmt.Errorf("%w with name %q from file system structure: %w", ErrGetOrCreateFolder, folderName, err) } provisioningMetadata, err := fr.saveDashboard(ctx, path, folderID, folderUID, fileInfo, dashboardRefs) diff --git a/pkg/services/provisioning/provisioning.go b/pkg/services/provisioning/provisioning.go index 82d48e2043b..13cdf4b5458 100644 --- a/pkg/services/provisioning/provisioning.go +++ b/pkg/services/provisioning/provisioning.go @@ -2,6 +2,7 @@ package provisioning import ( "context" + "errors" "fmt" "path/filepath" "sync" @@ -77,9 +78,8 @@ func ProvideService( folderService: folderService, } - err := s.setDashboardProvisioner() - if err != nil { - return nil, fmt.Errorf("%v: %w", "Failed to create provisioner", err) + if err := s.setDashboardProvisioner(); err != nil { + return nil, err } return s, nil @@ -106,30 +106,27 @@ type ProvisioningService interface { GetAllowUIUpdatesFromConfig(name string) bool } -// Add a public constructor for overriding service to be able to instantiate OSS as fallback -func NewProvisioningServiceImpl() *ProvisioningServiceImpl { - logger := log.New("provisioning") - return &ProvisioningServiceImpl{ - log: logger, - newDashboardProvisioner: dashboards.New, - provisionDatasources: datasources.Provision, - provisionPlugins: plugins.Provision, - } -} - // Used for testing purposes func newProvisioningServiceImpl( newDashboardProvisioner dashboards.DashboardProvisionerFactory, provisionDatasources func(context.Context, string, datasources.BaseDataSourceService, datasources.CorrelationsStore, org.Service) error, provisionPlugins func(context.Context, string, pluginstore.Store, pluginsettings.Service, org.Service) error, -) *ProvisioningServiceImpl { - return &ProvisioningServiceImpl{ + searchService searchV2.SearchService, +) (*ProvisioningServiceImpl, error) { + s := &ProvisioningServiceImpl{ log: log.New("provisioning"), newDashboardProvisioner: newDashboardProvisioner, provisionDatasources: provisionDatasources, provisionPlugins: provisionPlugins, Cfg: setting.NewCfg(), + searchService: searchService, } + + if err := s.setDashboardProvisioner(); err != nil { + return nil, err + } + + return s, nil } type ProvisioningServiceImpl struct { @@ -185,7 +182,11 @@ func (ps *ProvisioningServiceImpl) Run(ctx context.Context) error { err := ps.ProvisionDashboards(ctx) if err != nil { ps.log.Error("Failed to provision dashboard", "error", err) - return err + // Consider the allow list of errors for which running the provisioning service should not + // fail. For now this includes only dashboards.ErrGetOrCreateFolder. + if !errors.Is(err, dashboards.ErrGetOrCreateFolder) { + return err + } } if ps.dashboardProvisioner.HasDashboardSources() { ps.searchService.TriggerReIndex() diff --git a/pkg/services/provisioning/provisioning_test.go b/pkg/services/provisioning/provisioning_test.go index 93691f7b12a..c2e9e08fbaa 100644 --- a/pkg/services/provisioning/provisioning_test.go +++ b/pkg/services/provisioning/provisioning_test.go @@ -3,6 +3,7 @@ package provisioning import ( "context" "errors" + "fmt" "testing" "time" @@ -14,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/provisioning/dashboards" "github.com/grafana/grafana/pkg/services/provisioning/utils" + "github.com/grafana/grafana/pkg/services/searchV2" ) func TestProvisioningServiceImpl(t *testing.T) { @@ -66,6 +68,46 @@ func TestProvisioningServiceImpl(t *testing.T) { // Cancelling the root context and stopping the service serviceTest.cancel() }) + + t.Run("Should not return run error when dashboard provisioning fails because of folder", func(t *testing.T) { + serviceTest := setup(t) + provisioningErr := fmt.Errorf("%w: Test error", dashboards.ErrGetOrCreateFolder) + serviceTest.mock.ProvisionFunc = func(ctx context.Context) error { + return provisioningErr + } + err := serviceTest.service.ProvisionDashboards(context.Background()) + assert.NotNil(t, err) + serviceTest.startService() + + serviceTest.waitForPollChanges() + assert.Equal(t, 1, len(serviceTest.mock.Calls.PollChanges), "PollChanges should have been called") + + // Cancelling the root context and stopping the service + serviceTest.cancel() + serviceTest.waitForStop() + + assert.Equal(t, context.Canceled, serviceTest.serviceError) + }) + + t.Run("Should return run error when dashboard provisioning fails for non-allow-listed error", func(t *testing.T) { + serviceTest := setup(t) + provisioningErr := errors.New("Non-allow-listed error") + serviceTest.mock.ProvisionFunc = func(ctx context.Context) error { + return provisioningErr + } + err := serviceTest.service.ProvisionDashboards(context.Background()) + assert.NotNil(t, err) + serviceTest.startService() + + serviceTest.waitForPollChanges() + assert.Equal(t, 0, len(serviceTest.mock.Calls.PollChanges), "PollChanges should have been called") + + // Cancelling the root context and stopping the service + serviceTest.cancel() + serviceTest.waitForStop() + + assert.True(t, errors.Is(serviceTest.serviceError, provisioningErr)) + }) } type serviceTestStruct struct { @@ -95,14 +137,17 @@ func setup(t *testing.T) *serviceTestStruct { pollChangesChannel <- ctx } - serviceTest.service = newProvisioningServiceImpl( + searchStub := searchV2.NewStubSearchService() + + service, err := newProvisioningServiceImpl( func(context.Context, string, dashboardstore.DashboardProvisioningService, org.Service, utils.DashboardStore, folder.Service) (dashboards.DashboardProvisioner, error) { return serviceTest.mock, nil }, nil, nil, + searchStub, ) - err := serviceTest.service.setDashboardProvisioner() + serviceTest.service = service require.NoError(t, err) ctx, cancel := context.WithCancel(context.Background())