OCPBUGS-9050: Alert on failing conditional update risk evaluation #961

petr-muller · 2023-08-31T16:33:13Z

Add a new cluster_version_conditional_updates_recommended_conditions_seconds which, for each conditional update known to CVO, reports how long (in seconds) is the Recommended condition on that update in the current state (this is the time since the conditions' lastTransitionTime). The metric is labelled with version, reason and status. Note that lastTransitionTime was not correctly maintained by CVO until fixed in #964.

Using this metric, we create an alert that fires when there is an update that is in an Unknown status for more than 50 minutes and this state is maintained for 10 minutes.

The metric was created slightly more generic than the alert would have needed. Technically the CVO could compute the "bad" state right away and just flap a 0/1 metric, but I believe the metrics are more useful when slightly generic.

openshift-ci-robot · 2023-08-31T16:33:19Z

@petr-muller: This pull request references Jira Issue OCPBUGS-9050, which is invalid:

expected the bug to target the "4.14.0" version, but it targets "4.12.z" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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

Details

In response to this:

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.

openshift-ci · 2023-08-31T16:33:23Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

petr-muller · 2023-08-31T16:33:24Z

/test all

petr-muller · 2023-08-31T18:16:38Z

Haha we do not respect lastTransitionTime contract and reset it to zero even without a status change...

petr-muller · 2023-09-04T13:32:49Z

/test all

petr-muller · 2023-09-04T15:16:29Z

/test all

petr-muller · 2023-09-04T15:18:54Z

/test all

petr-muller · 2023-09-05T12:08:40Z

/retest

petr-muller · 2023-09-05T12:11:22Z

/test all

petr-muller · 2023-09-05T12:11:34Z

/jira refresh

openshift-ci-robot · 2023-09-05T12:11:37Z

@petr-muller: This pull request references Jira Issue OCPBUGS-9050, which is invalid:

expected the bug to target the "4.14.0" version, but it targets "4.15.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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.

petr-muller · 2023-09-05T16:56:54Z

Needs #964 first

petr-muller · 2023-09-08T14:45:15Z

/jira refresh

openshift-ci-robot · 2023-09-08T14:45:23Z

@petr-muller: This pull request references Jira Issue OCPBUGS-9050, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.15.0) matches configured target version for branch (4.15.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @shellyyang1989

Details

In response to this:

/jira refresh

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.

- Used `max by` in the alert to drop irrelevant labels from alert - Replaced further `"Recommended"` literals with new constant - Do not `ToLower` booleanish string labels - Report metrics labelled with `condition` and `status` instead of a specialized `recommended` label. We only export the `Recommended` condition status still. - Naming and code structure tweaks

…ition_seconds`

…on to ts

If the CVO is just starting up, it should populate its "known state" of available updates from the `ClusterVersion` status, if its contains them. Previous CVO may have evaluated the same graph data like the the current one is about to do and if it did, there are likely existing conditions in the status that we need to respect (for example, do not bump a `lastTransitionTime` field on a condition on a conditinal update that was already evaluated with the same result.

petr-muller · 2023-10-24T16:09:56Z

This should be now ready for another round of review and testing. The underlying metrics was pivoted to be timestamp-based rather than duration based (this is a followup of the Slack conversation where we discovered this upstream recommendation).

New commit 983b3d0 makes CVO persist conditions known about conditional updates from the ClusterVersion status when it starts and evaluates available updates for the first time.

wking · 2023-10-24T16:52:26Z

pkg/cvo/metrics.go

+	}
+}
+
+// Collect collects metrics from the operator into the channel ch


nit: truncated comment

wking

/lgtm

openshift-ci · 2023-10-24T16:57:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

~~OWNERS~~ [wking]

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

LalatenduMohanty · 2023-10-25T20:41:20Z

/label backport-risk-assessed

petr-muller · 2023-10-26T00:05:34Z

/hold cancel
/test e2e-hypershift

Lot of failures, nothing indicates CVO

petr-muller · 2023-10-26T00:06:48Z

/hold

This was held because it waits for testing (the backport-risk-assessed confused me, it's not needed here but in #985)

shellyyang1989 · 2023-10-26T13:04:00Z

Pre-merge testing

# oc adm upgrade 
Cluster version is 4.15.0-0.test-2023-10-26-103451-ci-ln-59brt82-latest

// Patch dummy cincy

# oc patch clusterversion/version --patch '{"spec":{"upstream":"https://raw.githubusercontent.com/shellyyang1989/upgrade-cincy/master/cincy-conditional-edge-invalid-promql.json"}}' --type=merge
clusterversion.config.openshift.io/version patched

# oc adm upgrade channel stable-4.15
warning: No channels known to be compatible with the current version "4.15.0-0.test-2023-10-26-103451-ci-ln-59brt82-latest"; unable to validate "stable-4.15". Setting the update channel to "stable-4.15" anyway.

// Recommended=Unknown update condition is present

conditionalUpdates:
    - conditions:
      - lastTransitionTime: "2023-10-26T11:54:21Z"
        message: |-
          Could not evaluate exposure to update risk InvalidPromQL (executing PromQL query: bad_data: 1:49: parse error: unexpected identifier "buggy" in label matching, expected string)
            InvalidPromQL description: Invalid Promql
            InvalidPromQL URL: https://invalid.com/a
        reason: EvaluationFailed
        status: Unknown
        type: Recommended
      release:
        image: registry.ci.openshift.org/ocp/release@sha256:d9759e7c8ec5e2555419d84ff36aff2a4c8f9367236c18e722a3fe4d7c4f6dee
        version: 4.15.0-0.nightly-2023-11-11-065245
      risks:
      - matchingRules:
        - promql:
            promql: group(cluster_version_available_updates{channel=buggy})
          type: PromQL
        message: Invalid Promql
        name: InvalidPromQL
        url: https://invalid.com/a

// After 1 hour, the alert CannotEvaluateConditionalUpdates fires

# curl -s -k -H "Authorization: Bearer $token"  https://$url/api/v1/alerts | jq -r '.data.alerts[]|select(.labels.alertname == "CannotEvaluateConditionalUpdates")'
{
  "labels": {
    "alertname": "CannotEvaluateConditionalUpdates",
    "condition": "Recommended",
    "reason": "EvaluationFailed",
    "severity": "warning",
    "status": "Unknown",
    "version": "4.15.0-0.nightly-2023-11-11-065245"
  },
  "annotations": {
    "description": "Failure to evaluate conditional update matches means that Cluster Version Operator cannot decide whether an update path is recommended or not.",
    "summary": "Cluster Version Operator cannot evaluate conditional update matches for 1h 0m 6s."
  },
  "state": "firing",
  "activeAt": "2023-10-26T12:54:27.319838523Z",
  "value": "3.606319000005722e+03"
}

// Delete CVO pod

# oc delete pod cluster-version-operator-5f94759dfb-hprlp -n openshift-cluster-version
pod "cluster-version-operator-5f94759dfb-hprlp" deleted

// The alert survives

# curl -s -k -H "Authorization: Bearer $token"  https://$url/api/v1/alerts | jq -r '.data.alerts[]|select(.labels.alertname == "CannotEvaluateConditionalUpdates")'
{
  "labels": {
    "alertname": "CannotEvaluateConditionalUpdates",
    "condition": "Recommended",
    "reason": "EvaluationFailed",
    "severity": "warning",
    "status": "Unknown",
    "version": "4.15.0-0.nightly-2023-11-11-065245"
  },
  "annotations": {
    "description": "Failure to evaluate conditional update matches means that Cluster Version Operator cannot decide whether an update path is recommended or not.",
    "summary": "Cluster Version Operator cannot evaluate conditional update matches for 1h 1m 6s."
  },
  "state": "firing",
  "activeAt": "2023-10-26T12:54:27.319838523Z",
  "value": "3.666319000005722e+03"
}

Looks good.

shellyyang1989 · 2023-10-26T13:33:01Z

/label qe-approved

petr-muller · 2023-10-26T14:10:35Z

/hold cancel
/test e2e-hypershift

openshift-ci · 2023-10-26T16:21:38Z

@petr-muller: 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/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot · 2023-10-26T16:28:57Z

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

openshift/cluster-version-operator#961

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

Details

In response to this:

Add a new cluster_version_conditional_updates_recommended_conditions_seconds which, for each conditional update known to CVO, reports how long (in seconds) is the Recommended condition on that update in the current state (this is the time since the conditions' lastTransitionTime). The metric is labelled with version, reason and status. Note that lastTransitionTime was not correctly maintained by CVO until fixed in #964.

Using this metric, we create an alert that fires when there is an update that is in an Unknown status for more than 50 minutes and this state is maintained for 10 minutes.

The metric was created slightly more generic than the alert would have needed. Technically the CVO could compute the "bad" state right away and just flap a 0/1 metric, but I believe the metrics are more useful when slightly generic.

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.

openshift-merge-robot · 2023-10-27T19:01:05Z

Fix included in accepted release 4.15.0-0.nightly-2023-10-27-135451

openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Aug 31, 2023

openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Aug 31, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 31, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2023

petr-muller mentioned this pull request Aug 31, 2023

validate-blocked-edges.py: enforce CamelCase names openshift/cincinnati-graph-data#4059

Merged

petr-muller force-pushed the ocpbugs-9050-cannot-evaluate-conditional-updates branch from 384f24b to 4261ee7 Compare September 4, 2023 10:21

petr-muller force-pushed the ocpbugs-9050-cannot-evaluate-conditional-updates branch from 4261ee7 to d032eeb Compare September 4, 2023 15:16

petr-muller force-pushed the ocpbugs-9050-cannot-evaluate-conditional-updates branch from d032eeb to bc442fd Compare September 4, 2023 15:18

petr-muller force-pushed the ocpbugs-9050-cannot-evaluate-conditional-updates branch from bc442fd to 14199a3 Compare September 5, 2023 12:10

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 8, 2023

openshift-ci bot requested a review from shellyyang1989 September 8, 2023 14:45

petr-muller force-pushed the ocpbugs-9050-cannot-evaluate-conditional-updates branch from 14199a3 to db1a301 Compare September 12, 2023 13:51

petr-muller marked this pull request as ready for review September 12, 2023 13:52

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 12, 2023

petr-muller added 3 commits October 24, 2023 17:57

OCPBUGS-9050: Alert on failing conditional update risk evaluation

59c1513

Improve the help message for `cluster_version_conditional_update_cond…

7225971

…ition_seconds`

petr-muller force-pushed the ocpbugs-9050-cannot-evaluate-conditional-updates branch 3 times, most recently from ed62379 to e30c47c Compare October 24, 2023 16:02

petr-muller added 2 commits October 24, 2023 18:05

metrics: pivot ..._conditional_update_condition_seconds from durati…

33be595

…on to ts

petr-muller force-pushed the ocpbugs-9050-cannot-evaluate-conditional-updates branch from e30c47c to 983b3d0 Compare October 24, 2023 16:05

wking reviewed Oct 24, 2023

View reviewed changes

pkg/cvo/metrics.go

}

}

// Collect collects metrics from the operator into the channel ch

Copy link

Member

wking Oct 24, 2023

Choose a reason for hiding this comment

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

nit: truncated comment

wking approved these changes Oct 24, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2023

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

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 26, 2023

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 26, 2023

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Oct 26, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 26, 2023

openshift-ci bot merged commit 00d0940 into openshift:master Oct 26, 2023

petr-muller deleted the ocpbugs-9050-cannot-evaluate-conditional-updates branch October 30, 2023 16:06

OCPBUGS-9050: Alert on failing conditional update risk evaluation #961

OCPBUGS-9050: Alert on failing conditional update risk evaluation #961

Uh oh!

Conversation

petr-muller commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Aug 31, 2023

Uh oh!

openshift-ci bot commented Aug 31, 2023

Uh oh!

petr-muller commented Aug 31, 2023

Uh oh!

petr-muller commented Aug 31, 2023

Uh oh!

petr-muller commented Sep 4, 2023

Uh oh!

petr-muller commented Sep 4, 2023

Uh oh!

petr-muller commented Sep 4, 2023

Uh oh!

petr-muller commented Sep 5, 2023

Uh oh!

petr-muller commented Sep 5, 2023

Uh oh!

petr-muller commented Sep 5, 2023

Uh oh!

openshift-ci-robot commented Sep 5, 2023

Uh oh!

petr-muller commented Sep 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petr-muller commented Sep 8, 2023

Uh oh!

openshift-ci-robot commented Sep 8, 2023

Uh oh!

petr-muller commented Oct 24, 2023

Uh oh!

wking Oct 24, 2023

Choose a reason for hiding this comment

Uh oh!

wking left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 24, 2023

Uh oh!

LalatenduMohanty commented Oct 25, 2023

Uh oh!

petr-muller commented Oct 26, 2023

Uh oh!

petr-muller commented Oct 26, 2023

Uh oh!

shellyyang1989 commented Oct 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shellyyang1989 commented Oct 26, 2023

Uh oh!

petr-muller commented Oct 26, 2023

Uh oh!

openshift-ci bot commented Oct 26, 2023

Uh oh!

openshift-ci-robot commented Oct 26, 2023

Uh oh!

openshift-merge-robot commented Oct 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

petr-muller commented Aug 31, 2023 •

edited

Loading

petr-muller commented Sep 5, 2023 •

edited

Loading

shellyyang1989 commented Oct 26, 2023 •

edited

Loading