Skip to content

Commit 642b609

Browse files
BaarsgaardBaarsgaard
authored
chore(AlertRuleGroup): Remove unnecessary requests during reconcile (#2051)
* chore: fix example-resources and add extra rule to ARG cr * chore(AlertRuleGroup): Remove excess requests in reconcile * chore: Set MinLength of alertrule array
1 parent ca0d8b4 commit 642b609

File tree

8 files changed

+225
-111
lines changed

8 files changed

+225
-111
lines changed

‎api/v1beta1/grafanaalertrulegroup_types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ type GrafanaAlertRuleGroupSpec struct {
4444
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
4545
FolderRef string `json:"folderRef,omitempty"`
4646

47+
// +kubebuilder:validation:MinItems=1
4748
Rules []AlertRule `json:"rules"`
4849

4950
// +kubebuilder:validation:Type=string

‎api/v1beta1/grafanaalertrulegroup_types_test.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,21 @@ package v1beta1
22

33
import (
44
"context"
5+
"time"
56

67
. "github.com/onsi/ginkgo/v2"
78
. "github.com/onsi/gomega"
8-
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
910
)
1011

1112
func newAlertRuleGroup(name string, editable *bool) *GrafanaAlertRuleGroup {
13+
noDataState := "NoData"
1214
return &GrafanaAlertRuleGroup{
13-
TypeMeta: v1.TypeMeta{
15+
TypeMeta: metav1.TypeMeta{
1416
APIVersion: APIVersion,
1517
Kind: "GrafanaAlertRuleGroup",
1618
},
17-
ObjectMeta: v1.ObjectMeta{
19+
ObjectMeta: metav1.ObjectMeta{
1820
Name: name,
1921
Namespace: "default",
2022
},
@@ -23,13 +25,22 @@ func newAlertRuleGroup(name string, editable *bool) *GrafanaAlertRuleGroup {
2325
Editable: editable,
2426
FolderRef: "DummyFolderRef",
2527
GrafanaCommonSpec: GrafanaCommonSpec{
26-
InstanceSelector: &v1.LabelSelector{
28+
InstanceSelector: &metav1.LabelSelector{
2729
MatchLabels: map[string]string{
2830
"test": "alertrulegroup",
2931
},
3032
},
3133
},
32-
Rules: []AlertRule{},
34+
Rules: []AlertRule{
35+
{
36+
Title: "TestRule",
37+
UID: "akdj-wonvo",
38+
ExecErrState: "KeepLast",
39+
NoDataState: &noDataState,
40+
For: &metav1.Duration{Duration: 60 * time.Second},
41+
Data: []*AlertQuery{},
42+
},
43+
},
3344
},
3445
}
3546
}

‎config/crd/bases/grafana.integreatly.org_grafanaalertrulegroups.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ spec:
261261
- title
262262
- uid
263263
type: object
264+
minItems: 1
264265
type: array
265266
required:
266267
- instanceSelector

‎controllers/alertrulegroup_controller.go

Lines changed: 100 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,17 @@ func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctr
105105
return ctrl.Result{}, fmt.Errorf("folder uid not found: %w", err)
106106
}
107107

108+
editable := "true" //nolint:goconst
109+
if group.Spec.Editable != nil && !*group.Spec.Editable {
110+
editable = "false"
111+
}
112+
113+
mGroup := crToModel(group, folderUID)
114+
log.V(1).Info("converted cr to api model")
115+
108116
applyErrors := make(map[string]string)
109117
for _, grafana := range instances {
110-
err := r.reconcileWithInstance(ctx, &grafana, group, folderUID)
118+
err := r.reconcileWithInstance(ctx, &grafana, group, &mGroup, editable)
111119
if err != nil {
112120
applyErrors[fmt.Sprintf("%s/%s", grafana.Namespace, grafana.Name)] = err.Error()
113121
}
@@ -123,26 +131,73 @@ func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctr
123131
return ctrl.Result{RequeueAfter: group.Spec.ResyncPeriod.Duration}, nil
124132
}
125133

126-
// SetupWithManager sets up the controller with the Manager.
127-
func (r *GrafanaAlertRuleGroupReconciler) SetupWithManager(mgr ctrl.Manager) error {
128-
return ctrl.NewControllerManagedBy(mgr).
129-
For(&grafanav1beta1.GrafanaAlertRuleGroup{}).
130-
WithEventFilter(ignoreStatusUpdates()).
131-
Complete(r)
134+
func crToModel(cr *grafanav1beta1.GrafanaAlertRuleGroup, folderUID string) models.AlertRuleGroup {
135+
groupName := cr.GroupName()
136+
137+
mRules := make(models.ProvisionedAlertRules, 0, len(cr.Spec.Rules))
138+
139+
for _, r := range cr.Spec.Rules {
140+
apiRule := &models.ProvisionedAlertRule{
141+
Annotations: r.Annotations,
142+
Condition: &r.Condition,
143+
Data: make([]*models.AlertQuery, len(r.Data)),
144+
ExecErrState: &r.ExecErrState,
145+
FolderUID: &folderUID,
146+
For: (*strfmt.Duration)(&r.For.Duration),
147+
IsPaused: r.IsPaused,
148+
Labels: r.Labels,
149+
NoDataState: r.NoDataState,
150+
RuleGroup: &groupName,
151+
Title: &r.Title,
152+
UID: r.UID,
153+
}
154+
155+
if r.NotificationSettings != nil {
156+
apiRule.NotificationSettings = &models.AlertRuleNotificationSettings{
157+
Receiver: &r.NotificationSettings.Receiver,
158+
GroupBy: r.NotificationSettings.GroupBy,
159+
GroupWait: r.NotificationSettings.GroupWait,
160+
MuteTimeIntervals: r.NotificationSettings.MuteTimeIntervals,
161+
GroupInterval: r.NotificationSettings.GroupInterval,
162+
RepeatInterval: r.NotificationSettings.RepeatInterval,
163+
}
164+
}
165+
166+
if r.Record != nil {
167+
apiRule.Record = &models.Record{
168+
From: &r.Record.From,
169+
Metric: &r.Record.Metric,
170+
}
171+
}
172+
173+
for idx, q := range r.Data {
174+
apiRule.Data[idx] = &models.AlertQuery{
175+
DatasourceUID: q.DatasourceUID,
176+
Model: q.Model,
177+
QueryType: q.QueryType,
178+
RefID: q.RefID,
179+
RelativeTimeRange: q.RelativeTimeRange,
180+
}
181+
}
182+
183+
mRules = append(mRules, apiRule)
184+
}
185+
186+
return models.AlertRuleGroup{
187+
FolderUID: folderUID,
188+
Interval: int64(cr.Spec.Interval.Seconds()),
189+
Rules: mRules,
190+
Title: groupName,
191+
}
132192
}
133193

134-
func (r *GrafanaAlertRuleGroupReconciler) reconcileWithInstance(ctx context.Context, instance *grafanav1beta1.Grafana, group *grafanav1beta1.GrafanaAlertRuleGroup, folderUID string) error {
194+
func (r *GrafanaAlertRuleGroupReconciler) reconcileWithInstance(ctx context.Context, instance *grafanav1beta1.Grafana, group *grafanav1beta1.GrafanaAlertRuleGroup, mGroup *models.AlertRuleGroup, disableProvenance string) error {
135195
cl, err := client2.NewGeneratedGrafanaClient(ctx, r.Client, instance)
136196
if err != nil {
137197
return fmt.Errorf("building grafana client: %w", err)
138198
}
139199

140-
trueRef := "true" //nolint:goconst
141-
editable := true //nolint:staticcheck
142-
if group.Spec.Editable != nil && !*group.Spec.Editable {
143-
editable = false
144-
}
145-
200+
folderUID := mGroup.FolderUID
146201
_, err = cl.Folders.GetFolderByUID(folderUID) //nolint:errcheck
147202
if err != nil {
148203
var folderNotFound *folders.GetFolderByUIDNotFound
@@ -152,122 +207,55 @@ func (r *GrafanaAlertRuleGroupReconciler) reconcileWithInstance(ctx context.Cont
152207
return fmt.Errorf("fetching folder: %w", err)
153208
}
154209

155-
groupName := group.GroupName()
156-
applied, err := cl.Provisioning.GetAlertRuleGroup(groupName, folderUID)
210+
applied, err := cl.Provisioning.GetAlertRuleGroup(mGroup.Title, folderUID)
157211
var ruleNotFound *provisioning.GetAlertRuleGroupNotFound
158212
if err != nil && !errors.As(err, &ruleNotFound) {
159213
return fmt.Errorf("fetching existing alert rule group: %w", err)
160214
}
161215

162-
currentRules := make(map[string]bool)
163-
if applied != nil {
164-
for _, rule := range applied.Payload.Rules {
165-
currentRules[rule.UID] = false
166-
}
216+
// Create an empty collection to loop over if group does not exist on remote
217+
remoteRules := models.ProvisionedAlertRules{}
218+
if applied != nil && applied.Payload != nil {
219+
remoteRules = applied.Payload.Rules
167220
}
168221

169-
for _, rule := range group.Spec.Rules {
170-
apiRule := &models.ProvisionedAlertRule{
171-
Annotations: rule.Annotations,
172-
Condition: &rule.Condition,
173-
Data: make([]*models.AlertQuery, len(rule.Data)),
174-
ExecErrState: &rule.ExecErrState,
175-
FolderUID: &folderUID,
176-
For: (*strfmt.Duration)(&rule.For.Duration),
177-
IsPaused: rule.IsPaused,
178-
Labels: rule.Labels,
179-
NoDataState: rule.NoDataState,
180-
RuleGroup: &groupName,
181-
Title: &rule.Title,
182-
UID: rule.UID,
183-
}
184-
if rule.NotificationSettings != nil {
185-
apiRule.NotificationSettings = &models.AlertRuleNotificationSettings{
186-
Receiver: &rule.NotificationSettings.Receiver,
187-
GroupBy: rule.NotificationSettings.GroupBy,
188-
GroupWait: rule.NotificationSettings.GroupWait,
189-
MuteTimeIntervals: rule.NotificationSettings.MuteTimeIntervals,
190-
GroupInterval: rule.NotificationSettings.GroupInterval,
191-
RepeatInterval: rule.NotificationSettings.RepeatInterval,
192-
}
193-
}
194-
if rule.Record != nil {
195-
apiRule.Record = &models.Record{
196-
From: &rule.Record.From,
197-
Metric: &rule.Record.Metric,
198-
}
199-
}
200-
for idx, q := range rule.Data {
201-
apiRule.Data[idx] = &models.AlertQuery{
202-
DatasourceUID: q.DatasourceUID,
203-
Model: q.Model,
204-
QueryType: q.QueryType,
205-
RefID: q.RefID,
206-
RelativeTimeRange: q.RelativeTimeRange,
222+
// Rules must be created individually
223+
// Find rules missing on the instance and create them
224+
for _, mRule := range mGroup.Rules {
225+
ruleExists := false
226+
for _, remoteRule := range remoteRules {
227+
if mRule.UID == remoteRule.UID {
228+
ruleExists = true
229+
break
207230
}
208231
}
209232

210-
if _, ok := currentRules[rule.UID]; ok {
211-
params := provisioning.NewPutAlertRuleParams().
212-
WithBody(apiRule).
213-
WithUID(rule.UID)
214-
if editable {
215-
params.SetXDisableProvenance(&trueRef)
216-
}
217-
_, err := cl.Provisioning.PutAlertRule(params) //nolint:errcheck
218-
if err != nil {
219-
return fmt.Errorf("updating rule: %w", err)
220-
}
221-
} else {
233+
if !ruleExists {
222234
params := provisioning.NewPostAlertRuleParams().
223-
WithBody(apiRule)
224-
if editable {
225-
params.SetXDisableProvenance(&trueRef)
226-
}
235+
WithBody(mRule).
236+
WithXDisableProvenance(&disableProvenance)
227237
_, err = cl.Provisioning.PostAlertRule(params) //nolint:errcheck
228238
if err != nil {
229239
return fmt.Errorf("creating rule: %w", err)
230240
}
231241
}
232-
233-
currentRules[rule.UID] = true
234-
}
235-
236-
for uid, present := range currentRules {
237-
if present {
238-
continue
239-
}
240-
params := provisioning.NewDeleteAlertRuleParams().
241-
WithUID(uid)
242-
if editable {
243-
params.SetXDisableProvenance(&trueRef)
244-
}
245-
_, err := cl.Provisioning.DeleteAlertRule(params) //nolint:errcheck
246-
if err != nil {
247-
return fmt.Errorf("deleting old alert rule %s: %w", uid, err)
248-
}
249242
}
250243

251-
mGroup := &models.AlertRuleGroup{
252-
FolderUID: folderUID,
253-
Interval: int64(group.Spec.Interval.Seconds()),
254-
Rules: []*models.ProvisionedAlertRule{},
255-
Title: "",
256-
}
244+
// Update whole group and all rules existing rules at once
245+
// Will delete rules not present in the body
257246
params := provisioning.NewPutAlertRuleGroupParams().
258247
WithBody(mGroup).
259-
WithGroup(groupName).
260-
WithFolderUID(folderUID)
261-
if editable {
262-
params.SetXDisableProvenance(&trueRef)
263-
}
248+
WithGroup(mGroup.Title).
249+
WithFolderUID(folderUID).
250+
WithXDisableProvenance(&disableProvenance)
251+
264252
_, err = cl.Provisioning.PutAlertRuleGroup(params) //nolint:errcheck
265253
if err != nil {
266254
return fmt.Errorf("updating group: %s", err.Error())
267255
}
268256

269257
// Update grafana instance Status
270-
instance.Status.AlertRuleGroups = instance.Status.AlertRuleGroups.Add(group.Namespace, group.Name, groupName)
258+
instance.Status.AlertRuleGroups = instance.Status.AlertRuleGroups.Add(group.Namespace, group.Name, mGroup.Title)
271259
return r.Client.Status().Update(ctx, instance)
272260
}
273261

@@ -306,3 +294,11 @@ func (r *GrafanaAlertRuleGroupReconciler) finalize(ctx context.Context, group *g
306294
}
307295
return nil
308296
}
297+
298+
// SetupWithManager sets up the controller with the Manager.
299+
func (r *GrafanaAlertRuleGroupReconciler) SetupWithManager(mgr ctrl.Manager) error {
300+
return ctrl.NewControllerManagedBy(mgr).
301+
For(&grafanav1beta1.GrafanaAlertRuleGroup{}).
302+
WithEventFilter(ignoreStatusUpdates()).
303+
Complete(r)
304+
}

‎controllers/alertrulegroup_controller_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package controllers
22

33
import (
44
"context"
5+
"time"
56

67
"github.com/grafana/grafana-operator/v5/api/v1beta1"
78
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -13,6 +14,7 @@ import (
1314
var _ = Describe("AlertRulegroup: Reconciler", func() {
1415
It("Results in NoMatchingInstances Condition", func() {
1516
// Create object
17+
noDataState := "NoData"
1618
cr := &v1beta1.GrafanaAlertRuleGroup{
1719
ObjectMeta: metav1.ObjectMeta{
1820
Name: "no-match",
@@ -21,7 +23,16 @@ var _ = Describe("AlertRulegroup: Reconciler", func() {
2123
Spec: v1beta1.GrafanaAlertRuleGroupSpec{
2224
GrafanaCommonSpec: instanceSelectorNoMatchingInstances,
2325
FolderUID: "GroupUID",
24-
Rules: []v1beta1.AlertRule{},
26+
Rules: []v1beta1.AlertRule{
27+
{
28+
Title: "TestRule",
29+
UID: "akdj-wonvo",
30+
ExecErrState: "KeepLast",
31+
NoDataState: &noDataState,
32+
For: &metav1.Duration{Duration: 60 * time.Second},
33+
Data: []*v1beta1.AlertQuery{},
34+
},
35+
},
2536
},
2637
}
2738
ctx := context.Background()

‎deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanaalertrulegroups.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ spec:
261261
- title
262262
- uid
263263
type: object
264+
minItems: 1
264265
type: array
265266
required:
266267
- instanceSelector

‎deploy/kustomize/base/crds.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ spec:
260260
- title
261261
- uid
262262
type: object
263+
minItems: 1
263264
type: array
264265
required:
265266
- instanceSelector

0 commit comments

Comments
 (0)