Skip to content

Commit 6307c54

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1746 from anik120/subs-metrics-4.4
Bug 1851095: Delete subscription metric when an operator is uninstalled
2 parents f2e7b9f + 92bbdd4 commit 6307c54

File tree

4 files changed

+202
-29
lines changed

4 files changed

+202
-29
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/metrics/metrics.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,19 @@ var (
168168
},
169169
[]string{NAMESPACE_LABEL, NAME_LABEL, VERSION_LABEL, PHASE_LABEL, REASON_LABEL},
170170
)
171+
172+
// subscriptionSyncCounters keeps a record of the promethues counters emitted by
173+
// Subscription objects. The key of a record is the Subscription name, while the value
174+
// is struct containing label values used in the counter
175+
subscriptionSyncCounters = make(map[string]subscriptionSyncLabelValues)
171176
)
172177

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

‎test/e2e/metrics_e2e_test.go

Lines changed: 136 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,33 @@
33
package e2e
44

55
import (
6+
"strings"
67
"testing"
8+
"time"
79

810
log "github.com/sirupsen/logrus"
911
"github.com/stretchr/testify/require"
12+
corev1 "k8s.io/api/core/v1"
1013
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1114
"k8s.io/apimachinery/pkg/util/net"
15+
"k8s.io/apimachinery/pkg/util/wait"
1216

1317
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
1418
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
1519
)
1620

17-
// TestMetrics tests the metrics endpoint of the OLM pod.
18-
func TestMetricsEndpoint(t *testing.T) {
21+
const (
22+
// RetryInterval defines the frequency at which we check for updates against the
23+
// k8s api when waiting for a specific condition to be true.
24+
RetryInterval = time.Second * 5
25+
26+
// Timeout defines the amount of time we should spend querying the k8s api
27+
// when waiting for a specific condition to be true.
28+
Timeout = time.Minute * 5
29+
)
30+
31+
// TestCSVMetrics tests the metrics endpoint of the OLM pod for metrics emitted by CSVs.
32+
func TestCSVMetrics(t *testing.T) {
1933
c := newKubeClient(t)
2034
crc := newCRClient(t)
2135

@@ -41,7 +55,7 @@ func TestMetricsEndpoint(t *testing.T) {
4155
_, err = fetchCSV(t, crc, failingCSV.Name, testNamespace, csvFailedChecker)
4256
require.NoError(t, err)
4357

44-
rawOutput, err := getMetricsFromPod(t, c, getOLMPodName(t, c), operatorNamespace, "8081")
58+
rawOutput, err := getMetricsFromPod(t, c, getPodWithLabel(t, c, "app=olm-operator"), operatorNamespace, "8081")
4559
if err != nil {
4660
t.Fatalf("Metrics test failed: %v\n", err)
4761
}
@@ -56,16 +70,128 @@ func TestMetricsEndpoint(t *testing.T) {
5670

5771
cleanupCSV()
5872

59-
rawOutput, err = getMetricsFromPod(t, c, getOLMPodName(t, c), operatorNamespace, "8081")
73+
rawOutput, err = getMetricsFromPod(t, c, getPodWithLabel(t, c, "app=olm-operator"), operatorNamespace, "8081")
6074
if err != nil {
6175
t.Fatalf("Failed to retrieve metrics from OLM pod because of: %v\n", err)
6276
}
6377
require.NotContains(t, rawOutput, "csv_abnormal{name=\""+failingCSV.Name+"\"")
6478
require.NotContains(t, rawOutput, "csv_succeeded{name=\""+failingCSV.Name+"\"")
6579
}
6680

67-
func getOLMPodName(t *testing.T, client operatorclient.ClientInterface) string {
68-
listOptions := metav1.ListOptions{LabelSelector: "app=olm-operator"}
81+
func TestSubscriptionMetrics(t *testing.T) {
82+
c := newKubeClient(t)
83+
crc := newCRClient(t)
84+
85+
subscriptionCleanup, subscription := createSubscription(t, crc, testNamespace, "metric-subscription", testPackageName, stableChannel, v1alpha1.ApprovalManual)
86+
87+
err := wait.PollImmediate(RetryInterval, Timeout, func() (done bool, err error) {
88+
rawOutput, err := getMetricsFromPod(t, c, getPodWithLabel(t, c, "app=catalog-operator"), operatorNamespace, "8081")
89+
if err != nil {
90+
return false, err
91+
}
92+
if strings.Contains(rawOutput, "subscription_sync_total") &&
93+
strings.Contains(rawOutput, "name=\"metric-subscription\"") &&
94+
strings.Contains(rawOutput, "channel=\""+stableChannel+"\"") &&
95+
strings.Contains(rawOutput, "package=\""+testPackageName+"\"") {
96+
return true, nil
97+
}
98+
return false, nil
99+
})
100+
require.NoError(t, err)
101+
102+
updatedSubscription, err := crc.OperatorsV1alpha1().Subscriptions(subscription.GetNamespace()).Get(subscription.GetName(), metav1.GetOptions{})
103+
require.NoError(t, err)
104+
105+
updatedSubscription.Spec.Channel = betaChannel
106+
updateSubscription(t, crc, updatedSubscription)
107+
108+
err = wait.PollImmediate(RetryInterval, Timeout, func() (done bool, err error) {
109+
rawOutput, err := getMetricsFromPod(t, c, getPodWithLabel(t, c, "app=catalog-operator"), operatorNamespace, "8081")
110+
if err != nil {
111+
return false, err
112+
}
113+
if strings.Contains(rawOutput, "subscription_sync_total") &&
114+
strings.Contains(rawOutput, "name=\"metric-subscription\"") &&
115+
strings.Contains(rawOutput, "channel=\""+stableChannel+"\"") &&
116+
strings.Contains(rawOutput, "package=\""+testPackageName+"\"") {
117+
return false, nil
118+
}
119+
return true, nil
120+
})
121+
require.NoError(t, err)
122+
123+
err = wait.PollImmediate(RetryInterval, Timeout, func() (done bool, err error) {
124+
rawOutput, err := getMetricsFromPod(t, c, getPodWithLabel(t, c, "app=catalog-operator"), operatorNamespace, "8081")
125+
if err != nil {
126+
return false, err
127+
}
128+
if strings.Contains(rawOutput, "subscription_sync_total") &&
129+
strings.Contains(rawOutput, "name=\"metric-subscription\"") &&
130+
strings.Contains(rawOutput, "channel=\""+betaChannel+"\"") &&
131+
strings.Contains(rawOutput, "package=\""+testPackageName+"\"") {
132+
return true, nil
133+
}
134+
return false, nil
135+
})
136+
require.NoError(t, err)
137+
138+
updatedSubscription, err = crc.OperatorsV1alpha1().Subscriptions(subscription.GetNamespace()).Get(subscription.GetName(), metav1.GetOptions{})
139+
require.NoError(t, err)
140+
141+
updatedSubscription.Spec.Channel = alphaChannel
142+
updateSubscription(t, crc, updatedSubscription)
143+
144+
err = wait.PollImmediate(RetryInterval, Timeout, func() (done bool, err error) {
145+
rawOutput, err := getMetricsFromPod(t, c, getPodWithLabel(t, c, "app=catalog-operator"), operatorNamespace, "8081")
146+
if err != nil {
147+
return false, err
148+
}
149+
if strings.Contains(rawOutput, "subscription_sync_total") &&
150+
strings.Contains(rawOutput, "name=\"metric-subscription\"") &&
151+
strings.Contains(rawOutput, "channel=\""+betaChannel+"\"") &&
152+
strings.Contains(rawOutput, "package=\""+testPackageName+"\"") {
153+
return false, nil
154+
}
155+
return true, nil
156+
})
157+
require.NoError(t, err)
158+
159+
err = wait.PollImmediate(RetryInterval, Timeout, func() (done bool, err error) {
160+
rawOutput, err := getMetricsFromPod(t, c, getPodWithLabel(t, c, "app=catalog-operator"), operatorNamespace, "8081")
161+
if err != nil {
162+
return false, err
163+
}
164+
if strings.Contains(rawOutput, "subscription_sync_total") &&
165+
strings.Contains(rawOutput, "name=\"metric-subscription\"") &&
166+
strings.Contains(rawOutput, "channel=\""+alphaChannel+"\"") &&
167+
strings.Contains(rawOutput, "package=\""+testPackageName+"\"") {
168+
return true, nil
169+
}
170+
return false, nil
171+
})
172+
require.NoError(t, err)
173+
174+
if subscriptionCleanup != nil {
175+
subscriptionCleanup()
176+
}
177+
err = wait.PollImmediate(RetryInterval, Timeout, func() (done bool, err error) {
178+
rawOutput, err := getMetricsFromPod(t, c, getPodWithLabel(t, c, "app=catalog-operator"), operatorNamespace, "8081")
179+
if err != nil {
180+
return false, err
181+
}
182+
if strings.Contains(rawOutput, "subscription_sync_total") &&
183+
strings.Contains(rawOutput, "name=metric-subscription") &&
184+
strings.Contains(rawOutput, "channel=\""+alphaChannel+"\"") &&
185+
strings.Contains(rawOutput, "package=\""+testPackageName+"\"") {
186+
return false, nil
187+
}
188+
return true, nil
189+
})
190+
require.NoError(t, err)
191+
}
192+
193+
func getPodWithLabel(t *testing.T, client operatorclient.ClientInterface, label string) *corev1.Pod {
194+
listOptions := metav1.ListOptions{LabelSelector: label}
69195
podList, err := client.KubernetesInterface().CoreV1().Pods(operatorNamespace).List(listOptions)
70196
if err != nil {
71197
log.Infof("Error %v\n", err)
@@ -74,15 +200,11 @@ func getOLMPodName(t *testing.T, client operatorclient.ClientInterface) string {
74200
if len(podList.Items) != 1 {
75201
t.Fatalf("Expected 1 olm-operator pod, got %v", len(podList.Items))
76202
}
77-
78-
podName := podList.Items[0].GetName()
79-
log.Infof("Looking at pod %v in namespace %v", podName, operatorNamespace)
80-
return podName
81-
203+
return &podList.Items[0]
82204
}
83205

84-
func getMetricsFromPod(t *testing.T, client operatorclient.ClientInterface, podName string, namespace string, port string) (string, error) {
85-
olmPod, err := client.KubernetesInterface().CoreV1().Pods(namespace).Get(podName, metav1.GetOptions{})
206+
func getMetricsFromPod(t *testing.T, client operatorclient.ClientInterface, pod *corev1.Pod, namespace string, port string) (string, error) {
207+
olmPod, err := client.KubernetesInterface().CoreV1().Pods(namespace).Get(pod.GetName(), metav1.GetOptions{})
86208
if err != nil {
87209
return "", err
88210
}
@@ -113,7 +235,7 @@ func getMetricsFromPod(t *testing.T, client operatorclient.ClientInterface, podN
113235
Namespace(namespace).
114236
Resource("pods").
115237
SubResource("proxy").
116-
Name(net.JoinSchemeNamePort(scheme, podName, port)).
238+
Name(net.JoinSchemeNamePort(scheme, pod.GetName(), port)).
117239
Suffix("metrics").
118240
Do().Raw()
119241
if err != nil {

‎test/e2e/subscription_e2e_test.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ func buildSubscriptionCleanupFunc(t *testing.T, crc versioned.Interface, subscri
386386
}
387387
}
388388

389-
func createSubscription(t *testing.T, crc versioned.Interface, namespace, name, packageName, channel string, approval v1alpha1.Approval) cleanupFunc {
389+
func createSubscription(t *testing.T, crc versioned.Interface, namespace, name, packageName, channel string, approval v1alpha1.Approval) (cleanupFunc, *v1alpha1.Subscription) {
390390
subscription := &v1alpha1.Subscription{
391391
TypeMeta: metav1.TypeMeta{
392392
Kind: v1alpha1.SubscriptionKind,
@@ -407,7 +407,12 @@ func createSubscription(t *testing.T, crc versioned.Interface, namespace, name,
407407

408408
subscription, err := crc.OperatorsV1alpha1().Subscriptions(namespace).Create(subscription)
409409
require.NoError(t, err)
410-
return buildSubscriptionCleanupFunc(t, crc, subscription)
410+
return buildSubscriptionCleanupFunc(t, crc, subscription), subscription
411+
}
412+
413+
func updateSubscription(t *testing.T, crc versioned.Interface, subscription *v1alpha1.Subscription) {
414+
_, err := crc.OperatorsV1alpha1().Subscriptions(subscription.GetNamespace()).Update(subscription)
415+
require.NoError(t, err)
411416
}
412417

413418
func createSubscriptionForCatalog(t *testing.T, crc versioned.Interface, namespace, name, catalog, packageName, channel, startingCSV string, approval v1alpha1.Approval) cleanupFunc {
@@ -465,7 +470,7 @@ func TestCreateNewSubscriptionNotInstalled(t *testing.T) {
465470
}()
466471
require.NoError(t, initCatalog(t, c, crc))
467472

468-
cleanup := createSubscription(t, crc, testNamespace, testSubscriptionName, testPackageName, betaChannel, v1alpha1.ApprovalAutomatic)
473+
cleanup, _ := createSubscription(t, crc, testNamespace, testSubscriptionName, testPackageName, betaChannel, v1alpha1.ApprovalAutomatic)
469474
defer cleanup()
470475

471476
subscription, err := fetchSubscription(t, crc, testNamespace, testSubscriptionName, subscriptionStateAtLatestChecker)
@@ -493,7 +498,7 @@ func TestCreateNewSubscriptionExistingCSV(t *testing.T) {
493498
_, err := createCSV(t, c, crc, stableCSV, testNamespace, false, false)
494499
require.NoError(t, err)
495500

496-
subscriptionCleanup := createSubscription(t, crc, testNamespace, testSubscriptionName, testPackageName, alphaChannel, v1alpha1.ApprovalAutomatic)
501+
subscriptionCleanup, _ := createSubscription(t, crc, testNamespace, testSubscriptionName, testPackageName, alphaChannel, v1alpha1.ApprovalAutomatic)
497502
defer subscriptionCleanup()
498503

499504
subscription, err := fetchSubscription(t, crc, testNamespace, testSubscriptionName, subscriptionStateAtLatestChecker)
@@ -600,7 +605,7 @@ func TestCreateNewSubscriptionManualApproval(t *testing.T) {
600605
}()
601606
require.NoError(t, initCatalog(t, c, crc))
602607

603-
subscriptionCleanup := createSubscription(t, crc, testNamespace, "manual-subscription", testPackageName, stableChannel, v1alpha1.ApprovalManual)
608+
subscriptionCleanup, _ := createSubscription(t, crc, testNamespace, "manual-subscription", testPackageName, stableChannel, v1alpha1.ApprovalManual)
604609
defer subscriptionCleanup()
605610

606611
subscription, err := fetchSubscription(t, crc, testNamespace, "manual-subscription", subscriptionStateUpgradePendingChecker)

0 commit comments

Comments
 (0)