Skip to content

[release-4.8] Bug 2074680: Emit CSV metric on startup #288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
May 2, 2022
Merged

[release-4.8] Bug 2074680: Emit CSV metric on startup #288

merged 3 commits into from
May 2, 2022

Conversation

anik120
Copy link
Contributor

@anik120 anik120 commented Apr 12, 2022

4.8 backport of #239

Josef Karasek and others added 2 commits April 12, 2022 15:05
@anik120
Emit CSV metric on startup (#2216)
46edd11 
Signed-off-by: Josef Karasek <[email protected]>

Upstream-repository: operator-lifecycle-manager
Upstream-commit: de4bebe06ba076f804d28c594d7dc52dfd95ef20
@akihikokuroda @anik120
fix e2e CSV metric is preserved failure (#2530)
363fe49 
Signed-off-by: akihikokuroda <[email protected]>

Upstream-repository: operator-lifecycle-manager
Upstream-commit: 33b081aca79dcfc33895a5c114976c4f284f027f
@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Apr 12, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 12, 2022

@anik120: This pull request references Bugzilla bug 2074680, which is invalid:

  • expected dependent Bugzilla bug 1952576 to target a release in 4.9.0, 4.9.z, but it targets "4.10.0" instead

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

In response to this:

[release-4.8] Bug 2074680: Emit CSV metric on startup

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/test-infra repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 12, 2022
@anik120
Copy link
Contributor Author

anik120 commented Apr 12, 2022

/bugzilla refresh

@openshift-ci openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Apr 12, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 12, 2022

@anik120: This pull request references Bugzilla bug 2074680, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

6 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.z) matches configured target release for branch (4.8.z)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
  • dependent bug Bugzilla bug 2072995 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE))
  • dependent Bugzilla bug 2072995 targets the "4.9.z" release, which is one of the valid target releases: 4.9.0, 4.9.z
  • bug has dependents

Requesting review from QA contact:
/cc @jianzhangbjz

In response to this:

/bugzilla refresh

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/test-infra repository.

@openshift-ci openshift-ci bot requested a review from jianzhangbjz April 12, 2022 19:18
@anik120
Copy link
Contributor Author

anik120 commented Apr 12, 2022

/test e2e-aws-olm

@perdasilva
Copy link
Contributor

/retest-required

@perdasilva
Copy link
Contributor

@anik120 sorry dude - I was just cycling some failed jobs - thought I'd be helpful. Hopefully, I didn't screw anything up...

@perdasilva
Copy link
Contributor

/test e2e-aws-olm
/test e2e-upgrade

1 similar comment
@perdasilva
Copy link
Contributor

/test e2e-aws-olm
/test e2e-upgrade

@perdasilva
Copy link
Contributor

/test e2e-aws-olm

@jianzhangbjz
Copy link
Contributor

/label cherry-pick-approved

@openshift-ci openshift-ci bot added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Apr 13, 2022
@perdasilva
Copy link
Contributor

/test e2e-upgrade
/test e2e-aws-olm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 13, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 13, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anik120, timflannagan

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:
  • OWNERS [anik120,timflannagan]

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

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@jianzhangbjz
Copy link
Contributor

/test e2e-aws-olm

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

4 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@anik120
Copy link
Contributor Author

anik120 commented Apr 28, 2022

/hold wow looks like this has been failing for a very long time now

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed lgtm Indicates that a PR is ready to be merged. labels Apr 28, 2022
@awgreene @anik120
Update getMetricsFromPort to infer port number
46c23c9 
Problem: The getMetricsFromPod function assumes that metrics are exposed
on port 8080. This function fails to retrieve metrics from the olm or
catalog operator when the port is changed.

Solution: Name the port in each of the deployments and update the
getMetricsFromPod function to infer the port number from the
deployments.

Signed-off-by: Alexander Greene <[email protected]>

Upstream-repository: operator-lifecycle-manager
Upstream-commit: 08a1d4b2f06fda1a6d6b4cadf779e1ce2259af3e
@anik120
Copy link
Contributor Author

anik120 commented Apr 28, 2022

/hold cancel
Turns out this was failing because this commit from master that has e2e changes was not backported along with the csv metric commits. Updated the PR to include the commit.

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 28, 2022
@anik120
Copy link
Contributor Author

anik120 commented Apr 29, 2022

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 29, 2022

@anik120: 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/test-infra repository. I understand the commands that are listed here.

@anik120
Copy link
Contributor Author

anik120 commented Apr 29, 2022

Nice @timflannagan @perdasilva this PR's finally green 🎉 just needs an lgtm

@bandrade
Copy link
Contributor 

oc version  
Client Version: 4.8.0-0.nightly-2021-05-03-072623
Server Version: 4.8.0-0.ci.test-2022-04-29-142539-ci-ln-mdrtg5k-latest
Kubernetes Version: v1.21.8-1466+ed4d8fd9e83b31-dirty


oc exec catalog-operator-6957f5496b-4lbb8  -n openshift-operator-lifecycle-manager -- olm --version
OLM version: 0.17.0
git commit: d2ddc316ced0342a680655fde4415b08d29a3891

1, install some operators
oc get csv -A | grep -v elasticsearch | grep -v nginx-ingress | grep -v must-gather
NAMESPACE                              NAME                        DISPLAY                     VERSION    REPLACES   PHASE
openshift-logging                      cluster-logging.5.3.6-46    Red Hat OpenShift Logging   5.3.6-46              Succeeded
openshift-operator-lifecycle-manager   packageserver               Package Server              0.17.0                Succeeded
test-1                                 anzo-operator               Anzo Operator               2.0.1                 Succeeded
test-3                                 cockroach-operator.v2.6.0   CockroachDB Operator        2.6.0                 Succeeded

2, port-fowarding to the olm-operator pod and curling the metrics endpoint
oc port-forward olm-operator-8c8c5458-8776c 8081 -n openshift-operator-lifecycle-manager                      1 ↵
Forwarding from 127.0.0.1:8081 -> 8081
Forwarding from [::1]:8081 -> 8081
Handling connection for 8081
Handling connection for 8081

curl -k https://localhost:8081/metrics | grep csv                                                             1 ↵
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  6890    0  6890    0     0  11445     # HELP csv_count Number of CSVs successfully registered
 0# TYPE csv_count gauge
 -csv_count 4
-:-# HELP csv_succeeded Successful CSV install
-:# TYPE csv_succeeded gauge
-- csv_succeeded{name="anzo-operator",namespace="test-1",version="2.0.1"} 1
--csv_succeeded{name="cluster-logging.5.3.6-46",namespace="openshift-logging",version="5.3.6-46"} 1
:-csv_succeeded{name="cockroach-operator.v2.6.0",namespace="test-3",version="2.6.0"} 1
-csv_succeeded{name="packageserver",namespace="openshift-operator-lifecycle-manager",version="0.17.0"} 1
:# HELP csv_upgrade_count Monotonic count of CSV upgrades
--# TYPE csv_upgrade_count counter
 -csv_upgrade_count 0


oc delete pod olm-operator-8c8c5458-8776c -n openshift-operator-lifecycle-manager       
pod "olm-operator-8c8c5458-8776c" deleted

oc port-forward olm-operator-8c8c5458-z5n2f 8081 -n openshift-operator-lifecycle-manager                    130 ↵
Forwarding from 127.0.0.1:8081 -> 8081
Forwarding from [::1]:8081 -> 8081
Handling connection for 8081

url -k https://localhost:8081/metrics | grep csv
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0# HELP csv_count Number of CSVs successfully registered
# TYPE csv_count gauge
csv_count 4
# HELP csv_succeeded Successful CSV install
# TYPE csv_succeeded gauge
csv_succeeded{name="anzo-operator",namespace="test-1",version="2.0.1"} 1
csv_succeeded{name="cluster-logging.5.3.6-46",namespace="openshift-logging",version="5.3.6-46"} 1
csv_succeeded{name="cockroach-operator.v2.6.0",namespace="test-3",version="2.6.0"} 1
csv_succeeded{name="packageserver",namespace="openshift-operator-lifecycle-manager",version="0.17.0"} 1
# HELP csv_upgrade_count Monotonic count of CSV upgrades
# TYPE csv_upgrade_count counter
csv_upgrade_count 0
100  6893    0  6893    0     0   9391      0 --:--:-- --:--:-- --:--:--  9391

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 29, 2022
@perdasilva
Copy link
Contributor

/label backport-risk-assessed

@openshift-ci openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label May 2, 2022
@openshift-merge-robot openshift-merge-robot merged commit 9e9736d into openshift:release-4.8 May 2, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 2, 2022

@anik120: All pull requests linked via external trackers have merged:

Bugzilla bug 2074680 has been moved to the MODIFIED state.

In response to this:

[release-4.8] Bug 2074680: Emit CSV metric on startup

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/test-infra repository.

@anik120
Copy link
Contributor Author

anik120 commented May 3, 2022

/cherry-pick release-4.7

@openshift-cherrypick-robot
Copy link

@anik120: #288 failed to apply on top of branch "release-4.7":

Applying: Emit CSV metric on startup (#2216)
Using index info to reconstruct a base tree...
M	staging/operator-lifecycle-manager/cmd/olm/main.go
M	staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go
M	staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go
A	vendor/github.com/operator-framework/operator-lifecycle-manager/cmd/olm/main.go
M	vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go
Falling back to patching base and 3-way merge...
Auto-merging vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go
Auto-merging staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go
Auto-merging staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go
Auto-merging staging/operator-lifecycle-manager/cmd/olm/main.go
Auto-merging cmd/olm/main.go
Applying: fix e2e CSV metric is preserved failure (#2530)
Applying: Update getMetricsFromPort to infer port number
Using index info to reconstruct a base tree...
M	staging/operator-lifecycle-manager/test/e2e/installplan_e2e_test.go
M	staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go
Falling back to patching base and 3-way merge...
Auto-merging staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go
CONFLICT (content): Merge conflict in staging/operator-lifecycle-manager/test/e2e/metrics_e2e_test.go
Auto-merging staging/operator-lifecycle-manager/test/e2e/installplan_e2e_test.go
CONFLICT (content): Merge conflict in staging/operator-lifecycle-manager/test/e2e/installplan_e2e_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 Update getMetricsFromPort to infer port number
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.7

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/test-infra repository.

This was referenced May 3, 2022
Closed
Merged
stevekuznetsov added a commit to stevekuznetsov/operator-framework-olm that referenced this pull request Aug 3, 2023
@stevekuznetsov
CatalogSource: add configurations for gRPC server memory footprint (o…
89c9cb7 
…penshift#288)

Signed-off-by: Steve Kuznetsov <[email protected]>
Upstream-repository: api
Upstream-commit: 704ae942c4a9a33a3326a7da9364bc59f20ae532
Signed-off-by: Steve Kuznetsov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timflannagan timflannagan timflannagan approved these changes

@ankitathomas ankitathomas Awaiting requested review from ankitathomas

@jianzhangbjz jianzhangbjz Awaiting requested review from jianzhangbjz

Assignees

@bandrade bandrade

@timflannagan timflannagan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@anik120 @perdasilva @jianzhangbjz @openshift-bot @bandrade @openshift-cherrypick-robot @timflannagan @openshift-merge-robot @akihikokuroda @awgreene