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 1829405: test/extended/operators/operators: Rework "start all core operators" #24948
Bug 1829405: test/extended/operators/operators: Rework "start all core operators" #24948
Conversation
@wking: This pull request references Bugzilla bug 1829405, which is invalid:
Comment 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. |
/bugzilla refresh |
@wking: This pull request references Bugzilla bug 1829405, 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. 3 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. |
b1c5bd4
to
a7b435d
Compare
/retest |
8f02850
to
fe2a717
Compare
I'd flipped the expected sense on Degraded (fixed with a7b435d -> fe2a717), but it shows the output formatting:
|
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.
Thanks, generally looks good. Some naming nits and do you have an example of the new output when this test fails?
I pushed fe2a717 -> 5819963 to fig working -> unready and use |
thanks, that looks nice |
Drop namespace from output strings, because ClusterOperator is cluster-scoped [1]. Shift the 'available' variable down to the block of code that actually consumes it, and convert it from a map to a counter, because we don't care about displaying available operators. Rename 'unavailable' to the more generic 'unready'. Also rephrase the "still doing work" message to hinge on "ready", because "working" can sound like "as expectd" not "still has stuff to do" [1]. An unready operator may be available but progressing, etc., so 'unavailable' was too specific. Remove the 'break' from the wait-loop's ClusterOperator iteration. The previous logic would only list the first unavailable operator. With this commit we now list each unavailable operator while we wait. Reformat the tab-writer block to print each operator and its worst condition. The previous format had columns for progressing and available, but none for degraded status. It also had a single message column, making it unclear which condition's message was being displayed. The new format displays the full type, status, reason, and message of the worst condition for each operator. The new code also includes the worst condition (or, if all existing conditions for an operator were expected, any missing condition types) in the failing error message. This should be more actionable with less digging than the current messages, which just list the unsettled operator names [2]. [1]: https://github.com/openshift/api/blob/b98a784d8e6dc93c416b40cdf54bf3102f2b61d2/config/v1/types_cluster_operator.go#L9 [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1829405#c1
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, 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. |
@wking: All pull requests linked via external trackers have merged: openshift/origin#24948. Bugzilla bug 1829405 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. |
Drop namespace from output strings, because
ClusterOperator
is cluster-scoped.Shift the
available
variable down to the block of code that actually consumes it, and convert it from a map to a counter, because we don't care about displaying available operators.Rename
unavailable
to the more genericworking
(to match the existing "still doing work" messages). Working operator may be available but progressing, etc., sounavailable
was too specific.Remove the
break
from the wait-loop'sClusterOperator
iteration. The previous logic would only list the first unavailable operator. With this commit we now list each unavailable operator while we wait.Reformat the tab-writer block to print each operator and its worst condition. The previous format had columns for progressing and available, but none for degraded status. It also had a single message column, making it unclear which condition's message was being displayed. The new format displays the full type, status, reason, and message of the worst condition for each operator.
The new code also includes the worst condition (or, if all existing conditions for an operator were expected, any missing condition types) in the failing error message. This should be more actionable with less digging than the current messages, which just list the unsettled operator names.