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

Deprecate cpu and memory exceeds alerts #172

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Deprecate cpu and memory exceeds alerts #172

merged 1 commit into from
Apr 10, 2024

Conversation

avlitman
Copy link
Contributor

@avlitman avlitman commented Mar 25, 2024 •
edited

Deprecate the KubeVirtComponentExceedsRequestedCPU and KubeVirtComponentExceedsRequestedMemory alerts, which the user can't fix.

jira-ticket: https://issues.redhat.com/browse/CNV-37900

Special notes for your reviewer:

  • CNV-38320 will handle adding a metric to be able to track this issue without creating noise to the user.
KubeVirtComponentExceedsRequestedCPU and KubeVirtComponentExceedsRequestedMemory alerts are deprecated

@avlitman avlitman force-pushed the deprecate-cpu-mem-alerts branch 2 times, most recently from 1fca059 to 8434876 Compare March 25, 2024 09:44
orenc1
orenc1 approved these changes Mar 25, 2024
View reviewed changes
Copy link

@orenc1 orenc1 left a comment

Choose a reason for hiding this comment

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

/approve

@orenc1
Copy link

orenc1 commented Mar 25, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 25, 2024
@sradco
Copy link
Contributor

sradco commented Mar 25, 2024

@ctomasko please let me know who should review and update the run books at this point. Thanks.

@avlitman
Copy link
Contributor Author

/hold until doc team will approve this.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 25, 2024
@ctomasko
Copy link

ctomasko commented Mar 25, 2024 •
edited by openshift-ci bot

@ctomasko please let me know who should review and update the run books at this point. Thanks.

@sradco @avlitman I have 2 writers who are updating the runbooks for v4.16. We have not sync'd the upstream/dowstream yet, see epic https://issues.redhat.com/browse/CNV-36859.
I located CNV-37900 and see that the alerts are being deprecated for versions 4.13, 4.14, 4.15, & 4.16. I cloned the bug https://issues.redhat.com/browse/CNV-39980 to update the documentation.

@avlitman
Copy link
Contributor Author

avlitman commented Apr 1, 2024

/unhold since doc is handling @sradco

@avlitman
Copy link
Contributor Author

avlitman commented Apr 1, 2024

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 1, 2024
@sradco
Copy link
Contributor

sradco commented Apr 1, 2024 •
edited

/hold
@ctomasko Should we merge updates to this repo or only create PRs and ask for your review?

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 1, 2024
ousleyp
ousleyp reviewed Apr 1, 2024
View reviewed changes
Copy link
Member

@ousleyp ousleyp left a comment

Choose a reason for hiding this comment

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

Hi @sradco, I'll be happy to provide doc review. Just one comment that applies to both files. Thanks!

alerts/openshift-virtualization-operator/KubeVirtComponentExceedsRequestedCPU.md Outdated
Comment on lines 3 to 4
If triggered, it may be safely ignored and silenced.
Copy link
Member

Choose a reason for hiding this comment

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

Is this the established text for a deprecated alert? If you're open to changing it, I think we can make it clearer by using minimalism principles.

  • I'd suggest using the active voice ("You can safely ignore or silence this alert") instead of the passive voice ("it may be safely ignored and silenced").

  • I'd remove "it does not indicate an actual issue" because it adds some confusion, IMO, and "safely" implies that it's a non-issue.

Suggestion:

Suggested change
This alert has been deprecated; it does not indicate an actual issue.
If triggered, it may be safely ignored and silenced.
This alert is deprecated. You can safely ignore or silence it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed thank you! and sorry for late response

@avlitman
Deprecate cpu and memory exceeds alerts
79f4eef 
Deprecate the KubeVirtComponentExceedsRequestedCPU and
KubeVirtComponentExceedsRequestedMemory alerts, which the user can't
fix.

Signed-off-by: avlitman <alitman@redhat.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2024
@avlitman avlitman requested review from ousleyp and orenc1 April 9, 2024 13:08
@avlitman
Copy link
Contributor Author

avlitman commented Apr 9, 2024 •
edited

Hi @ousleyp will appreciate if you can review again.

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Apr 9, 2024

@avlitman: all tests passed!

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.

@sradco
Copy link
Contributor

sradco commented Apr 9, 2024

/lgtm

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 9, 2024
@sradco
Copy link
Contributor

sradco commented Apr 9, 2024

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 9, 2024
@ousleyp
Copy link
Member

ousleyp commented Apr 10, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Apr 10, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: avlitman, orenc1, ousleyp, sradco

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

@openshift-merge-bot openshift-merge-bot bot merged commit c89f434 into openshift:master Apr 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiraboschi tiraboschi Awaiting requested review from tiraboschi

@ousleyp ousleyp Awaiting requested review from ousleyp

@orenc1 orenc1 Awaiting requested review from orenc1

Assignees

@ousleyp ousleyp

@orenc1 orenc1

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@avlitman @orenc1 @sradco @ctomasko @ousleyp