Skip to content

test/e2e: Make sure Prometheus only fires Watchdog alert #266

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
Closed
wants to merge 2 commits into from
Closed

test/e2e: Make sure Prometheus only fires Watchdog alert #266

wants to merge 2 commits into from

Conversation

mxinden
Copy link
Contributor

@mxinden mxinden commented Feb 25, 2019 •
edited
Loading

In my local cluster Prometheus fires the KubeClientCertificateExpiration alert. Once we resolved that we can continue here. Opening it up for early feedback.

@mxinden mxinden added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 25, 2019
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 25, 2019
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 25, 2019
@mxinden
Copy link
Contributor Author

mxinden commented Feb 26, 2019

Added 9883d68 to address minor after-merge comments in #263.

I am sorry for interleaving the two concerns, but given that this is such a minor change, going through the end-to-end-retest process would be a lot of toil.

@brancz
Copy link
Contributor

brancz commented Feb 26, 2019

This needs a rebase, otherwise lgtm.

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 27, 2019
@mxinden
Copy link
Contributor Author

mxinden commented Feb 27, 2019

/retest

@s-urbaniak
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 28, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mxinden, s-urbaniak

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

@s-urbaniak
Copy link
Contributor

/hold

@s-urbaniak
Copy link
Contributor

whoops, I missed that we are still firing KubeClientCertificateExpiration. I am 👍 on this change.

@mxinden
Copy link
Contributor Author

mxinden commented Feb 28, 2019

No worries @s-urbaniak, I added the [do-not-merge/hold] anyways. I will look into templating proper expiration intervals into KubeClientCertificateExpiration.

@mxinden
Copy link
Contributor Author

mxinden commented Apr 3, 2019

Kicking off a retest as #275 is merged.

/retest

@mxinden
{pkg/client,test/e2e}: Minor function and error msg renaming
4662fb5 
- Rename `pkg/client.DeleteIfExists` to `DeleteNamespaceIfExists`.
- Fix `pkg/clinet.DeleteNamespaceIfExists` error message on failure.
@mxinden mxinden force-pushed the no-alerts-firing branch from f00462d to 4662fb5 Compare April 3, 2019 11:45
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 3, 2019
@openshift-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@brancz
Copy link
Contributor

brancz commented Apr 3, 2019

I think we were talking about that this should rather be part of the e2e-aws test suite right? Because otherwise if another team breaks something that makes an alert fire we are blocked. I think @paulfantom was already looking into moving some of our e2e tests to e2e-aws, could you take care of this one as well?

@mxinden
Copy link
Contributor Author

mxinden commented Apr 3, 2019

Yes, that sounds good @brancz.

On a current Openshift 4.0 cluster TargetDown alert is firing for job="catalog-operator-metrics", job="sdn" and job="olm-operator-metrics".

@mxinden mxinden closed this Apr 3, 2019
@paulfantom
Copy link
Contributor

could you take care of this one as well?

This one might be a bit trickier, but I will move it into e2e-aws.

@brancz
Copy link
Contributor

brancz commented Apr 3, 2019

I don't think we've ever added a test to that suite, keep us updated :)

@paulfantom paulfantom mentioned this pull request Apr 9, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brancz brancz Awaiting requested review from brancz

@squat squat Awaiting requested review from squat

Assignees

@s-urbaniak s-urbaniak

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mxinden @brancz @s-urbaniak @openshift-ci-robot @paulfantom