test/extended/prometheus: Validate alerting rules #26504

bison · 2021-10-05T11:37:13Z

The OpenShift Alerting Consistency enhancement defines a style
guide for the alerts shipped as part of OpenShift. This adds a test
validating some of the guidelines considered required.

This was originally added in #26476, but was reverted in #26499 due to
failures with OVN and vSphere clusters. This adds the tests back, but
adds exceptions for the non-compliant alerts as well as marking the
failing tests as flakes for now. We'll gather data and make the tests
required once we're reasonably sure things are passing with all the
existing alerts.

bison · 2021-10-05T11:41:49Z

I've run this on an OVN cluster and it passed. I haven't been able to get a vSphere cluster to spin up successfully, but I added exceptions for the problematic rules I saw in CI and that are listed in the bug that was created.

simonpasquier · 2021-10-05T11:43:38Z

/lgtm

bison · 2021-10-05T13:30:00Z

/retest

The [OpenShift Alerting Consistency][1] enhancement defines a style guide for the alerts shipped as part of OpenShift. This adds a test validating some of the guidelines considered required. This was originally added in #26476, but was reverted in #26499 due to failures with OVN and vSphere clusters. This adds the tests back, but adds exceptions for the non-compliant alerts as well as marking the failing tests as flakes for now. We'll gather data and make the tests required once we're reasonably sure things are passing with all the existing alerts. [1]: https://github.com/openshift/enhancements/blob/master/enhancements/monitoring/alerting-consistency.md

bison · 2021-10-05T13:40:57Z

@simonpasquier: I found a new alert that needed an exception, so lost the LGTM. Can you have another look? That's the only change.

bison · 2021-10-05T15:44:37Z

/test e2e-gcp-upgrade

simonpasquier

/lgtm

bison · 2021-10-07T08:34:01Z

/test e2e-gcp-upgrade

bison · 2021-10-07T12:43:07Z

@deads2k, this adds the tests for alerting rules again after they were reverted. The problematic tests are marked as flakes now, in addition to having added temporary exceptions for the OVN and vSphere alerts. Can you have a look?

The e2e-gcp-upgrade tests are failing, but it looks unrelated.

bison · 2021-10-07T13:54:22Z

/test e2e-gcp-upgrade

simonpasquier · 2021-10-08T09:11:48Z

/test e2e-gcp-upgrade

jan--f · 2021-10-08T09:44:26Z

/lgtm

bison · 2021-10-11T09:47:23Z

/retest

bison · 2021-10-11T09:48:20Z

We need an owner to approve this. Could maybe @deads2k, @smarterclayton, or @bparees have a look if you have a minute?

bparees · 2021-10-11T18:20:17Z

test/extended/prometheus/prometheus.go

+		})
+
+		if err != nil {
+			e2e.Failf(err.Error())


you indicated it would flake on failure, but this one is failing on failure....deliberate?

Yep, sorry, only the tests for runbooks and the description + summary labels are flakes for now, because lots of existing alerts don't conform. AFAICT, no existing alerts fail the test for the severity label, so we can just make it enforcing immediately. I am happy to make it flake also if we want to be extra sure though.

bparees · 2021-10-11T19:56:44Z

/approve

openshift-ci · 2021-10-11T19:57:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bison, bparees, jan--f, simonpasquier

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/extended/OWNERS~~ [bparees]

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

openshift-bot · 2021-10-11T20:17:44Z

/retest-required