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

monitoring: Add KubeVirtDeprecatedAPIsRequested alert #9724

Merged
merged 2 commits into from
May 26, 2023
Merged

monitoring: Add KubeVirtDeprecatedAPIsRequested alert #9724

merged 2 commits into from
May 26, 2023

Conversation

0xFelix
Copy link
Member

@0xFelix 0xFelix commented May 9, 2023 •
edited

What this PR does / why we need it:

This adds an alert which triggers when KubeVirt APIs marked as deprecated
are used. It makes use of the Kubernetes apiserver_requested_deprecated_apis
and apiserver_request_total metrics.

The PromQL query is based on the Kubernetes API deprecation docs [1].

[1] https://kubernetes.io/docs/reference/using-api/deprecation-policy/#rest-resources-aka-api-objects

Release note:

An alert which triggers when KubeVirt APIs marked as deprecated are used was added.

@kubevirt-bot 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 May 9, 2023
@kubevirt-bot kubevirt-bot requested review from dhiller and rmohr May 9, 2023 13:54
@0xFelix 0xFelix mentioned this pull request May 9, 2023
Merged
@0xFelix
Copy link
Member Author

0xFelix commented May 9, 2023

/hold Wait until kubevirt/monitoring#172 was merged

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 9, 2023
@0xFelix 0xFelix mentioned this pull request May 9, 2023
Merged
@apinnick
Copy link

apinnick commented May 9, 2023

When will this be released downstream?

@0xFelix
Copy link
Member Author

0xFelix commented May 9, 2023

@apinnick If this is merged soon it will go into KubeVirt v1.0.0 resp. CNV 4.14 in downstream.

@xpivarc
Copy link
Member

xpivarc commented May 9, 2023

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xpivarc

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 9, 2023
@sradco
Copy link
Contributor

sradco commented May 10, 2023

/hold

@sradco
Copy link
Contributor

sradco commented May 10, 2023 •
edited

@0xFelix Please add the information about the new recording rule, kubevirt_api_request_deprecated_total, also to
https://github.com/kubevirt/kubevirt/blob/main/tools/doc-generator/doc-generator.go
and to the https://github.com/kubevirt/kubevirt/blob/main/docs/metrics.md

Also, Can the value of kubevirt_api_request_deprecated_total go up and down(Gauge) or is it constantly increasing(Counter)? This will impacts the name of the new recording rule.

@sradco
Copy link
Contributor

sradco commented May 10, 2023 •
edited

@0xFelix Please also add to the PR description that the recording rule is based on https://kubernetes.io/docs/reference/using-api/deprecation-policy/#rest-resources-aka-api-objects

@sradco
Copy link
Contributor

sradco commented May 10, 2023

@avlitman This new recording rule is a counter. Please validate its name based on the new linter that it meets the naming conventions. Thanks.

@avlitman
Copy link
Contributor

@avlitman This new recording rule is a counter. Please validate its name based on the new linter that it meets the naming conventions. Thanks.

Name passed the linter.

@0xFelix 0xFelix force-pushed the deprecated-api-alert branch 2 times, most recently from fcdabaf to bc80b97 Compare May 10, 2023 09:39
@0xFelix
Copy link
Member Author

0xFelix commented May 10, 2023

@sradco Done, updated the PR.

@0xFelix
Copy link
Member Author

0xFelix commented May 10, 2023

/retest

sradco
sradco reviewed May 11, 2023
View reviewed changes
pkg/virt-operator/resource/generate/components/prometheus.go Outdated
},
{
Alert: "KubeVirtDeprecatedAPIsRequested",
Expr: intstr.FromString("sum(kubevirt_api_request_deprecated_total) > 0"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the increase function here, so that the alert will be cleared after the defined time.
Example:

sum(increase(kubevirt_api_request_deprecated_total[5m]) )> 0

And please update the tests to check its being cleared after the defined period.

@sradco
Copy link
Contributor

sradco commented May 15, 2023

/lgtm

@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 23, 2023
@0xFelix
prom-rule-ci: Update PROM_IMAGE to v2.44.0
cac9948 
The PROM_IMAGE is updated to the latest stable release version so
promtool accepts the latest rule syntax.

Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>
@sradco
Copy link
Contributor

sradco commented May 23, 2023

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label May 23, 2023
@0xFelix
Copy link
Member Author

0xFelix commented May 23, 2023

/unhold

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 23, 2023
@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

2 similar comments
@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@0xFelix
Copy link
Member Author

0xFelix commented May 24, 2023

/hold

Investigating the CI failures

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 24, 2023
@0xFelix
monitoring: Add KubeVirtDeprecatedAPIsRequested alert
38aa8bd 
This adds an alert which triggers when KubeVirt APIs marked as deprecated
are used. It makes use of the Kubernetes apiserver_requested_deprecated_apis
and apiserver_request_total metrics.

The PromQL query is based on the Kubernetes API deprecation docs [1].

[1] https://kubernetes.io/docs/reference/using-api/deprecation-policy/#rest-resources-aka-api-objects

Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label May 24, 2023
@0xFelix
Copy link
Member Author

0xFelix commented May 24, 2023

/unhold

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 24, 2023
@sradco
Copy link
Contributor

sradco commented May 24, 2023

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label May 24, 2023
@0xFelix
Copy link
Member Author

0xFelix commented May 24, 2023

/retest

@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@brianmcarey
Copy link
Member

/retest

@0xFelix 0xFelix mentioned this pull request May 25, 2023
Merged
@0xFelix
Copy link
Member Author

0xFelix commented May 25, 2023

/hold Wait until kubevirt/project-infra#2785 is merged

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 25, 2023
@0xFelix
Copy link
Member Author

0xFelix commented May 25, 2023

/unhold
/retest

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 25, 2023
@kubevirt-bot
Copy link
Contributor

kubevirt-bot commented May 25, 2023 •
edited

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

Test name Commit Details Required Rerun command
pull-kubevirt-fossa c285f91 link false /test pull-kubevirt-fossa
pull-kubevirt-e2e-arm64 c285f91 link false /test pull-kubevirt-e2e-arm64
pull-kubevirt-e2e-k8s-1.27-sig-compute 38aa8bd link unknown /test pull-kubevirt-e2e-k8s-1.27-sig-compute

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.

@kubevirt-bot kubevirt-bot merged commit 113c2da into kubevirt:main May 26, 2023
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lyarwood lyarwood lyarwood left review comments

@sradco sradco sradco left review comments

@dhiller dhiller Awaiting requested review from dhiller

@rmohr rmohr Awaiting requested review from rmohr

Assignees

@sradco sradco

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/monitoring dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@0xFelix @apinnick @xpivarc @kubevirt-bot @sradco @avlitman @lyarwood @kubevirt-commenter-bot @brianmcarey