Skip to content

Commit 6b83f3f

Browse files
anik120anik120
committed
Bug 1850237: Delete subscription metric when an operator is uninstalled
When an operator was subscribed to using a Subscription Object, the subscription_sync_total metric was emitted whenever the Subscription Object was created/updated/deleted. This PR updates that behaviour to emit the metric only when the Subscription object is created/updated, and deletes the time series for that particular subscription when the subscription object is deleted.
1 parent 0bcd497 commit 6b83f3f

File tree

5 files changed

+254
-63
lines changed

5 files changed

+254
-63
lines changed

‎pkg/controller/operators/catalog/subscription/syncer.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func (s *subscriptionSyncer) Sync(ctx context.Context, event kubestate.ResourceE
5050
return err
5151
}
5252

53-
s.recordMetrics(res)
53+
metrics.EmitSubMetric(res)
5454

5555
logger := s.logger.WithFields(logrus.Fields{
5656
"reconciling": fmt.Sprintf("%T", res),
@@ -68,8 +68,10 @@ func (s *subscriptionSyncer) Sync(ctx context.Context, event kubestate.ResourceE
6868
initial = initial.Add()
6969
case kubestate.ResourceUpdated:
7070
initial = initial.Update()
71+
metrics.UpdateSubsSyncCounterStorage(res)
7172
case kubestate.ResourceDeleted:
7273
initial = initial.Delete()
74+
metrics.DeleteSubsMetric(res)
7375
}
7476

7577
reconciled, err := s.reconcilers.Reconcile(ctx, initial)
@@ -85,15 +87,6 @@ func (s *subscriptionSyncer) Sync(ctx context.Context, event kubestate.ResourceE
8587
return nil
8688
}
8789

88-
func (s *subscriptionSyncer) recordMetrics(sub *v1alpha1.Subscription) {
89-
// sub.Spec is not a required field.
90-
if sub.Spec == nil {
91-
return
92-
}
93-
94-
metrics.CounterForSubscription(sub.GetName(), sub.Status.InstalledCSV, sub.Spec.Channel, sub.Spec.Package).Inc()
95-
}
96-
9790
func (s *subscriptionSyncer) Notify(event kubestate.ResourceEvent) {
9891
s.notify(event)
9992
}

‎pkg/controller/operators/catalog/subscription/syncer_test.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,19 @@ import (
66

77
"github.com/sirupsen/logrus"
88
"github.com/stretchr/testify/require"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
910

1011
"github.com/operator-framework/api/pkg/operators/v1alpha1"
1112
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubestate"
1213
)
1314

15+
const (
16+
name = "test-subscription"
17+
packageName = "test-package"
18+
channel = "test-channel"
19+
installedCSVName = "test-csv"
20+
)
21+
1422
func TestSync(t *testing.T) {
1523
type fields struct {
1624
syncer kubestate.Syncer
@@ -39,7 +47,22 @@ func TestSync(t *testing.T) {
3947
args: args{
4048
event: kubestate.NewResourceEvent(
4149
kubestate.ResourceAdded,
42-
&v1alpha1.Subscription{},
50+
&v1alpha1.Subscription{
51+
TypeMeta: metav1.TypeMeta{
52+
Kind: v1alpha1.SubscriptionKind,
53+
APIVersion: v1alpha1.SubscriptionCRDAPIVersion,
54+
},
55+
ObjectMeta: metav1.ObjectMeta{
56+
Name: name,
57+
},
58+
Spec: &v1alpha1.SubscriptionSpec{
59+
Package: packageName,
60+
Channel: channel,
61+
},
62+
Status: v1alpha1.SubscriptionStatus{
63+
InstalledCSV: installedCSVName,
64+
},
65+
},
4366
),
4467
},
4568
want: want{

‎pkg/metrics/metrics.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package metrics
22

33
import (
44
"context"
5+
56
"github.com/prometheus/client_golang/prometheus"
67
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
78
"k8s.io/apimachinery/pkg/labels"
@@ -168,8 +169,19 @@ var (
168169
},
169170
[]string{NAMESPACE_LABEL, NAME_LABEL, VERSION_LABEL, PHASE_LABEL, REASON_LABEL},
170171
)
172+
173+
// subscriptionSyncCounters keeps a record of the promethues counters emitted by
174+
// Subscription objects. The key of a record is the Subscription name, while the value
175+
// is struct containing label values used in the counter
176+
subscriptionSyncCounters = make(map[string]subscriptionSyncLabelValues)
171177
)
172178

179+
type subscriptionSyncLabelValues struct {
180+
installedCSV string
181+
pkg string
182+
channel string
183+
}
184+
173185
func RegisterOLM() {
174186
prometheus.MustRegister(csvCount)
175187
prometheus.MustRegister(csvSucceeded)
@@ -217,3 +229,45 @@ func EmitCSVMetric(oldCSV *olmv1alpha1.ClusterServiceVersion, newCSV *olmv1alpha
217229
csvAbnormal.WithLabelValues(newCSV.Namespace, newCSV.Name, newCSV.Spec.Version.String(), string(newCSV.Status.Phase), string(newCSV.Status.Reason)).Set(1)
218230
}
219231
}
232+
233+
func EmitSubMetric(sub *olmv1alpha1.Subscription) {
234+
if sub.Spec == nil {
235+
return
236+
}
237+
SubscriptionSyncCount.WithLabelValues(sub.GetName(), sub.Status.InstalledCSV, sub.Spec.Channel, sub.Spec.Package).Inc()
238+
if _, present := subscriptionSyncCounters[sub.GetName()]; !present {
239+
subscriptionSyncCounters[sub.GetName()] = subscriptionSyncLabelValues{
240+
installedCSV: sub.Status.InstalledCSV,
241+
pkg: sub.Spec.Package,
242+
channel: sub.Spec.Channel,
243+
}
244+
}
245+
}
246+
247+
func DeleteSubsMetric(sub *olmv1alpha1.Subscription) {
248+
if sub.Spec == nil {
249+
return
250+
}
251+
SubscriptionSyncCount.DeleteLabelValues(sub.GetName(), sub.Status.InstalledCSV, sub.Spec.Channel, sub.Spec.Package)
252+
}
253+
254+
func UpdateSubsSyncCounterStorage(sub *olmv1alpha1.Subscription) {
255+
if sub.Spec == nil {
256+
return
257+
}
258+
counterValues := subscriptionSyncCounters[sub.GetName()]
259+
260+
if sub.Spec.Channel != counterValues.channel ||
261+
sub.Spec.Package != counterValues.pkg ||
262+
sub.Status.InstalledCSV != counterValues.installedCSV {
263+
264+
// Delete metric will label values of old Subscription first
265+
SubscriptionSyncCount.DeleteLabelValues(sub.GetName(), counterValues.installedCSV, counterValues.channel, counterValues.pkg)
266+
267+
counterValues.installedCSV = sub.Status.InstalledCSV
268+
counterValues.pkg = sub.Spec.Package
269+
counterValues.channel = sub.Spec.Channel
270+
271+
subscriptionSyncCounters[sub.GetName()] = counterValues
272+
}
273+
}

‎test/e2e/metrics_e2e_test.go

Lines changed: 156 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"context"
77
"fmt"
88
"regexp"
9+
"time"
910

1011
. "github.com/onsi/ginkgo"
1112
. "github.com/onsi/gomega"
@@ -15,61 +16,170 @@ import (
1516
"k8s.io/apimachinery/pkg/util/net"
1617

1718
"github.com/operator-framework/api/pkg/operators/v1alpha1"
19+
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
1820
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
21+
"github.com/operator-framework/operator-lifecycle-manager/pkg/metrics"
1922
)
2023

21-
var _ = Describe("Metrics", func() {
22-
It("endpoint", func() {
23-
24-
// TestMetrics tests the metrics endpoint of the OLM pod.
25-
26-
c := newKubeClient()
27-
crc := newCRClient()
28-
29-
failingCSV := v1alpha1.ClusterServiceVersion{
30-
TypeMeta: metav1.TypeMeta{
31-
Kind: v1alpha1.ClusterServiceVersionKind,
32-
APIVersion: v1alpha1.ClusterServiceVersionAPIVersion,
33-
},
34-
ObjectMeta: metav1.ObjectMeta{
35-
Name: genName("failing-csv-test-"),
36-
},
37-
Spec: v1alpha1.ClusterServiceVersionSpec{
38-
InstallStrategy: v1alpha1.NamedInstallStrategy{
39-
StrategyName: v1alpha1.InstallStrategyNameDeployment,
40-
StrategySpec: strategy,
41-
},
42-
},
43-
}
24+
var _ = FDescribe("Metrics", func() {
4425

45-
cleanupCSV, err := createCSV(GinkgoT(), c, crc, failingCSV, testNamespace, false, false)
46-
Expect(err).ToNot(HaveOccurred())
26+
var (
27+
c operatorclient.ClientInterface
28+
crc versioned.Interface
29+
)
4730

48-
_, err = fetchCSV(GinkgoT(), crc, failingCSV.Name, testNamespace, csvFailedChecker)
49-
Expect(err).ToNot(HaveOccurred())
31+
BeforeEach(func() {
32+
c = newKubeClient()
33+
crc = newCRClient()
34+
35+
})
36+
37+
Context("CSV metrics", func() {
38+
It("endpoint", func() {
39+
40+
// TestMetrics tests the metrics endpoint of the OLM pod.
41+
42+
failingCSV := v1alpha1.ClusterServiceVersion{
43+
TypeMeta: metav1.TypeMeta{
44+
Kind: v1alpha1.ClusterServiceVersionKind,
45+
APIVersion: v1alpha1.ClusterServiceVersionAPIVersion,
46+
},
47+
ObjectMeta: metav1.ObjectMeta{
48+
Name: genName("failing-csv-test-"),
49+
},
50+
Spec: v1alpha1.ClusterServiceVersionSpec{
51+
InstallStrategy: v1alpha1.NamedInstallStrategy{
52+
StrategyName: v1alpha1.InstallStrategyNameDeployment,
53+
StrategySpec: strategy,
54+
},
55+
},
56+
}
57+
58+
cleanupCSV, err := createCSV(GinkgoT(), c, crc, failingCSV, testNamespace, false, false)
59+
Expect(err).ToNot(HaveOccurred())
60+
61+
_, err = fetchCSV(GinkgoT(), crc, failingCSV.Name, testNamespace, csvFailedChecker)
62+
Expect(err).ToNot(HaveOccurred())
63+
64+
// Verify metrics have been emitted for packageserver csv
65+
Expect(getMetricsFromPod(c, getPodWithLabel(c, "app=olm-operator"), "8081")).To(And(
66+
ContainSubstring("csv_abnormal"),
67+
ContainSubstring(fmt.Sprintf("name=\"%s\"", failingCSV.Name)),
68+
ContainSubstring("phase=\"Failed\""),
69+
ContainSubstring("reason=\"UnsupportedOperatorGroup\""),
70+
ContainSubstring("version=\"0.0.0\""),
71+
ContainSubstring("csv_succeeded"),
72+
))
73+
74+
cleanupCSV()
75+
76+
// Verify that when the csv has been deleted, it deletes the corresponding CSV metrics
77+
Expect(getMetricsFromPod(c, getPodWithLabel(c, "app=olm-operator"), "8081")).ToNot(And(
78+
ContainSubstring("csv_abnormal{name=\"%s\"", failingCSV.Name),
79+
ContainSubstring("csv_succeeded{name=\"%s\"", failingCSV.Name),
80+
))
81+
})
82+
})
5083

51-
// Verify metrics have been emitted for packageserver csv
52-
Expect(getMetricsFromPod(c, getOLMPod(c), "8081")).To(And(
53-
ContainSubstring("csv_abnormal"),
54-
ContainSubstring(fmt.Sprintf("name=\"%s\"", failingCSV.Name)),
55-
ContainSubstring("phase=\"Failed\""),
56-
ContainSubstring("reason=\"UnsupportedOperatorGroup\""),
57-
ContainSubstring("version=\"0.0.0\""),
58-
ContainSubstring("csv_succeeded"),
59-
))
60-
61-
cleanupCSV()
62-
63-
// Verify that when the csv has been deleted, it deletes the corresponding CSV metrics
64-
Expect(getMetricsFromPod(c, getOLMPod(c), "8081")).ToNot(And(
65-
ContainSubstring("csv_abnormal{name=\"%s\"", failingCSV.Name),
66-
ContainSubstring("csv_succeeded{name=\"%s\"", failingCSV.Name),
67-
))
84+
Context("Subscription Metric", func() {
85+
var (
86+
subscriptionCleanup cleanupFunc
87+
subscription *v1alpha1.Subscription
88+
)
89+
When("A subscription object is created", func() {
90+
91+
BeforeEach(func() {
92+
subscriptionCleanup, subscription = createSubscription(GinkgoT(), crc, testNamespace, "metric-subscription", testPackageName, stableChannel, v1alpha1.ApprovalManual)
93+
})
94+
95+
It("generates subscription_sync_total metric", func() {
96+
97+
// Verify metrics have been emitted for subscription
98+
Eventually(func() string {
99+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
100+
}, time.Minute, 5*time.Second).Should(And(
101+
ContainSubstring("subscription_sync_total"),
102+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.NAME_LABEL, "metric-subscription")),
103+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, stableChannel)),
104+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.PACKAGE_LABEL, testPackageName))))
105+
})
106+
107+
When("The subscription object is updated", func() {
108+
109+
BeforeEach(func() {
110+
updatedSubscription := getSubscription(GinkgoT(), crc, subscription.GetNamespace(), subscription.GetName())
111+
updatedSubscription.Spec.Channel = betaChannel
112+
updateSubscription(GinkgoT(), crc, updatedSubscription)
113+
})
114+
115+
It("deletes the old Subscription metric and emits the new metric", func() {
116+
Eventually(func() string {
117+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
118+
}, time.Minute, 5*time.Second).ShouldNot(And(
119+
ContainSubstring("subscription_sync_total{name=\"metric-subscription\""),
120+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, stableChannel))))
121+
122+
Eventually(func() string {
123+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
124+
}, time.Minute, 5*time.Second).Should(And(
125+
ContainSubstring("subscription_sync_total"),
126+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.NAME_LABEL, "metric-subscription")),
127+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, betaChannel)),
128+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.PACKAGE_LABEL, testPackageName))))
129+
})
130+
131+
When("The Subscription object is updated again", func() {
132+
133+
BeforeEach(func() {
134+
updatedSubscription := getSubscription(GinkgoT(), crc, subscription.GetNamespace(), subscription.GetName())
135+
updatedSubscription.Spec.Channel = alphaChannel
136+
updateSubscription(GinkgoT(), crc, updatedSubscription)
137+
})
138+
139+
It("deletes the old subscription metric and emits the new metric(there is only one metric for the subscription)", func() {
140+
Eventually(func() string {
141+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
142+
}, time.Minute, 5*time.Second).ShouldNot(And(
143+
ContainSubstring("subscription_sync_total{name=\"metric-subscription-for-update\""),
144+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, stableChannel)),
145+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, betaChannel))))
146+
147+
Eventually(func() string {
148+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
149+
}, time.Minute, 5*time.Second).Should(And(
150+
ContainSubstring("subscription_sync_total"),
151+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.NAME_LABEL, "metric-subscription")),
152+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, alphaChannel)),
153+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.PACKAGE_LABEL, testPackageName))))
154+
})
155+
})
156+
})
157+
158+
AfterEach(func() {
159+
if subscriptionCleanup != nil {
160+
subscriptionCleanup()
161+
}
162+
})
163+
})
164+
When("A Subscription object is deleted", func() {
165+
166+
BeforeEach(func() {
167+
subscriptionCleanup, _ = createSubscription(GinkgoT(), crc, testNamespace, "metric-subscription-for-deletion", testPackageName, stableChannel, v1alpha1.ApprovalManual)
168+
if subscriptionCleanup != nil {
169+
subscriptionCleanup()
170+
}
171+
})
172+
It("deletes the Subscription metric", func() {
173+
Eventually(func() string {
174+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
175+
}, time.Minute, 5*time.Second).ShouldNot(ContainSubstring("subscription_sync_total{name=\"metric-subscription-for-deletion\""))
176+
})
177+
})
68178
})
69179
})
70180

71-
func getOLMPod(client operatorclient.ClientInterface) *corev1.Pod {
72-
listOptions := metav1.ListOptions{LabelSelector: "app=olm-operator"}
181+
func getPodWithLabel(client operatorclient.ClientInterface, label string) *corev1.Pod {
182+
listOptions := metav1.ListOptions{LabelSelector: label}
73183
var podList *corev1.PodList
74184
Eventually(func() (err error) {
75185
podList, err = client.KubernetesInterface().CoreV1().Pods(operatorNamespace).List(context.TODO(), listOptions)

0 commit comments

Comments
 (0)