-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 1866480: pkg/cvo/status: Raise Operator leveling grace-period to 20 minutes #427
Bug 1866480: pkg/cvo/status: Raise Operator leveling grace-period to 20 minutes #427
Conversation
Because every Failing=True condition should have a reason. Also wordsmith the user-facing docs to replace "synchronize" with "reconcile", because our merge logic is more nuanced than the complete match "synchronize" implies for me. The ClusterOperatorNotAvailable special casing landed with convertErrorToProgressing in c2ac20f (status: Report the operators that have not yet deployed, 2019-04-09, openshift#158).
Reduce false-positives when operators take a while to level (like the machine-config operator, which has to roll the control plane machines). We may want to raise this further in the future, but baby steps ;). The previous 10-minute value is from c2ac20f (status: Report the operators that have not yet deployed, 2019-04-09, openshift#158), which doesn't make a case for that specific value. So the bump is unlikely to break anything unexpected.
@wking: This pull request references Bugzilla bug 1866480, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 6 validation(s) were run on this bug
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm in general, one (possible) nitpick
@@ -93,22 +93,22 @@ So the graph nodes are all parallelized with the by-number ordering flattened ou | |||
|
|||
For the usual reconciliation loop (neither an upgrade between releases nor a fresh install), the flattened graph is also randomly permuted to avoid hanging on ordering bugs. | |||
|
|||
## Synchronizing the graph | |||
## Reconciling the graph |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Reconciling the graph | |
## Reconciling the cluster version |
nit: "graph" sounds a bit ambiguous to me - I've immediately though of "updates graph" instead "manifests graph"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a backport PR. Drop an /lgtm
, and then PR master with your change to "manifest graph", which sounds good to me.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrutkovs, 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:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
@wking: All pull requests linked via external trackers have merged: openshift/cluster-version-operator#427. Bugzilla bug 1866480 has been moved to the MODIFIED state. 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. |
Backporting #422 and, to avoid rebase conflicts, the doc-only #371 to 4.5.