fix(controller): correct metrics handling (#174)

Signed-off-by: Oliver Bähler <[email protected]>

Author: oliverbaehler

README.md

@@ -27,9 +27,7 @@ This Operators introduces the concept of [SopsProviders](./docs/usage.md#provide
 
 With this option an Kubernetes users may manage their own keys and [`SopsSecrets`](./docs/usage.md#sopssecrets). The implementation of `SopsSecrets` allows them to be applied to the Kubernetes API with sops encryption-meta. The entire decryption happens within the cluster. So a `SopsSecret` is applied the way it's stored eg. in git.
 
-
-![Sops Operator](./docs/assets/sops-operator.gif)
-
+![Sops Operator](./docs/assets/sops-operator.drawio.png)
 
 ## Documentation
 

charts/sops-operator/README.md

@@ -93,7 +93,7 @@ The following Values are available for this chart.
 | monitoring.enabled | bool | `false` | Enable Monitoring of the Operator |
 | monitoring.rules.annotations | object | `{}` | Assign additional Annotations |
 | monitoring.rules.enabled | bool | `true` | Enable deployment of PrometheusRules |
-| monitoring.rules.groups | list | `[{"name":"SopsAlerts","rules":[{"alert":"ProviderNotReady","annotations":{"description":"Secret {{ $labels.name }} has been in a NotReady state for over 15 minutes.","summary":"Provider {{ $labels.name }} is not ready"},"expr":"sops_provider_condition{status=\"NotReady\"} == 1","for":"15m","labels":{"severity":"warning"}},{"alert":"SecretNotReady","annotations":{"description":"Secret {{ $labels.name }} in {{ $labels.namespace }} has been in a NotReady state for over 15 minutes.","summary":"Secret {{ $labels.name }} in {{ $labels.namespace }} is not ready"},"expr":"sops_secret_condition{status=\"NotReady\"} == 1","for":"15m","labels":{"severity":"warning"}}]}]` | Prometheus Groups for the rule |
+| monitoring.rules.groups | list | `[{"name":"SopsAlerts","rules":[{"alert":"ProviderNotReady","annotations":{"description":"Secret {{ $labels.name }} has been in a NotReady state for over 15 minutes.","summary":"Provider {{ $labels.name }} is not ready"},"expr":"sops_provider_condition{status=\"NotReady\"} == 1","for":"15m","labels":{"severity":"warning"}},{"alert":"SecretNotReady","annotations":{"description":"Secret {{ $labels.name }} in {{ $labels.namespace }} has been in a NotReady state for over 15 minutes.","summary":"Secret {{ $labels.name }} in {{ $labels.namespace }} is not ready"},"expr":"sops_secret_condition{status=\"NotReady\"} == 1","for":"15m","labels":{"severity":"warning"}},{"alert":"GlobalSecretNotReady","annotations":{"description":"Global Secret {{ $labels.name }} has been in a NotReady state for over 15 minutes.","summary":"Global Secret {{ $labels.name }} is not ready"},"expr":"sops_global_secret_condition{status=\"NotReady\"} == 1","for":"15m","labels":{"severity":"warning"}}]}]` | Prometheus Groups for the rule |
 | monitoring.rules.labels | object | `{}` | Assign additional labels |
 | monitoring.rules.namespace | string | `""` | Install the rules into a different Namespace, as the monitoring stack one (default: the release one) |
 | monitoring.serviceMonitor.annotations | object | `{}` | Assign additional Annotations |

charts/sops-operator/values.yaml

@@ -199,6 +199,14 @@ monitoring:
           annotations:
             summary: "Secret {{ $labels.name }} in {{ $labels.namespace }} is not ready"
             description: "Secret {{ $labels.name }} in {{ $labels.namespace }} has been in a NotReady state for over 15 minutes."
+        - alert: GlobalSecretNotReady
+          expr: sops_global_secret_condition{status="NotReady"} == 1
+          for: 15m
+          labels:
+            severity: warning
+          annotations:
+            summary: "Global Secret {{ $labels.name }} is not ready"
+            description: "Global Secret {{ $labels.name }} has been in a NotReady state for over 15 minutes."
 
   # ServiceMonitor
   serviceMonitor:

docs/assets/sops-operator.drawio

@@ -3,10 +3,20 @@
 Via the `/metrics` endpoint and the dedicated port you can scrape Prometheus Metrics. Amongst the standard [Kubebuilder Metrics](https://book-v1.book.kubebuilder.io/beyond_basics/controller_metrics) we provide metrics, to give you oversight of what's currently working and what's broken. This way you can always be informed, when something is not working as expected. Our custom metrics are prefixed with `sops_`:
 
 ```shell
-sops_provider_condition{name="default-onboarding",status="NotReady"} 0
-sops_provider_condition{name="default-onboarding",status="Ready"} 1
-sops_secret_condition{name="dev-onboarding",namespace="secret-namespace",status="NotReady"} 0
-sops_secret_condition{name="dev-onboarding",namespace="secret-namespace",status="Ready"} 1
+# HELP sops_provider_condition The current condition status of a Provider.
+# TYPE sops_provider_condition gauge
+sops_provider_condition{name="sample-provider",status="NotReady"} 0
+sops_provider_condition{name="sample-provider",status="Ready"} 1
+
+# HELP sops_secret_condition The current condition status of a Secret.
+# TYPE sops_secret_condition gauge
+sops_secret_condition{name="secret-key-1",namespace="default",status="NotReady"} 0
+sops_secret_condition{name="secret-key-1",namespace="default",status="Ready"} 1
+
+# HELP sops_global_secret_condition The current condition status of a Global Secret.
+# TYPE sops_global_secret_condition gauge
+sops_global_secret_condition{name="global-secret-key-1",status="NotReady"} 1
+sops_global_secret_condition{name="global-secret-key-1",status="Ready"} 0
 ```
 
 The Helm-Chart comes with a [ServiceMonitor](https://github.com/prometheus-operator/prometheus-operator/blob/main/Documentation/api.md#servicemonitor) and [PrometheusRules](https://github.com/prometheus-operator/prometheus-operator/blob/main/Documentation/api.md#monitoring.coreos.com/v1.PrometheusRule)

docs/assets/sops-operator.drawio.png

@@ -6,6 +6,7 @@ package controllers
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"github.com/go-logr/logr"
 	sopsv1alpha1 "github.com/peak-scale/sops-operator/api/v1alpha1"
@@ -23,7 +24,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -102,26 +102,36 @@ func (r *SopsProviderReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return reconcile.Result{}, nil
 	}
 
+	defer func() {
+		r.Metrics.RecordProviderCondition(instance)
+	}()
+
 	reconcileErr := r.reconcile(ctx, log, instance)
 	if reconcileErr != nil {
 		instance.Status.Condition = meta.NewNotReadyCondition(instance, reconcileErr.Error())
 	}
 
-	r.Metrics.RecordProviderCondition(instance)
-
 	// Always Post Status
-	err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
-		log.V(10).Info("updating", "status", instance.Status)
-		_, err = controllerutil.CreateOrUpdate(ctx, r.Client, instance.DeepCopy(), func() error {
-			return r.Client.Status().Update(ctx, instance, &client.SubResourceUpdateOptions{})
-		})
+	err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		current := &sopsv1alpha1.SopsProvider{}
+		if err := r.Get(ctx, client.ObjectKeyFromObject(instance), current); err != nil {
+			return fmt.Errorf("failed to refetch instance before update: %w", err)
+		}
+
+		current.Status = instance.Status
 
-		return
+		log.V(7).Info("updating status", "status", current.Status)
+
+		return r.Client.Status().Update(ctx, current)
 	})
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 
+	if reconcileErr != nil {
+		return ctrl.Result{}, reconcileErr
+	}
+
 	return ctrl.Result{}, nil
 }
 

docs/assets/sops-operator.gif

@@ -117,7 +117,7 @@ func (r *SopsSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	defer func() {
-		r.Metrics.DeleteSecretCondition(instance)
+		r.Metrics.RecordSecretCondition(instance)
 	}()
 
 	// Main Reconciler

docs/monitoring.md

@@ -7,7 +7,6 @@ import (
 	sopsv1alpha1 "github.com/peak-scale/sops-operator/api/v1alpha1"
 	"github.com/peak-scale/sops-operator/internal/meta"
 	"github.com/prometheus/client_golang/prometheus"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	crtlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
 )
 
@@ -67,7 +66,7 @@ func (r *Recorder) Collectors() []prometheus.Collector {
 func (r *Recorder) RecordProviderCondition(provider *sopsv1alpha1.SopsProvider) {
 	for _, status := range []string{meta.ReadyCondition, meta.NotReadyCondition} {
 		var value float64
-		if provider.Status.Condition.Status == metav1.ConditionTrue {
+		if provider.Status.Condition.Type == status {
 			value = 1
 		}
 
@@ -79,7 +78,7 @@ func (r *Recorder) RecordProviderCondition(provider *sopsv1alpha1.SopsProvider)
 func (r *Recorder) RecordSecretCondition(secret *sopsv1alpha1.SopsSecret) {
 	for _, status := range []string{meta.ReadyCondition, meta.NotReadyCondition} {
 		var value float64
-		if secret.Status.Condition.Status == metav1.ConditionTrue {
+		if secret.Status.Condition.Type == status {
 			value = 1
 		}
 
@@ -91,7 +90,7 @@ func (r *Recorder) RecordSecretCondition(secret *sopsv1alpha1.SopsSecret) {
 func (r *Recorder) RecordGlobalSecretCondition(secret *sopsv1alpha1.GlobalSopsSecret) {
 	for _, status := range []string{meta.ReadyCondition, meta.NotReadyCondition} {
 		var value float64
-		if secret.Status.Condition.Status == metav1.ConditionTrue {
+		if secret.Status.Condition.Type == status {
 			value = 1
 		}