[v11.0.x] Provisioning: Prevent provisioning folder errors from failing startup (#92588)
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)
Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com>
This commit is contained in:
co-authored by
Arati R.
parent
b690b4f245
commit
e31ff0175b
@@ -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"
|
||||
@@ -75,9 +76,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
|
||||
@@ -104,30 +104,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 {
|
||||
@@ -183,7 +180,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,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())
|
||||
|
||||
Reference in New Issue
Block a user