[release-4.16] OCPBUGS-33917: add alert data to upgrade health in oc adm upgrade status #1794

petr-muller · 2024-06-03T10:05:38Z

Replaces automated #1771 because multiple PRs were needed for OCPBUGS-33896.

Backports #1771 #1782 and #1787 to release 4.16:

add alerts to update health in oc adm upgrade status
add mock tests for alerts in oc adm upgrade status
OCPBUGS-33896: status/inspect-alerts: handle non-200 by Thanos
inspectalerts: use client-go wrappers for thanos call debugging
inspectalerts: refactor getWithBearer to try all urls in route
upgrade status: polish alert insights

this commit adds alerts that fire during the upgrade to `upgrade health` section. by default all the alerts that started firing after intiating the upgrade will appear in the upgrade health section we also have allowed alerts that will show alerts that firing before the upgrade was started. We've not added examples for alerts and are just testing it with unit tests. This is to reduce the input data in the examples.

Previously, the method iterated over URIs from a route but instead of searching for success it actually searched until first failures, which is against the point of iterating over possible URIs in the first place. Refactor the method so that it does not return on error, and return on success instead. Only return with an error if all URIs failed to yield a workable result. Slightly optimize the error for the common case where there is only a single URI to try, and shorten the string by using a namespace/name nnotation.

- skip alerts without required labels - add context on why we show the insight (started firing during update or is known to affect updates) - skip alerts with info level - explicitly mention the alert does not have a runbook - fix `shortDuration` for more cases, including `now`, add tests - handle also `message` annotation on alerts

openshift-ci-robot · 2024-06-03T10:05:44Z

@petr-muller: This pull request references Jira Issue OCPBUGS-33917, which is valid.

7 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.16.0) matches configured target version for branch (4.16.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
release note type set to "Release Note Not Required"
dependent bug Jira Issue OCPBUGS-33896 is in the state Verified, which is one of the valid states (MODIFIED, ON_QA, VERIFIED)
dependent Jira Issue OCPBUGS-33896 targets the "4.17.0" version, which is one of the valid target versions: 4.17.0
bug has dependents

Requesting review from QA contact:
/cc @evakhoni

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Backports #1771 #1782 and #1787 to release 4.16:

add alerts to update health in oc adm upgrade status

add mock tests for alerts in oc adm upgrade status

OCPBUGS-33896: status/inspect-alerts: handle non-200 by Thanos

inspectalerts: use client-go wrappers for thanos call debugging

inspectalerts: refactor getWithBearer to try all urls in route

upgrade status: polish alert insights

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2024-06-03T13:24:53Z

@petr-muller: 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-metal-ipi-ovn-ipv6	`3955f41`	link	false	`/test e2e-metal-ipi-ovn-ipv6`

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.

petr-muller · 2024-06-03T15:06:59Z

/cc @PratikMahajan @wking @jan--f
/uncc @deads2k

wking

Clean backports; nothing but some context difference vs. the dev-branch changes:

$ cherry-pick-diff origin/release-4.16..origin/pr/1794 origin/master

7515161 -> 3955f41 upgrade status: polish alert insights
51487f9 -> b2c1189 inspectalerts: refactor getWithBearer to try all urls in route
2715cc4 -> 579e757 inspectalerts: use client-go wrappers for thanos call debugging
518c0e7 -> c3cee5d OCPBUGS-33896: status/inspect-alerts: handle non-200 by Thanos
d5f52ff -> 48c7595 add mock tests for alerts in oc adm upgrade status
10c41f5 -> 8b0dc50 add alerts to update health in oc adm upgrade status

751516199 -> 3955f41d7 `upgrade status`: polish alert insights
--- 751516199
+++ 3955f41d7
@@ -11,3 +11,3 @@
 diff --git a/pkg/cli/admin/upgrade/status/alerts.go b/pkg/cli/admin/upgrade/status/alerts.go
-index cf66178d9..a9387b865 100644
+index 503c4ebf9..bc690baec 100644
 --- a/pkg/cli/admin/upgrade/status/alerts.go
@@ -23,3 +23,3 @@
 @@ -57,27 +58,72 @@ func parseAlertDataToInsights(alertData AlertData, startedAt time.Time) []update
-       var updateInsights []updateInsight
+       var updateInsights []updateInsight = []updateInsight{}
  
@@ -102,3 +102,3 @@
 diff --git a/pkg/cli/admin/upgrade/status/alerts_test.go b/pkg/cli/admin/upgrade/status/alerts_test.go
-index db68ab2a0..654976165 100644
+index bb79a3e8b..be25cd932 100644
 --- a/pkg/cli/admin/upgrade/status/alerts_test.go

d5f52ff69 -> 48c75952f add mock tests for alerts in oc adm upgrade status
--- d5f52ff69
+++ 48c75952f
@@ -269,6 +269,6 @@
 diff --git a/pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-unavailable-mco-20m.detailed-output b/pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-unavailable-mco-20m.detailed-output
-index 6225bf52a..a7381ef27 100644
+index 6c3c5bcc0..1c6d86694 100644
 --- a/pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-unavailable-mco-20m.detailed-output
 +++ b/pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-unavailable-mco-20m.detailed-output
-@@ -33,3 +33,21 @@ Message: Cluster Operator machine-config is unavailable (MachineConfigController
+@@ -34,3 +34,21 @@ Message: Cluster Operator machine-config is unavailable (MachineConfigController
    Resources:
@@ -295,6 +295,6 @@
 diff --git a/pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-unavailable-mco-20m.output b/pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-unavailable-mco-20m.output
-index 5289e9e55..c40a74b8c 100644
+index 155bfa393..f230be862 100644
 --- a/pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-unavailable-mco-20m.output
 +++ b/pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-unavailable-mco-20m.output
-@@ -25,7 +25,9 @@ ip-10-0-4-159.us-east-2.compute.internal    Outdated     Pending   4.14.0-rc.3
+@@ -26,7 +26,9 @@ ip-10-0-4-159.us-east-2.compute.internal    Outdated     Pending   4.14.0-rc.3
  ip-10-0-99-40.us-east-2.compute.internal    Outdated     Pending   4.14.0-rc.3   ?     
@@ -311,3 +311,3 @@
 diff --git a/pkg/cli/admin/upgrade/status/status.go b/pkg/cli/admin/upgrade/status/status.go
-index ad24f7e14..d79113d0f 100644
+index b54e530b3..a56e60459 100644
 --- a/pkg/cli/admin/upgrade/status/status.go
@@ -322,3 +322,3 @@
  
-@@ -283,7 +284,6 @@ func (o *options) Run(ctx context.Context) error {
+@@ -280,7 +281,6 @@ func (o *options) Run(ctx context.Context) error {
                if err := json.Unmarshal(alertBytes, &alertData); err != nil {

10c41f526 -> 8b0dc5057 add alerts to update health in oc adm upgrade status
--- 10c41f526
+++ 8b0dc5057
@@ -277,3 +277,3 @@
 diff --git a/pkg/cli/admin/upgrade/status/health.go b/pkg/cli/admin/upgrade/status/health.go
-index ef047b0c4..07e895352 100644
+index 8d77250b6..24c23b5e4 100644
 --- a/pkg/cli/admin/upgrade/status/health.go
@@ -309,3 +309,3 @@
 diff --git a/pkg/cli/admin/upgrade/status/status.go b/pkg/cli/admin/upgrade/status/status.go
-index 0612a0745..ad24f7e14 100644
+index 82d0de1c4..b54e530b3 100644
 --- a/pkg/cli/admin/upgrade/status/status.go
@@ -371,3 +371,3 @@
                if err != nil {
-@@ -245,6 +266,27 @@ func (o *options) Run(ctx context.Context) error {
+@@ -242,6 +263,27 @@ func (o *options) Run(ctx context.Context) error {
        }

/lgtm

evakhoni · 2024-06-03T16:27:12Z

pre-merge verified
/label qe-approved

petr-muller · 2024-06-03T16:29:21Z

@evakhoni can you slap the cherry-pick-approved here too?

evakhoni · 2024-06-03T19:21:05Z

np.
/label cherry-pick-approved

soltysh · 2024-06-04T14:48:21Z

/label backport-risk-assessed

simonpasquier

/lgtm

simonpasquier · 2024-06-04T15:04:53Z

/approve

openshift-ci · 2024-06-04T15:08:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, simonpasquier, wking

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:

~~pkg/cli/admin/inspectalerts/OWNERS~~ [simonpasquier]
~~pkg/cli/admin/upgrade/OWNERS~~ [petr-muller,wking]

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

openshift-ci · 2024-06-04T15:09:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, simonpasquier, wking

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:

~~pkg/cli/admin/inspectalerts/OWNERS~~ [simonpasquier]
~~pkg/cli/admin/upgrade/OWNERS~~ [petr-muller,wking]

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

openshift-ci-robot · 2024-06-04T18:14:25Z

@petr-muller: Jira Issue OCPBUGS-33917: All pull requests linked via external trackers have merged:

openshift/oc#1794

Jira Issue OCPBUGS-33917 has been moved to the MODIFIED state.

Details

In response to this:

Replaces automated #1771 because multiple PRs were needed for OCPBUGS-33896.

Backports #1771 #1782 and #1787 to release 4.16:

add alerts to update health in oc adm upgrade status

add mock tests for alerts in oc adm upgrade status

OCPBUGS-33896: status/inspect-alerts: handle non-200 by Thanos

inspectalerts: use client-go wrappers for thanos call debugging

inspectalerts: refactor getWithBearer to try all urls in route

upgrade status: polish alert insights

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-bot · 2024-06-04T21:52:20Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-tools-container-v4.16.0-202406042036.p0.gd9c3227.assembly.stream.el9 for distgit ose-tools.
All builds following this will include this PR.

PratikMahajan and others added 6 commits June 3, 2024 12:02

add mock tests for alerts in oc adm upgrade status

48c7595

OCPBUGS-33896: status/inspect-alerts: handle non-200 by Thanos

c3cee5d

inspectalerts: use client-go wrappers for thanos call debugging

579e757

openshift-ci bot requested review from deads2k, evakhoni and marioferh June 3, 2024 10:05

petr-muller mentioned this pull request Jun 3, 2024

[release-4.16] OCPBUGS-33917: add alert data to upgrade health in oc adm upgrade status #1771

Closed

openshift-ci bot requested review from PratikMahajan, jan--f and wking and removed request for deads2k June 3, 2024 15:07

wking approved these changes Jun 3, 2024

View reviewed changes

openshift-ci bot assigned wking Jun 3, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 3, 2024

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jun 3, 2024

openshift-ci bot added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jun 3, 2024

openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Jun 4, 2024

openshift-ci bot assigned evakhoni, kasturinarra, xingxingxia and zhouying7780 Jun 4, 2024

simonpasquier reviewed Jun 4, 2024

View reviewed changes

openshift-ci bot assigned simonpasquier Jun 4, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 4, 2024

openshift-merge-bot bot merged commit d9c3227 into openshift:release-4.16 Jun 4, 2024

petr-muller deleted the cherry-pick-1740-1787-1782-to-release-4.16 branch June 4, 2024 18:21

[release-4.16] OCPBUGS-33917: add alert data to upgrade health in oc adm upgrade status #1794

[release-4.16] OCPBUGS-33917: add alert data to upgrade health in oc adm upgrade status #1794

Uh oh!

Conversation

petr-muller commented Jun 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Jun 3, 2024

Uh oh!

openshift-ci bot commented Jun 3, 2024

Uh oh!

petr-muller commented Jun 3, 2024

Uh oh!

wking left a comment

Choose a reason for hiding this comment

Uh oh!

evakhoni commented Jun 3, 2024

Uh oh!

petr-muller commented Jun 3, 2024

Uh oh!

evakhoni commented Jun 3, 2024

Uh oh!

soltysh commented Jun 4, 2024

Uh oh!

simonpasquier left a comment

Choose a reason for hiding this comment

Uh oh!

simonpasquier commented Jun 4, 2024

Uh oh!

openshift-ci bot commented Jun 4, 2024

Uh oh!

openshift-ci bot commented Jun 4, 2024

Uh oh!

openshift-ci-robot commented Jun 4, 2024

Uh oh!

openshift-bot commented Jun 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

petr-muller commented Jun 3, 2024 •

edited

Loading