Skip to content
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

fix e2e CSV metric is preserved failure #2530

Merged
merged 1 commit into from
Dec 20, 2021
Merged

fix e2e CSV metric is preserved failure #2530

merged 1 commit into from
Dec 20, 2021

Conversation

akihikokuroda
Copy link
Member

Signed-off-by: akihikokuroda akihikokuroda2020@gmail.com

Description of the change:

Adding wait for the metric are reemitted in the new pod in the e2e test.

Motivation for the change:
Close #2529

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /doc
  • Commit messages sensible and descriptive

@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 17, 2021
@openshift-ci
Copy link

openshift-ci bot commented Dec 17, 2021

Hi @akihikokuroda. Thanks for your PR.

I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@timflannagan
Copy link
Contributor

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 17, 2021
@timflannagan
Copy link
Contributor

I'm going to give the e2e-tests action a couple of spins in any case to weed out any instability in any case - but these types of changes are super appreciated while we work towards reducing the overall CI noise in our current e2e suite. We should have a couple of changes inflight that will make the experience better (both in CI and local environments).

@timflannagan timflannagan linked an issue Dec 17, 2021 that may be closed by this pull request
"Metrics are generated for OLM managed resources Given an OperatorGroup that supports all namespaces when a CSV is created when the OLM pod restarts CSV metric is preserved" #2441
Closed
@timflannagan timflannagan mentioned this pull request Dec 17, 2021
Closed
awgreene
awgreene previously requested changes Dec 17, 2021
View reviewed changes
Copy link
Member

@awgreene awgreene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @akihikokuroda, mostly just echoed what @timflannagan said.

test/e2e/metrics_e2e_test.go Outdated Show resolved Hide resolved
test/e2e/metrics_e2e_test.go Show resolved Hide resolved
@akihikokuroda
fix e2e CSV metric is preserved failure
cf4d928 
Signed-off-by: akihikokuroda <akihikokuroda2020@gmail.com>
@timflannagan
Copy link
Contributor

Thanks for tackling this again - this is definitely one of the flakes that pops up frequently.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 17, 2021
@awgreene
Copy link
Member

/approve
Thanks again for the contribution towards OLM @akihikokuroda, these PRs are very impactful to the health of the project!

@openshift-ci
Copy link

openshift-ci bot commented Dec 17, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akihikokuroda, awgreene

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 17, 2021
@openshift-merge-robot openshift-merge-robot merged commit 33b081a into operator-framework:master Dec 20, 2021
@akihikokuroda akihikokuroda mentioned this pull request Jan 11, 2022
Closed
6 tasks
anik120 pushed a commit to anik120/operator-lifecycle-manager that referenced this pull request May 3, 2022
@akihikokuroda @anik120
fix e2e CSV metric is preserved failure (operator-framework#2530)
1d3acd1 
Signed-off-by: akihikokuroda <akihikokuroda2020@gmail.com>
anik120 pushed a commit to anik120/operator-lifecycle-manager that referenced this pull request May 3, 2022
@akihikokuroda @anik120
fix e2e CSV metric is preserved failure (operator-framework#2530)
5cf1d5b 
Signed-off-by: akihikokuroda <akihikokuroda2020@gmail.com>
openshift-merge-robot pushed a commit that referenced this pull request May 12, 2022
@anik120 @awgreene @akihikokuroda
Bug 2081483: Emit csv_succeeded metrics on startup (#2768)
64cff34 
* Update getMetricsFromPort to infer port number

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 <greene.al1991@gmail.com>

* Emit CSV metric on startup (#2216)

Signed-off-by: Josef Karasek <jkarasek@redhat.com>

* fix e2e CSV metric is preserved failure (#2530)

Signed-off-by: akihikokuroda <akihikokuroda2020@gmail.com>

Co-authored-by: Alexander Greene <greene.al1991@gmail.com>
Co-authored-by: Josef Karasek <jkarasek@redhat.com>
Co-authored-by: Akihiko (Aki) Kuroda <16141898+akihikokuroda@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hasbro17 hasbro17 Awaiting requested review from hasbro17

@njhale njhale Awaiting requested review from njhale

@awgreene awgreene Awaiting requested review from awgreene

@timflannagan timflannagan Awaiting requested review from timflannagan

Assignees

@timflannagan timflannagan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Milestone
No milestone
4 participants
@akihikokuroda @timflannagan @awgreene @openshift-merge-robot