Skip to content

OSD-22394 osdctl alert silence --all does not silence#580

Merged
openshift-merge-bot[bot] merged 8 commits intoopenshift:masterfrom
itsmitul9:test1
Jun 28, 2024
Merged

OSD-22394 osdctl alert silence --all does not silence#580
openshift-merge-bot[bot] merged 8 commits intoopenshift:masterfrom
itsmitul9:test1

Conversation

@itsmitul9
Copy link
Contributor

  • Fixed issue for making all alerts silenced using --all
  • Refactored code
  • Silence list display namespace in AlertName
  • Using namespace matcher to alert all silence

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 22, 2024
@AlexVulaj
Copy link
Contributor

Some thoughts as I look over this PR:

  • ExecWithPod should take a "namespace" as a parameter and be moved to a more generic "utils" package, as it's a useful command all around and doesn't have any logic specific to alerts.
  • ExecInPod should have a more specific name, maybe something like "ExecInAlertManagerPod", and a more descriptive comment to go along with that. Right now it's unclear to someone looking at the function name and comments that it will try on both AlertManager pods specifically.

@itsmitul9 itsmitul9 requested a review from AlexVulaj June 27, 2024 13:14
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 28, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 28, 2024

@itsmitul9: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

@devppratik
Copy link
Contributor

/lgtm

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

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 28, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlexVulaj, devppratik, itsmitul9

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 28, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit b8a4413 into openshift:master Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

4 participants