From 6e060441699ba2778a38f42dfc14546652f98dd5 Mon Sep 17 00:00:00 2001
From: Muhammad Talal Anwar <muhammad.talal.anwar@sap.com>
Date: Wed, 16 Sep 2020 05:56:06 +0200
Subject: [PATCH 1/2] controller: use one loop for worker

---
 CHANGELOG.md                                 |  9 ++++++-
 internal/controller/absent_prometheusrule.go |  1 +
 internal/controller/controller.go            | 28 ++++++++------------
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b8539089..281f5b47 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.6.1] - 2020-09-16
+
+### Fixed
+
+- Use a single loop for controller worker.
+
 ## [0.6.0] - 2020-08-26
 
 ### Added
@@ -63,7 +69,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Initial release.
 
-[unreleased]: https://github.com/sapcc/absent-metrics-operator/compare/v0.6.0...HEAD
+[unreleased]: https://github.com/sapcc/absent-metrics-operator/compare/v0.6.1...HEAD
+[0.6.1]: https://github.com/sapcc/absent-metrics-operator/compare/v0.6.0...v0.6.1
 [0.6.0]: https://github.com/sapcc/absent-metrics-operator/compare/v0.5.2...v0.6.0
 [0.5.2]: https://github.com/sapcc/absent-metrics-operator/compare/v0.5.1...v0.5.2
 [0.5.1]: https://github.com/sapcc/absent-metrics-operator/compare/v0.5.0...v0.5.1
diff --git a/internal/controller/absent_prometheusrule.go b/internal/controller/absent_prometheusrule.go
index 75d84612..82333e52 100644
--- a/internal/controller/absent_prometheusrule.go
+++ b/internal/controller/absent_prometheusrule.go
@@ -149,6 +149,7 @@ OuterLoop:
 		// This RuleGroup should be carried over as is.
 		new = append(new, oldG)
 	}
+
 	// Add the pending RuleGroups.
 	for _, g := range absentAlertRuleGroups {
 		if !updated[g.Name] {
diff --git a/internal/controller/controller.go b/internal/controller/controller.go
index ff97de53..07d18eeb 100644
--- a/internal/controller/controller.go
+++ b/internal/controller/controller.go
@@ -235,27 +235,21 @@ func (c *Controller) runWorker() {
 	defer reconcileT.Stop()
 	maintenanceT := time.NewTicker(maintenancePeriod)
 	defer maintenanceT.Stop()
-	done := make(chan struct{})
-	go func() {
-		for {
-			select {
-			case <-done:
+	for {
+		select {
+		case <-reconcileT.C:
+			c.enqueueAllObjects()
+		case <-maintenanceT.C:
+			if err := c.cleanUpOrphanedAbsentAlertsCluster(); err != nil {
+				c.logger.ErrorWithBackoff("msg", "could not cleanup orphaned absent alerts from cluster",
+					"err", err)
+			}
+		default:
+			if ok := c.processNextWorkItem(); !ok {
 				return
-			case <-reconcileT.C:
-				c.enqueueAllObjects()
-			case <-maintenanceT.C:
-				if err := c.cleanUpOrphanedAbsentAlertsCluster(); err != nil {
-					c.logger.ErrorWithBackoff("msg", "could not cleanup orphaned absent alerts from cluster",
-						"err", err)
-				}
 			}
 		}
-	}()
-
-	for c.processNextWorkItem() {
 	}
-
-	done <- struct{}{}
 }
 
 // processNextWorkItem will read a single work item off the workqueue and

From 7b8df51193e376519ac98b03e6bc3f1a9e97e05e Mon Sep 17 00:00:00 2001
From: Muhammad Talal Anwar <muhammad.talal.anwar@sap.com>
Date: Wed, 16 Sep 2020 08:10:00 +0200
Subject: [PATCH 2/2] controller: delete reconcile metric for orphaned PromRule
 in advance

---
 CHANGELOG.md                      | 5 +++++
 internal/controller/cleanup.go    | 2 +-
 internal/controller/controller.go | 4 +---
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 281f5b47..44ead62a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [0.6.1] - 2020-09-16
 
+### Changed
+
+- Delete the reconcile time metric for orphaned `PrometheusRule` in advance,
+  regardless of the cleanup error status.
+
 ### Fixed
 
 - Use a single loop for controller worker.
diff --git a/internal/controller/cleanup.go b/internal/controller/cleanup.go
index 54ac268a..0619425a 100644
--- a/internal/controller/cleanup.go
+++ b/internal/controller/cleanup.go
@@ -84,10 +84,10 @@ func (c *Controller) cleanUpOrphanedAbsentAlertsCluster() error {
 
 			aPR := &absentPrometheusRule{PrometheusRule: pr}
 			for n := range cleanup {
+				c.metrics.SuccessfulPrometheusRuleReconcileTime.DeleteLabelValues(namespace, n)
 				if err := c.cleanUpOrphanedAbsentAlerts(n, aPR); err != nil {
 					return err
 				}
-				c.metrics.SuccessfulPrometheusRuleReconcileTime.DeleteLabelValues(namespace, n)
 			}
 		}
 	}
diff --git a/internal/controller/controller.go b/internal/controller/controller.go
index 07d18eeb..eb7c9ca9 100644
--- a/internal/controller/controller.go
+++ b/internal/controller/controller.go
@@ -309,10 +309,8 @@ func (c *Controller) syncHandler(key string) error {
 		// The resource may no longer exist, in which case we clean up any
 		// orphaned absent alerts.
 		c.logger.Debug("msg", "PrometheusRule no longer exists", "key", key)
+		c.metrics.SuccessfulPrometheusRuleReconcileTime.DeleteLabelValues(namespace, name)
 		err = c.cleanUpOrphanedAbsentAlertsNamespace(name, namespace)
-		if err == nil {
-			c.metrics.SuccessfulPrometheusRuleReconcileTime.DeleteLabelValues(namespace, name)
-		}
 	default:
 		// Requeue object for later processing.
 		return err