Skip to content

Commit 56976b6

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1591 from anik120/sub_sync_metric_4.5
Bug 1850237: Delete subscription metric when an operator is uninstalled
2 parents 454ee08 + c72bee4 commit 56976b6

File tree

5 files changed

+249
-62
lines changed

5 files changed

+249
-62
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: 157 additions & 45 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,172 @@ 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

2124
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-
}
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, err := crc.OperatorsV1alpha1().Subscriptions(subscription.GetNamespace()).Get(context.TODO(), subscription.GetName(), metav1.GetOptions{})
111+
Expect(err).ToNot(HaveOccurred())
112+
updatedSubscription.Spec.Channel = betaChannel
113+
updateSubscription(GinkgoT(), crc, updatedSubscription)
114+
})
115+
116+
It("deletes the old Subscription metric and emits the new metric", func() {
117+
Eventually(func() string {
118+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
119+
}, time.Minute, 5*time.Second).ShouldNot(And(
120+
ContainSubstring("subscription_sync_total{name=\"metric-subscription\""),
121+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, stableChannel))))
122+
123+
Eventually(func() string {
124+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
125+
}, time.Minute, 5*time.Second).Should(And(
126+
ContainSubstring("subscription_sync_total"),
127+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.NAME_LABEL, "metric-subscription")),
128+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, betaChannel)),
129+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.PACKAGE_LABEL, testPackageName))))
130+
})
131+
132+
When("The Subscription object is updated again", func() {
133+
134+
BeforeEach(func() {
135+
updatedSubscription, err := crc.OperatorsV1alpha1().Subscriptions(subscription.GetNamespace()).Get(context.TODO(), subscription.GetName(), metav1.GetOptions{})
136+
Expect(err).ToNot(HaveOccurred())
137+
updatedSubscription.Spec.Channel = alphaChannel
138+
updateSubscription(GinkgoT(), crc, updatedSubscription)
139+
})
140+
141+
It("deletes the old subscription metric and emits the new metric(there is only one metric for the subscription)", func() {
142+
Eventually(func() string {
143+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
144+
}, time.Minute, 5*time.Second).ShouldNot(And(
145+
ContainSubstring("subscription_sync_total{name=\"metric-subscription-for-update\""),
146+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, stableChannel)),
147+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, betaChannel))))
148+
149+
Eventually(func() string {
150+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
151+
}, time.Minute, 5*time.Second).Should(And(
152+
ContainSubstring("subscription_sync_total"),
153+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.NAME_LABEL, "metric-subscription")),
154+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.CHANNEL_LABEL, alphaChannel)),
155+
ContainSubstring(fmt.Sprintf("%s=\"%s\"", metrics.PACKAGE_LABEL, testPackageName))))
156+
})
157+
})
158+
})
159+
160+
AfterEach(func() {
161+
if subscriptionCleanup != nil {
162+
subscriptionCleanup()
163+
}
164+
})
165+
})
166+
When("A Subscription object is deleted", func() {
167+
168+
BeforeEach(func() {
169+
subscriptionCleanup, _ = createSubscription(GinkgoT(), crc, testNamespace, "metric-subscription-for-deletion", testPackageName, stableChannel, v1alpha1.ApprovalManual)
170+
if subscriptionCleanup != nil {
171+
subscriptionCleanup()
172+
}
173+
})
174+
It("deletes the Subscription metric", func() {
175+
Eventually(func() string {
176+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
177+
}, time.Minute, 5*time.Second).ShouldNot(ContainSubstring("subscription_sync_total{name=\"metric-subscription-for-deletion\""))
178+
})
179+
})
68180
})
69181
})
70182

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

0 commit comments

Comments
 (0)