services/provisioning: Various cleanup (#30396)

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

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)
 		}
 	}
 

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 {

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

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)