Skip to content

Commit 5d4801d

Browse files
authored
chore(DatasourceController): Remove code that deletes missing CRs from instances (#1991)
1 parent 891455f commit 5d4801d

File tree

2 files changed

+49
-141
lines changed

2 files changed

+49
-141
lines changed

controllers/datasource_controller.go

Lines changed: 49 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -58,109 +58,59 @@ type GrafanaDatasourceReconciler struct {
5858
//+kubebuilder:rbac:groups=grafana.integreatly.org,resources=grafanadatasources/status,verbs=get;update;patch
5959
//+kubebuilder:rbac:groups=grafana.integreatly.org,resources=grafanadatasources/finalizers,verbs=update
6060

61-
func (r *GrafanaDatasourceReconciler) syncDatasources(ctx context.Context) (ctrl.Result, error) {
61+
func (r *GrafanaDatasourceReconciler) syncStatuses(ctx context.Context) error {
6262
log := logf.FromContext(ctx)
63-
datasourcesSynced := 0
6463

6564
// get all grafana instances
6665
grafanas := &v1beta1.GrafanaList{}
6766
var opts []client.ListOption
6867
err := r.List(ctx, grafanas, opts...)
6968
if err != nil {
70-
return ctrl.Result{}, err
69+
return err
7170
}
72-
7371
// no instances, no need to sync
7472
if len(grafanas.Items) == 0 {
75-
return ctrl.Result{Requeue: false}, nil
73+
return nil
7674
}
7775

7876
// get all datasources
79-
allDatasources := &v1beta1.GrafanaDatasourceList{}
80-
err = r.List(ctx, allDatasources, opts...)
77+
allDatasource := &v1beta1.GrafanaDatasourceList{}
78+
err = r.List(ctx, allDatasource, opts...)
8179
if err != nil {
82-
return ctrl.Result{}, err
80+
return err
8381
}
8482

85-
datasourcesToDelete := getDatasourcesToDelete(allDatasources, grafanas.Items)
86-
87-
// delete all datasources that no longer have a cr
88-
for grafana, existingDatasources := range datasourcesToDelete {
89-
grafanaClient, err := client2.NewGeneratedGrafanaClient(ctx, r.Client, grafana)
90-
if err != nil {
91-
return ctrl.Result{}, err
92-
}
93-
94-
for syncCounter, datasource := range existingDatasources {
95-
// avoid bombarding the grafana instance with a large number of requests at once, limit
96-
// the sync to ten datasources per cycle. This means that it will take longer to sync
97-
// a large number of deleted datasource crs, but that should be an edge case.
98-
if syncCounter >= syncBatchSize {
99-
return ctrl.Result{Requeue: true}, nil
83+
// delete datasources datasourcegrafana statuses that no longer have a CR
84+
datasourcesSynced := 0
85+
for _, grafana := range grafanas.Items {
86+
statusUpdated := false
87+
for _, ds := range grafana.Status.Datasources {
88+
namespace, name, _ := ds.Split()
89+
if allDatasource.Find(namespace, name) == nil {
90+
grafana.Status.Datasources = grafana.Status.Datasources.Remove(namespace, name)
91+
datasourcesSynced += 1
92+
statusUpdated = true
10093
}
94+
}
10195

102-
namespace, name, uid := datasource.Split()
103-
instanceDatasource, err := grafanaClient.Datasources.GetDataSourceByUID(uid)
96+
if statusUpdated {
97+
err = r.Status().Update(ctx, &grafana)
10498
if err != nil {
105-
var notFound *datasources.GetDataSourceByUIDNotFound
106-
if !errors.As(err, &notFound) {
107-
return ctrl.Result{}, err
108-
}
109-
log.Info("datasource no longer exists", "namespace", namespace, "name", name)
110-
} else {
111-
_, err = grafanaClient.Datasources.DeleteDataSourceByUID(instanceDatasource.Payload.UID) //nolint
112-
if err != nil {
113-
var notFound *datasources.DeleteDataSourceByUIDNotFound
114-
if !errors.As(err, &notFound) {
115-
return ctrl.Result{}, err
116-
}
117-
}
99+
return err
118100
}
119-
120-
grafana.Status.Datasources = grafana.Status.Datasources.Remove(namespace, name)
121-
datasourcesSynced += 1
122-
}
123-
124-
// one update per grafana - this will trigger a reconcile of the grafana controller
125-
// so we should minimize those updates
126-
err = r.Client.Status().Update(ctx, grafana)
127-
if err != nil {
128-
return ctrl.Result{}, err
129101
}
130102
}
131103

132104
if datasourcesSynced > 0 {
133105
log.Info("successfully synced datasources", "datasources", datasourcesSynced)
134106
}
135-
return ctrl.Result{Requeue: false}, nil
136-
}
137-
138-
// return a list of datasources from the grafana cr that do no longer have a datasource cr
139-
func getDatasourcesToDelete(allDatasources *v1beta1.GrafanaDatasourceList, grafanas []v1beta1.Grafana) map[*v1beta1.Grafana][]v1beta1.NamespacedResource {
140-
datasourcesToDelete := map[*v1beta1.Grafana][]v1beta1.NamespacedResource{}
141-
for _, grafana := range grafanas {
142-
for _, datasource := range grafana.Status.Datasources {
143-
if allDatasources.Find(datasource.Namespace(), datasource.Name()) == nil {
144-
datasourcesToDelete[&grafana] = append(datasourcesToDelete[&grafana], datasource)
145-
}
146-
}
147-
}
148-
return datasourcesToDelete
107+
return nil
149108
}
150109

151110
func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
152111
log := logf.FromContext(ctx).WithName("GrafanaDatasourceReconciler")
153112
ctx = logf.IntoContext(ctx, log)
154113

155-
// periodic sync reconcile
156-
if req.Namespace == "" && req.Name == "" {
157-
start := time.Now()
158-
syncResult, err := r.syncDatasources(ctx)
159-
elapsed := time.Since(start).Milliseconds()
160-
metrics.InitialDatasourceSyncDuration.Set(float64(elapsed))
161-
return syncResult, err
162-
}
163-
164114
cr := &v1beta1.GrafanaDatasource{}
165115
err := r.Get(ctx, client.ObjectKey{
166116
Namespace: req.Namespace,
@@ -303,7 +253,7 @@ func (r *GrafanaDatasourceReconciler) deleteOldDatasource(ctx context.Context, c
303253
}
304254

305255
grafana.Status.Datasources = grafana.Status.Datasources.Remove(cr.Namespace, cr.Name)
306-
err = r.Client.Status().Update(ctx, &grafana)
256+
err = r.Status().Update(ctx, &grafana)
307257
if err != nil {
308258
return err
309259
}
@@ -347,7 +297,7 @@ func (r *GrafanaDatasourceReconciler) finalize(ctx context.Context, cr *v1beta1.
347297
}
348298

349299
grafana.Status.Datasources = grafana.Status.Datasources.Remove(cr.Namespace, cr.Name)
350-
err = r.Client.Status().Update(ctx, &grafana)
300+
err = r.Status().Update(ctx, &grafana)
351301
if err != nil {
352302
return err
353303
}
@@ -405,7 +355,7 @@ func (r *GrafanaDatasourceReconciler) onDatasourceCreated(ctx context.Context, g
405355
}
406356

407357
grafana.Status.Datasources = grafana.Status.Datasources.Add(cr.Namespace, cr.Name, datasource.UID)
408-
return r.Client.Status().Update(ctx, grafana)
358+
return r.Status().Update(ctx, grafana)
409359
}
410360

411361
func (r *GrafanaDatasourceReconciler) Exists(client *genapi.GrafanaHTTPAPI, uid, name string) (bool, string, error) {
@@ -429,32 +379,35 @@ func (r *GrafanaDatasourceReconciler) SetupWithManager(ctx context.Context, mgr
429379
For(&v1beta1.GrafanaDatasource{}).
430380
WithEventFilter(ignoreStatusUpdates()).
431381
Complete(r)
382+
if err != nil {
383+
return err
384+
}
432385

433-
if err == nil {
434-
go func() {
435-
log := logf.FromContext(ctx).WithName("GrafanaDatasourceReconciler")
436-
for {
437-
select {
438-
case <-ctx.Done():
439-
return
440-
case <-time.After(initialSyncDelay):
441-
result, err := r.Reconcile(ctx, ctrl.Request{})
442-
if err != nil {
443-
log.Error(err, "error synchronizing datasources")
444-
continue
445-
}
446-
if result.Requeue {
447-
log.Info("more datasources left to synchronize")
448-
continue
449-
}
450-
log.Info("datasources sync complete")
451-
return
386+
go func() {
387+
// periodic sync reconcile
388+
log := logf.FromContext(ctx).WithName("GrafanaDatasourceReconciler")
389+
390+
for {
391+
select {
392+
case <-ctx.Done():
393+
return
394+
case <-time.After(initialSyncDelay):
395+
start := time.Now()
396+
err := r.syncStatuses(ctx)
397+
elapsed := time.Since(start).Milliseconds()
398+
metrics.InitialDatasourceSyncDuration.Set(float64(elapsed))
399+
if err != nil {
400+
log.Error(err, "error synchronizing datasources")
401+
continue
452402
}
403+
404+
log.Info("datasource sync complete")
405+
return
453406
}
454-
}()
455-
}
407+
}
408+
}()
456409

457-
return err
410+
return nil
458411
}
459412

460413
func (r *GrafanaDatasourceReconciler) buildDatasourceModel(ctx context.Context, cr *v1beta1.GrafanaDatasource) (*models.UpdateDataSourceCommand, string, error) {

controllers/datasource_controller_test.go

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -41,51 +41,6 @@ func TestGetDatasourceContent(t *testing.T) {
4141
})
4242
}
4343

44-
func TestGetDatasourcesToDelete(t *testing.T) {
45-
f := func(ds []v1beta1.GrafanaDatasource, grafana v1beta1.Grafana, expected []v1beta1.NamespacedResource) {
46-
t.Helper()
47-
datasourcesList := v1beta1.GrafanaDatasourceList{
48-
TypeMeta: metav1.TypeMeta{},
49-
ListMeta: metav1.ListMeta{},
50-
Items: ds,
51-
}
52-
datasourcesToDelete := getDatasourcesToDelete(&datasourcesList, []v1beta1.Grafana{grafana})
53-
for _, out := range datasourcesToDelete {
54-
assert.Equal(t, out, expected)
55-
}
56-
}
57-
58-
f([]v1beta1.GrafanaDatasource{
59-
{
60-
TypeMeta: metav1.TypeMeta{},
61-
ObjectMeta: metav1.ObjectMeta{
62-
Name: "datasource-a",
63-
Namespace: "namespace",
64-
},
65-
Status: v1beta1.GrafanaDatasourceStatus{
66-
UID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
67-
},
68-
},
69-
},
70-
v1beta1.Grafana{
71-
TypeMeta: metav1.TypeMeta{},
72-
ObjectMeta: metav1.ObjectMeta{
73-
Name: "grafana-1",
74-
Namespace: "namespace",
75-
},
76-
Status: v1beta1.GrafanaStatus{
77-
Datasources: v1beta1.NamespacedResourceList{
78-
"namespace/datasource-a/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
79-
"namespace/datasource-c/cccccccc-cccc-cccc-cccc-cccccccccccc",
80-
},
81-
},
82-
},
83-
[]v1beta1.NamespacedResource{
84-
"namespace/datasource-c/cccccccc-cccc-cccc-cccc-cccccccccccc",
85-
},
86-
)
87-
}
88-
8944
var _ = Describe("Datasource: Reconciler", func() {
9045
It("Results in NoMatchingInstances Condition", func() {
9146
// Create object

0 commit comments

Comments
 (0)