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
[release-4.5] Bug 1851839: Make e2e tests more stable #130
[release-4.5] Bug 1851839: Make e2e tests more stable #130
Conversation
/retitle [release-4.5] Bug 1851839: Make e2e tests more stable |
@openshift-cherrypick-robot: This pull request references Bugzilla bug 1851839, 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. |
@martinkunc: This pull request references Bugzilla bug 1851839, 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. |
@martinkunc: This pull request references Bugzilla bug 1851839, 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. |
@martinkunc: This pull request references Bugzilla bug 1851839, 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 |
@martinkunc: This pull request references Bugzilla bug 1851839, 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 |
@martinkunc: This pull request references Bugzilla bug 1851839, 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 |
@martinkunc: This pull request references Bugzilla bug 1851839, 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 |
@martinkunc: This pull request references Bugzilla bug 1851839, 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. |
/test e2e-aws |
What was the change that actually makes the tests more stable? Also I would love for @lina-nikiforova to give her blessing, as main author of this area of the repo. |
@@ -7,6 +7,7 @@ test-unit: | |||
.PHONY: test-unit | |||
|
|||
test-e2e: | |||
go test ./test/integration -run ^\(TestPullSecretExists\)$$ -timeout 2h |
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.
do we need 2h here?
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.
I have created another PR with shorter period here: #135
@@ -102,7 +133,7 @@ func restartInsightsOperator(t *testing.T) { | |||
|
|||
for _, pod := range pods.Items { | |||
clientset.CoreV1().Pods("openshift-insights").Delete(pod.Name, &metav1.DeleteOptions{}) | |||
err := wait.PollImmediate(1*time.Second, 20*time.Minute, func() (bool, error) { | |||
err := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (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.
I don't think pods are usually deleted within 1 minute, I'd give it more time
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.
Actually 1 minute is with quite a reserve. Normally it takes few seconds and Kubernetes is making sure that deletion wont be longer then Grace period, which is 30s in IO pod.
You can see moure about it here: https://cloud.google.com/blog/products/gcp/kubernetes-best-practices-terminating-with-grace
@@ -115,7 +146,7 @@ func restartInsightsOperator(t *testing.T) { | |||
} | |||
|
|||
// check new pods are created and running | |||
errPod := wait.PollImmediate(1*time.Second, 10*time.Minute, func() (bool, error) { | |||
errPod := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (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.
same, I don't think 1 minute is enough
@iNecas Actually this is just a backport of #124, so I would need to create another PR to master with changes suggested here, if there will be some. |
/test e2e-aws |
2 similar comments
/test e2e-aws |
/test e2e-aws |
@lina-nikiforova Can you please approve, so that Ivan could add the label ? |
lgtm |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: iNecas, openshift-cherrypick-robot 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 |
/bugzilla refresh |
@martinkunc: This pull request references Bugzilla bug 1851839, which is valid. 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. |
@openshift-cherrypick-robot: All pull requests linked via external trackers have merged: openshift/insights-operator#130. Bugzilla bug 1851839 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. |
This is an automated cherry-pick of #124
/assign martinkunc