Skip to content

WIP: pkg/cvo/status: Always set 'reason' for Available, Failing, and Progressing#867

Closed
wking wants to merge 1 commit intoopenshift:masterfrom
wking:happy-condition-reasons
Closed

WIP: pkg/cvo/status: Always set 'reason' for Available, Failing, and Progressing#867
wking wants to merge 1 commit intoopenshift:masterfrom
wking:happy-condition-reasons

Conversation

@wking
Copy link
Copy Markdown
Member

@wking wking commented Nov 18, 2022

From our org guidelines:

Operators should set reason and message for both happy and sad conditions. For sad conditions, the strings help explain what is going wrong. For happy conditions, the strings help convince users that things are going well.

`AsExpected` is a common choice for happy reasons with messages like `All is well` or `NodeInstallerProgressing: 3 nodes are at revision 7`.

Having an explicit happy reasons also make it easier to do things like:

sort_desc(count by (reason) (cluster_operator_conditions{name="cloud-credential",condition="Degraded"}))

for convenient aggregation, without having to mix in the time-series values to distinguish happy and sad cases.

@wking
Copy link
Copy Markdown
Member Author

wking commented Nov 18, 2022

/hold while we green up CI

@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 Nov 18, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Nov 18, 2022

[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:

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 Nov 18, 2022
@wking wking changed the title pkg/cvo/status: Always set 'reason' for Failing and Progressing WIP: pkg/cvo/status: Always set 'reason' for Failing and Progressing Nov 18, 2022
@openshift-ci 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 Nov 18, 2022
@wking wking force-pushed the happy-condition-reasons branch 3 times, most recently from d2383be to 084f7bf Compare November 18, 2022 04:31
@wking wking changed the title WIP: pkg/cvo/status: Always set 'reason' for Failing and Progressing WIP: pkg/cvo/status: Always set 'reason' for Available, Failing, and Progressing Nov 18, 2022
@wking wking force-pushed the happy-condition-reasons branch from 084f7bf to ba222ec Compare November 18, 2022 04:37
…essing

From [1]:

> Operators should set `reason` and `message` for both happy and sad
> conditions.  For sad conditions, the strings help explain what is
> going wrong.  For happy conditions, the strings help convince users
> that things are going well.
>
> `AsExpected` is a common choice for happy reasons with messages like `All is well` or `NodeInstallerProgressing: 3 nodes are at revision 7`.
>
> Having an explicit happy reasons also make it easier to do things like:
>
>   sort_desc(count by (reason) (cluster_operator_conditions{name="cloud-credential",condition="Degraded"}))
>
> for convenient aggregation, without having to mix in the time-series
> values to distinguish happy and sad cases.

[1]: openshift/enhancements#1266
@wking wking force-pushed the happy-condition-reasons branch from ba222ec to 0d89b90 Compare November 18, 2022 05:38
@openshift-bot
Copy link
Copy Markdown
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 16, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Mar 10, 2023

@wking: 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
ci/prow/unit 0d89b90 link true /test unit
ci/prow/e2e-agnostic 0d89b90 link true /test e2e-agnostic
ci/prow/e2e-agnostic-upgrade-out-of-change 0d89b90 link true /test e2e-agnostic-upgrade-out-of-change
ci/prow/e2e-agnostic-ovn-upgrade-out-of-change 0d89b90 link true /test e2e-agnostic-ovn-upgrade-out-of-change
ci/prow/e2e-agnostic-ovn-upgrade-into-change 0d89b90 link true /test e2e-agnostic-ovn-upgrade-into-change

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-bot
Copy link
Copy Markdown
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci Bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 10, 2023
@openshift-bot
Copy link
Copy Markdown
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci Bot closed this May 10, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 10, 2023

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants