OTA-542: pkg/cvo/internal: Do not block on Degraded=True ClusterOperator

[wking]

We have blocking on this condition since 545c342 (#31) when it was Failing. We'd softened our install-time handling to act this way back in b0b4902 (#136), motivated by install speed. And a degraded operator may slow dependent components in their own transitions. But as long as the operator/operand are available at all, it should not block depndent components from transitioning, so this commit removes the Degraded=True block from the remaining modes.

We still have the critical ClusterOperatorDegraded waking admins up when an operator goes Degraded=True for a while, we will just no longer block updates at that point. We won't block ReconcilingMode manifest application either, but since that's already flattened and permuted, and ClusterOperator tend to be towards the end of their TaskNode, the impact on ReconcilingMode is minimal (except that we will no longer go Failing=True in ClusterVersion when the only issue is some Degraded=True ClusterOperator).

CC @abhinavdahiya, @deads2k, @smarterclayton as folks who were involved in the logic I'm removing here.

[openshift-ci-robot]

[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

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

[wking]

/hold

Big change; we want to give folks time to weigh in.

[sdodson]

I would personally prefer that we tighten up the definition of what degraded means such that it's only used when things are critical enough that they should block the forward progress of upgrades.

[wking]

I would personally prefer that we tighten up the definition of what degraded means...

Current definition hinges on quality-of-service. Available definition requires the operand to be "functional and available". Can you float an example of component behavior that would be Available=True but still sufficiently severe to need to block later-manifest reconciliation?

[sdodson]

Thanks for referencing that, I'd say my view was mostly inline with the statement A service should not report Degraded during the course of a normal upgrade. rather than a deep consideration of available versus degraded. I'm concerned that we're being too eager to move into a degraded state during normal upgrade behavior. I'd prefer those operators tune themselves to align more closely with real world cluster behavior, I fear their timeouts are tuned based on behaviors observed from unloaded or only CI clusters.

[smarterclayton]

I agree with Scott (I think) - I don’t believe degraded is an acceptable state for operators ever, especially during upgrade, as we have historically defined it in practice.

[wking]

I don’t believe degraded is an acceptable state for operators ever, especially during upgrade, as we have historically defined it in practice.

I'm not arguing for it to be acceptable, I'm arguing about it being non-blocking. We will still alert if operators go Degraded=True in the wild. We can build CI to fail operators that go Degraded=True during a run, if we don't do that already. But if an operator goes Degraded=True in a customer cluster, it's not clear to me how sticking mid-update is helping the customer resolve that situation, vs. pushing through with the rest of the update, as long as the operator is Available=True, and letting admins sort out the degrading issue orthogonally.

[openshift-merge-robot]

@wking: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/integration	02fad45	link	/test integration
ci/prow/e2e-agnostic-operator	02fad45	link	/test e2e-agnostic-operator

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.

[openshift-bot]

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-bot]

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-bot]

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]

@wking: PR needs rebase.

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]

@openshift-bot: Closed this PR.

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.

[wking]

Picking this one back up for more discussion.

[openshift-ci]

[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

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

[openshift-ci-robot]

@wking: This pull request references OTA-542 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

We have blocking on this condition since 545c342 (#31) when it was Failing. We'd softened our install-time handling to act this way back in b0b4902 (#136), motivated by install speed. And a degraded operator may slow dependent components in their own transitions. But as long as the operator/operand are available at all, it should not block depndent components from transitioning, so this commit removes the Degraded=True block from the remaining modes.

We still have the critical ClusterOperatorDegraded waking admins up when an operator goes Degraded=True for a while, we will just no longer block updates at that point. We won't block ReconcilingMode manifest application either, but since that's already flattened and permuted, and ClusterOperator tend to be towards the end of their TaskNode, the impact on ReconcilingMode is minimal (except that we will no longer go Failing=True in ClusterVersion when the only issue is some Degraded=True ClusterOperator).

CC @abhinavdahiya, @deads2k, @smarterclayton as folks who were involved in the logic I'm removing here.

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]

@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/e2e-hypershift	cf0f1e3	link	true	/test e2e-hypershift
ci/prow/okd-scos-e2e-aws-ovn	cf0f1e3	link	false	/test okd-scos-e2e-aws-ovn
ci/prow/unit	cf0f1e3	link	true	/test unit
ci/prow/e2e-agnostic-ovn-upgrade-out-of-change	cf0f1e3	link	true	/test e2e-agnostic-ovn-upgrade-out-of-change
ci/prow/gofmt	cf0f1e3	link	true	/test gofmt

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