-
Notifications
You must be signed in to change notification settings - Fork 165
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
OCPBUGS-50478: Increase waitForFallbackDegradedConditionTimeout #1789
OCPBUGS-50478: Increase waitForFallbackDegradedConditionTimeout #1789
Conversation
@wangke19: This pull request references OCPQE-28167 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 sub-task to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
25f5271
to
69bdf11
Compare
/assign @p0lyn0mial |
@wangke19: This pull request references OCPQE-28167 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 sub-task to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin 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.
Thanks for fixing the broken test. This is awesome!
I left a few minor comments. Overall it LGTM.
setUnsupportedConfig(t, cs, getDefaultUnsupportedConfigForCurrentPlatform(t, cs)) | ||
err := waitForClusterInGoodState(t, cs, clusterStateWaitPollTimeout, clusterMustBeReadyFor) | ||
err := waitForClusterInGoodState(t, cs, clusterStateWaitPollTimeout, 5*clusterMustBeReadyFor) |
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.
is this increase in 5*clusterMustBeReadyFor
necessary ?
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.
Yes, After setUnsupportedConfig
, KAS operator still was not in stable, it will take some time to recover.
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.
OK, so maybe we should update waitForFallbackDegradedConditionTimeout
to return the modified time for all cases ?
|
||
return wait.Poll(10*time.Second, waitPollTimeout, func() (bool, error) { | ||
return wait.PollUntilContextTimeout(context.Background(), 20*time.Second, waitPollTimeout, true, func(cxt context.Context) (bool, error) { |
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.
could use update the signature of the waitForClusterInGoodState
function to accept a context instead ?
waitForClusterInGoodState(ctx context.Context, t testing.TB, cs clientSet, waitPollTimeout, mustBeReadyFor time.Duration) error
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.
Updated.
69bdf11
to
58a23f3
Compare
Test results in local,
|
58a23f3
to
5d6451d
Compare
/test e2e-gcp-operator-single-node |
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.
a few more nits, overall LGTM, thanks!
setUnsupportedConfig(t, cs, getDefaultUnsupportedConfigForCurrentPlatform(t, cs)) | ||
err := waitForClusterInGoodState(t, cs, clusterStateWaitPollTimeout, clusterMustBeReadyFor) | ||
err := waitForClusterInGoodState(t, cs, clusterStateWaitPollTimeout, 5*clusterMustBeReadyFor) |
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.
OK, so maybe we should update waitForFallbackDegradedConditionTimeout
to return the modified time for all cases ?
5d6451d
to
1e4aa28
Compare
I updated |
Test results in local for update,
|
/retest |
308bfef
to
4fc7f87
Compare
4fc7f87
to
707728f
Compare
/test e2e-gcp-operator-single-node |
@wangke19: The following test failed, say
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. |
This is awesome! The test had been broken for nearly two years, and thanks to your work, it is finally fixed. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: p0lyn0mial, wangke19 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 |
/label acknowledge-critical-fixes-only |
c3e5c90
into
openshift:master
/cherry-pick release-4.18 |
@wangke19: new pull request created: #1798 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-sigs/prow repository. |
/cherry-pick release-4.17 |
/cherry-pick release-4.16 |
/cherry-pick release-4.15 |
/cherry-pick release-4.14 |
@wangke19: new pull request created: #1799 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-sigs/prow repository. |
@wangke19: new pull request created: #1800 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-sigs/prow repository. |
@wangke19: new pull request created: #1801 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-sigs/prow repository. |
@wangke19: new pull request created: #1802 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-sigs/prow repository. |
[ART PR BUILD NOTIFIER] Distgit: ose-cluster-kube-apiserver-operator |
@wangke19: Jira Issue OCPBUGS-50478: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-50478 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 openshift-eng/jira-lifecycle-plugin repository. |
PR fixed:
The case execution time takes about 20mins,
StaticPodsAvailable
,NodeInstallerProgressing
andNodeControllerDegraded
to make sure the final KAS operator status.