UPSTREAM: <carry>: OCPBUGS-49410: Enable OCP metrics collection by default #252

azych · 2025-01-30T11:22:01Z

Enables OCP to collect Prometheus metrics for both catalogd and operator-controller by default. This is accomplished via ServiceMonitor CRs which are now created for both projects.

Tested on both 4.19 and 4.19-nightly (aka main) versions:

Fixes OCPBUGS-49410

openshift-ci · 2025-01-30T11:22:05Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci-robot · 2025-01-30T11:29:32Z

@azych: This pull request references Jira Issue OCPBUGS-49410, which is invalid:

expected the bug to target the "4.19.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift/catalogd/manifests/20-role-openshift-catalogd-metrics.yml

joelanford

Two things:

We should do a similar thing for operator-controller
We'll need these included in the kustomize directory somewhere. The manifests directories are generated from kustomize.

azych · 2025-01-30T14:48:45Z

Two things:

1. We should do a similar thing for operator-controller

yes, I intend to, it's a Draft-WIP at this point until I can verify that everything is working and come up with manifests for operator-controller

2. We'll need these included in the kustomize directory somewhere. The manifests directories are generated from kustomize.

good point, I'll do that before removing Draft-WIP status of this PR

openshift/catalogd/manifests/22-servicemonitor-catalogd.yml

tmshort · 2025-01-30T15:21:47Z

Fundamentally, these files are supposed to be created through the kustomize mechanism, which means adding resources to the openshift/catalogd/kustomize/overlays/openshift directory and appropriate kustomization.yaml updates and patches.

openshift/catalogd/manifests/20-role-openshift-catalogd-metrics.yml

azych · 2025-01-31T07:29:09Z

Fundamentally, these files are supposed to be created through the kustomize mechanism, which means adding resources to the openshift/catalogd/kustomize/overlays/openshift directory and appropriate kustomization.yaml updates and patches.

good point, I'll do that before removing Draft-WIP status of this PR

azych · 2025-01-31T07:43:49Z

@camilamacedo86 @joelanford @tmshort

To clarify & try to find the way forward:

this general setup and tlsconfig has been verified prior to issuing this Draft PR on a live 4.19 cluster to work correctly (at least for catalogd - it's a WIP)
if the setup changed and we're using self-signed cert in 'main' (probably a bug in itself), then should we want to go ahead with the changes described here just for 4.19? do we want to do it in this PR?
what about UPSTREAM: <carry>: OCPBUGS-49410: Enable OCP metrics collection by default #252 (comment) ? Is there a way not to have the changes done in main automatically synced down?
If we want and need to solve this in 'main' first for some reason, this means dependency on OCPBUGS-49683: CA cert and catalog cache bug fixes #255 potentially, true?

...ft/catalogd/kustomize/overlays/openshift/olmv1-ns/metrics/catalogd_metrics_role_binding.yaml

openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/metrics/catalogd_metrics_role.yaml

openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/metrics/catalogd_servicemonitor.yaml

azych · 2025-01-31T11:17:35Z

/test all

openshift/catalogd/manifests/11-rolebinding-openshift-config-catalogd-manager-rolebinding.yml

openshift-ci-robot · 2025-02-10T09:46:51Z

@azych: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

In response to this:

Enables OCP to collect Prometheus metrics for both catalogd and operator-controller by default. This is accomplished via ServiceMonitor CRs which are now created for both projects.

Tested on both 4.19 and 4.19-nightly (aka main) versions:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2025-02-10T09:48:46Z

@azych: This pull request references Jira Issue OCPBUGS-49410, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.19.0) matches configured target version for branch (4.19.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @Xia-Zhao-rh

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Enables OCP to collect Prometheus metrics for both catalogd and operator-controller by default. This is accomplished via ServiceMonitor CRs which are now created for both projects.

Tested on both 4.19 and 4.19-nightly (aka main) versions:

Fixes OCPBUGS-49410

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

tmshort · 2025-02-14T13:13:35Z

@azych this likely needs to be rebased.

Enables OCP to collect Prometheus metrics for both catalogd and operator-controller by default. This is accomplished via ServiceMonitor CRs which are now created for both projects.

tmshort · 2025-02-14T14:39:11Z

/approve
/lgtm
Looks as though @joelanford's requested changes might block merging.

camilamacedo86

I'm not sure how or where we can add E2E tests that won’t conflict with the bumper and are valid only for downstream OCP.

@dtfranz @tmshort might have a better idea on how to approach this. I’d also like to assess whether implementing this downstream-only test is worthwhile. If maintaining such a test exclusively downstream proves too difficult, then:

a) It might not be worth the effort

OR

b) It could still be valuable as a small E2E test—serving as an example or a foundation for expanding test coverage in the future.

What test we could add here?

Example: Check that the ServiceMonitor was created with success

Otherwise, I'm fine with it. So, /lgtm.

c/c @joelanford

tmshort · 2025-02-14T14:47:29Z

Yeah, I noticed no tests... the best place might be part of the origin tests:
https://github.com/openshift/origin

Xia-Zhao-rh · 2025-02-17T07:29:04Z

/label qe-approved

openshift-ci · 2025-02-17T13:09:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: azych, joelanford, tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~DOWNSTREAM_OWNERS~~ [joelanford,tmshort]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

azych · 2025-02-17T14:47:17Z

+1 I'll be following up with a test in https://github.com/openshift/origin once merged

joelanford · 2025-02-17T21:40:38Z

/label acknowledge-critical-fixes-only

openshift-ci · 2025-02-18T00:12:24Z

@azych: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot · 2025-02-18T00:15:13Z

@azych: Jira Issue OCPBUGS-49410: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-49410 has been moved to the MODIFIED state.

In response to this:

Enables OCP to collect Prometheus metrics for both catalogd and operator-controller by default. This is accomplished via ServiceMonitor CRs which are now created for both projects.

Tested on both 4.19 and 4.19-nightly (aka main) versions:

Fixes OCPBUGS-49410

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-bot · 2025-02-18T04:40:58Z

[ART PR BUILD NOTIFIER]

Distgit: ose-olm-catalogd
This PR has been included in build ose-olm-catalogd-container-v4.19.0-202502180408.p0.gbeab335.assembly.stream.el9.
All builds following this will include this PR.

openshift-bot · 2025-02-18T04:40:58Z

[ART PR BUILD NOTIFIER]

Distgit: ose-olm-operator-controller
This PR has been included in build ose-olm-operator-controller-container-v4.19.0-202502180408.p0.gbeab335.assembly.stream.el9.
All builds following this will include this PR.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 30, 2025

azych force-pushed the enable-metrics-collection-by-default branch 3 times, most recently from 82a5757 to 80839da Compare January 30, 2025 11:27

azych changed the title ~~[WIP] Enable OCP metrics collection for catalogd~~ [WIP] Enable OCP metrics collection by default Jan 30, 2025

azych changed the title ~~[WIP] Enable OCP metrics collection by default~~ [WIP] OCPBUGS-49410: Enable OCP metrics collection by default Jan 30, 2025

camilamacedo86 reviewed Jan 30, 2025

View reviewed changes

openshift/catalogd/manifests/20-role-openshift-catalogd-metrics.yml Show resolved Hide resolved

joelanford requested changes Jan 30, 2025

View reviewed changes

openshift-ci bot assigned joelanford Jan 30, 2025

tmshort reviewed Jan 30, 2025

View reviewed changes

openshift/catalogd/manifests/22-servicemonitor-catalogd.yml Show resolved Hide resolved

joelanford reviewed Jan 30, 2025

View reviewed changes

openshift/catalogd/manifests/20-role-openshift-catalogd-metrics.yml Show resolved Hide resolved

azych force-pushed the enable-metrics-collection-by-default branch from 80839da to c81e1e0 Compare January 31, 2025 09:32

camilamacedo86 reviewed Jan 31, 2025

View reviewed changes

...ft/catalogd/kustomize/overlays/openshift/olmv1-ns/metrics/catalogd_metrics_role_binding.yaml Outdated Show resolved Hide resolved

camilamacedo86 reviewed Jan 31, 2025

View reviewed changes

openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/metrics/catalogd_metrics_role.yaml Show resolved Hide resolved

camilamacedo86 reviewed Jan 31, 2025

View reviewed changes

openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/metrics/catalogd_servicemonitor.yaml Show resolved Hide resolved

camilamacedo86 reviewed Jan 31, 2025

View reviewed changes

openshift/catalogd/manifests/11-rolebinding-openshift-config-catalogd-manager-rolebinding.yml Show resolved Hide resolved

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 31, 2025

azych force-pushed the enable-metrics-collection-by-default branch 2 times, most recently from 8924ee8 to a6a7e96 Compare February 3, 2025 08:53

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2025

azych changed the title ~~UPSTREAM: <carry>: Enable OCP metrics collection by default~~ UPSTREAM: <carry>: OCPBUGS-49410: Enable OCP metrics collection by default Feb 10, 2025

azych force-pushed the enable-metrics-collection-by-default branch from 1521ec4 to 982bfc3 Compare February 10, 2025 12:31

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2025

UPSTREAM: <carry>: Enable OCP metrics collection by default

77f9d42

Enables OCP to collect Prometheus metrics for both catalogd and operator-controller by default. This is accomplished via ServiceMonitor CRs which are now created for both projects.

azych force-pushed the enable-metrics-collection-by-default branch from 982bfc3 to 77f9d42 Compare February 14, 2025 13:21

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2025

openshift-ci bot assigned tmshort Feb 14, 2025

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 14, 2025

camilamacedo86 reviewed Feb 14, 2025

View reviewed changes

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Feb 17, 2025

joelanford approved these changes Feb 17, 2025

View reviewed changes

openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Feb 17, 2025

openshift-merge-bot bot merged commit beab335 into openshift:main Feb 18, 2025
9 checks passed

UPSTREAM: <carry>: OCPBUGS-49410: Enable OCP metrics collection by default #252

UPSTREAM: <carry>: OCPBUGS-49410: Enable OCP metrics collection by default #252

Uh oh!

Conversation

azych commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jan 30, 2025

Uh oh!

openshift-ci-robot commented Jan 30, 2025

Uh oh!

Uh oh!

joelanford left a comment

Choose a reason for hiding this comment

Uh oh!

azych commented Jan 30, 2025

Uh oh!

Uh oh!

tmshort commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

azych commented Jan 31, 2025

Uh oh!

azych commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

azych commented Jan 31, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Feb 10, 2025

Uh oh!

openshift-ci-robot commented Feb 10, 2025

Uh oh!

tmshort commented Feb 14, 2025

Uh oh!

tmshort commented Feb 14, 2025

Uh oh!

camilamacedo86 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmshort commented Feb 14, 2025

Uh oh!

Xia-Zhao-rh commented Feb 17, 2025

Uh oh!

openshift-ci bot commented Feb 17, 2025

Uh oh!

azych commented Feb 17, 2025

Uh oh!

joelanford commented Feb 17, 2025

Uh oh!

openshift-ci bot commented Feb 18, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Feb 18, 2025

Uh oh!

openshift-bot commented Feb 18, 2025

Uh oh!

openshift-bot commented Feb 18, 2025

Uh oh!

Uh oh!

azych commented Jan 30, 2025 •

edited

Loading

tmshort commented Jan 30, 2025 •

edited

Loading

azych commented Jan 31, 2025 •

edited

Loading

camilamacedo86 left a comment •

edited

Loading