Bug 1822396: Update metric when Subscription is updated

In #1519, the subscription_sync_total metric was updated when the
related subscription object was updated. However, the PR had a bug,
in which the map used to store metric related information was not
being updated when the metric was udpated. As a result of which any
update after the first update was not removing the old metric. This
PR fixes that bug.

Author: anik120

pkg/metrics/metrics.go

@@ -267,5 +267,7 @@ func UpdateSubsSyncCounterStorage(sub *olmv1alpha1.Subscription) {
 		counterValues.installedCSV = sub.Status.InstalledCSV
 		counterValues.pkg = sub.Spec.Package
 		counterValues.channel = sub.Spec.Channel
+
+		subscriptionSyncCounters[sub.GetName()] = counterValues
 	}
 }

test/e2e/like_metric_matcher_test.go

@@ -47,6 +47,30 @@ func WithLabel(n, v string) MetricPredicate {
 	}
 }
 
+func WithName(name string) MetricPredicate {
+	return WithLabel("name", name)
+}
+
+func WithChannel(channel string) MetricPredicate {
+	return WithLabel("channel", channel)
+}
+
+func WithPackage(pkg string) MetricPredicate {
+	return WithLabel("package", pkg)
+}
+
+func WithPhase(phase string) MetricPredicate {
+	return WithLabel("phase", phase)
+}
+
+func WithReason(reason string) MetricPredicate {
+	return WithLabel("reason", reason)
+}
+
+func WithVersion(version string) MetricPredicate {
+	return WithLabel("version", version)
+}
+
 func WithValue(v float64) MetricPredicate {
 	return MetricPredicate{
 		name: fmt.Sprintf("WithValue(%g)", v),

test/e2e/metrics_e2e_test.go

@@ -75,15 +75,15 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
 				Expect(getMetricsFromPod(c, getPodWithLabel(c, "app=olm-operator"), "8081")).To(And(
 					ContainElement(LikeMetric(
 						WithFamily("csv_abnormal"),
-						WithLabel("name", failingCSV.Name),
-						WithLabel("phase", "Failed"),
-						WithLabel("reason", "UnsupportedOperatorGroup"),
-						WithLabel("version", "0.0.0"),
+						WithName(failingCSV.Name),
+						WithPhase("Failed"),
+						WithReason("UnsupportedOperatorGroup"),
+						WithVersion("0.0.0"),
 					)),
 					ContainElement(LikeMetric(
 						WithFamily("csv_succeeded"),
 						WithValue(0),
-						WithLabel("name", failingCSV.Name),
+						WithName(failingCSV.Name),
 					)),
 				))
 
@@ -101,8 +101,8 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
 				It("deletes its associated CSV metrics", func() {
 					// Verify that when the csv has been deleted, it deletes the corresponding CSV metrics
 					Expect(getMetricsFromPod(c, getPodWithLabel(c, "app=olm-operator"), "8081")).ToNot(And(
-						ContainElement(LikeMetric(WithFamily("csv_abnormal"), WithLabel("name", failingCSV.Name))),
-						ContainElement(LikeMetric(WithFamily("csv_succeeded"), WithLabel("name", failingCSV.Name))),
+						ContainElement(LikeMetric(WithFamily("csv_abnormal"), WithName(failingCSV.Name))),
+						ContainElement(LikeMetric(WithFamily("csv_succeeded"), WithName(failingCSV.Name))),
 					))
 				})
 			})
@@ -133,9 +133,9 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
 					return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
 				}).Should(ContainElement(LikeMetric(
 					WithFamily("subscription_sync_total"),
-					WithLabel("name", "metric-subscription-for-create"),
-					WithLabel("channel", stableChannel),
-					WithLabel("package", testPackageName),
+					WithName("metric-subscription-for-create"),
+					WithChannel(stableChannel),
+					WithPackage(testPackageName),
 				)))
 			})
 		})
@@ -152,7 +152,7 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
 					if err != nil {
 						return err
 					}
-					s.Spec.Channel = "beta"
+					s.Spec.Channel = betaChannel
 					_, err = crc.OperatorsV1alpha1().Subscriptions(s.GetNamespace()).Update(context.TODO(), s, metav1.UpdateOptions{})
 					return err
 				}).Should(Succeed())
@@ -170,17 +170,55 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
 				}).Should(And(
 					Not(ContainElement(LikeMetric(
 						WithFamily("subscription_sync_total"),
-						WithLabel("name", "metric-subscription-for-update"),
-						WithLabel("channel", stableChannel),
+						WithName("metric-subscription-for-update"),
+						WithChannel(stableChannel),
+						WithPackage(testPackageName),
 					))),
 					ContainElement(LikeMetric(
 						WithFamily("subscription_sync_total"),
-						WithLabel("name", "metric-subscription-for-update"),
-						WithLabel("channel", "beta"),
-						WithLabel("package", testPackageName),
+						WithName("metric-subscription-for-update"),
+						WithChannel(betaChannel),
+						WithPackage(testPackageName),
 					)),
 				))
 			})
+			When("The subscription object is updated again", func() {
+
+				BeforeEach(func() {
+					Eventually(func() error {
+						s, err := crc.OperatorsV1alpha1().Subscriptions(subscription.GetNamespace()).Get(context.TODO(), subscription.GetName(), metav1.GetOptions{})
+						if err != nil {
+							return err
+						}
+						s.Spec.Channel = alphaChannel
+						_, err = crc.OperatorsV1alpha1().Subscriptions(s.GetNamespace()).Update(context.TODO(), s, metav1.UpdateOptions{})
+						return err
+					}).Should(Succeed())
+				})
+
+				It("deletes the old subscription metric and emits the new metric(there is only one metric for the subscription)", func() {
+					Eventually(func() []Metric {
+						return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
+					}).Should(And(
+						Not(ContainElement(LikeMetric(
+							WithFamily("subscription_sync_total"),
+							WithName("metric-subscription-for-update"),
+							WithChannel(stableChannel),
+						))),
+						Not(ContainElement(LikeMetric(
+							WithFamily("subscription_sync_total"),
+							WithName("metric-subscription-for-update"),
+							WithChannel(betaChannel),
+							WithPackage(testPackageName),
+						))),
+						ContainElement(LikeMetric(
+							WithFamily("subscription_sync_total"),
+							WithName("metric-subscription-for-update"),
+							WithChannel(alphaChannel),
+							WithPackage(testPackageName),
+						))))
+				})
+			})
 		})
 
 		When("A subscription object is deleted after emitting metrics", func() {
@@ -205,7 +243,7 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
 			It("deletes the Subscription metric", func() {
 				Eventually(func() []Metric {
 					return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
-				}).ShouldNot(ContainElement(LikeMetric(WithFamily("subscription_sync_total"), WithLabel("name", "metric-subscription-for-delete"))))
+				}).ShouldNot(ContainElement(LikeMetric(WithFamily("subscription_sync_total"), WithName("metric-subscription-for-delete"))))
 			})
 		})
 	})