From d8c0fcabec84dea474f202b11877ce819bc67b8f Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 13:15:51 +0200 Subject: [PATCH 01/18] fix(datasource): calculate hash with referenced values --- api/v1beta1/grafanadatasource_types.go | 51 +------------------------- controllers/datasource_controller.go | 37 ++++++++----------- 2 files changed, 18 insertions(+), 70 deletions(-) diff --git a/api/v1beta1/grafanadatasource_types.go b/api/v1beta1/grafanadatasource_types.go index 753a72dcb..a1c7d9869 100644 --- a/api/v1beta1/grafanadatasource_types.go +++ b/api/v1beta1/grafanadatasource_types.go @@ -18,7 +18,6 @@ package v1beta1 import ( "bytes" - "crypto/sha256" "encoding/json" "errors" "fmt" @@ -125,52 +124,6 @@ type GrafanaDatasourceList struct { Items []GrafanaDatasource `json:"items"` } -func (in *GrafanaDatasource) Hash() string { - hash := sha256.New() - - if in.Spec.Datasource != nil { - hash.Write([]byte(in.Spec.Datasource.Name)) - hash.Write([]byte(in.Spec.Datasource.Access)) - hash.Write([]byte(in.Spec.Datasource.BasicAuthUser)) - hash.Write(in.Spec.Datasource.JSONData) - hash.Write(in.Spec.Datasource.SecureJSONData) - hash.Write([]byte(in.Spec.Datasource.Database)) - hash.Write([]byte(in.Spec.Datasource.Type)) - hash.Write([]byte(in.Spec.Datasource.User)) - hash.Write([]byte(in.Spec.Datasource.URL)) - - for _, valueRef := range in.Spec.ValuesFrom { - hash.Write([]byte(valueRef.TargetPath)) - if valueRef.ValueFrom.ConfigMapKeyRef != nil { - hash.Write([]byte(valueRef.ValueFrom.ConfigMapKeyRef.Name)) - hash.Write([]byte(valueRef.ValueFrom.ConfigMapKeyRef.Key)) - } - if valueRef.ValueFrom.SecretKeyRef != nil { - hash.Write([]byte(valueRef.ValueFrom.SecretKeyRef.Name)) - hash.Write([]byte(valueRef.ValueFrom.SecretKeyRef.Key)) - } - } - - if in.Spec.Datasource.BasicAuth != nil && *in.Spec.Datasource.BasicAuth { - hash.Write([]byte("_")) - } - - if in.Spec.Datasource.IsDefault != nil && *in.Spec.Datasource.IsDefault { - hash.Write([]byte("_")) - } - - if in.Spec.Datasource.OrgID != nil { - hash.Write([]byte(fmt.Sprint(*in.Spec.Datasource.OrgID))) - } - - if in.Spec.Datasource.Editable != nil && *in.Spec.Datasource.Editable { - hash.Write([]byte("_")) - } - } - - return fmt.Sprintf("%x", hash.Sum(nil)) -} - func (in *GrafanaDatasource) GetResyncPeriod() time.Duration { if in.Spec.ResyncPeriod == "" { in.Spec.ResyncPeriod = DefaultResyncPeriod @@ -186,8 +139,8 @@ func (in *GrafanaDatasource) GetResyncPeriod() time.Duration { return duration } -func (in *GrafanaDatasource) Unchanged() bool { - return in.Hash() == in.Status.Hash +func (in *GrafanaDatasource) Unchanged(hash string) bool { + return in.Status.Hash == hash } func (in *GrafanaDatasource) ExpandVariables(variables map[string][]byte) ([]byte, error) { diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index dcf94cd5e..4fa5446ec 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "crypto/sha256" "encoding/json" "fmt" "strings" @@ -215,10 +216,10 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re // if the datasource was successfully synced in all instances, wait for its re-sync period if success { datasource.Status.LastMessage = "" - return ctrl.Result{RequeueAfter: datasource.GetResyncPeriod()}, r.UpdateStatus(ctx, datasource) + return ctrl.Result{RequeueAfter: datasource.GetResyncPeriod()}, r.Client.Status().Update(ctx, datasource) } else { // if there was an issue with the datasource, update the status - return ctrl.Result{RequeueAfter: RequeueDelay}, r.UpdateStatus(ctx, datasource) + return ctrl.Result{RequeueAfter: RequeueDelay}, r.Client.Status().Update(ctx, datasource) } } @@ -286,7 +287,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g // always use the same uid for CR and datasource cr.Spec.Datasource.UID = string(cr.UID) - unmarshalledDatasource, err := r.getDatasourceContent(ctx, cr) + unmarshalledDatasource, hash, err := r.getDatasourceContent(ctx, cr) if err != nil { return err } @@ -297,7 +298,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g if err != nil && !strings.Contains(err.Error(), "status: 409") { return err } - case !cr.Unchanged(): + case !cr.Unchanged(hash): err := grafanaClient.UpdateDataSource(unmarshalledDatasource) if err != nil { return err @@ -307,17 +308,8 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g return nil } - err = r.UpdateStatus(ctx, cr) - if err != nil { - return err - } - grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, string(cr.UID)) - return r.Client.Status().Update(ctx, grafana) -} - -func (r *GrafanaDatasourceReconciler) UpdateStatus(ctx context.Context, cr *v1beta1.GrafanaDatasource) error { - cr.Status.Hash = cr.Hash() + cr.Status.Hash = hash return r.Client.Status().Update(ctx, cr) } @@ -388,37 +380,40 @@ func (r *GrafanaDatasourceReconciler) GetMatchingDatasourceInstances(ctx context return instances, err } -func (r *GrafanaDatasourceReconciler) getDatasourceContent(ctx context.Context, cr *v1beta1.GrafanaDatasource) (*gapi.DataSource, error) { +func (r *GrafanaDatasourceReconciler) getDatasourceContent(ctx context.Context, cr *v1beta1.GrafanaDatasource) (*gapi.DataSource, string, error) { initialBytes, err := json.Marshal(cr.Spec.Datasource) if err != nil { - return nil, err + return nil, "", err } simpleContent, err := simplejson.NewJson(initialBytes) if err != nil { - return nil, err + return nil, "", err } for _, ref := range cr.Spec.ValuesFrom { val, err := r.getReferencedValue(ctx, cr, &ref.ValueFrom) if err != nil { - return nil, err + return nil, "", err } simpleContent.SetPath(strings.Split(ref.TargetPath, "."), val) } newBytes, err := simpleContent.MarshalJSON() if err != nil { - return nil, err + return nil, "", err } var res gapi.DataSource err = json.Unmarshal(newBytes, &res) if err != nil { - return nil, err + return nil, "", err } - return &res, nil + hash := sha256.New() + hash.Write(newBytes) + + return &res, fmt.Sprintf("%x", hash.Sum(nil)), nil } func (r *GrafanaDatasourceReconciler) getReferencedValue(ctx context.Context, cr *v1beta1.GrafanaDatasource, source *v1beta1.GrafanaDatasourceValueFromSource) (string, error) { From 05c476d7bfb19d4de45795052592212882673d1c Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 13:47:01 +0200 Subject: [PATCH 02/18] feat(datasource): add last resync --- api/v1beta1/grafanadatasource_types.go | 8 ++++ api/v1beta1/zz_generated.deepcopy.go | 3 +- ...na.integreatly.org_grafanadatasources.yaml | 7 ++++ ...na.integreatly.org_grafanadatasources.yaml | 7 ++++ ...na.integreatly.org_grafanadatasources.yaml | 8 ++++ controllers/datasource_controller.go | 40 ++++++++++--------- ...na.integreatly.org_grafanadatasources.yaml | 7 ++++ deploy/kustomize/base/crds.yaml | 8 ++++ docs/docs/api.md | 9 +++++ 9 files changed, 78 insertions(+), 19 deletions(-) diff --git a/api/v1beta1/grafanadatasource_types.go b/api/v1beta1/grafanadatasource_types.go index a1c7d9869..eb37e8542 100644 --- a/api/v1beta1/grafanadatasource_types.go +++ b/api/v1beta1/grafanadatasource_types.go @@ -99,6 +99,8 @@ type GrafanaDatasourceStatus struct { LastMessage string `json:"lastMessage,omitempty"` // The datasource instanceSelector can't find matching grafana instances NoMatchingInstances bool `json:"NoMatchingInstances,omitempty"` + // Last time the datasource was resynced + LastResync metav1.Time `json:"lastResync,omitempty"` } //+kubebuilder:object:root=true @@ -106,6 +108,7 @@ type GrafanaDatasourceStatus struct { // GrafanaDatasource is the Schema for the grafanadatasources API // +kubebuilder:printcolumn:name="No matching instances",type="boolean",JSONPath=".status.NoMatchingInstances",description="" +// +kubebuilder:printcolumn:name="Last resync",type="date",format="date-time",JSONPath=".status.lastResync",description="" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="" type GrafanaDatasource struct { metav1.TypeMeta `json:",inline"` @@ -139,6 +142,11 @@ func (in *GrafanaDatasource) GetResyncPeriod() time.Duration { return duration } +func (in *GrafanaDatasource) ResyncPeriodHasElapsed() bool { + deadline := in.Status.LastResync.Add(in.GetResyncPeriod()) + return time.Now().After(deadline) +} + func (in *GrafanaDatasource) Unchanged(hash string) bool { return in.Status.Hash == hash } diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 506a64de6..52216f215 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -522,7 +522,7 @@ func (in *GrafanaDatasource) DeepCopyInto(out *GrafanaDatasource) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GrafanaDatasource. @@ -665,6 +665,7 @@ func (in *GrafanaDatasourceSpec) DeepCopy() *GrafanaDatasourceSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GrafanaDatasourceStatus) DeepCopyInto(out *GrafanaDatasourceStatus) { *out = *in + in.LastResync.DeepCopyInto(&out.LastResync) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GrafanaDatasourceStatus. diff --git a/bundle/manifests/grafana.integreatly.org_grafanadatasources.yaml b/bundle/manifests/grafana.integreatly.org_grafanadatasources.yaml index e0eafe36a..91a78ff65 100644 --- a/bundle/manifests/grafana.integreatly.org_grafanadatasources.yaml +++ b/bundle/manifests/grafana.integreatly.org_grafanadatasources.yaml @@ -18,6 +18,10 @@ spec: - jsonPath: .status.NoMatchingInstances name: No matching instances type: boolean + - format: date-time + jsonPath: .status.lastResync + name: Last resync + type: date - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -155,6 +159,9 @@ spec: type: string lastMessage: type: string + lastResync: + format: date-time + type: string type: object type: object served: true diff --git a/config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml b/config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml index c30decd5e..79b949444 100644 --- a/config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml +++ b/config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml @@ -19,6 +19,10 @@ spec: - jsonPath: .status.NoMatchingInstances name: No matching instances type: boolean + - format: date-time + jsonPath: .status.lastResync + name: Last resync + type: date - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -156,6 +160,9 @@ spec: type: string lastMessage: type: string + lastResync: + format: date-time + type: string type: object type: object served: true diff --git a/config/grafana.integreatly.org_grafanadatasources.yaml b/config/grafana.integreatly.org_grafanadatasources.yaml index 0fa75eee4..95f42159b 100644 --- a/config/grafana.integreatly.org_grafanadatasources.yaml +++ b/config/grafana.integreatly.org_grafanadatasources.yaml @@ -19,6 +19,10 @@ spec: - jsonPath: .status.NoMatchingInstances name: No matching instances type: boolean + - format: date-time + jsonPath: .status.lastResync + name: Last resync + type: date - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -207,6 +211,10 @@ spec: type: string lastMessage: type: string + lastResync: + description: Last time the datasource was resynced + format: date-time + type: string type: object type: object served: true diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index 4fa5446ec..7b96d81b0 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -33,6 +33,7 @@ import ( client2 "github.com/grafana-operator/grafana-operator/v5/controllers/client" gapi "github.com/grafana/grafana-api-golang-client" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -280,50 +281,53 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g return err } - id, err := r.ExistingId(grafanaClient, cr) + // always use the same uid for CR and datasource + cr.Spec.Datasource.UID = string(cr.UID) + unmarshalledDatasource, hash, err := r.getDatasourceContent(ctx, cr) if err != nil { return err } - // always use the same uid for CR and datasource - cr.Spec.Datasource.UID = string(cr.UID) - unmarshalledDatasource, hash, err := r.getDatasourceContent(ctx, cr) + exists, err := r.Exists(grafanaClient, cr) if err != nil { return err } - switch { - case id == nil: - _, err = grafanaClient.NewDataSource(unmarshalledDatasource) - if err != nil && !strings.Contains(err.Error(), "status: 409") { - return err - } - case !cr.Unchanged(hash): + if exists && cr.Unchanged(hash) && !cr.ResyncPeriodHasElapsed() { + return nil + } + + if exists { err := grafanaClient.UpdateDataSource(unmarshalledDatasource) if err != nil { return err } - default: - // datasource exists and is unchanged, nothing to do - return nil + } else { + _, err = grafanaClient.NewDataSource(unmarshalledDatasource) + if err != nil && !strings.Contains(err.Error(), "status: 409") { + return err + } } grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, string(cr.UID)) cr.Status.Hash = hash + cr.Status.LastResync = metav1.Time{Time: time.Now()} return r.Client.Status().Update(ctx, cr) } -func (r *GrafanaDatasourceReconciler) ExistingId(client *gapi.Client, cr *v1beta1.GrafanaDatasource) (*int64, error) { +func (r *GrafanaDatasourceReconciler) Exists(client *gapi.Client, cr *v1beta1.GrafanaDatasource) (bool, error) { datasources, err := client.DataSources() if err != nil { - return nil, err + return false, err } + for _, datasource := range datasources { if datasource.UID == string(cr.UID) { - return &datasource.ID, nil + return true, nil } } - return nil, nil + + return false, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadatasources.yaml b/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadatasources.yaml index c30decd5e..79b949444 100644 --- a/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadatasources.yaml +++ b/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadatasources.yaml @@ -19,6 +19,10 @@ spec: - jsonPath: .status.NoMatchingInstances name: No matching instances type: boolean + - format: date-time + jsonPath: .status.lastResync + name: Last resync + type: date - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -156,6 +160,9 @@ spec: type: string lastMessage: type: string + lastResync: + format: date-time + type: string type: object type: object served: true diff --git a/deploy/kustomize/base/crds.yaml b/deploy/kustomize/base/crds.yaml index 3f9fb0cae..18a6910ad 100644 --- a/deploy/kustomize/base/crds.yaml +++ b/deploy/kustomize/base/crds.yaml @@ -207,6 +207,10 @@ spec: - jsonPath: .status.NoMatchingInstances name: No matching instances type: boolean + - format: date-time + jsonPath: .status.lastResync + name: Last resync + type: date - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -395,6 +399,10 @@ spec: type: string lastMessage: type: string + lastResync: + description: Last time the datasource was resynced + format: date-time + type: string type: object type: object served: true diff --git a/docs/docs/api.md b/docs/docs/api.md index 40c7b64ea..9ea4ba598 100644 --- a/docs/docs/api.md +++ b/docs/docs/api.md @@ -965,6 +965,15 @@ GrafanaDatasourceStatus defines the observed state of GrafanaDatasource
false + + lastResync + string + + Last time the datasource was resynced
+
+ Format: date-time
+ + false From 38674c9e624731f14d3f8d5d5899d35678abca0f Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 13:55:16 +0200 Subject: [PATCH 03/18] fix(datasource): fail on 409 code --- controllers/datasource_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index 7b96d81b0..0fa9c9fb8 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -304,7 +304,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g } } else { _, err = grafanaClient.NewDataSource(unmarshalledDatasource) - if err != nil && !strings.Contains(err.Error(), "status: 409") { + if err != nil { return err } } From 72b5f2e68e23c7c14ce4779a53eef083fd3e25bd Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 14:16:00 +0200 Subject: [PATCH 04/18] fix(datasource): get content once, update hash and lastresync once --- controllers/datasource_controller.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index 0fa9c9fb8..573b38f27 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -178,6 +178,12 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re controllerLog.Info("found matching Grafana instances for datasource", "count", len(instances.Items)) + unmarshalledDatasource, hash, err := r.getDatasourceContent(ctx, datasource) + if err != nil { + controllerLog.Error(err, "could not retrieve datasource contents", "name", datasource.Name, "namespace", datasource.Namespace) + return ctrl.Result{RequeueAfter: RequeueDelay}, err + } + success := true for _, grafana := range instances.Items { // check if this is a cross namespace import @@ -206,7 +212,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re } // then import the datasource into the matching grafana instances - err = r.onDatasourceCreated(ctx, &grafana, datasource) + err = r.onDatasourceCreated(ctx, &grafana, datasource, unmarshalledDatasource, hash) if err != nil { success = false datasource.Status.LastMessage = err.Error() @@ -217,6 +223,8 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re // if the datasource was successfully synced in all instances, wait for its re-sync period if success { datasource.Status.LastMessage = "" + datasource.Status.Hash = hash + datasource.Status.LastResync = metav1.Time{Time: time.Now()} return ctrl.Result{RequeueAfter: datasource.GetResyncPeriod()}, r.Client.Status().Update(ctx, datasource) } else { // if there was an issue with the datasource, update the status @@ -267,15 +275,15 @@ func (r *GrafanaDatasourceReconciler) onDatasourceDeleted(ctx context.Context, n return nil } -func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaDatasource) error { - if cr.Spec.Datasource == nil { - return nil - } - +func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaDatasource, unmarshalledDatasource *gapi.DataSource, hash string) error { if grafana.IsExternal() && cr.Spec.Plugins != nil { return fmt.Errorf("external grafana instances don't support plugins, please remove spec.plugins from your datasource cr") } + if cr.Spec.Datasource == nil { + return nil + } + grafanaClient, err := client2.NewGrafanaClient(ctx, r.Client, grafana) if err != nil { return err @@ -283,10 +291,6 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g // always use the same uid for CR and datasource cr.Spec.Datasource.UID = string(cr.UID) - unmarshalledDatasource, hash, err := r.getDatasourceContent(ctx, cr) - if err != nil { - return err - } exists, err := r.Exists(grafanaClient, cr) if err != nil { @@ -310,8 +314,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g } grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, string(cr.UID)) - cr.Status.Hash = hash - cr.Status.LastResync = metav1.Time{Time: time.Now()} + // TODO: do we need to update status at all? return r.Client.Status().Update(ctx, cr) } From 642c4938d425e81c99493f2b85ed0cf602786d0b Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 14:21:28 +0200 Subject: [PATCH 05/18] fix(datasource): update status of grafana in onDatasourceCreated (added bug in my branch) --- controllers/datasource_controller.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index 573b38f27..a9fe0563d 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -314,8 +314,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g } grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, string(cr.UID)) - // TODO: do we need to update status at all? - return r.Client.Status().Update(ctx, cr) + return r.Client.Status().Update(ctx, grafana) } func (r *GrafanaDatasourceReconciler) Exists(client *gapi.Client, cr *v1beta1.GrafanaDatasource) (bool, error) { From 4fd57324ce20d67bb1aba24232cc416ded969bd1 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 15:22:13 +0200 Subject: [PATCH 06/18] feat(datasource): hardcoded uid --- api/v1beta1/grafanadatasource_types.go | 10 ++++ ...na.integreatly.org_grafanadatasources.yaml | 2 + ...na.integreatly.org_grafanadatasources.yaml | 2 + ...na.integreatly.org_grafanadatasources.yaml | 2 + controllers/datasource_controller.go | 48 ++++++++++++++++--- ...na.integreatly.org_grafanadatasources.yaml | 2 + deploy/kustomize/base/crds.yaml | 2 + docs/docs/api.md | 7 +++ 8 files changed, 68 insertions(+), 7 deletions(-) diff --git a/api/v1beta1/grafanadatasource_types.go b/api/v1beta1/grafanadatasource_types.go index eb37e8542..5d2d31c06 100644 --- a/api/v1beta1/grafanadatasource_types.go +++ b/api/v1beta1/grafanadatasource_types.go @@ -101,6 +101,7 @@ type GrafanaDatasourceStatus struct { NoMatchingInstances bool `json:"NoMatchingInstances,omitempty"` // Last time the datasource was resynced LastResync metav1.Time `json:"lastResync,omitempty"` + UID string `json:"uid,omitempty"` } //+kubebuilder:object:root=true @@ -151,6 +152,15 @@ func (in *GrafanaDatasource) Unchanged(hash string) bool { return in.Status.Hash == hash } +func (in *GrafanaDatasource) IsUpdatedUID(uid string) bool { + // Datasource has just been created, status is not yet updated + if in.Status.UID == "" { + return false + } + + return in.Status.UID != uid +} + func (in *GrafanaDatasource) ExpandVariables(variables map[string][]byte) ([]byte, error) { if in.Spec.Datasource == nil { return nil, errors.New("data source is empty, can't expand variables") diff --git a/bundle/manifests/grafana.integreatly.org_grafanadatasources.yaml b/bundle/manifests/grafana.integreatly.org_grafanadatasources.yaml index 91a78ff65..98b49cb3d 100644 --- a/bundle/manifests/grafana.integreatly.org_grafanadatasources.yaml +++ b/bundle/manifests/grafana.integreatly.org_grafanadatasources.yaml @@ -162,6 +162,8 @@ spec: lastResync: format: date-time type: string + uid: + type: string type: object type: object served: true diff --git a/config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml b/config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml index 79b949444..05f521715 100644 --- a/config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml +++ b/config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml @@ -163,6 +163,8 @@ spec: lastResync: format: date-time type: string + uid: + type: string type: object type: object served: true diff --git a/config/grafana.integreatly.org_grafanadatasources.yaml b/config/grafana.integreatly.org_grafanadatasources.yaml index 95f42159b..6cfed7cbb 100644 --- a/config/grafana.integreatly.org_grafanadatasources.yaml +++ b/config/grafana.integreatly.org_grafanadatasources.yaml @@ -215,6 +215,8 @@ spec: description: Last time the datasource was resynced format: date-time type: string + uid: + type: string type: object type: object served: true diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index a9fe0563d..76d73ae72 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -170,6 +170,13 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re controllerLog.Error(err, "error getting grafana datasource cr") return ctrl.Result{RequeueAfter: RequeueDelay}, err } + + if datasource.Spec.Datasource == nil { + controllerLog.Info("skipped datasource with empty spec", datasource.Name, datasource.Namespace) + // TODO: add a custom status around that? + return ctrl.Result{}, nil + } + instances, err := r.GetMatchingDatasourceInstances(ctx, datasource, r.Client) if err != nil { controllerLog.Error(err, "could not find matching instances", "name", datasource.Name, "namespace", datasource.Namespace) @@ -184,6 +191,27 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{RequeueAfter: RequeueDelay}, err } + uid := unmarshalledDatasource.UID + + if datasource.IsUpdatedUID(uid) { + controllerLog.Info("datasource uid got updated, deleting datasources with the old uid") + err = r.onDatasourceDeleted(ctx, req.Namespace, req.Name) + if err != nil { + return ctrl.Result{RequeueAfter: RequeueDelay}, err + } + + // Clean up uid, so further reconcilications can track changes there + datasource.Status.UID = "" + err = r.Client.Status().Update(ctx, datasource) + + if err != nil { + return ctrl.Result{RequeueAfter: RequeueDelay}, err + } + + // Status update should trigger the next reconciliation right away, no need to requeue for dashboard creation + return ctrl.Result{}, nil + } + success := true for _, grafana := range instances.Items { // check if this is a cross namespace import @@ -225,6 +253,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re datasource.Status.LastMessage = "" datasource.Status.Hash = hash datasource.Status.LastResync = metav1.Time{Time: time.Now()} + datasource.Status.UID = uid return ctrl.Result{RequeueAfter: datasource.GetResyncPeriod()}, r.Client.Status().Update(ctx, datasource) } else { // if there was an issue with the datasource, update the status @@ -284,15 +313,14 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g return nil } + uid := unmarshalledDatasource.UID + grafanaClient, err := client2.NewGrafanaClient(ctx, r.Client, grafana) if err != nil { return err } - // always use the same uid for CR and datasource - cr.Spec.Datasource.UID = string(cr.UID) - - exists, err := r.Exists(grafanaClient, cr) + exists, err := r.Exists(grafanaClient, uid) if err != nil { return err } @@ -313,18 +341,19 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g } } - grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, string(cr.UID)) + grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, uid) return r.Client.Status().Update(ctx, grafana) } -func (r *GrafanaDatasourceReconciler) Exists(client *gapi.Client, cr *v1beta1.GrafanaDatasource) (bool, error) { +func (r *GrafanaDatasourceReconciler) Exists(client *gapi.Client, uid string) (bool, error) { datasources, err := client.DataSources() if err != nil { return false, err } for _, datasource := range datasources { - if datasource.UID == string(cr.UID) { + // TODO: should we inspect name (case-sensitive) as well? + if datasource.UID == uid { return true, nil } } @@ -397,6 +426,11 @@ func (r *GrafanaDatasourceReconciler) getDatasourceContent(ctx context.Context, return nil, "", err } + // TODO: test + if cr.Spec.Datasource.UID == "" { + simpleContent.Set("uid", string(cr.UID)) + } + for _, ref := range cr.Spec.ValuesFrom { val, err := r.getReferencedValue(ctx, cr, &ref.ValueFrom) if err != nil { diff --git a/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadatasources.yaml b/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadatasources.yaml index 79b949444..05f521715 100644 --- a/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadatasources.yaml +++ b/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadatasources.yaml @@ -163,6 +163,8 @@ spec: lastResync: format: date-time type: string + uid: + type: string type: object type: object served: true diff --git a/deploy/kustomize/base/crds.yaml b/deploy/kustomize/base/crds.yaml index 18a6910ad..a6cbe702e 100644 --- a/deploy/kustomize/base/crds.yaml +++ b/deploy/kustomize/base/crds.yaml @@ -403,6 +403,8 @@ spec: description: Last time the datasource was resynced format: date-time type: string + uid: + type: string type: object type: object served: true diff --git a/docs/docs/api.md b/docs/docs/api.md index 9ea4ba598..5198fb261 100644 --- a/docs/docs/api.md +++ b/docs/docs/api.md @@ -974,6 +974,13 @@ GrafanaDatasourceStatus defines the observed state of GrafanaDatasource Format: date-time
false + + uid + string + +
+ + false From 4d448b2fbd0a3256217a3761111b77a3f707a71c Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 16:01:42 +0200 Subject: [PATCH 07/18] fix(datasource): handle manually recreated datasource, update uid for existing datasource --- controllers/datasource_controller.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index 76d73ae72..42bde80cb 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -278,15 +278,17 @@ func (r *GrafanaDatasourceReconciler) onDatasourceDeleted(ctx context.Context, n } datasource, err := grafanaClient.DataSourceByUID(*uid) - if err != nil { - return err - } - - err = grafanaClient.DeleteDataSource(datasource.ID) if err != nil { if !strings.Contains(err.Error(), "status: 404") { return err } + } else { + err = grafanaClient.DeleteDataSource(datasource.ID) + if err != nil { + if !strings.Contains(err.Error(), "status: 404") { + return err + } + } } if grafana.IsInternal() { @@ -320,7 +322,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g return err } - exists, err := r.Exists(grafanaClient, uid) + exists, id, err := r.Exists(grafanaClient, unmarshalledDatasource.UID, unmarshalledDatasource.Name) if err != nil { return err } @@ -330,6 +332,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g } if exists { + unmarshalledDatasource.ID = id err := grafanaClient.UpdateDataSource(unmarshalledDatasource) if err != nil { return err @@ -345,20 +348,20 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g return r.Client.Status().Update(ctx, grafana) } -func (r *GrafanaDatasourceReconciler) Exists(client *gapi.Client, uid string) (bool, error) { +func (r *GrafanaDatasourceReconciler) Exists(client *gapi.Client, uid, name string) (bool, int64, error) { datasources, err := client.DataSources() if err != nil { - return false, err + return false, 0, err } for _, datasource := range datasources { // TODO: should we inspect name (case-sensitive) as well? - if datasource.UID == uid { - return true, nil + if datasource.UID == uid || datasource.Name == name { + return true, datasource.ID, nil } } - return false, nil + return false, 0, nil } // SetupWithManager sets up the controller with the Manager. From 42443de3ca9b597e13fa39f15cf710d9f7a16840 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 16:36:50 +0200 Subject: [PATCH 08/18] chore(datasource): improve naming in onDatasourceCreated --- controllers/datasource_controller.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index 42bde80cb..0276cc38c 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -306,7 +306,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceDeleted(ctx context.Context, n return nil } -func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaDatasource, unmarshalledDatasource *gapi.DataSource, hash string) error { +func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaDatasource, datasource *gapi.DataSource, hash string) error { if grafana.IsExternal() && cr.Spec.Plugins != nil { return fmt.Errorf("external grafana instances don't support plugins, please remove spec.plugins from your datasource cr") } @@ -315,14 +315,12 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g return nil } - uid := unmarshalledDatasource.UID - grafanaClient, err := client2.NewGrafanaClient(ctx, r.Client, grafana) if err != nil { return err } - exists, id, err := r.Exists(grafanaClient, unmarshalledDatasource.UID, unmarshalledDatasource.Name) + exists, id, err := r.Exists(grafanaClient, datasource.UID, datasource.Name) if err != nil { return err } @@ -332,19 +330,19 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g } if exists { - unmarshalledDatasource.ID = id - err := grafanaClient.UpdateDataSource(unmarshalledDatasource) + datasource.ID = id + err := grafanaClient.UpdateDataSource(datasource) if err != nil { return err } } else { - _, err = grafanaClient.NewDataSource(unmarshalledDatasource) + _, err = grafanaClient.NewDataSource(datasource) if err != nil { return err } } - grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, uid) + grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, datasource.UID) return r.Client.Status().Update(ctx, grafana) } @@ -355,7 +353,6 @@ func (r *GrafanaDatasourceReconciler) Exists(client *gapi.Client, uid, name stri } for _, datasource := range datasources { - // TODO: should we inspect name (case-sensitive) as well? if datasource.UID == uid || datasource.Name == name { return true, datasource.ID, nil } From bd942726235506bf9a6e5abcf946b419b5da377e Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 16:41:56 +0200 Subject: [PATCH 09/18] chore(datasource): improve naming in Reconcile --- controllers/datasource_controller.go | 46 ++++++++++++++-------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index 0276cc38c..206c7eb5b 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -154,11 +154,11 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re return syncResult, err } - datasource := &v1beta1.GrafanaDatasource{} + cr := &v1beta1.GrafanaDatasource{} err := r.Client.Get(ctx, client.ObjectKey{ Namespace: req.Namespace, Name: req.Name, - }, datasource) + }, cr) if err != nil { if errors.IsNotFound(err) { err = r.onDatasourceDeleted(ctx, req.Namespace, req.Name) @@ -171,29 +171,29 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{RequeueAfter: RequeueDelay}, err } - if datasource.Spec.Datasource == nil { - controllerLog.Info("skipped datasource with empty spec", datasource.Name, datasource.Namespace) + if cr.Spec.Datasource == nil { + controllerLog.Info("skipped datasource with empty spec", cr.Name, cr.Namespace) // TODO: add a custom status around that? return ctrl.Result{}, nil } - instances, err := r.GetMatchingDatasourceInstances(ctx, datasource, r.Client) + instances, err := r.GetMatchingDatasourceInstances(ctx, cr, r.Client) if err != nil { - controllerLog.Error(err, "could not find matching instances", "name", datasource.Name, "namespace", datasource.Namespace) + controllerLog.Error(err, "could not find matching instances", "name", cr.Name, "namespace", cr.Namespace) return ctrl.Result{RequeueAfter: RequeueDelay}, err } controllerLog.Info("found matching Grafana instances for datasource", "count", len(instances.Items)) - unmarshalledDatasource, hash, err := r.getDatasourceContent(ctx, datasource) + unmarshalledDatasource, hash, err := r.getDatasourceContent(ctx, cr) if err != nil { - controllerLog.Error(err, "could not retrieve datasource contents", "name", datasource.Name, "namespace", datasource.Namespace) + controllerLog.Error(err, "could not retrieve datasource contents", "name", cr.Name, "namespace", cr.Namespace) return ctrl.Result{RequeueAfter: RequeueDelay}, err } uid := unmarshalledDatasource.UID - if datasource.IsUpdatedUID(uid) { + if cr.IsUpdatedUID(uid) { controllerLog.Info("datasource uid got updated, deleting datasources with the old uid") err = r.onDatasourceDeleted(ctx, req.Namespace, req.Name) if err != nil { @@ -201,8 +201,8 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re } // Clean up uid, so further reconcilications can track changes there - datasource.Status.UID = "" - err = r.Client.Status().Update(ctx, datasource) + cr.Status.UID = "" + err = r.Client.Status().Update(ctx, cr) if err != nil { return ctrl.Result{RequeueAfter: RequeueDelay}, err @@ -215,7 +215,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re success := true for _, grafana := range instances.Items { // check if this is a cross namespace import - if grafana.Namespace != datasource.Namespace && !datasource.IsAllowCrossNamespaceImport() { + if grafana.Namespace != cr.Namespace && !cr.IsAllowCrossNamespaceImport() { continue } @@ -232,32 +232,32 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re // first reconcile the plugins // append the requested datasources to a configmap from where the // grafana reconciler will pick them upi - err = ReconcilePlugins(ctx, r.Client, r.Scheme, &grafana, datasource.Spec.Plugins, fmt.Sprintf("%v-datasource", datasource.Name)) + err = ReconcilePlugins(ctx, r.Client, r.Scheme, &grafana, cr.Spec.Plugins, fmt.Sprintf("%v-datasource", cr.Name)) if err != nil { success = false - controllerLog.Error(err, "error reconciling plugins", "datasource", datasource.Name, "grafana", grafana.Name) + controllerLog.Error(err, "error reconciling plugins", "datasource", cr.Name, "grafana", grafana.Name) } } // then import the datasource into the matching grafana instances - err = r.onDatasourceCreated(ctx, &grafana, datasource, unmarshalledDatasource, hash) + err = r.onDatasourceCreated(ctx, &grafana, cr, unmarshalledDatasource, hash) if err != nil { success = false - datasource.Status.LastMessage = err.Error() - controllerLog.Error(err, "error reconciling datasource", "datasource", datasource.Name, "grafana", grafana.Name) + cr.Status.LastMessage = err.Error() + controllerLog.Error(err, "error reconciling datasource", "datasource", cr.Name, "grafana", grafana.Name) } } // if the datasource was successfully synced in all instances, wait for its re-sync period if success { - datasource.Status.LastMessage = "" - datasource.Status.Hash = hash - datasource.Status.LastResync = metav1.Time{Time: time.Now()} - datasource.Status.UID = uid - return ctrl.Result{RequeueAfter: datasource.GetResyncPeriod()}, r.Client.Status().Update(ctx, datasource) + cr.Status.LastMessage = "" + cr.Status.Hash = hash + cr.Status.LastResync = metav1.Time{Time: time.Now()} + cr.Status.UID = uid + return ctrl.Result{RequeueAfter: cr.GetResyncPeriod()}, r.Client.Status().Update(ctx, cr) } else { // if there was an issue with the datasource, update the status - return ctrl.Result{RequeueAfter: RequeueDelay}, r.Client.Status().Update(ctx, datasource) + return ctrl.Result{RequeueAfter: RequeueDelay}, r.Client.Status().Update(ctx, cr) } } From 8382943aa022b0da9259edf87334f4e9a618c259 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 16:43:26 +0200 Subject: [PATCH 10/18] chore(datasource): improve naming in Reconcile --- controllers/datasource_controller.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index 206c7eb5b..f876248ea 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -185,15 +185,13 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re controllerLog.Info("found matching Grafana instances for datasource", "count", len(instances.Items)) - unmarshalledDatasource, hash, err := r.getDatasourceContent(ctx, cr) + datasource, hash, err := r.getDatasourceContent(ctx, cr) if err != nil { controllerLog.Error(err, "could not retrieve datasource contents", "name", cr.Name, "namespace", cr.Namespace) return ctrl.Result{RequeueAfter: RequeueDelay}, err } - uid := unmarshalledDatasource.UID - - if cr.IsUpdatedUID(uid) { + if cr.IsUpdatedUID(datasource.UID) { controllerLog.Info("datasource uid got updated, deleting datasources with the old uid") err = r.onDatasourceDeleted(ctx, req.Namespace, req.Name) if err != nil { @@ -240,7 +238,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re } // then import the datasource into the matching grafana instances - err = r.onDatasourceCreated(ctx, &grafana, cr, unmarshalledDatasource, hash) + err = r.onDatasourceCreated(ctx, &grafana, cr, datasource, hash) if err != nil { success = false cr.Status.LastMessage = err.Error() @@ -253,7 +251,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re cr.Status.LastMessage = "" cr.Status.Hash = hash cr.Status.LastResync = metav1.Time{Time: time.Now()} - cr.Status.UID = uid + cr.Status.UID = datasource.UID return ctrl.Result{RequeueAfter: cr.GetResyncPeriod()}, r.Client.Status().Update(ctx, cr) } else { // if there was an issue with the datasource, update the status From 67eae60f9dd8e448eb0a03c833b4869b47fb819a Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Sun, 28 May 2023 17:07:09 +0200 Subject: [PATCH 11/18] chore: update formatting, remove unneeded comments --- controllers/datasource_controller.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/controllers/datasource_controller.go b/controllers/datasource_controller.go index f876248ea..9f27206b5 100644 --- a/controllers/datasource_controller.go +++ b/controllers/datasource_controller.go @@ -200,8 +200,8 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re // Clean up uid, so further reconcilications can track changes there cr.Status.UID = "" - err = r.Client.Status().Update(ctx, cr) + err = r.Client.Status().Update(ctx, cr) if err != nil { return ctrl.Result{RequeueAfter: RequeueDelay}, err } @@ -424,7 +424,6 @@ func (r *GrafanaDatasourceReconciler) getDatasourceContent(ctx context.Context, return nil, "", err } - // TODO: test if cr.Spec.Datasource.UID == "" { simpleContent.Set("uid", string(cr.UID)) } From 97f876e3738903c7945ba98530dc366f025280d2 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Mon, 29 May 2023 13:02:24 +0200 Subject: [PATCH 12/18] fix(datasource): use CR uid in comparison --- api/v1beta1/grafanadatasource_types.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/v1beta1/grafanadatasource_types.go b/api/v1beta1/grafanadatasource_types.go index 5d2d31c06..d073bc6dd 100644 --- a/api/v1beta1/grafanadatasource_types.go +++ b/api/v1beta1/grafanadatasource_types.go @@ -158,6 +158,10 @@ func (in *GrafanaDatasource) IsUpdatedUID(uid string) bool { return false } + if uid == "" { + uid = string(in.UID) + } + return in.Status.UID != uid } From 2b186e1ec1a9ee44120fccbe105480b42d5ebd14 Mon Sep 17 00:00:00 2001 From: Edvin N Date: Mon, 5 Jun 2023 22:01:54 +0200 Subject: [PATCH 13/18] [docs] blog/script to migrate from v4-v5 (#1069) --- docs/blog/v4-v5-migration.md | 79 ++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 docs/blog/v4-v5-migration.md diff --git a/docs/blog/v4-v5-migration.md b/docs/blog/v4-v5-migration.md new file mode 100644 index 000000000..059210dfa --- /dev/null +++ b/docs/blog/v4-v5-migration.md @@ -0,0 +1,79 @@ +--- +author: "Edvin 'NissesSenap' Norling" +date: 2023-05-27 +title: "v4 to v5 migration" +linkTitle: "v4 to v5 migration" +description: "How to migrate from grafana-operator v4 to v5" +--- + +We are getting close to releasing version 5 of grafana-operator. + +Version 5 comes with a number of breaking changes in API which were required to introduce support for managing multiple Grafana instances (including externally-hosted), arbitrary Grafana configuration (we no longer need to explicitly support each configuration field in grafana.ini), and other useful features. A more comprehensive list of changes can be found in our [intro blog post]({{< ref "/blog/v5-intro.md" >}}), and full documentation - [here]({{< ref "/docs/">}}). + +Unfortunately, there is no automated way to migrate between the API versions due to fundamental changes in how CRs are described and managed. + +The best way to prepare for the migration though would be by looking at our [example library]({{< ref "/docs/examples/">}}) and adjusting those examples according to your needs. For anything that is not covered there, our [API documentation]({{< ref "/docs/api.md">}}) should be very helpful. + +**NOTE:** Since v4 and v5 use different `apiVersion` in CRs (`integreatly.org/v1alpha1` -> `grafana.integreatly.org/v1beta1`), you can run those versions in parallel during the transition period. + +The biggest difference to keep in mind is that the label selector isn't in a `Grafana` spec anymore (`dashboardLabelSelector`), it's in other CRs instead (`instanceSelector`). Basically, it means that, say, a `GrafanaDashboard` "chooses" which `Grafana` instance to apply to, not the other way around. In terms of manifests, you get from: + +```yaml +# v4 +apiVersion: integreatly.org/v1alpha1 +kind: Grafana +metadata: + name: example-grafana +spec: + # [...] + dashboardLabelSelector: + - matchExpressions: + - { key: app, operator: In, values: [grafana] } +--- +apiVersion: integreatly.org/v1alpha1 +kind: GrafanaDashboard +metadata: + name: simple-dashboard + labels: + app: grafana +spec: + json: > + { + "id": null, + "title": "Simple Dashboard", + "tags": [], + "style": "dark", + } +``` + +to: + +```yaml +# v5 +apiVersion: grafana.integreatly.org/v1beta1 +kind: Grafana +metadata: + name: grafana + labels: + dashboards: "grafana" +spec: + # [...] +--- +apiVersion: grafana.integreatly.org/v1beta1 +kind: GrafanaDashboard +metadata: + name: grafanadashboard-sample +spec: + instanceSelector: + matchLabels: + dashboards: "grafana" + json: > + { + "id": null, + "title": "Simple Dashboard", + "tags": [], + "style": "dark", + } +``` + +As you can see, in a basic scenario of the dashboard with inline-JSON, a very few changes were needed. Although, other scenarios are likely to require a bit more effort. From a75afca326f5722b3145f29b9450f1d58e200b3b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 5 Jun 2023 22:07:06 +0200 Subject: [PATCH 14/18] chore(deps): bump golangci/golangci-lint-action from 3.4.0 to 3.5.0 (#1083) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.4.0 to 3.5.0. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](https://github.com/golangci/golangci-lint-action/compare/v3.4.0...v3.5.0) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Hubert Stefanski <35736504+HubertStefanski@users.noreply.github.com> --- .github/workflows/pr-validation.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-validation.yaml b/.github/workflows/pr-validation.yaml index 6fcc4ead0..1efde59bb 100644 --- a/.github/workflows/pr-validation.yaml +++ b/.github/workflows/pr-validation.yaml @@ -97,7 +97,7 @@ jobs: go-version-file: "go.mod" - name: golangci-lint - uses: golangci/golangci-lint-action@v3.4.0 + uses: golangci/golangci-lint-action@v3.5.0 with: version: "v1.51.2" From 72c5728f0734a7bc8f4e1acb2e6769711c1494be Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 5 Jun 2023 20:12:41 +0000 Subject: [PATCH 15/18] chore(deps): bump tj-actions/changed-files from 35.9.2 to 36.0.17 (#1086) Bumps [tj-actions/changed-files](https://github.com/tj-actions/changed-files) from 35.9.2 to 36.0.17. - [Release notes](https://github.com/tj-actions/changed-files/releases) - [Changelog](https://github.com/tj-actions/changed-files/blob/main/HISTORY.md) - [Commits](https://github.com/tj-actions/changed-files/compare/b2d17f51244a144849c6b37a3a6791b98a51d86f...b13786805affca18e536ed489687d3d8d1f05d21) --- updated-dependencies: - dependency-name: tj-actions/changed-files dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/e2e.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 53371632a..b38b69472 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -23,7 +23,7 @@ jobs: fetch-depth: 0 - id: changed-files name: Get changed files - uses: tj-actions/changed-files@b2d17f51244a144849c6b37a3a6791b98a51d86f #v35.9.2 + uses: tj-actions/changed-files@b13786805affca18e536ed489687d3d8d1f05d21 #v36.0.17 with: files_ignore: | **/*.md From 36f94b0946c384a7f845eb1bbf8bbb0a7259dd70 Mon Sep 17 00:00:00 2001 From: Craig Rodrigues Date: Mon, 5 Jun 2023 13:18:29 -0700 Subject: [PATCH 16/18] doc: Try expanding the version variable from hugo in the docs (#1085) Co-authored-by: Hubert Stefanski <35736504+HubertStefanski@users.noreply.github.com> --- docs/docs/installation/kustomize.md | 2 +- hugo/config.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/docs/installation/kustomize.md b/docs/docs/installation/kustomize.md index 30d979e1b..2efd79a3f 100644 --- a/docs/docs/installation/kustomize.md +++ b/docs/docs/installation/kustomize.md @@ -16,7 +16,7 @@ After you have downloaded Flux you can use `flux pull artifact` to download the ```shell mkdir grafana-operator -flux pull artifact oci://ghcr.io/grafana-operator/kustomize/grafana-operator:v5.0.0-rc3 --output ./grafana-operator/ +flux pull artifact oci://ghcr.io/grafana-operator/kustomize/grafana-operator:{{ .version }} --output ./grafana-operator/ ``` This will provide you the manifest files unpacked and ready to use. diff --git a/hugo/config.toml b/hugo/config.toml index cb3903ad1..1dfa8aec2 100644 --- a/hugo/config.toml +++ b/hugo/config.toml @@ -59,7 +59,7 @@ archived_version = false # The version number for the version of the docs represented in this doc set. # Used in the "version-banner" partial to display a version number for the # current doc set. -version = "0.0" +version = "v5.0.0-rc3" # A link to latest version of the docs. Used in the "version-banner" partial to # point people to the main doc site. From b16174e1021eecd71a299bedf42bacd47a55c568 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 5 Jun 2023 20:37:37 +0000 Subject: [PATCH 17/18] chore(deps): bump github.com/prometheus/client_golang (#1060) Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.15.0 to 1.15.1. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](https://github.com/prometheus/client_golang/compare/v1.15.0...v1.15.1) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Hubert Stefanski <35736504+HubertStefanski@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 341ca1840..8e6fd30c4 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/onsi/gomega v1.27.6 github.com/openshift/api v3.9.0+incompatible github.com/pkg/errors v0.9.1 - github.com/prometheus/client_golang v1.15.0 + github.com/prometheus/client_golang v1.15.1 github.com/stretchr/testify v1.8.3 k8s.io/api v0.25.2 k8s.io/apimachinery v0.27.2 diff --git a/go.sum b/go.sum index 9c07e9dc9..19ba72f77 100644 --- a/go.sum +++ b/go.sum @@ -279,8 +279,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/prometheus/client_golang v1.15.0 h1:5fCgGYogn0hFdhyhLbw7hEsWxufKtY9klyvdNfFlFhM= -github.com/prometheus/client_golang v1.15.0/go.mod h1:e9yaBhRPU2pPNsZwE+JdQl0KEt1N9XgF6zxWmaC0xOk= +github.com/prometheus/client_golang v1.15.1 h1:8tXpTmJbyH5lydzFPoxSIJ0J46jdh3tylbvM1xCv0LI= +github.com/prometheus/client_golang v1.15.1/go.mod h1:e9yaBhRPU2pPNsZwE+JdQl0KEt1N9XgF6zxWmaC0xOk= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/client_model v0.3.0 h1:UBgGFHqYdG/TPFD1B1ogZywDqEkwp3fBMvqdiQ7Xew4= github.com/prometheus/client_model v0.3.0/go.mod h1:LDGWKZIo7rky3hgvBe+caln+Dr3dPggB5dvjtD7w9+w= From 72a052151221443810dc5a273a82fe66526f81f3 Mon Sep 17 00:00:00 2001 From: Igor Beliakov <46579601+weisdd@users.noreply.github.com> Date: Tue, 6 Jun 2023 12:42:46 +0200 Subject: [PATCH 18/18] fix(dashboard): status updates, replacements when the same title & folderID, but different uid (#1077) * fix: update dashboard status at Reconcile level * chore: rename vars * fix(dashboard): correctly replace dashboards with the same title in the same folder * chore: ignore 404 --------- Co-authored-by: Hubert Stefanski <35736504+HubertStefanski@users.noreply.github.com> Co-authored-by: Edvin N --- api/v1beta1/grafanadashboard_types.go | 28 +---- api/v1beta1/grafanadashboard_types_test.go | 81 ++++++++++++ controllers/dashboard_controller.go | 137 ++++++++++++--------- controllers/dashboard_controller_test.go | 102 --------------- 4 files changed, 167 insertions(+), 181 deletions(-) diff --git a/api/v1beta1/grafanadashboard_types.go b/api/v1beta1/grafanadashboard_types.go index 04d6d13e8..4e324fa8e 100644 --- a/api/v1beta1/grafanadashboard_types.go +++ b/api/v1beta1/grafanadashboard_types.go @@ -19,9 +19,6 @@ package v1beta1 import ( "bytes" "compress/gzip" - "crypto/sha256" - "encoding/json" - "fmt" "io" "time" @@ -140,12 +137,6 @@ type GrafanaDashboardList struct { Items []GrafanaDashboard `json:"items"` } -func (in *GrafanaDashboard) Hash(dashboardJson []byte) string { - hash := sha256.New() - hash.Write(dashboardJson) - return fmt.Sprintf("%x", hash.Sum(nil)) -} - func (in *GrafanaDashboard) Unchanged(hash string) bool { return in.Status.Hash == hash } @@ -226,28 +217,17 @@ func (in *GrafanaDashboard) IsAllowCrossNamespaceImport() bool { return false } -func (in *GrafanaDashboard) IsUpdatedUID(dashboardJson []byte) bool { +func (in *GrafanaDashboard) IsUpdatedUID(uid string) bool { // Dashboard has just been created, status is not yet updated if in.Status.UID == "" { return false } - type DashboardUID struct { - UID string `json:"uid,omitempty"` - } - - dashboardUID := DashboardUID{} - err := json.Unmarshal(dashboardJson, &dashboardUID) - // here, we don't really care about catching json errors - if err != nil { - return false - } - - if dashboardUID.UID == "" { - dashboardUID.UID = string(in.UID) + if uid == "" { + uid = string(in.UID) } - return in.Status.UID != dashboardUID.UID + return in.Status.UID != uid } func Gunzip(compressed []byte) ([]byte, error) { diff --git a/api/v1beta1/grafanadashboard_types_test.go b/api/v1beta1/grafanadashboard_types_test.go index a58fb3d80..86ea5e21a 100644 --- a/api/v1beta1/grafanadashboard_types_test.go +++ b/api/v1beta1/grafanadashboard_types_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) func TestGrafanaDashboardStatus_getContentCache(t *testing.T) { @@ -101,3 +102,83 @@ func Test_Gzip(t *testing.T) { assert.Equal(t, dashboardJSON, decompressed, "Decompressed dashboard should match the original") } + +func TestGrafanaDashboardIsUpdatedUID(t *testing.T) { + crUID := "crUID" + tests := []struct { + name string + crUID string + statusUID string + dashboardUID string + want bool + }{ + { + name: "Status.UID and dashboard UID are empty", + crUID: crUID, + statusUID: "", + dashboardUID: "newUID", + want: false, + }, + { + name: "Status.UID is empty, dashboard UID is not", + crUID: crUID, + statusUID: "", + dashboardUID: "newUID", + want: false, + }, + { + name: "Status.UID is not empty (same as CR uid), new UID is empty", + crUID: crUID, + statusUID: crUID, + dashboardUID: "", + want: false, + }, + { + name: "Status.UID is not empty (different from CR uid), new UID is empty", + crUID: crUID, + statusUID: "oldUID", + dashboardUID: "", + want: true, + }, + { + name: "Status.UID is not empty, new UID is different", + crUID: crUID, + statusUID: "oldUID", + dashboardUID: "newUID", + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cr := getDashboardCR(t, tt.crUID, tt.statusUID) + got := cr.IsUpdatedUID(tt.dashboardUID) + assert.Equal(t, tt.want, got) + }) + } +} + +func getDashboardCR(t *testing.T, crUID string, statusUID string) GrafanaDashboard { + t.Helper() + + cr := GrafanaDashboard{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "mydashboard", + Namespace: "grafana-operator-system", + UID: types.UID(crUID), + }, + Spec: GrafanaDashboardSpec{ + InstanceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "dashboard": "grafana", + }, + }, + }, + Status: GrafanaDashboardStatus{ + UID: statusUID, + }, + } + + return cr +} diff --git a/controllers/dashboard_controller.go b/controllers/dashboard_controller.go index 72ec276b2..2bca69091 100644 --- a/controllers/dashboard_controller.go +++ b/controllers/dashboard_controller.go @@ -19,6 +19,7 @@ package controllers import ( "bytes" "context" + "crypto/sha256" "encoding/json" "fmt" "net/http" @@ -160,11 +161,11 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req return syncResult, err } - dashboard := &v1beta1.GrafanaDashboard{} + cr := &v1beta1.GrafanaDashboard{} err := r.Client.Get(ctx, client.ObjectKey{ Namespace: req.Namespace, Name: req.Name, - }, dashboard) + }, cr) if err != nil { if errors.IsNotFound(err) { err = r.onDashboardDeleted(ctx, req.Namespace, req.Name) @@ -177,22 +178,30 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{RequeueAfter: RequeueDelay}, err } - instances, err := r.GetMatchingDashboardInstances(ctx, dashboard, r.Client) + instances, err := r.GetMatchingDashboardInstances(ctx, cr, r.Client) if err != nil { - controllerLog.Error(err, "could not find matching instances", "name", dashboard.Name, "namespace", dashboard.Namespace) + controllerLog.Error(err, "could not find matching instances", "name", cr.Name, "namespace", cr.Namespace) return ctrl.Result{RequeueAfter: RequeueDelay}, err } controllerLog.Info("found matching Grafana instances for dashboard", "count", len(instances.Items)) - dashboardJson, err := r.fetchDashboardJson(dashboard) + dashboardJson, err := r.fetchDashboardJson(cr) if err != nil { - controllerLog.Error(err, "error fetching dashboard", "dashboard", dashboard.Name) + controllerLog.Error(err, "error fetching dashboard", "dashboard", cr.Name) return ctrl.Result{RequeueAfter: RequeueDelay}, nil } + dashboardModel, hash, err := r.getDashboardModel(cr, dashboardJson) + if err != nil { + controllerLog.Error(err, "failed to prepare dashboard model", "dashboard", cr.Name) + return ctrl.Result{RequeueAfter: RequeueDelay}, nil + } + + uid := fmt.Sprintf("%s", dashboardModel["uid"]) + // Garbage collection for a case where dashboard uid get changed, dashboard creation is expected to happen in a separate reconcilication cycle - if dashboard.IsUpdatedUID(dashboardJson) { + if cr.IsUpdatedUID(uid) { controllerLog.Info("dashboard uid got updated, deleting dashboards with the old uid") err = r.onDashboardDeleted(ctx, req.Namespace, req.Name) if err != nil { @@ -200,8 +209,8 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req } // Clean up uid, so further reconcilications can track changes there - dashboard.Status.UID = "" - err = r.Client.Status().Update(ctx, dashboard) + cr.Status.UID = "" + err = r.Client.Status().Update(ctx, cr) if err != nil { return ctrl.Result{RequeueAfter: RequeueDelay}, err } @@ -213,7 +222,7 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req success := true for _, grafana := range instances.Items { // check if this is a cross namespace import - if grafana.Namespace != dashboard.Namespace && !dashboard.IsAllowCrossNamespaceImport() { + if grafana.Namespace != cr.Namespace && !cr.IsAllowCrossNamespaceImport() { continue } @@ -230,24 +239,27 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req // first reconcile the plugins // append the requested dashboards to a configmap from where the // grafana reconciler will pick them up - err = ReconcilePlugins(ctx, r.Client, r.Scheme, &grafana, dashboard.Spec.Plugins, fmt.Sprintf("%v-dashboard", dashboard.Name)) + err = ReconcilePlugins(ctx, r.Client, r.Scheme, &grafana, cr.Spec.Plugins, fmt.Sprintf("%v-dashboard", cr.Name)) if err != nil { - controllerLog.Error(err, "error reconciling plugins", "dashboard", dashboard.Name, "grafana", grafana.Name) + controllerLog.Error(err, "error reconciling plugins", "dashboard", cr.Name, "grafana", grafana.Name) success = false } } // then import the dashboard into the matching grafana instances - err = r.onDashboardCreated(ctx, &grafana, dashboard, dashboardJson) + err = r.onDashboardCreated(ctx, &grafana, cr, dashboardModel, hash) if err != nil { - controllerLog.Error(err, "error reconciling dashboard", "dashboard", dashboard.Name, "grafana", grafana.Name) + controllerLog.Error(err, "error reconciling dashboard", "dashboard", cr.Name, "grafana", grafana.Name) success = false } } // if the dashboard was successfully synced in all instances, wait for its re-sync period if success { - return ctrl.Result{RequeueAfter: dashboard.GetResyncPeriod()}, nil + cr.Status.Hash = hash + cr.Status.LastResync = metav1.Time{Time: time.Now()} + cr.Status.UID = uid + return ctrl.Result{RequeueAfter: cr.GetResyncPeriod()}, r.Client.Status().Update(ctx, cr) } return ctrl.Result{RequeueAfter: RequeueDelay}, nil @@ -314,56 +326,53 @@ func (r *GrafanaDashboardReconciler) onDashboardDeleted(ctx context.Context, nam return nil } -func (r *GrafanaDashboardReconciler) onDashboardCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaDashboard, dashboardJson []byte) error { +func (r *GrafanaDashboardReconciler) onDashboardCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaDashboard, dashboardModel map[string]interface{}, hash string) error { if grafana.IsExternal() && cr.Spec.Plugins != nil { return fmt.Errorf("external grafana instances don't support plugins, please remove spec.plugins from your dashboard cr") } - dashboardJson, err := r.resolveDatasources(cr, dashboardJson) + grafanaClient, err := client2.NewGrafanaClient(ctx, r.Client, grafana) if err != nil { return err } - grafanaClient, err := client2.NewGrafanaClient(ctx, r.Client, grafana) + folderID, err := r.GetOrCreateFolder(grafanaClient, cr) if err != nil { - return err + return errors.NewInternalError(err) } - var dashboardFromJson map[string]interface{} - err = json.Unmarshal(dashboardJson, &dashboardFromJson) + uid := fmt.Sprintf("%s", dashboardModel["uid"]) + title := fmt.Sprintf("%s", dashboardModel["title"]) + + exists, remoteUID, err := r.Exists(grafanaClient, uid, title, folderID) if err != nil { return err } - uid, _ := dashboardFromJson["uid"].(string) //nolint:errcheck - if uid == "" { - uid = string(cr.UID) - } - - dashboardFromJson["uid"] = uid + if exists && remoteUID != uid { + // If there's already a dashboard with the same title in the same folder, grafana preserves that dashboard's uid, so we should remove it first + r.Log.Info("found dashboard with the same title (in the same folder) but different uid, removing the dashboard before recreating it with a new uid") + err = grafanaClient.DeleteDashboardByUID(remoteUID) + if err != nil { + if !strings.Contains(err.Error(), "status: 404") { + return err + } + } - // update/create the dashboard if it doesn't exist in the instance or has been changed - hash := cr.Hash(dashboardJson) - exists, err := r.Exists(grafanaClient, uid) - if err != nil { - return err + exists = false } + if exists && cr.Unchanged(hash) && !cr.ResyncPeriodHasElapsed() { return nil } - folderID, err := r.GetOrCreateFolder(grafanaClient, cr) - if err != nil { - return errors.NewInternalError(err) - } - resp, err := grafanaClient.NewDashboard(grapi.Dashboard{ Meta: grapi.DashboardMeta{ IsStarred: false, Slug: cr.Name, Folder: folderID, }, - Model: dashboardFromJson, + Model: dashboardModel, FolderID: folderID, Overwrite: true, Message: "", @@ -376,18 +385,8 @@ func (r *GrafanaDashboardReconciler) onDashboardCreated(ctx context.Context, gra return errors.NewBadRequest(fmt.Sprintf("error creating dashboard, status was %v", resp.Status)) } - // NOTE: When dashboard is uploaded with a new dashboardFromJson["uid"], but with an old name, the old uid is preserved. So, better to rely on the resp.UID here - grafana.Status.Dashboards = grafana.Status.Dashboards.Add(cr.Namespace, cr.Name, resp.UID) - err = r.Client.Status().Update(ctx, grafana) - if err != nil { - return err - } - - cr.Status.Hash = hash - cr.Status.LastResync = metav1.Time{Time: time.Now()} - cr.Status.UID = resp.UID - - return r.Client.Status().Update(ctx, cr) + grafana.Status.Dashboards = grafana.Status.Dashboards.Add(cr.Namespace, cr.Name, uid) + return r.Client.Status().Update(ctx, grafana) } // map data sources that are required in the dashboard to data sources that exist in the instance @@ -437,17 +436,45 @@ func (r *GrafanaDashboardReconciler) fetchDashboardJson(dashboard *v1beta1.Grafa } } -func (r *GrafanaDashboardReconciler) Exists(client *grapi.Client, uid string) (bool, error) { +// getDashboardModel resolves datasources, updates uid (if needed) and converts raw json to type grafana client accepts +func (r *GrafanaDashboardReconciler) getDashboardModel(cr *v1beta1.GrafanaDashboard, dashboardJson []byte) (map[string]interface{}, string, error) { + dashboardJson, err := r.resolveDatasources(cr, dashboardJson) + if err != nil { + return map[string]interface{}{}, "", err + } + + hash := sha256.New() + hash.Write(dashboardJson) + + var dashboardModel map[string]interface{} + err = json.Unmarshal(dashboardJson, &dashboardModel) + if err != nil { + return map[string]interface{}{}, "", err + } + + uid, _ := dashboardModel["uid"].(string) //nolint:errcheck + if uid == "" { + uid = string(cr.UID) + } + + dashboardModel["uid"] = uid + + return dashboardModel, fmt.Sprintf("%x", hash.Sum(nil)), nil +} + +func (r *GrafanaDashboardReconciler) Exists(client *grapi.Client, uid string, title string, folderID int64) (bool, string, error) { dashboards, err := client.Dashboards() if err != nil { - return false, err + return false, "", err } + for _, dashboard := range dashboards { - if dashboard.UID == uid { - return true, nil + if dashboard.UID == uid || (dashboard.Title == title && dashboard.FolderID == uint(folderID)) { + return true, dashboard.UID, nil } } - return false, nil + + return false, "", nil } func (r *GrafanaDashboardReconciler) GetOrCreateFolder(client *grapi.Client, cr *v1beta1.GrafanaDashboard) (int64, error) { diff --git a/controllers/dashboard_controller_test.go b/controllers/dashboard_controller_test.go index 6f9099d71..d88b0fbe1 100644 --- a/controllers/dashboard_controller_test.go +++ b/controllers/dashboard_controller_test.go @@ -17,13 +17,11 @@ limitations under the License. package controllers import ( - "encoding/json" "testing" "github.com/grafana-operator/grafana-operator/v5/api/v1beta1" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) func TestGetDashboardsToDelete(t *testing.T) { @@ -119,103 +117,3 @@ func TestGetDashboardsToDelete(t *testing.T) { } } } - -func TestGrafanaDashboardIsUpdatedUID(t *testing.T) { - crUID := "crUID" - tests := []struct { - name string - crUID string - statusUID string - dashboardUID string - want bool - }{ - { - name: "Status.UID and dashboard UID are empty", - crUID: crUID, - statusUID: "", - dashboardUID: "newUID", - want: false, - }, - { - name: "Status.UID is empty, dashboard UID is not", - crUID: crUID, - statusUID: "", - dashboardUID: "newUID", - want: false, - }, - { - name: "Status.UID is not empty (same as CR uid), new UID is empty", - crUID: crUID, - statusUID: crUID, - dashboardUID: "", - want: false, - }, - { - name: "Status.UID is not empty (different from CR uid), new UID is empty", - crUID: crUID, - statusUID: "oldUID", - dashboardUID: "", - want: true, - }, - { - name: "Status.UID is not empty, new UID is different", - crUID: crUID, - statusUID: "oldUID", - dashboardUID: "newUID", - want: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - cr := getDashboardCR(t, tt.crUID, tt.statusUID) - dashboardJson := getDashboardJSONWithUID(t, tt.dashboardUID) - got := cr.IsUpdatedUID(dashboardJson) - assert.Equal(t, tt.want, got) - }) - } -} - -func getDashboardCR(t *testing.T, crUID string, statusUID string) v1beta1.GrafanaDashboard { - t.Helper() - - cr := v1beta1.GrafanaDashboard{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: "mydashboard", - Namespace: "grafana-operator-system", - UID: types.UID(crUID), - }, - Spec: v1beta1.GrafanaDashboardSpec{ - InstanceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "dashboard": "grafana", - }, - }, - }, - Status: v1beta1.GrafanaDashboardStatus{ - UID: statusUID, - }, - } - - return cr -} - -func getDashboardJSONWithUID(t *testing.T, uid string) []byte { - t.Helper() - - type DashboardUID struct { - UID string `json:"uid,omitempty"` - } - - dashboardUID := DashboardUID{ - UID: uid, - } - - dashboardJson, err := json.Marshal(dashboardUID) - if err != nil { - t.Errorf("failed to generate dashboard JSON with uid: %s", uid) - } - - return dashboardJson -}