Remove controller predicate, add externalSecretCondition metric

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -28,7 +28,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
 	"github.com/external-secrets/external-secrets/pkg/provider"
@@ -80,7 +79,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	if err != nil {
 		log.Error(err, "could not get store reference")
 		conditionSynced := NewExternalSecretCondition(esv1alpha1.ExternalSecretReady, corev1.ConditionFalse, esv1alpha1.ConditionReasonSecretSyncedError, err.Error())
-		SetExternalSecretCondition(&externalSecret.Status, *conditionSynced)
+		SetExternalSecretCondition(&externalSecret, *conditionSynced)
+
 		err = r.Status().Update(ctx, &externalSecret)
 		syncCallsError.With(syncCallsMetricLabels).Inc()
 		return ctrl.Result{RequeueAfter: requeueAfter}, nil
@@ -105,7 +105,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	if err != nil {
 		log.Error(err, "could not get provider client")
 		conditionSynced := NewExternalSecretCondition(esv1alpha1.ExternalSecretReady, corev1.ConditionFalse, esv1alpha1.ConditionReasonSecretSyncedError, err.Error())
-		SetExternalSecretCondition(&externalSecret.Status, *conditionSynced)
+		SetExternalSecretCondition(&externalSecret, *conditionSynced)
 		err = r.Status().Update(ctx, &externalSecret)
 		syncCallsError.With(syncCallsMetricLabels).Inc()
 		return ctrl.Result{RequeueAfter: requeueAfter}, nil
@@ -131,7 +131,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	if err != nil {
 		log.Error(err, "could not reconcile ExternalSecret")
 		conditionSynced := NewExternalSecretCondition(esv1alpha1.ExternalSecretReady, corev1.ConditionFalse, esv1alpha1.ConditionReasonSecretSyncedError, err.Error())
-		SetExternalSecretCondition(&externalSecret.Status, *conditionSynced)
+		SetExternalSecretCondition(&externalSecret, *conditionSynced)
 		err = r.Status().Update(ctx, &externalSecret)
 		if err != nil {
 			log.Error(err, "unable to update status")
@@ -146,7 +146,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 
 	conditionSynced := NewExternalSecretCondition(esv1alpha1.ExternalSecretReady, corev1.ConditionTrue, esv1alpha1.ConditionReasonSecretSynced, "Secret was synced")
-	SetExternalSecretCondition(&externalSecret.Status, *conditionSynced)
+	SetExternalSecretCondition(&externalSecret, *conditionSynced)
 	externalSecret.Status.RefreshTime = metav1.NewTime(time.Now())
 	err = r.Status().Update(ctx, &externalSecret)
 	if err != nil {
@@ -209,13 +209,8 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient p
 }
 
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
-	// Skip running Reconcile() when there are NO changes to the .spec field
-	// Useful to not trigger Reconcile() when only updating the object's status/metadata
-	p := predicate.GenerationChangedPredicate{}
-
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&esv1alpha1.ExternalSecret{}).
 		Owns(&corev1.Secret{}).
-		WithEventFilter(p).
 		Complete(r)
 }

pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -36,6 +36,8 @@ import (
 var (
 	fakeProvider *fake.Client
 	metric       dto.Metric
+	timeout      = time.Second * 5
+	interval     = time.Millisecond * 250
 )
 
 var _ = Describe("ExternalSecret controller", func() {
@@ -43,8 +45,6 @@ var _ = Describe("ExternalSecret controller", func() {
 		ExternalSecretName             = "test-es"
 		ExternalSecretStore            = "test-store"
 		ExternalSecretTargetSecretName = "test-secret"
-		timeout                        = time.Second * 5
-		interval                       = time.Millisecond * 250
 	)
 
 	var ExternalSecretNamespace string
@@ -70,6 +70,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		metric.Reset()
 		syncCallsTotal.Reset()
 		syncCallsError.Reset()
+		externalSecretCondition.Reset()
 	})
 
 	AfterEach(func() {
@@ -117,35 +118,16 @@ var _ = Describe("ExternalSecret controller", func() {
 				}
 				return true
 			}, timeout, interval).Should(BeTrue())
-		})
 
-		It("should increment the syncCallsTotal metric", func() {
-			ctx := context.Background()
-			es := &esv1alpha1.ExternalSecret{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      ExternalSecretName,
-					Namespace: ExternalSecretNamespace,
-				},
-				Spec: esv1alpha1.ExternalSecretSpec{
-					SecretStoreRef: esv1alpha1.SecretStoreRef{
-						Name: ExternalSecretStore,
-					},
-					Target: esv1alpha1.ExternalSecretTarget{
-						Name: ExternalSecretTargetSecretName,
-					},
-				},
-			}
-
-			Expect(k8sClient.Create(ctx, es)).Should(Succeed())
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 1.0)).To(BeTrue())
 
 			// When creating a new ExternalSecret, the Reconcile loop executes twice
 			// due the call to `controllerutil.SetControllerReference`
-			expectedValue := 2.0
-
 			Eventually(func() float64 {
 				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
 				return metric.GetCounter().GetValue()
-			}, timeout, interval).Should(Equal(expectedValue))
+			}, timeout, interval).Should(Equal(2.0))
 		})
 	})
 
@@ -194,13 +176,10 @@ var _ = Describe("ExternalSecret controller", func() {
 				return nil
 			}, timeout, interval).Should(Succeed())
 
-			// 2 from the Create() + 1 from Update()
-			expectedValue := 3.0
-
 			Eventually(func() float64 {
 				Expect(syncCallsTotal.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
 				return metric.GetCounter().GetValue()
-			}, timeout, interval).Should(Equal(expectedValue))
+			}, timeout, interval).Should(Equal(3.0))
 		})
 	})
 
@@ -407,7 +386,10 @@ var _ = Describe("ExternalSecret controller", func() {
 			Eventually(func() float64 {
 				Expect(syncCallsError.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
 				return metric.GetCounter().GetValue()
-			}, timeout, interval).Should(Equal(1.0))
+			}, timeout, interval).Should(Equal(2.0))
+
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 1.0)).To(BeTrue())
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 0.0)).To(BeTrue())
 		})
 
 		It("should set an error condition when store does not exist", func() {
@@ -458,7 +440,10 @@ var _ = Describe("ExternalSecret controller", func() {
 			Eventually(func() float64 {
 				Expect(syncCallsError.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
 				return metric.GetCounter().GetValue()
-			}, timeout, interval).Should(Equal(1.0))
+			}, timeout, interval).Should(Equal(2.0))
+
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 1.0)).To(BeTrue())
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 0.0)).To(BeTrue())
 		})
 
 		It("should set an error condition when store provider constructor fails", func() {
@@ -513,7 +498,10 @@ var _ = Describe("ExternalSecret controller", func() {
 			Eventually(func() float64 {
 				Expect(syncCallsError.WithLabelValues(ExternalSecretName, ExternalSecretNamespace).Write(&metric)).To(Succeed())
 				return metric.GetCounter().GetValue()
-			}, timeout, interval).Should(Equal(1.0))
+			}, timeout, interval).Should(Equal(2.0))
+
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 1.0)).To(BeTrue())
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 0.0)).To(BeTrue())
 		})
 
 		It("should not process stores with mismatching controller field", func() {
@@ -581,6 +569,20 @@ var _ = Describe("ExternalSecret controller", func() {
 				cond := GetExternalSecretCondition(createdES.Status, esv1alpha1.ExternalSecretReady)
 				return cond == nil
 			}, timeout, interval).Should(BeTrue())
+
+			// Condition True and False should be 0, since the Condition was not created
+			Eventually(func() float64 {
+				Expect(externalSecretCondition.WithLabelValues(ExternalSecretName, ExternalSecretNamespace, string(esv1alpha1.ExternalSecretReady), string(v1.ConditionTrue)).Write(&metric)).To(Succeed())
+				return metric.GetGauge().GetValue()
+			}, timeout, interval).Should(Equal(0.0))
+
+			Eventually(func() float64 {
+				Expect(externalSecretCondition.WithLabelValues(ExternalSecretName, ExternalSecretNamespace, string(esv1alpha1.ExternalSecretReady), string(v1.ConditionFalse)).Write(&metric)).To(Succeed())
+				return metric.GetGauge().GetValue()
+			}, timeout, interval).Should(Equal(0.0))
+
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionFalse, 0.0)).To(BeTrue())
+			Expect(externalSecretConditionShouldBe(ExternalSecretName, ExternalSecretNamespace, esv1alpha1.ExternalSecretReady, v1.ConditionTrue, 0.0)).To(BeTrue())
 		})
 	})
 })
@@ -607,6 +609,13 @@ func CreateNamespace(baseName string, c client.Client) (string, error) {
 	return ns.Name, nil
 }
 
+func externalSecretConditionShouldBe(name string, ns string, ct esv1alpha1.ExternalSecretConditionType, cs v1.ConditionStatus, v float64) bool {
+	return Eventually(func() float64 {
+		Expect(externalSecretCondition.WithLabelValues(name, ns, string(ct), string(cs)).Write(&metric)).To(Succeed())
+		return metric.GetGauge().GetValue()
+	}, timeout, interval).Should(Equal(v))
+}
+
 func init() {
 	fakeProvider = fake.New()
 	schema.ForceRegister(fakeProvider, &esv1alpha1.SecretStoreProvider{

pkg/controllers/externalsecret/metrics.go

@@ -15,14 +15,17 @@ limitations under the License.
 package externalsecret
 
 import (
+	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
+
 	"github.com/prometheus/client_golang/prometheus"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 )
 
 const (
-	ExternalSecretSubsystem = "external_secret"
-	SyncCallsKey            = "sync_calls_total"
-	SyncCallsErrorKey       = "sync_calls_error"
+	ExternalSecretSubsystem          = "externalsecret"
+	SyncCallsKey                     = "sync_calls_total"
+	SyncCallsErrorKey                = "sync_calls_error"
+	externalSecretStatusConditionKey = "status_condition"
 )
 
 var (
@@ -37,8 +40,23 @@ var (
 		Name:      SyncCallsErrorKey,
 		Help:      "Total number of the External Secret sync errors",
 	}, []string{"name", "namespace"})
+
+	externalSecretCondition = prometheus.NewGaugeVec(prometheus.GaugeOpts{
+		Subsystem: ExternalSecretSubsystem,
+		Name:      externalSecretStatusConditionKey,
+		Help:      "The status condition of a specific External Secret",
+	}, []string{"name", "namespace", "condition", "status"})
 )
 
+func updateExternalSecretCondition(es *esv1alpha1.ExternalSecret, condition *esv1alpha1.ExternalSecretStatusCondition, value float64) {
+	externalSecretCondition.With(prometheus.Labels{
+		"name":      es.Name,
+		"namespace": es.Namespace,
+		"condition": string(condition.Type),
+		"status":    string(condition.Status),
+	}).Set(value)
+}
+
 func init() {
-	metrics.Registry.MustRegister(syncCallsTotal, syncCallsError)
+	metrics.Registry.MustRegister(syncCallsTotal, syncCallsError, externalSecretCondition)
 }

pkg/controllers/externalsecret/util.go

@@ -43,16 +43,26 @@ func GetExternalSecretCondition(status esv1alpha1.ExternalSecretStatus, condType
 
 // SetExternalSecretCondition updates the external secret to include the provided
 // condition.
-func SetExternalSecretCondition(status *esv1alpha1.ExternalSecretStatus, condition esv1alpha1.ExternalSecretStatusCondition) {
-	currentCond := GetExternalSecretCondition(*status, condition.Type)
+func SetExternalSecretCondition(es *esv1alpha1.ExternalSecret, condition esv1alpha1.ExternalSecretStatusCondition) {
+	currentCond := GetExternalSecretCondition(es.Status, condition.Type)
+
 	if currentCond != nil && currentCond.Status == condition.Status && currentCond.Reason == condition.Reason {
+		updateExternalSecretCondition(es, &condition, 1.0)
 		return
 	}
+
 	// Do not update lastTransitionTime if the status of the condition doesn't change.
 	if currentCond != nil && currentCond.Status == condition.Status {
 		condition.LastTransitionTime = currentCond.LastTransitionTime
 	}
-	status.Conditions = append(filterOutCondition(status.Conditions, condition.Type), condition)
+
+	es.Status.Conditions = append(filterOutCondition(es.Status.Conditions, condition.Type), condition)
+
+	if currentCond != nil {
+		updateExternalSecretCondition(es, currentCond, 0.0)
+	}
+
+	updateExternalSecretCondition(es, &condition, 1.0)
 }
 
 func filterOutCondition(conditions []esv1alpha1.ExternalSecretStatusCondition, condType esv1alpha1.ExternalSecretConditionType) []esv1alpha1.ExternalSecretStatusCondition {