-
Notifications
You must be signed in to change notification settings - Fork 192
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-17359: test/e2e: Don't use openshift/origin-node #970
OCPBUGS-17359: test/e2e: Don't use openshift/origin-node #970
Conversation
@Miciah: This pull request references Jira Issue OCPBUGS-17359, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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. |
/retest-required |
/lgtm |
/test e2e-azure-operator |
Using a Cluster Bot cluster to run
|
Hm, sometimes the test is able to pull the image, and sometimes it gets that auth error, during repeated tests on the same cluster. |
Watching
And then the image registry is allowing the pull in the next run:
|
The successful request has |
@Miciah: This pull request references Jira Issue OCPBUGS-17359, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
The two new commits seem to prevent the "authentication required" errors. In manual testing with these changes, I have seen the test pass over 20 times and fail 0 times. |
e2e-aws-ovn-serial failed because
I believe this is a known issue: OCPBUGS-17151. Also,
This is also a known issue: OCPBUGS-14930. e2e-aws-operator failed because must-gather failed. |
Use the "openshift/tools" image from the cluster image registry instead of using the "openshift/origin-node" image pullspec in E2E tests. Before this commit, the E2E tests were inadvertently pulling the "openshift/origin-node" image from Docker Hub and getting rate-limited. The choice to use "openshift/tools" is based on a similar change here: openshift/origin@4cbb844 Follow-up to commit 167bcc2 and commit a635566. This commit fixes OCPBUGS-17359. https://issues.redhat.com/browse/OCPBUGS-17359 * test/e2e/util_test.go (buildEchoPod, buildSlowHTTPDPod): Replace the "openshift/origin-node" image pullspec with "image-registry.openshift-image-registry.svc:5000/openshift/tools:latest".
839d1b2
to
4216df0
Compare
e2e-aws-ovn-serial failed with same failure for test
|
e2e-aws-operator failed because must-gather failed. e2e-gcp-operator failed because
Similarly,
The
We might need to adjust these timeouts, but if ELBs are consistently taking longer to provision than they used to, we also should look into that. Looking into the kube-controller logs, there is a rather conspicuous 3m39s gap from 16:14 to 16:17, between
/test e2e-gcp-operator |
test/e2e/util_test.go
Outdated
@@ -712,3 +712,19 @@ func getRouteHost(t *testing.T, route *routev1.Route, router string) string { | |||
t.Fatalf("failed to find host name for default router in route: %#v", route) | |||
return "" | |||
} | |||
|
|||
// dumpEventsInNamespace gets the namespaces in the specified namespace and logs |
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 supposed to be "dumpEventsInNamespace gets the events"?
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.
* test/e2e/hsts_policy_test.go (TestHstsPolicyWorks): Dump events in case of test failure, using the new dumpEventsInNamespace helper. * test/e2e/util_test.go (dumpEventsInNamespace): New helper function to log all events in a namespace.
When creating a new namespace for the TestHstsPolicyWorks test, wait for the "default" ServiceAccount and the "system:image-pullers" RoleBinding to be provisioned in the newly created namespace before proceeding with the test. Make a similar change for the TestMTLSWithCRLsCerts test. Before this commit, TestHstsPolicyWorks sometimes failed because it tried to create a pod before the ServiceAccount had been provisioned and granted access to pull images. As a result, the test would randomly fail with the following error: Failed to pull image "image-registry.openshift-image-registry.svc:5000/openshift/tools:latest": rpc error: code = Unknown desc = reading manifest This change should prevent such failures. Because TestMTLSWithCRLsCerts also creates a namespace and then creates pods in this namespace, this commit makes the same change to this test as well. Some other tests create namespaces but do not create pods in those namespaces; those tests do not necessarily need to wait for the ServiceAccount and RoleBinding. Inspired by openshift/origin@877c652. * test/e2e/client_tls_test.go (TestMTLSWithCRLs): * test/e2e/hsts_policy_test.go (TestHstsPolicyWorks): Use the new createNamespace helper. * test/e2e/util_test.go (createNamespace): New helper function. Create a namespace with the specified name, register a cleanup handler to delete the namespace when the test finishes, wait for the "default" ServiceAccount and "system:image-pullers" RoleBinding to be created, and return the namespace.
4216df0
to
42c4f82
Compare
https://github.com/openshift/cluster-ingress-operator/compare/4216df06b6a9731693f8d227123992e1e59b6a59..42c4f827df32ca5166e26dc2a2d75aea4dd8ebc4 fixes the godoc comment for |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: frobware 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 |
e2e-gcp-operator failed because e2e-hypershift failed because e2e-aws-ovn-single-node failed because 338 of 3821 tests failed. The first failure in the list was an
I filed OCPBUGS-17632 for this panic. Let's see what happens if we retry the job. e2e-gcp-ovn failed because |
e2e-gcp-operator failed because |
e2e-gcp-operator failed because |
I filed OCPBUGS-17670 for the issue that LBs can take over 5 minutes to provision on GCP. |
@Miciah: 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/test-infra repository. I understand the commands that are listed here. |
e2e-azure-operator failed because |
@Miciah: Jira Issue OCPBUGS-17359: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-17359 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. |
/cherry-pick release-4.13 |
@Miciah: #970 failed to apply on top of branch "release-4.13":
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
: Don't use "openshift/origin-node" imageUse the "openshift/tools" image from the cluster image registry instead of using the "openshift/origin-node" image pullspec in E2E tests.
Before this change, the E2E tests were inadvertently pulling the "openshift/origin-node" image from Docker Hub and getting rate-limited.
The choice to use "openshift/tools" is based on a similar change here: openshift/origin@4cbb844
Follow-up to #410 and #451.
test/e2e/util_test.go
(buildEchoPod
,buildSlowHTTPDPod
): Replace the "openshift/origin-node" image pullspec with "image-registry.openshift-image-registry.svc:5000/openshift/tools:latest".TestHstsPolicyWorks
: Dump events if test failstest/e2e/hsts_policy_test.go
(TestHstsPolicyWorks
): Dump events in case of test failure, using the newdumpEventsInNamespace
helper.test/e2e/util_test.go
(dumpEventsInNamespace
): New helper function to log all events in a namespace.TestHstsPolicyWorks
: Wait for namespace to be provisionedWhen creating a new namespace for the
TestHstsPolicyWorks
test, wait for the "default" ServiceAccount and the "system:image-pullers" RoleBinding to be provisioned in the newly created namespace before proceeding with the test. Make a similar change for theTestMTLSWithCRLsCerts
test.Before this change,
TestHstsPolicyWorks
sometimes failed because it tried to create a pod before the ServiceAccount had been provisioned and granted access to pull images. As a result, the test would randomly fail with the following error:This change should prevent such failures.
Because
TestMTLSWithCRLsCerts
also creates a namespace and then creates pods in this namespace, this PR makes the same change to this test as well. Some other tests create namespaces but do not create pods in thosenamespaces; those tests do not necessarily need to wait for the ServiceAccount and RoleBinding.
Inspired by openshift/origin@877c652.
test/e2e/client_tls_test.go
(TestMTLSWithCRLs
):test/e2e/hsts_policy_test.go
(TestHstsPolicyWorks
): Use the newcreateNamespace
helper.test/e2e/util_test.go
(createNamespace
): New helper function. Create a namespace with the specified name, register a cleanup handler to delete the namespace when the test finishes, wait for the "default" ServiceAccount and "system:image-pullers" RoleBinding to be created, and return the namespace.