Apply monitoring linters #932
Conversation
kubevirt-bot
added
the
release-note-none
kubevirt-bot
added
dco-signoff: yes
|
/retest |
Makefile
Outdated
| @@ -326,6 +326,11 @@ lint: | |||
| lint-metrics: | |||
| ./hack/prom_metric_linter.sh --operator-name="kubevirt" --sub-operator-name="ssp" | |||
|
|
|||
| .PHONY: lint-monitoring | |||
| lint-monitoring: | |||
| go install github.com/kubevirt/monitoring/monitoringlinter/cmd/monitoringlinter@e2be790 | |||
There was a problem hiding this comment.
Please install it to the $(LOCALBIN) directory. Similarly to how controller-gen is installed on line 250.
As a future task, can you create a release tag in the github.com/kubevirt/monitoring repo for the submodule monitoringlinter? Then we can use a released version instead of a git commit e2be790.
There was a problem hiding this comment.
Updated to use $(LOCALBIN), thanks.
We did create a release tag in https://github.com/kubevirt/monitoring/releases, but there is an issue with installing it using v0.0.5 tag, probably because it's a submodule:
go install github.com/kubevirt/monitoring/monitoringlinter/cmd/monitoringlinter@v0.0.5
go: github.com/kubevirt/monitoring/monitoringlinter/cmd/monitoringlinter@v0.0.5: module github.com/kubevirt/monitoring@v0.0.5 found, but does not contain package github.com/kubevirt/monitoring/monitoringlinter/cmd/monitoringlinter
We decided to use commit hash because of that issue, and also because this way we don't need to release all kubevirt/monitoring repo for changes in the linter.
There was a problem hiding this comment.
Yes, you need to create a separate tag for submodule. Like we do in ssp-operator for the api submoule: https://github.com/kubevirt/ssp-operator/tags
assafad
force-pushed
the
monitoring-linters
branch
from
241b2af to
592d33a
Compare
assafad
force-pushed
the
monitoring-linters
branch
from
592d33a to
0eb10bf
Compare
|
/approve |
|
[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:
Approvers can indicate their approval by writing |
kubevirt-bot
added
the
approved
Makefile
Outdated
| @@ -326,6 +328,11 @@ lint: | |||
| lint-metrics: | |||
| ./hack/prom_metric_linter.sh --operator-name="kubevirt" --sub-operator-name="ssp" | |||
|
|
|||
| .PHONY: lint-monitoring | |||
| lint-monitoring: $(LOCALBIN) | |||
| test -s $(LOCALBIN)/monitoringlinter || GOBIN=$(LOCALBIN) go install github.com/kubevirt/monitoring/monitoringlinter/cmd/monitoringlinter@e2be790 | |||
There was a problem hiding this comment.
Can we put the version / rev into a variable?
pkg/monitoring/rules/rules_test.go
Outdated
| testutil.ValidateAlertHealthImpactLabel, | ||
| testutil.ValidateAlertPartOfAndComponentLabels) | ||
|
|
||
| alerts := ListAlerts() |
pkg/monitoring/rules/rules_test.go
Outdated
| }) | ||
|
|
||
| It("Should validate recording rules", func() { | ||
| recordingRules := ListRecordingRules() |
Apply monitoringlinter, which is designed to enforce kubevirt/community#219 by ensuring that monitoring-related practices are implemented within the pkg/monitoring directory using operator-observability methods. Signed-off-by: assafad <aadmi@redhat.com>
Apply operator-observability rules linter which ensures that kubevirt alerts and recording rules definitions are following the monitoring best practices. Signed-off-by: assafad <aadmi@redhat.com>
assafad
force-pushed
the
monitoring-linters
branch
from
0eb10bf to
8f57722
Compare
|
What this PR does / why we need it:
Apply the following linters:
monitoringlinter that was implemented in Add monitoring linter monitoring#221, and was designed to enforce proposal: Monitoring code refactor community#219.
This linter ensures that all monitoring-related practices are implemented within the
pkg/monitoringdirectory using operator-observability methods. It verifies that all metrics, alerts and recording rules registrations are centralized in this directory, and restricts the direct use of Prometheus registration methods.Example for a message reported by the linter:
operator-observability rules linter - ensures that alerts and recording rules definitions are following the monitoring best practices, e.g. alerts have all the required annotations and labels with expected values, recording rules have expressions, etc.
Fixes https://issues.redhat.com/browse/CNV-36766
Other PRs references: kubevirt/hyperconverged-cluster-operator#2827, kubevirt/kubevirt#11454
Release note: