Simplify folder controller schema, change folder uid if needed

api/v1beta1/grafanafolder_types.go

@@ -28,7 +28,8 @@ import (
 
 // GrafanaFolderSpec defines the desired state of GrafanaFolder
 type GrafanaFolderSpec struct {
-	Json string `json:"json,omitempty"`
+	// +optional
+	Title string `json:"title,omitempty"`
 
 	// selects Grafanas for import
 	InstanceSelector *metav1.LabelSelector `json:"instanceSelector"`
@@ -83,7 +84,7 @@ func (in *GrafanaFolderList) Find(namespace string, name string) *GrafanaFolder
 
 func (in *GrafanaFolder) Hash() string {
 	hash := sha256.New()
-	hash.Write([]byte(in.Spec.Json))
+	hash.Write([]byte(in.Spec.Title))
 	return fmt.Sprintf("%x", hash.Sum(nil))
 }
 
@@ -97,3 +98,11 @@ func (in *GrafanaFolder) IsAllowCrossNamespaceImport() bool {
 	}
 	return false
 }
+
+func (in *GrafanaFolder) GetTitle() string {
+	if in.Spec.Title != "" {
+		return in.Spec.Title
+	}
+
+	return in.Name
+}

api/v1beta1/grafanafolder_types_test.go

@@ -0,0 +1,41 @@
+package v1beta1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestGrafanaFolder_GetTitle(t *testing.T) {
+	tests := []struct {
+		name string
+		cr   GrafanaFolder
+		want string
+	}{
+		{
+			name: "No custom title",
+			cr: GrafanaFolder{
+				ObjectMeta: metav1.ObjectMeta{Name: "cr-name"},
+			},
+			want: "cr-name",
+		},
+		{
+			name: "Custom title",
+			cr: GrafanaFolder{
+				ObjectMeta: metav1.ObjectMeta{Name: "cr-name"},
+				Spec: GrafanaFolderSpec{
+					Title: "custom-title",
+				},
+			},
+			want: "custom-title",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.cr.GetTitle()
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

bundle/manifests/grafana.integreatly.org_grafanafolders.yaml

@@ -52,7 +52,7 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
-              json:
+              title:
                 type: string
             required:
             - instanceSelector

config/crd/bases/grafana.integreatly.org_grafanafolders.yaml

@@ -53,7 +53,7 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
-              json:
+              title:
                 type: string
             required:
             - instanceSelector

config/grafana.integreatly.org_grafanafolders.yaml

@@ -84,7 +84,7 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
-              json:
+              title:
                 type: string
             required:
             - instanceSelector

controllers/grafanafolder_controller.go

@@ -18,7 +18,6 @@ package controllers
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"strings"
 	"time"
@@ -178,6 +177,7 @@ func (r *GrafanaFolderReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	controllerLog.Info("found matching Grafana instances for folder", "count", len(instances.Items))
 
+	success := true
 	for _, grafana := range instances.Items {
 		// check if this is a cross namespace import
 		if grafana.Namespace != folder.Namespace && !folder.IsAllowCrossNamespaceImport() {
@@ -193,9 +193,14 @@ func (r *GrafanaFolderReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		err = r.onFolderCreated(ctx, &grafana, folder)
 		if err != nil {
 			controllerLog.Error(err, "error reconciling folder", "folder", folder.Name, "grafana", grafana.Name)
+			success = false
 		}
 	}
 
+	if !success {
+		return ctrl.Result{RequeueAfter: RequeueDelay}, nil
+	}
+
 	return ctrl.Result{}, nil
 }
 
@@ -270,64 +275,59 @@ func (r *GrafanaFolderReconciler) onFolderDeleted(ctx context.Context, namespace
 }
 
 func (r *GrafanaFolderReconciler) onFolderCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaFolder) error {
-	if cr.Spec.Json == "" {
-		return nil
-	}
+	title := cr.GetTitle()
+	uid := string(cr.UID)
 
 	grafanaClient, err := client2.NewGrafanaClient(ctx, r.Client, grafana)
 	if err != nil {
 		return err
 	}
 
-	exists, err := r.Exists(grafanaClient, cr)
+	exists, remoteUID, err := r.Exists(grafanaClient, cr)
 	if err != nil {
 		return err
 	}
-	if exists && cr.Unchanged() {
-		return nil
-	}
 
-	var folderFromJson map[string]interface{}
-	err = json.Unmarshal([]byte(cr.Spec.Json), &folderFromJson)
-	if err != nil {
-		return err
-	}
+	if exists {
+		// Add to status to cover cases:
+		// - operator have previously failed to update status
+		// - the folder was created outside of operator
+		// - the folder was created through dashboard controller
+		if found, _ := grafana.Status.Folders.Find(cr.Namespace, cr.Name); !found {
+			grafana.Status.Folders = grafana.Status.Folders.Add(cr.Namespace, cr.Name, uid)
+			err = r.Client.Status().Update(ctx, grafana)
+			if err != nil {
+				return err
+			}
+		}
 
-	title := fmt.Sprintf("%v", folderFromJson["title"])
-	if title == "" {
-		title = cr.Name
-	}
+		if cr.Unchanged() && uid == remoteUID {
+			return nil
+		}
 
-	// folder exists, update only
-	if exists && !cr.Unchanged() {
-		err = grafanaClient.UpdateFolder(string(cr.UID), title)
+		// Update title and replace uid if needed
+		err = grafanaClient.UpdateFolder(remoteUID, title, uid)
 		if err != nil {
 			return err
 		}
-
-		return r.UpdateStatus(ctx, cr)
-	}
-
-	folderFromClient, err := grafanaClient.NewFolder(title, string(cr.UID))
-	if err != nil {
-		// folder already exists in grafana, do nothing
-		if strings.Contains(err.Error(), "status: 409") {
-			return nil
+	} else {
+		folderFromClient, err := grafanaClient.NewFolder(title, uid)
+		if err != nil {
+			return err
 		}
-		return err
-	}
 
-	// FIXME our current version of the client doesn't return response codes, or any response for
-	// FIXME that matter, this needs an issue/feature request upstream
-	// FIXME for now, use the returned URL as an indicator that the folder was created instead
-	if folderFromClient.URL == "" && len(folderFromClient.URL) == 0 {
-		return errors.NewBadRequest(fmt.Sprintf("something went wrong trying to create folder %s in grafana %s", cr.Name, grafana.Name))
-	}
+		// FIXME our current version of the client doesn't return response codes, or any response for
+		// FIXME that matter, this needs an issue/feature request upstream
+		// FIXME for now, use the returned URL as an indicator that the folder was created instead
+		if folderFromClient.URL == "" {
+			return errors.NewBadRequest(fmt.Sprintf("something went wrong trying to create folder %s in grafana %s", cr.Name, grafana.Name))
+		}
 
-	grafana.Status.Folders = grafana.Status.Folders.Add(cr.Namespace, cr.Name, folderFromClient.UID)
-	err = r.Client.Status().Update(ctx, grafana)
-	if err != nil {
-		return err
+		grafana.Status.Folders = grafana.Status.Folders.Add(cr.Namespace, cr.Name, folderFromClient.UID)
+		err = r.Client.Status().Update(ctx, grafana)
+		if err != nil {
+			return err
+		}
 	}
 
 	return r.UpdateStatus(ctx, cr)
@@ -338,17 +338,22 @@ func (r *GrafanaFolderReconciler) UpdateStatus(ctx context.Context, cr *v1beta1.
 	return r.Client.Status().Update(ctx, cr)
 }
 
-func (r *GrafanaFolderReconciler) Exists(client *grapi.Client, cr *v1beta1.GrafanaFolder) (bool, error) {
+func (r *GrafanaFolderReconciler) Exists(client *grapi.Client, cr *v1beta1.GrafanaFolder) (bool, string, error) {
+	title := cr.GetTitle()
+	uid := string(cr.UID)
+
 	folders, err := client.Folders()
 	if err != nil {
-		return false, err
+		return false, "", err
 	}
+
 	for _, folder := range folders {
-		if folder.UID == string(cr.UID) {
-			return true, nil
+		if folder.UID == uid || strings.EqualFold(folder.Title, title) {
+			return true, folder.UID, nil
 		}
 	}
-	return false, nil
+
+	return false, "", nil
 }
 
 func (r *GrafanaFolderReconciler) GetMatchingFolderInstances(ctx context.Context, folder *v1beta1.GrafanaFolder, k8sClient client.Client) (v1beta1.GrafanaList, error) {

deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanafolders.yaml

@@ -53,7 +53,7 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
-              json:
+              title:
                 type: string
             required:
             - instanceSelector

deploy/kustomize/base/crds.yaml

@@ -423,7 +423,7 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
-              json:
+              title:
                 type: string
             required:
             - instanceSelector

docs/docs/api.md

@@ -903,7 +903,7 @@ GrafanaFolderSpec defines the desired state of GrafanaFolder
         </td>
         <td>false</td>
       </tr><tr>
-        <td><b>json</b></td>
+        <td><b>title</b></td>
         <td>string</td>
         <td>
           <br/>