From 116809ed7f6f1df23b5b4a968aaee0d7f41b234e Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 19 Jan 2021 18:57:09 +0100 Subject: [PATCH] services/provisioning: Various cleanup (#30396) Signed-off-by: Arve Knudsen --- pkg/services/dashboards/dashboard_service.go | 9 ++- pkg/services/ldap/settings.go | 2 +- .../provisioning/dashboards/config_reader.go | 2 +- .../provisioning/dashboards/dashboard.go | 15 +++-- .../provisioning/dashboards/file_reader.go | 55 +++++++++++-------- .../dashboards/file_reader_test.go | 24 ++++---- .../sqlstore/dashboard_provisioning.go | 1 - 7 files changed, 58 insertions(+), 50 deletions(-) diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index 4d0559a3469..e55536af5e2 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -81,7 +81,8 @@ func (dr *dashboardServiceImpl) GetProvisionedDashboardDataByDashboardID(dashboa return cmd.Result, nil } -func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, validateAlerts bool, validateProvisionedDashboard bool) (*models.SaveDashboardCommand, error) { +func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, validateAlerts bool, + validateProvisionedDashboard bool) (*models.SaveDashboardCommand, error) { dash := dto.Dashboard dash.Title = strings.TrimSpace(dash.Title) @@ -216,9 +217,11 @@ func (dr *dashboardServiceImpl) updateAlerting(cmd *models.SaveDashboardCommand, return bus.Dispatch(&alertCmd) } -func (dr *dashboardServiceImpl) SaveProvisionedDashboard(dto *SaveDashboardDTO, provisioning *models.DashboardProvisioning) (*models.Dashboard, error) { +func (dr *dashboardServiceImpl) SaveProvisionedDashboard(dto *SaveDashboardDTO, + provisioning *models.DashboardProvisioning) (*models.Dashboard, error) { if err := validateDashboardRefreshInterval(dto.Dashboard); err != nil { - dr.log.Warn("Changing refresh interval for provisioned dashboard to minimum refresh interval", "dashboardUid", dto.Dashboard.Uid, "dashboardTitle", dto.Dashboard.Title, "minRefreshInterval", setting.MinRefreshInterval) + dr.log.Warn("Changing refresh interval for provisioned dashboard to minimum refresh interval", "dashboardUid", + dto.Dashboard.Uid, "dashboardTitle", dto.Dashboard.Title, "minRefreshInterval", setting.MinRefreshInterval) dto.Dashboard.Data.Set("refresh", setting.MinRefreshInterval) } diff --git a/pkg/services/ldap/settings.go b/pkg/services/ldap/settings.go index ffd5d48c893..9ed021519ab 100644 --- a/pkg/services/ldap/settings.go +++ b/pkg/services/ldap/settings.go @@ -74,7 +74,7 @@ func IsEnabled() bool { return setting.LDAPEnabled } -// ReloadConfig reads the config from the disc and caches it. +// ReloadConfig reads the config from the disk and caches it. func ReloadConfig() error { if !IsEnabled() { return nil diff --git a/pkg/services/provisioning/dashboards/config_reader.go b/pkg/services/provisioning/dashboards/config_reader.go index 92eb9a10eae..ca469cecf2e 100644 --- a/pkg/services/provisioning/dashboards/config_reader.go +++ b/pkg/services/provisioning/dashboards/config_reader.go @@ -110,7 +110,7 @@ func (cr *configReader) readConfig() ([]*config, error) { for uid, times := range uidUsage { if times > 1 { - cr.log.Error("the same 'folderUid' is used more than once", "folderUid", uid) + cr.log.Error("the same folder UID is used more than once", "folderUid", uid) } } diff --git a/pkg/services/provisioning/dashboards/dashboard.go b/pkg/services/provisioning/dashboards/dashboard.go index dfd5298980b..f15903d336c 100644 --- a/pkg/services/provisioning/dashboards/dashboard.go +++ b/pkg/services/provisioning/dashboards/dashboard.go @@ -11,8 +11,8 @@ import ( "github.com/grafana/grafana/pkg/util/errutil" ) -// DashboardProvisioner is responsible for syncing dashboard from disc to -// Grafanas database. +// DashboardProvisioner is responsible for syncing dashboard from disk to +// Grafana's database. type DashboardProvisioner interface { Provision() error PollChanges(ctx context.Context) @@ -24,7 +24,7 @@ type DashboardProvisioner interface { // DashboardProvisionerFactory creates DashboardProvisioners based on input type DashboardProvisionerFactory func(string) (DashboardProvisioner, error) -// Provisioner is responsible for syncing dashboard from disc to Grafanas database. +// Provisioner is responsible for syncing dashboard from disk to Grafana's database. type Provisioner struct { log log.Logger fileReaders []*FileReader @@ -42,7 +42,6 @@ func New(configDirectory string) (*Provisioner, error) { } fileReaders, err := getFileReaders(configs, logger) - if err != nil { return nil, errutil.Wrap("Failed to initialize file readers", err) } @@ -56,11 +55,11 @@ func New(configDirectory string) (*Provisioner, error) { return d, nil } -// Provision starts scanning the disc for dashboards and updates -// the database with the latest versions of those dashboards +// Provision scans the disk for dashboards and updates +// the database with the latest versions of those dashboards. func (provider *Provisioner) Provision() error { for _, reader := range provider.fileReaders { - if err := reader.startWalkingDisk(); err != nil { + if err := reader.walkDisk(); err != nil { if os.IsNotExist(err) { // don't stop the provisioning service in case the folder is missing. The folder can appear after the startup provider.log.Warn("Failed to provision config", "name", reader.Cfg.Name, "error", err) @@ -87,7 +86,7 @@ func (provider *Provisioner) CleanUpOrphanedDashboards() { } } -// PollChanges starts polling for changes in dashboard definition files. It creates goroutine for each provider +// PollChanges starts polling for changes in dashboard definition files. It creates a goroutine for each provider // defined in the config. func (provider *Provisioner) PollChanges(ctx context.Context) { for _, reader := range provider.fileReaders { diff --git a/pkg/services/provisioning/dashboards/file_reader.go b/pkg/services/provisioning/dashboards/file_reader.go index 576b62d4cf0..32ebd3bb9a3 100644 --- a/pkg/services/provisioning/dashboards/file_reader.go +++ b/pkg/services/provisioning/dashboards/file_reader.go @@ -23,9 +23,9 @@ var ( ErrFolderNameMissing = errors.New("folder name missing") ) -// FileReader is responsible for reading dashboards from disc and +// FileReader is responsible for reading dashboards from disk and // insert/update dashboards to the Grafana database using -// `dashboards.DashboardProvisioningService` +// `dashboards.DashboardProvisioningService`. type FileReader struct { Cfg *config Path string @@ -61,13 +61,13 @@ func NewDashboardFileReader(cfg *config, log log.Logger) (*FileReader, error) { }, nil } -// pollChanges periodically runs startWalkingDisk based on interval specified in the config. +// pollChanges periodically runs walkDisk based on interval specified in the config. func (fr *FileReader) pollChanges(ctx context.Context) { ticker := time.NewTicker(time.Duration(int64(time.Second) * fr.Cfg.UpdateIntervalSeconds)) for { select { case <-ticker.C: - if err := fr.startWalkingDisk(); err != nil { + if err := fr.walkDisk(); err != nil { fr.log.Error("failed to search for dashboards", "error", err) } case <-ctx.Done(): @@ -76,23 +76,23 @@ func (fr *FileReader) pollChanges(ctx context.Context) { } } -// startWalkingDisk traverses the file system for defined path, reads dashboard definition files and applies any change -// to the database. -func (fr *FileReader) startWalkingDisk() error { +// walkDisk traverses the file system for the defined path, reading dashboard definition files, +// and applies any change to the database. +func (fr *FileReader) walkDisk() error { fr.log.Debug("Start walking disk", "path", fr.Path) resolvedPath := fr.resolvedPath() if _, err := os.Stat(resolvedPath); err != nil { return err } - provisionedDashboardRefs, err := getProvisionedDashboardByPath(fr.dashboardProvisioningService, fr.Cfg.Name) + provisionedDashboardRefs, err := getProvisionedDashboardsByPath(fr.dashboardProvisioningService, fr.Cfg.Name) if err != nil { return err } + // Find relevant files filesFoundOnDisk := map[string]os.FileInfo{} - err = filepath.Walk(resolvedPath, createWalkFn(filesFoundOnDisk)) - if err != nil { + if err := filepath.Walk(resolvedPath, createWalkFn(filesFoundOnDisk)); err != nil { return err } @@ -115,7 +115,8 @@ func (fr *FileReader) startWalkingDisk() error { } // storeDashboardsInFolder saves dashboards from the filesystem on disk to the folder from config -func (fr *FileReader) storeDashboardsInFolder(filesFoundOnDisk map[string]os.FileInfo, dashboardRefs map[string]*models.DashboardProvisioning, sanityChecker *provisioningSanityChecker) error { +func (fr *FileReader) storeDashboardsInFolder(filesFoundOnDisk map[string]os.FileInfo, + dashboardRefs map[string]*models.DashboardProvisioning, sanityChecker *provisioningSanityChecker) error { folderID, err := getOrCreateFolderID(fr.Cfg, fr.dashboardProvisioningService, fr.Cfg.Folder) if err != nil && !errors.Is(err, ErrFolderNameMissing) { return err @@ -124,17 +125,20 @@ func (fr *FileReader) storeDashboardsInFolder(filesFoundOnDisk map[string]os.Fil // save dashboards based on json files for path, fileInfo := range filesFoundOnDisk { provisioningMetadata, err := fr.saveDashboard(path, folderID, fileInfo, dashboardRefs) - sanityChecker.track(provisioningMetadata) if err != nil { fr.log.Error("failed to save dashboard", "error", err) + continue } + + sanityChecker.track(provisioningMetadata) } return nil } // storeDashboardsInFoldersFromFilesystemStructure saves dashboards from the filesystem on disk to the same folder -// in grafana as they are in on the filesystem -func (fr *FileReader) storeDashboardsInFoldersFromFileStructure(filesFoundOnDisk map[string]os.FileInfo, dashboardRefs map[string]*models.DashboardProvisioning, resolvedPath string, sanityChecker *provisioningSanityChecker) error { +// in Grafana as they are in on the filesystem. +func (fr *FileReader) storeDashboardsInFoldersFromFileStructure(filesFoundOnDisk map[string]os.FileInfo, + dashboardRefs map[string]*models.DashboardProvisioning, resolvedPath string, sanityChecker *provisioningSanityChecker) error { for path, fileInfo := range filesFoundOnDisk { folderName := "" @@ -158,20 +162,21 @@ func (fr *FileReader) storeDashboardsInFoldersFromFileStructure(filesFoundOnDisk } // handleMissingDashboardFiles will unprovision or delete dashboards which are missing on disk. -func (fr *FileReader) handleMissingDashboardFiles(provisionedDashboardRefs map[string]*models.DashboardProvisioning, filesFoundOnDisk map[string]os.FileInfo) { +func (fr *FileReader) handleMissingDashboardFiles(provisionedDashboardRefs map[string]*models.DashboardProvisioning, + filesFoundOnDisk map[string]os.FileInfo) { // find dashboards to delete since json file is missing - var dashboardToDelete []int64 + var dashboardsToDelete []int64 for path, provisioningData := range provisionedDashboardRefs { _, existsOnDisk := filesFoundOnDisk[path] if !existsOnDisk { - dashboardToDelete = append(dashboardToDelete, provisioningData.DashboardId) + dashboardsToDelete = append(dashboardsToDelete, provisioningData.DashboardId) } } if fr.Cfg.DisableDeletion { // If deletion is disabled for the provisioner we just remove provisioning metadata about the dashboard // so afterwards the dashboard is considered unprovisioned. - for _, dashboardID := range dashboardToDelete { + for _, dashboardID := range dashboardsToDelete { fr.log.Debug("unprovisioning provisioned dashboard. missing on disk", "id", dashboardID) err := fr.dashboardProvisioningService.UnprovisionDashboard(dashboardID) if err != nil { @@ -179,9 +184,9 @@ func (fr *FileReader) handleMissingDashboardFiles(provisionedDashboardRefs map[s } } } else { - // delete dashboard that are missing json file - for _, dashboardID := range dashboardToDelete { - fr.log.Debug("deleting provisioned dashboard. missing on disk", "id", dashboardID) + // delete dashboards missing JSON file + for _, dashboardID := range dashboardsToDelete { + fr.log.Debug("deleting provisioned dashboard, missing on disk", "id", dashboardID) err := fr.dashboardProvisioningService.DeleteProvisionedDashboard(dashboardID, fr.Cfg.OrgID) if err != nil { fr.log.Error("failed to delete dashboard", "id", dashboardID, "error", err) @@ -191,7 +196,8 @@ func (fr *FileReader) handleMissingDashboardFiles(provisionedDashboardRefs map[s } // saveDashboard saves or updates the dashboard provisioning file at path. -func (fr *FileReader) saveDashboard(path string, folderID int64, fileInfo os.FileInfo, provisionedDashboardRefs map[string]*models.DashboardProvisioning) (provisioningMetadata, error) { +func (fr *FileReader) saveDashboard(path string, folderID int64, fileInfo os.FileInfo, + provisionedDashboardRefs map[string]*models.DashboardProvisioning) (provisioningMetadata, error) { provisioningMetadata := provisioningMetadata{} resolvedFileInfo, err := resolveSymlink(fileInfo, path) if err != nil { @@ -211,7 +217,7 @@ func (fr *FileReader) saveDashboard(path string, folderID int64, fileInfo os.Fil upToDate = true } - // keeps track of what uid's and title's we have already provisioned + // keeps track of which UIDs and titles we have already provisioned dash := jsonFile.dashboard provisioningMetadata.uid = dash.Dashboard.Uid provisioningMetadata.identity = dashboardIdentity{title: dash.Dashboard.Title, folderID: dash.Dashboard.FolderId} @@ -241,7 +247,8 @@ func (fr *FileReader) saveDashboard(path string, folderID int64, fileInfo os.Fil return provisioningMetadata, err } -func getProvisionedDashboardByPath(service dashboards.DashboardProvisioningService, name string) (map[string]*models.DashboardProvisioning, error) { +func getProvisionedDashboardsByPath(service dashboards.DashboardProvisioningService, name string) ( + map[string]*models.DashboardProvisioning, error) { arr, err := service.GetProvisionedDashboardData(name) if err != nil { return nil, err diff --git a/pkg/services/provisioning/dashboards/file_reader_test.go b/pkg/services/provisioning/dashboards/file_reader_test.go index 33fe6a0a683..946d487d5f5 100644 --- a/pkg/services/provisioning/dashboards/file_reader_test.go +++ b/pkg/services/provisioning/dashboards/file_reader_test.go @@ -18,17 +18,17 @@ import ( . "github.com/smartystreets/goconvey/convey" ) -var ( +const ( defaultDashboards = "testdata/test-dashboards/folder-one" brokenDashboards = "testdata/test-dashboards/broken-dashboards" oneDashboard = "testdata/test-dashboards/one-dashboard" containingID = "testdata/test-dashboards/containing-id" unprovision = "testdata/test-dashboards/unprovision" foldersFromFilesStructure = "testdata/test-dashboards/folders-from-files-structure" - - fakeService *fakeDashboardProvisioningService ) +var fakeService *fakeDashboardProvisioningService + func TestCreatingNewDashboardFileReader(t *testing.T) { Convey("creating new dashboard file reader", t, func() { cfg := &config{ @@ -111,7 +111,7 @@ func TestDashboardFileReader(t *testing.T) { reader, err := NewDashboardFileReader(cfg, logger) So(err, ShouldBeNil) - err = reader.startWalkingDisk() + err = reader.walkDisk() So(err, ShouldBeNil) folders := 0 @@ -142,7 +142,7 @@ func TestDashboardFileReader(t *testing.T) { reader, err := NewDashboardFileReader(cfg, logger) So(err, ShouldBeNil) - err = reader.startWalkingDisk() + err = reader.walkDisk() So(err, ShouldBeNil) So(len(fakeService.inserted), ShouldEqual, 1) @@ -154,7 +154,7 @@ func TestDashboardFileReader(t *testing.T) { reader, err := NewDashboardFileReader(cfg, logger) So(err, ShouldBeNil) - err = reader.startWalkingDisk() + err = reader.walkDisk() So(err, ShouldBeNil) So(len(fakeService.inserted), ShouldEqual, 1) @@ -167,7 +167,7 @@ func TestDashboardFileReader(t *testing.T) { reader, err := NewDashboardFileReader(cfg, logger) So(err, ShouldBeNil) - err = reader.startWalkingDisk() + err = reader.walkDisk() So(err, ShouldBeNil) So(len(fakeService.inserted), ShouldEqual, 5) @@ -226,13 +226,13 @@ func TestDashboardFileReader(t *testing.T) { reader1, err := NewDashboardFileReader(cfg1, logger) So(err, ShouldBeNil) - err = reader1.startWalkingDisk() + err = reader1.walkDisk() So(err, ShouldBeNil) reader2, err := NewDashboardFileReader(cfg2, logger) So(err, ShouldBeNil) - err = reader2.startWalkingDisk() + err = reader2.walkDisk() So(err, ShouldBeNil) var folderCount int @@ -319,7 +319,7 @@ func TestDashboardFileReader(t *testing.T) { absPath1, err := filepath.Abs(unprovision + "/dashboard1.json") So(err, ShouldBeNil) - // This one does not exist on disc, simulating a deleted file + // This one does not exist on disk, simulating a deleted file absPath2, err := filepath.Abs(unprovision + "/dashboard2.json") So(err, ShouldBeNil) @@ -336,7 +336,7 @@ func TestDashboardFileReader(t *testing.T) { reader, err := NewDashboardFileReader(cfg, logger) So(err, ShouldBeNil) - err = reader.startWalkingDisk() + err = reader.walkDisk() So(err, ShouldBeNil) So(len(fakeService.provisioned["Default"]), ShouldEqual, 1) @@ -347,7 +347,7 @@ func TestDashboardFileReader(t *testing.T) { reader, err := NewDashboardFileReader(cfg, logger) So(err, ShouldBeNil) - err = reader.startWalkingDisk() + err = reader.walkDisk() So(err, ShouldBeNil) So(len(fakeService.provisioned["Default"]), ShouldEqual, 1) diff --git a/pkg/services/sqlstore/dashboard_provisioning.go b/pkg/services/sqlstore/dashboard_provisioning.go index 77f9b2b9498..6c6f81154ae 100644 --- a/pkg/services/sqlstore/dashboard_provisioning.go +++ b/pkg/services/sqlstore/dashboard_provisioning.go @@ -38,7 +38,6 @@ func GetProvisionedDataByDashboardId(cmd *models.GetProvisionedDashboardDataById func SaveProvisionedDashboard(cmd *models.SaveProvisionedDashboardCommand) error { return inTransaction(func(sess *DBSession) error { err := saveDashboard(sess, cmd.DashboardCmd) - if err != nil { return err }