[v10.4.x] Provisioning: Prevent provisioning folder errors from failing startup (#92591)

Provisioning: Prevent provisioning folder errors from failing startup (#92560) * Prevent provisioning folder errors from failing startup * Refactor setting of dashboard provisioner (cherry picked from commit 34149c86d0)
2024-08-28 15:40:37 +02:00
parent 596e03f56e
commit 11d5696ed0
3 changed files with 88 additions and 23 deletions
@@ -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)
@@ -2,6 +2,7 @@ package provisioning

 import (
 	"context"
+	"errors"
 	"fmt"
 	"path/filepath"
 	"sync"
@@ -78,9 +79,24 @@ func ProvideService(
 		orgService:                   orgService,
 		folderService:                folderService,
 	}
+
+	if err := s.setDashboardProvisioner(); err != nil {
+		return nil, err
+	}
+
 	return s, nil
 }

+func (ps *ProvisioningServiceImpl) setDashboardProvisioner() error {
+	dashboardPath := filepath.Join(ps.Cfg.ProvisioningPath, "dashboards")
+	dashProvisioner, err := ps.newDashboardProvisioner(context.Background(), dashboardPath, ps.dashboardProvisioningService, ps.orgService, ps.dashboardService, ps.folderService)
+	if err != nil {
+		return fmt.Errorf("%v: %w", "Failed to create provisioner", err)
+	}
+	ps.dashboardProvisioner = dashProvisioner
+	return nil
+}
+
 type ProvisioningService interface {
 	registry.BackgroundService
 	RunInitProvisioners(ctx context.Context) error
@@ -93,32 +109,29 @@ 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,
-		provisionNotifiers:      notifiers.Provision,
-		provisionDatasources:    datasources.Provision,
-		provisionPlugins:        plugins.Provision,
-	}
-}
-
 // Used for testing purposes
 func newProvisioningServiceImpl(
 	newDashboardProvisioner dashboards.DashboardProvisionerFactory,
 	provisionNotifiers func(context.Context, *setting.Cfg, string, notifiers.Manager, org.Service, encryption.Internal, *notifications.NotificationService) error,
 	provisionDatasources func(context.Context, string, datasources.Store, 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,
 		provisionNotifiers:      provisionNotifiers,
 		provisionDatasources:    provisionDatasources,
 		provisionPlugins:        provisionPlugins,
+		Cfg:                     setting.NewCfg(),
+		searchService:           searchService,
 	}
+
+	if err := s.setDashboardProvisioner(); err != nil {
+		return nil, err
+	}
+
+	return s, nil
 }

 type ProvisioningServiceImpl struct {
@@ -182,7 +195,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()
@@ -3,22 +3,24 @@ package provisioning
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 	"time"

 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"

 	dashboardstore "github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/folder"
 	"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/setting"
+	"github.com/grafana/grafana/pkg/services/searchV2"
 )

 func TestProvisioningServiceImpl(t *testing.T) {
 	t.Run("Restart dashboard provisioning and stop service", func(t *testing.T) {
-		serviceTest := setup()
+		serviceTest := setup(t)
 		err := serviceTest.service.ProvisionDashboards(context.Background())
 		assert.Nil(t, err)
 		serviceTest.startService()
@@ -45,7 +47,7 @@ func TestProvisioningServiceImpl(t *testing.T) {
 	})

 	t.Run("Failed reloading does not stop polling with old provisioned", func(t *testing.T) {
-		serviceTest := setup()
+		serviceTest := setup(t)
 		err := serviceTest.service.ProvisionDashboards(context.Background())
 		assert.Nil(t, err)
 		serviceTest.startService()
@@ -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 {
@@ -83,7 +125,7 @@ type serviceTestStruct struct {
 	service *ProvisioningServiceImpl
 }

-func setup() *serviceTestStruct {
+func setup(t *testing.T) *serviceTestStruct {
 	serviceTest := &serviceTestStruct{}
 	serviceTest.waitTimeout = time.Second

@@ -95,15 +137,19 @@ func setup() *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,
 		nil,
+		searchStub,
 	)
-	serviceTest.service.Cfg = setting.NewCfg()
+	serviceTest.service = service
+	require.NoError(t, err)

 	ctx, cancel := context.WithCancel(context.Background())
 	serviceTest.cancel = cancel