Add ServiceMonitor and E2E tests for Prometheus #286

borod108 · 2022-01-11T13:33:50Z

Add ServiceMonitor to seperate from kubevirts ServiceMonitor.
Add e2e tests for all Alerts

Signed-off-by: borod108 boris.od@gmail.com

ServiceMonitor added for the ssp-operator. A service added to the operator and TemplateValidator namespaces.
In case they are the same - only one service will be present.

borod108 · 2022-03-16T11:05:40Z

Changes applied, thank you!

borod108 · 2022-03-16T19:55:05Z

/retest

borod108 · 2022-03-17T07:23:15Z

/retest

internal/operands/metrics/reconcile.go

internal/operands/template-validator/reconcile.go

tests/misc_test.go

tests/monitoring_test.go

tests/tests_suite_test.go

akrejcir · 2022-03-17T16:30:39Z

tests/validator_test.go

@@ -895,6 +908,22 @@ func eventuallyFailToCreateVm(vm *kubevirtv1.VirtualMachine) bool {
 	}, shortTimeout).Should(Equal(metav1.StatusReasonInvalid), "Should have given the invalid error")
 }

+func failVmCreationToIncreaseRejectedVmsMetrics() {
+	rejectedCountBefore := totalRejectedVmsMetricsValue()
+	template := TemplateWithRules()


This template is not deleted after the test. It will be deleted once the namespace is removed, but that will not happen, if the tests are executed with existing SSP CR.

Can you add code to delete it, so the namspace is not polluted?

In the current code the template would not be deleted, if one of the Expect() calls panics.

Instead, can you create the template before this function is called, and pass it as a parameter? Then it can be deleted in AfterEach().

tests/validator_test.go

akrejcir

Thanks for the update. I have just 3 more small comments.

internal/operands/metrics/reconcile.go

tests/tests_suite_test.go

akrejcir · 2022-03-22T15:01:56Z

tests/validator_test.go

@@ -895,6 +908,22 @@ func eventuallyFailToCreateVm(vm *kubevirtv1.VirtualMachine) bool {
 	}, shortTimeout).Should(Equal(metav1.StatusReasonInvalid), "Should have given the invalid error")
 }

+func failVmCreationToIncreaseRejectedVmsMetrics() {
+	rejectedCountBefore := totalRejectedVmsMetricsValue()
+	template := TemplateWithRules()


In the current code the template would not be deleted, if one of the Expect() calls panics.

Instead, can you create the template before this function is called, and pass it as a parameter? Then it can be deleted in AfterEach().

tests/monitoring_test.go

Add ServiceMonitor to seperate from kubevirts ServiceMonitor. Add e2e tests for all Alerts. Signed-off-by: borod108 <boris.od@gmail.com>

sonarcloud · 2022-03-23T08:03:10Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

openshift-ci · 2022-03-23T10:21:13Z

@borod108: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-upgrade-from-old-operator-functests	`d60ec2b`	link	true	`/test e2e-upgrade-from-old-operator-functests`

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.

akrejcir · 2022-03-23T10:48:09Z

The CI failure probably means that CI is not testing what we expect. There is no template with os.template.kubevirt.io/ubuntu21.10 label in the current PR, or in the latest released version of SSP.

I will look into it.

akrejcir · 2022-03-23T11:09:33Z

Ignore my previous comment. The CI is correct. It is not testing this PR, but a merge commit of master branch and this PR.

Can you try rebasing this PR on top of master? Maybe the the merge in CI is incorrect.

borod108 · 2022-03-23T13:44:02Z

/retest

akrejcir · 2022-03-23T15:47:03Z

/lgtm

marceloamaral · 2022-03-24T01:58:05Z

internal/operands/metrics/resources.go

-	componentAlertLabelKey   = "kubernetes_operator_component"
-	componentAlertLabelValue = "ssp-operator"
+	PrometheusRuleName           = "prometheus-k8s-rules-cnv"
+	MonitorNamespace             = "openshift-monitoring"


Will it only work in OpenShift?
I think it should work for both monitoring and openshift-monitoring namespaces...

this will only work in openshift (the operator is working with templates that only work on openshift anyway)

and it should run on okd, since it has the same namespace (https://docs.okd.io/latest/monitoring/monitoring-overview.html#default-monitoring-components_monitoring-overview)

akrejcir · 2022-03-24T09:49:36Z

/approve

kubevirt-bot · 2022-03-24T09:49:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akrejcir

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~~ [akrejcir]

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

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jan 11, 2022

kubevirt-bot requested review from ksimon1 and rnetser January 11, 2022 13:33

borod108 changed the title ~~Add ServiceMonitor and E2E tests for Prometheus~~ [WIP] Add ServiceMonitor and E2E tests for Prometheus Jan 11, 2022

kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL labels Jan 11, 2022

borod108 force-pushed the tests/prom_e2e branch 11 times, most recently from 4402400 to bf53528 Compare January 19, 2022 10:02

borod108 force-pushed the tests/prom_e2e branch 4 times, most recently from 054898b to 8c7102c Compare January 20, 2022 13:57

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 26, 2022

borod108 force-pushed the tests/prom_e2e branch from 8c7102c to c7941b3 Compare January 27, 2022 09:20

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 27, 2022

borod108 force-pushed the tests/prom_e2e branch 5 times, most recently from dfce1c8 to a3709e1 Compare January 30, 2022 16:12

borod108 force-pushed the tests/prom_e2e branch 4 times, most recently from dbf2d6d to 0cc57f6 Compare March 16, 2022 10:44

borod108 force-pushed the tests/prom_e2e branch from 0cc57f6 to 77e2076 Compare March 16, 2022 18:21

borod108 force-pushed the tests/prom_e2e branch from 9556f85 to fb7beba Compare March 17, 2022 15:04

akrejcir requested changes Mar 17, 2022

View reviewed changes

borod108 force-pushed the tests/prom_e2e branch 2 times, most recently from a44c926 to 4e47891 Compare March 22, 2022 14:16

akrejcir requested changes Mar 22, 2022

View reviewed changes

borod108 force-pushed the tests/prom_e2e branch 2 times, most recently from f29675b to fd55d0d Compare March 23, 2022 04:24

Add ServiceMonitor and E2E tests for Prometheus

6a20a3f

Add ServiceMonitor to seperate from kubevirts ServiceMonitor. Add e2e tests for all Alerts. Signed-off-by: borod108 <boris.od@gmail.com>

borod108 force-pushed the tests/prom_e2e branch from fd55d0d to 6a20a3f Compare March 23, 2022 08:02

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 23, 2022

marceloamaral reviewed Mar 24, 2022

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 24, 2022

kubevirt-bot merged commit 15b23ca into kubevirt:master Mar 24, 2022

borod108 deleted the tests/prom_e2e branch July 12, 2022 10:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ServiceMonitor and E2E tests for Prometheus #286

Add ServiceMonitor and E2E tests for Prometheus #286

borod108 commented Jan 11, 2022 •

edited

borod108 commented Mar 16, 2022

borod108 commented Mar 16, 2022

borod108 commented Mar 17, 2022

akrejcir Mar 17, 2022

borod108 Mar 22, 2022

akrejcir Mar 22, 2022

akrejcir left a comment

akrejcir Mar 22, 2022

sonarcloud bot commented Mar 23, 2022

openshift-ci bot commented Mar 23, 2022 •

edited

akrejcir commented Mar 23, 2022

akrejcir commented Mar 23, 2022

borod108 commented Mar 23, 2022

akrejcir commented Mar 23, 2022

marceloamaral Mar 24, 2022

borod108 Mar 24, 2022

borod108 Mar 24, 2022

akrejcir commented Mar 24, 2022

kubevirt-bot commented Mar 24, 2022

Add ServiceMonitor and E2E tests for Prometheus #286

Add ServiceMonitor and E2E tests for Prometheus #286

Conversation

borod108 commented Jan 11, 2022 • edited

borod108 commented Mar 16, 2022

borod108 commented Mar 16, 2022

borod108 commented Mar 17, 2022

akrejcir Mar 17, 2022

Choose a reason for hiding this comment

borod108 Mar 22, 2022

Choose a reason for hiding this comment

akrejcir Mar 22, 2022

Choose a reason for hiding this comment

akrejcir left a comment

Choose a reason for hiding this comment

akrejcir Mar 22, 2022

Choose a reason for hiding this comment

sonarcloud bot commented Mar 23, 2022

openshift-ci bot commented Mar 23, 2022 • edited

akrejcir commented Mar 23, 2022

akrejcir commented Mar 23, 2022

borod108 commented Mar 23, 2022

akrejcir commented Mar 23, 2022

marceloamaral Mar 24, 2022

Choose a reason for hiding this comment

borod108 Mar 24, 2022

Choose a reason for hiding this comment

borod108 Mar 24, 2022

Choose a reason for hiding this comment

akrejcir commented Mar 24, 2022

kubevirt-bot commented Mar 24, 2022

borod108 commented Jan 11, 2022 •

edited

openshift-ci bot commented Mar 23, 2022 •

edited