From 43bde21a0ab9bb1085cb5d20266e1dc1496dfc3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20S=C3=BC=C3=9F?= Date: Wed, 17 Jul 2024 14:37:33 +0200 Subject: [PATCH] feat: use annotation to prevent policy conflicts --- .../grafananotificationpolicy_types.go | 6 +++++ ...eatly.org_grafananotificationpolicies.yaml | 1 + ...ana_v1beta1_grafananotificationpolicy.yaml | 6 ++--- controllers/controller_shared.go | 2 ++ controllers/notificationpolicy_controller.go | 25 ++++++++++++++++++- ...eatly.org_grafananotificationpolicies.yaml | 1 + 6 files changed, 37 insertions(+), 4 deletions(-) diff --git a/api/v1beta1/grafananotificationpolicy_types.go b/api/v1beta1/grafananotificationpolicy_types.go index 4ff507cf4..cdabfa386 100644 --- a/api/v1beta1/grafananotificationpolicy_types.go +++ b/api/v1beta1/grafananotificationpolicy_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "fmt" + "github.com/grafana/grafana-openapi-client-go/models" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -144,6 +146,10 @@ type GrafanaNotificationPolicy struct { Status GrafanaNotificationPolicyStatus `json:"status,omitempty"` } +func (np *GrafanaNotificationPolicy) NamespacedResource() string { + return fmt.Sprintf("%v/%v/%v", np.ObjectMeta.Namespace, np.ObjectMeta.Name, np.ObjectMeta.UID) +} + //+kubebuilder:object:root=true // GrafanaNotificationPolicyList contains a list of GrafanaNotificationPolicy diff --git a/config/crd/bases/grafana.integreatly.org_grafananotificationpolicies.yaml b/config/crd/bases/grafana.integreatly.org_grafananotificationpolicies.yaml index d52bc493a..a1ed76257 100644 --- a/config/crd/bases/grafana.integreatly.org_grafananotificationpolicies.yaml +++ b/config/crd/bases/grafana.integreatly.org_grafananotificationpolicies.yaml @@ -96,6 +96,7 @@ spec: pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ type: string route: + description: Routes for alerts to match against properties: continue: description: continue diff --git a/config/samples/grafana_v1beta1_grafananotificationpolicy.yaml b/config/samples/grafana_v1beta1_grafananotificationpolicy.yaml index a9747c11f..6075d0ec6 100644 --- a/config/samples/grafana_v1beta1_grafananotificationpolicy.yaml +++ b/config/samples/grafana_v1beta1_grafananotificationpolicy.yaml @@ -7,18 +7,18 @@ spec: matchLabels: dashboards: "grafana" route: - receiver: grafana cloud oncall + receiver: Grafana Cloud OnCall group_by: - grafana_folder - alertname routes: - - receiver: some-other-receiver + - receiver: grafana-default-email object_matchers: - - foo - = - bar routes: - - receiver: a_third_receiver + - receiver: Grafana Cloud OnCall object_matchers: - - severity - = diff --git a/controllers/controller_shared.go b/controllers/controller_shared.go index 454661367..7b02719a7 100644 --- a/controllers/controller_shared.go +++ b/controllers/controller_shared.go @@ -27,6 +27,8 @@ const ( conditionInvalidSpec = "InvalidSpec" ) +const annotationAppliedNotificationPolicy = "operator.grafana.com/applied-notificationpolicy" + //+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;watch;create;update;patch;delete func GetMatchingInstances(ctx context.Context, k8sClient client.Client, labelSelector *metav1.LabelSelector) (v1beta1.GrafanaList, error) { diff --git a/controllers/notificationpolicy_controller.go b/controllers/notificationpolicy_controller.go index 790453ab6..8cb465d6f 100644 --- a/controllers/notificationpolicy_controller.go +++ b/controllers/notificationpolicy_controller.go @@ -121,9 +121,17 @@ func (r *GrafanaNotificationPolicyReconciler) Reconcile(ctx context.Context, req removeNoMatchingInstance(¬ificationPolicy.Status.Conditions) applyErrors := make(map[string]string) + appliedCount := 0 for _, grafana := range instances.Items { // can be removed in go 1.22+ grafana := grafana + appliedPolicy := grafana.Annotations[annotationAppliedNotificationPolicy] + if appliedPolicy != "" && appliedPolicy != notificationPolicy.NamespacedResource() { + controllerLog.Info("instance already has a different notification policy applied - skipping", "grafana", grafana.Name) + continue + } + appliedCount++ + if grafana.Status.Stage != grafanav1beta1.OperatorStageComplete || grafana.Status.StageStatus != grafanav1beta1.OperatorStageResultSuccess { controllerLog.Info("grafana instance not ready", "grafana", grafana.Name) continue @@ -134,7 +142,7 @@ func (r *GrafanaNotificationPolicyReconciler) Reconcile(ctx context.Context, req applyErrors[fmt.Sprintf("%s/%s", grafana.Namespace, grafana.Name)] = err.Error() } } - condition := buildSynchronizedCondition("Notification Policy", conditionFolderSynchronized, notificationPolicy.Generation, applyErrors, len(instances.Items)) + condition := buildSynchronizedCondition("Notification Policy", conditionNotificationPolicySynchronized, notificationPolicy.Generation, applyErrors, appliedCount) meta.SetStatusCondition(¬ificationPolicy.Status.Conditions, condition) return ctrl.Result{RequeueAfter: notificationPolicy.Spec.ResyncPeriod.Duration}, nil @@ -151,6 +159,10 @@ func (r *GrafanaNotificationPolicyReconciler) reconcileWithInstance(ctx context. if _, err := cl.Provisioning.PutPolicyTree(params); err != nil { //nolint:errcheck return fmt.Errorf("applying notification policy: %w", err) } + instance.Annotations[annotationAppliedNotificationPolicy] = notificationPolicy.NamespacedResource() + if err := r.Client.Update(ctx, instance); err != nil { + return fmt.Errorf("saving applied policy to instance CR: %w", err) + } return nil } @@ -162,6 +174,11 @@ func (r *GrafanaNotificationPolicyReconciler) resetInstance(ctx context.Context, if _, err := cl.Provisioning.ResetPolicyTree(); err != nil { //nolint:errcheck return fmt.Errorf("resetting policy tree") } + delete(instance.Annotations, annotationAppliedNotificationPolicy) + if err := r.Client.Update(ctx, instance); err != nil { + return fmt.Errorf("removing applied policy from instance CR: %w", err) + } + return nil } @@ -174,6 +191,12 @@ func (r *GrafanaNotificationPolicyReconciler) finalize(ctx context.Context, noti } for _, i := range instances.Items { instance := i + appliedPolicy := i.Annotations[annotationAppliedNotificationPolicy] + if appliedPolicy != "" && appliedPolicy != notificationPolicy.NamespacedResource() { + r.Log.Info("instance already has a different notification policy applied - skipping", "grafana", instance.Name) + continue + } + if err := r.resetInstance(ctx, &instance); err != nil { return fmt.Errorf("resetting instance notification policy: %w", err) } diff --git a/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafananotificationpolicies.yaml b/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafananotificationpolicies.yaml index d52bc493a..a1ed76257 100644 --- a/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafananotificationpolicies.yaml +++ b/deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafananotificationpolicies.yaml @@ -96,6 +96,7 @@ spec: pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ type: string route: + description: Routes for alerts to match against properties: continue: description: continue