Skip to content

Conversation

@frobware
Copy link
Contributor

@frobware frobware commented May 6, 2022

This PR changes the entry point for the e2e test to the new function TestAll.

The new test/e2e/TestAll function groups tests that can be run in parallel and those that must run in series via two overarching sub-tests. The new TestAll function guarantees consistent ordering of all e2e tests; serial tests will not start until the parallel tests run to completion. Parallel tests are those that do not mutate the ingressconfig object, the default ingress controller, the infrastructure object, default certificates, et al. Those tests that stand up a private ingress controller, for example, are deemed to be self-contained and are grouped into the parallel set. Each test that can run in parallel must call t.Parallel().

  • All tests can be run via make test-e2e.
  • The set of all serial tests can be run with make test-e2e TEST=TestAll/serial
  • The set of all parallel tests can be run with make test-e2e TEST=TestAll/parallel
  • And individual tests (e.g., during development) can be run with make test-e2e TEST=TestHstsPolicyWorks

I have been soak testing both the serial and parallel tests to use go test -count=20. The intent is to flush out cases where, in particular, the serial tests continue to flake. Typically any error has show up to be a conflict error on update. For those cases I have identified I have updated the tests to retry on conflict errors.

Running some of the e2e tests in parallel reduces the elapsed time for the e2e tests from ~55 minutes to ~10 mins. The reduction in time for the set of identified parallel tests is ~28 mins => ~5 mins.

There is a new step to the make verify target. If a new test is added it must be added to the set of parallel or serial tests defined in ./test/e2e/all_test.go. Running make verify will identify any tests (via the ./hack/verify-e2e-test-all-presence.sh script) that are not listed in all_test.go. For example:

$ make verify
hack/verify-gofmt.sh
hack/verify-generated-crd.sh
hack/verify-profile-manifests.sh
hack/verify-generated-bindata.sh
hack/verify-deps.sh
hack/verify-e2e-test-all-presence.sh
error: test function 'TestTunableMaxConnectionsValidValues' not called by 'TestAll' in /home/aim/src/github.com/openshift/cluster-ingress-operator/hack/../test/e2e/all_test.go
make: *** [Makefile:70: verify] Error 1

[Possible] Future follow-ups: The set of e2e tests can by dynamically discovered (in a similar way to go test -list and we can use Go's AST packages to parse those _test.go files and dynamically build the set of parallel and serial tests by identifying which test functions make a call to t.Parallel(). It's not possible to call MethodByName using reflection on top-level functions in packages: #756 (comment)

There are currently some tests (e.g., hard stop after) that could also run in parallel. Such tests mutate the default ingresscontroller but they could equally use a private ingresscontroller. Rather than increase the review footprint of this PR I plan to address those at a later date. Right now there is already a substantial win in overall e2e execution time.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 6, 2022
@openshift-ci openshift-ci bot requested review from candita and gcs278 May 6, 2022 10:21
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 6, 2022
@frobware frobware force-pushed the run-tests-in-parallel branch from d3847db to 15f576b Compare May 6, 2022 10:22
@frobware
Copy link
Contributor Author

frobware commented May 6, 2022

/test e2e-gcp-operator

@frobware
Copy link
Contributor Author

frobware commented May 6, 2022

/test e2e-azure-operator

@frobware
Copy link
Contributor Author

frobware commented May 6, 2022

/test e2e-azure-operator

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-ingress-operator/756/pull-ci-openshift-cluster-ingress-operator-master-e2e-azure-operator/1522526997438271488/artifacts/e2e-azure-operator/test/build-log.txt

--- PASS: TestAll (267.44s)
    --- PASS: TestAll/parallel (0.00s)
        --- PASS: TestAll/parallel/TestUniqueDomainRejection (1.33s)
        --- PASS: TestAll/parallel/TestContainerLogging (39.34s)
        --- PASS: TestAll/parallel/TestTunableRouterKubeletProbesForCustomIngressController (39.55s)
        --- PASS: TestAll/parallel/TestUserDefinedIngressController (41.29s)
        --- PASS: TestAll/parallel/TestScopeChange (53.98s)
        --- SKIP: TestAll/parallel/TestNetworkLoadBalancer (0.00s)
        --- PASS: TestAll/parallel/TestUniqueIdHeader (60.53s)
        --- PASS: TestAll/parallel/TestHeaderNameCaseAdjustment (61.01s)
        --- PASS: TestAll/parallel/TestHostNetworkEndpointPublishingStrategy (2.30s)
        --- PASS: TestAll/parallel/TestReloadIntervalUnsupportedConfigOverride (38.60s)
        --- PASS: TestAll/parallel/TestProxyProtocolAPI (39.59s)
        --- PASS: TestAll/parallel/TestNodePortServiceEndpointPublishingStrategy (41.45s)
        --- PASS: TestAll/parallel/TestHealthCheckIntervalIngressController (38.82s)
        --- PASS: TestAll/parallel/TestCustomIngressClass (37.40s)
        --- PASS: TestAll/parallel/TestRouterCompressionParsing (124.90s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyNever (49.24s)
        --- PASS: TestAll/parallel/TestIngressControllerScale (74.59s)
        --- PASS: TestAll/parallel/TestHostNetworkPortBinding (68.71s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyIfNone (56.47s)
        --- PASS: TestAll/parallel/TestHTTPHeaderCapture (39.69s)
        --- PASS: TestAll/parallel/TestRouteAdmissionPolicy (146.05s)
        --- PASS: TestAll/parallel/TestDynamicConfigManagerUnsupportedConfigOverride (38.59s)
        --- SKIP: TestAll/parallel/TestInternalLoadBalancerGlobalAccessGCP (0.00s)
        --- PASS: TestAll/parallel/TestHAProxyTimeouts (37.65s)
        --- PASS: TestAll/parallel/TestCreateSecretThenIngressController (2.51s)
        --- PASS: TestAll/parallel/TestHTTPCookieCapture (41.84s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyAppend (54.01s)
        --- PASS: TestAll/parallel/TestCreateIngressControllerThenSecret (2.54s)
        --- PASS: TestAll/parallel/TestHAProxyTimeoutsRejection (38.54s)
        --- PASS: TestAll/parallel/TestLoadBalancingAlgorithmUnsupportedConfigOverride (38.69s)
        --- PASS: TestAll/parallel/TestMaxConnectionsUnsupportedConfigOverride (38.79s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyReplace (49.23s)
        --- PASS: TestAll/parallel/TestCustomErrorpages (45.83s)
        --- PASS: TestAll/parallel/TestHTTPHeaderBufferSize (133.94s)
        --- PASS: TestAll/parallel/TestIngressControllerServiceNameCollision (96.89s)
    --- SKIP: TestAll/serial (0.00s)
PASS
ok  	github.com/openshift/cluster-ingress-operator/test/e2e	270.292s

@frobware
Copy link
Contributor Author

frobware commented May 6, 2022

e2e-aws-operator:

=== RUN   TestAll/serial
    ci_test.go:65: 
--- PASS: TestAll (236.07s)
    --- PASS: TestAll/parallel (0.00s)
        --- PASS: TestAll/parallel/TestUniqueDomainRejection (1.17s)
        --- PASS: TestAll/parallel/TestContainerLogging (36.16s)
        --- PASS: TestAll/parallel/TestUserDefinedIngressController (39.17s)
        --- PASS: TestAll/parallel/TestTunableRouterKubeletProbesForCustomIngressController (43.27s)
        --- PASS: TestAll/parallel/TestUniqueIdHeader (57.79s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyReplace (60.64s)
        --- PASS: TestAll/parallel/TestScopeChange (66.51s)
        --- PASS: TestAll/parallel/TestHostNetworkEndpointPublishingStrategy (2.14s)
        --- PASS: TestAll/parallel/TestHostNetworkPortBinding (71.38s)
        --- PASS: TestAll/parallel/TestReloadIntervalUnsupportedConfigOverride (39.29s)
        --- PASS: TestAll/parallel/TestProxyProtocolAPI (41.29s)
        --- PASS: TestAll/parallel/TestNodePortServiceEndpointPublishingStrategy (38.22s)
        --- PASS: TestAll/parallel/TestHTTPHeaderCapture (40.43s)
        --- PASS: TestAll/parallel/TestHealthCheckIntervalIngressController (39.43s)
        --- PASS: TestAll/parallel/TestNetworkLoadBalancer (38.18s)
        --- SKIP: TestAll/parallel/TestInternalLoadBalancerGlobalAccessGCP (0.00s)
        --- PASS: TestAll/parallel/TestMaxConnectionsUnsupportedConfigOverride (38.39s)
        --- PASS: TestAll/parallel/TestRouterCompressionParsing (123.93s)
        --- PASS: TestAll/parallel/TestHeaderNameCaseAdjustment (47.50s)
        --- PASS: TestAll/parallel/TestHTTPCookieCapture (41.37s)
        --- PASS: TestAll/parallel/TestLoadBalancingAlgorithmUnsupportedConfigOverride (38.32s)
        --- PASS: TestAll/parallel/TestRouteAdmissionPolicy (119.02s)
        --- PASS: TestAll/parallel/TestHAProxyTimeoutsRejection (37.54s)
        --- PASS: TestAll/parallel/TestHAProxyTimeouts (38.47s)
        --- PASS: TestAll/parallel/TestCreateSecretThenIngressController (2.21s)
        --- PASS: TestAll/parallel/TestDynamicConfigManagerUnsupportedConfigOverride (38.29s)
        --- PASS: TestAll/parallel/TestCreateIngressControllerThenSecret (2.26s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyNever (42.63s)
        --- PASS: TestAll/parallel/TestIngressControllerScale (76.05s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyIfNone (50.72s)
        --- PASS: TestAll/parallel/TestIngressControllerServiceNameCollision (96.48s)
        --- PASS: TestAll/parallel/TestCustomErrorpages (43.35s)
        --- PASS: TestAll/parallel/TestCustomIngressClass (37.19s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyAppend (56.93s)
        --- PASS: TestAll/parallel/TestHTTPHeaderBufferSize (127.99s)
    --- SKIP: TestAll/serial (0.00s)
PASS
ok  	github.com/openshift/cluster-ingress-operator/test/e2e	238.832s

@frobware
Copy link
Contributor Author

frobware commented May 6, 2022

/test e2e-gcp-operator

=== RUN   TestAll/serial
    ci_test.go:65: 
--- PASS: TestAll (314.06s)
    --- PASS: TestAll/parallel (0.00s)
        --- SKIP: TestAll/parallel/TestNetworkLoadBalancer (0.00s)
        --- PASS: TestAll/parallel/TestProxyProtocolAPI (41.59s)
        --- PASS: TestAll/parallel/TestLoadBalancingAlgorithmUnsupportedConfigOverride (42.66s)
        --- PASS: TestAll/parallel/TestNodePortServiceEndpointPublishingStrategy (44.49s)
        --- PASS: TestAll/parallel/TestMaxConnectionsUnsupportedConfigOverride (45.77s)
        --- PASS: TestAll/parallel/TestContainerLogging (47.32s)
        --- PASS: TestAll/parallel/TestReloadIntervalUnsupportedConfigOverride (49.59s)
        --- PASS: TestAll/parallel/TestHostNetworkEndpointPublishingStrategy (7.25s)
        --- PASS: TestAll/parallel/TestInternalLoadBalancerGlobalAccessGCP (60.51s)
        --- PASS: TestAll/parallel/TestHostNetworkPortBinding (82.75s)
        --- PASS: TestAll/parallel/TestHealthCheckIntervalIngressController (44.77s)
        --- PASS: TestAll/parallel/TestHTTPHeaderCapture (45.64s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyReplace (68.10s)
        --- PASS: TestAll/parallel/TestHeaderNameCaseAdjustment (63.86s)
        --- PASS: TestAll/parallel/TestIngressControllerScale (84.37s)
        --- PASS: TestAll/parallel/TestHTTPCookieCapture (46.64s)
        --- PASS: TestAll/parallel/TestHAProxyTimeoutsRejection (43.40s)
        --- PASS: TestAll/parallel/TestIngressControllerServiceNameCollision (101.82s)
        --- PASS: TestAll/parallel/TestHAProxyTimeouts (46.43s)
        --- PASS: TestAll/parallel/TestCreateSecretThenIngressController (2.41s)
        --- PASS: TestAll/parallel/TestDynamicConfigManagerUnsupportedConfigOverride (39.51s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyNever (45.02s)
        --- PASS: TestAll/parallel/TestCustomErrorpages (44.62s)
        --- PASS: TestAll/parallel/TestUniqueDomainRejection (1.27s)
        --- PASS: TestAll/parallel/TestCustomIngressClass (41.30s)
        --- PASS: TestAll/parallel/TestTunableRouterKubeletProbesForCustomIngressController (39.44s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyIfNone (60.29s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyAppend (60.57s)
        --- PASS: TestAll/parallel/TestCreateIngressControllerThenSecret (4.50s)
        --- PASS: TestAll/parallel/TestUserDefinedIngressController (40.26s)
        --- PASS: TestAll/parallel/TestHTTPHeaderBufferSize (135.53s)
        --- PASS: TestAll/parallel/TestUniqueIdHeader (54.20s)
        --- PASS: TestAll/parallel/TestScopeChange (54.79s)
        --- PASS: TestAll/parallel/TestRouterCompressionParsing (125.75s)
        --- PASS: TestAll/parallel/TestRouteAdmissionPolicy (126.75s)
    --- SKIP: TestAll/serial (0.00s)
PASS
ok  	github.com/openshift/cluster-ingress-operator/test/e2e	316.900s

@frobware
Copy link
Contributor Author

frobware commented May 6, 2022

/test e2e-gcp-operator
/test e2e-azure-operator

@frobware
Copy link
Contributor Author

frobware commented May 9, 2022

/test e2e-aws-operator
/test e2e-azure-operator
/test e2e-gcp-operator

@frobware frobware changed the title [WIP] Group all e2e tests as either parallel or serial Bug 2080379: Group all e2e tests as parallel or serial May 9, 2022
@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels May 9, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 9, 2022

@frobware: This pull request references Bugzilla bug 2080379, 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
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @quarterpin

Details

In response to this:

Bug 2080379: Group all e2e tests as parallel or serial

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-ci openshift-ci bot requested a review from quarterpin May 9, 2022 11:34
@frobware
Copy link
Contributor Author

frobware commented May 9, 2022

/test e2e-aws-operator

--- FAIL: TestAll (218.13s)
    --- FAIL: TestAll/parallel (0.00s)
        --- PASS: TestAll/parallel/TestUniqueDomainRejection (1.19s)
        --- PASS: TestAll/parallel/TestContainerLogging (39.19s)
        --- PASS: TestAll/parallel/TestNodePortServiceEndpointPublishingStrategy (39.25s)
        --- PASS: TestAll/parallel/TestUserDefinedIngressController (40.19s)
        --- PASS: TestAll/parallel/TestTunableRouterKubeletProbesForCustomIngressController (40.30s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyIfNone (51.84s)
        --- FAIL: TestAll/parallel/TestHTTPHeaderBufferSize (53.00s)
        --- PASS: TestAll/parallel/TestUniqueIdHeader (58.78s)
        --- PASS: TestAll/parallel/TestScopeChange (64.54s)
        --- PASS: TestAll/parallel/TestReloadIntervalUnsupportedConfigOverride (39.28s)
        --- SKIP: TestAll/parallel/TestInternalLoadBalancerGlobalAccessGCP (0.00s)
        --- PASS: TestAll/parallel/TestProxyProtocolAPI (39.31s)
        --- PASS: TestAll/parallel/TestNetworkLoadBalancer (37.17s)
        --- PASS: TestAll/parallel/TestHostNetworkEndpointPublishingStrategy (2.14s)
        --- PASS: TestAll/parallel/TestMaxConnectionsUnsupportedConfigOverride (38.38s)
        --- PASS: TestAll/parallel/TestLoadBalancingAlgorithmUnsupportedConfigOverride (37.32s)
        --- PASS: TestAll/parallel/TestHealthCheckIntervalIngressController (39.42s)
        --- PASS: TestAll/parallel/TestIngressControllerScale (76.05s)
        --- PASS: TestAll/parallel/TestHAProxyTimeouts (38.38s)
        --- PASS: TestAll/parallel/TestHTTPCookieCapture (42.40s)
        --- PASS: TestAll/parallel/TestHAProxyTimeoutsRejection (37.94s)
        --- PASS: TestAll/parallel/TestRouteAdmissionPolicy (118.06s)
        --- PASS: TestAll/parallel/TestRouterCompressionParsing (123.07s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyReplace (47.62s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyNever (44.64s)
        --- PASS: TestAll/parallel/TestHeaderNameCaseAdjustment (42.51s)
        --- PASS: TestAll/parallel/TestCreateSecretThenIngressController (2.23s)
        --- PASS: TestAll/parallel/TestCreateIngressControllerThenSecret (3.28s)
        --- PASS: TestAll/parallel/TestCustomIngressClass (38.21s)
        --- PASS: TestAll/parallel/TestIngressControllerServiceNameCollision (99.46s)
        --- PASS: TestAll/parallel/TestDynamicConfigManagerUnsupportedConfigOverride (40.28s)
        --- PASS: TestAll/parallel/TestHostNetworkPortBinding (68.35s)
        --- PASS: TestAll/parallel/TestCustomErrorpages (44.37s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyAppend (54.01s)
        --- PASS: TestAll/parallel/TestHTTPHeaderCapture (43.36s)
FAIL
FAIL	github.com/openshift/cluster-ingress-operator/test/e2e	1467.456s
FAIL

@frobware
Copy link
Contributor Author

frobware commented May 9, 2022

10 runs: Success on azure

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-ingress-operator/756/pull-ci-openshift-cluster-ingress-operator-master-e2e-azure-operator/1523619667317362688/artifacts/e2e-azure-operator/test/build-log.txt

--- PASS: TestAll (255.77s)
    --- PASS: TestAll/parallel (0.00s)
        --- PASS: TestAll/parallel/TestHostNetworkEndpointPublishingStrategy (6.42s)
        --- PASS: TestAll/parallel/TestContainerLogging (37.37s)
        --- PASS: TestAll/parallel/TestUserDefinedIngressController (41.37s)
        --- PASS: TestAll/parallel/TestHealthCheckIntervalIngressController (42.93s)
        --- PASS: TestAll/parallel/TestHAProxyTimeouts (43.73s)
        --- PASS: TestAll/parallel/TestHeaderNameCaseAdjustment (45.98s)
        --- PASS: TestAll/parallel/TestHostNetworkPortBinding (73.76s)
        --- PASS: TestAll/parallel/TestHTTPHeaderCapture (39.71s)
        --- PASS: TestAll/parallel/TestIngressControllerScale (80.63s)
        --- PASS: TestAll/parallel/TestHAProxyTimeoutsRejection (40.61s)
        --- PASS: TestAll/parallel/TestHTTPCookieCapture (43.73s)
        --- PASS: TestAll/parallel/TestDynamicConfigManagerUnsupportedConfigOverride (36.53s)
        --- PASS: TestAll/parallel/TestCreateSecretThenIngressController (2.51s)
        --- PASS: TestAll/parallel/TestIngressControllerServiceNameCollision (96.93s)
        --- PASS: TestAll/parallel/TestCreateIngressControllerThenSecret (1.47s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyReplace (47.22s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyNever (45.18s)
        --- PASS: TestAll/parallel/TestUniqueDomainRejection (0.28s)
        --- PASS: TestAll/parallel/TestCustomIngressClass (36.34s)
        --- PASS: TestAll/parallel/TestCustomErrorpages (44.77s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyIfNone (54.45s)
        --- PASS: TestAll/parallel/TestReloadIntervalUnsupportedConfigOverride (38.61s)
        --- SKIP: TestAll/parallel/TestNetworkLoadBalancer (0.00s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyAppend (58.01s)
        --- PASS: TestAll/parallel/TestTunableRouterKubeletProbesForCustomIngressController (37.50s)
        --- PASS: TestAll/parallel/TestUniqueIdHeader (50.62s)
        --- PASS: TestAll/parallel/TestHTTPHeaderBufferSize (131.86s)
        --- SKIP: TestAll/parallel/TestInternalLoadBalancerGlobalAccessGCP (0.00s)
        --- PASS: TestAll/parallel/TestProxyProtocolAPI (37.62s)
        --- PASS: TestAll/parallel/TestNodePortServiceEndpointPublishingStrategy (37.44s)
        --- PASS: TestAll/parallel/TestScopeChange (55.07s)
        --- PASS: TestAll/parallel/TestLoadBalancingAlgorithmUnsupportedConfigOverride (36.65s)
        --- PASS: TestAll/parallel/TestMaxConnectionsUnsupportedConfigOverride (41.82s)
        --- PASS: TestAll/parallel/TestRouteAdmissionPolicy (118.00s)
        --- PASS: TestAll/parallel/TestRouterCompressionParsing (121.82s)
    --- SKIP: TestAll/serial (0.00s)
PASS
ok  	github.com/openshift/cluster-ingress-operator/test/e2e	2524.846s

@frobware
Copy link
Contributor Author

frobware commented May 9, 2022

10 runs: success on gcp

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-ingress-operator/756/pull-ci-openshift-cluster-ingress-operator-master-e2e-gcp-operator/1523619667346722816/artifacts/e2e-gcp-operator/test/build-log.txt

--- PASS: TestAll (280.44s)
    --- PASS: TestAll/parallel (0.00s)
        --- PASS: TestAll/parallel/TestUniqueDomainRejection (1.31s)
        --- PASS: TestAll/parallel/TestContainerLogging (38.29s)
        --- PASS: TestAll/parallel/TestTunableRouterKubeletProbesForCustomIngressController (42.46s)
        --- PASS: TestAll/parallel/TestUserDefinedIngressController (43.27s)
        --- PASS: TestAll/parallel/TestHealthCheckIntervalIngressController (50.79s)
        --- PASS: TestAll/parallel/TestHeaderNameCaseAdjustment (52.96s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyNever (52.08s)
        --- PASS: TestAll/parallel/TestUniqueIdHeader (55.49s)
        --- PASS: TestAll/parallel/TestHTTPHeaderCapture (42.61s)
        --- PASS: TestAll/parallel/TestHTTPCookieCapture (42.62s)
        --- PASS: TestAll/parallel/TestHAProxyTimeoutsRejection (39.38s)
        --- PASS: TestAll/parallel/TestHAProxyTimeouts (39.12s)
        --- PASS: TestAll/parallel/TestMaxConnectionsUnsupportedConfigOverride (40.64s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyReplace (51.04s)
        --- SKIP: TestAll/parallel/TestNetworkLoadBalancer (0.00s)
        --- PASS: TestAll/parallel/TestReloadIntervalUnsupportedConfigOverride (38.50s)
        --- PASS: TestAll/parallel/TestProxyProtocolAPI (40.46s)
        --- PASS: TestAll/parallel/TestNodePortServiceEndpointPublishingStrategy (39.38s)
        --- PASS: TestAll/parallel/TestScopeChange (154.84s)
        --- PASS: TestAll/parallel/TestLoadBalancingAlgorithmUnsupportedConfigOverride (39.54s)
        --- PASS: TestAll/parallel/TestHTTPHeaderBufferSize (130.52s)
        --- PASS: TestAll/parallel/TestCustomIngressClass (40.32s)
        --- PASS: TestAll/parallel/TestInternalLoadBalancerGlobalAccessGCP (70.42s)
        --- PASS: TestAll/parallel/TestIngressControllerServiceNameCollision (98.73s)
        --- PASS: TestAll/parallel/TestRouterCompressionParsing (123.81s)
        --- PASS: TestAll/parallel/TestHostNetworkEndpointPublishingStrategy (3.32s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyIfNone (52.54s)
        --- PASS: TestAll/parallel/TestCreateSecretThenIngressController (3.42s)
        --- PASS: TestAll/parallel/TestCreateIngressControllerThenSecret (2.42s)
        --- PASS: TestAll/parallel/TestDynamicConfigManagerUnsupportedConfigOverride (38.49s)
        --- PASS: TestAll/parallel/TestRouteAdmissionPolicy (128.81s)
        --- PASS: TestAll/parallel/TestForwardedHeaderPolicyAppend (57.67s)
        --- PASS: TestAll/parallel/TestHostNetworkPortBinding (71.62s)
        --- PASS: TestAll/parallel/TestCustomErrorpages (45.76s)
        --- PASS: TestAll/parallel/TestIngressControllerScale (77.48s)
    --- SKIP: TestAll/serial (0.00s)
PASS
ok  	github.com/openshift/cluster-ingress-operator/test/e2e	2848.399s

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 10, 2022
@frobware
Copy link
Contributor Author

/hold

still experimenting.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 10, 2022
@frobware frobware force-pushed the run-tests-in-parallel branch from 16d0e7c to 09066bf Compare May 10, 2022 11:07
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 10, 2022
@frobware
Copy link
Contributor Author

/test e2e-azure-operator
/test e2e-gcp-operator

frobware and others added 5 commits May 27, 2022 13:49
e2e tests that can run in parallel must create resource names with
unique names.
Explicitly assert tests TestHTTPHeaderCapture and
TestHTTPCookieCapture only find 1 pod.

These two tests were flaking in CI because they sometimes coincided
with tests that modified the ingress config object. When that object
is modified a rolling deployment of all ingresscontroller pods can
take place and these two tests fell foul of the expectation that only
one pod was running. The CI logs show that "the pod does not
exist"--and that's because the pod was in terminating state post the
rolling deployment.
Some tests, notably in test teardown (i.e., defer blocks) fail hard
because the object that is being updated has been changed since it was
last refreshed.

We use the new update*SpecWithRetryOnConflict() functions that allow
finer grained updates on the Spec itself and, if the update fails with
a conflict error, the sequence of get, mutate, update will be retried.
This tests uses an unsupported annotation.
Previously we ran all tests via the wildcard: TEST='.*'.

The new TestALL e2e function groups tests that can be run in parallel
and those that must run in series via two overarching subtests.

The new TestALL function provides for consistent ordering of all e2e
tests; serial tests will not start until the parallel tests complete.
And those tests that can run in parallel can, and should, run in any
order because, by definition, they are deemed to be self-contained.

But some tests which are run in parallel are sensitive to any other
test that modifies the ingress config object; changes to that object
will cause a rolling update of all ingresscontroller's and some of the
parallel tests do not accommodate such an event. Any test that does
modify the ingress config object are always grouped into the serial
subtests.
@frobware frobware force-pushed the run-tests-in-parallel branch from da48409 to a449e49 Compare May 27, 2022 12:49
@gcs278
Copy link
Contributor

gcs278 commented May 27, 2022

Looks good to me. @Miciah want to apply the final LGTM?

@frobware
Copy link
Contributor Author

/retest

@frobware
Copy link
Contributor Author

/skip

@Miciah
Copy link
Contributor

Miciah commented May 31, 2022

Thanks! I don't see any major issues, and this is an excellent performance improvement.
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 31, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 31, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: frobware, gcs278, Miciah, rfredette

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [Miciah,frobware,gcs278,rfredette]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 2 against base HEAD 09afd2a and 8 for PR HEAD a449e49 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 1 against base HEAD 09afd2a and 7 for PR HEAD a449e49 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 09afd2a and 6 for PR HEAD a449e49 in total

@frobware
Copy link
Contributor Author

frobware commented Jun 1, 2022

/skip
/retest

@frobware
Copy link
Contributor Author

frobware commented Jun 1, 2022

level=info msg=Waiting up to 20m0s (until 8:36AM) for the Kubernetes API at https://api.ci-op-78vv9b67-43abb.origin-ci-int-aws.dev.rhcloud.com:6443...
level=info msg=API v1.24.0-alpha.3.3617+beaaed625dfb47-dirty up
level=info msg=Waiting up to 30m0s (until 8:49AM) for bootstrapping to complete...
level=info msg=Pulling VM console logs
level=info msg=Pulling debug logs from the bootstrap machine
level=error msg=Cluster operator authentication Degraded is True with APIServerDeployment_PreconditionNotFulfilled::IngressStateEndpoints_MissingSubsets::OAuthAPIServerConfigObservation_Error::OAuthServerServiceEndpointAccessibleController_SyncError::OAuthServerServiceEndpointsEndpointAccessibleController_SyncError: APIServerDeploymentDegraded: waiting for observed configuration to have mandatory apiServerArguments.etcd-servers
level=error msg=APIServerDeploymentDegraded: 
level=error msg=IngressStateEndpointsDegraded: No subsets found for the endpoints of oauth-server
level=error msg=OAuthAPIServerConfigObservationDegraded: configmaps openshift-etcd/etcd-endpoints: no etcd endpoint addresses found
level=error msg=OAuthServerServiceEndpointAccessibleControllerDegraded: Get "https://172.30.198.6:443/healthz": dial tcp 172.30.198.6:443: connect: connection refused
level=error msg=OAuthServerServiceEndpointsEndpointAccessibleControllerDegraded: oauth service endpoints are not ready

/retest

@Miciah
Copy link
Contributor

Miciah commented Jun 1, 2022

It occurs to me that we should make sure with a large change to the E2E tests like this that CI still passes on other platforms.
/test e2e-azure-operator
/test e2e-gcp-operator

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 1, 2022

@frobware: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-single-node a449e49 link false /test e2e-aws-single-node

Full PR test history. Your PR dashboard.

Details

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.

@frobware
Copy link
Contributor Author

frobware commented Jun 1, 2022

one or more errors occurred while gathering container data for pod prometheus-k8s-1:

/test e2e-aws-operator

@frobware
Copy link
Contributor Author

frobware commented Jun 1, 2022

It occurs to me that we should make sure with a large change to the E2E tests like this > that CI still passes on other platforms.
/test e2e-azure-operator
/test e2e-gcp-operator

If you look back through the PR comments I have been doing this all the time.

@frobware
Copy link
Contributor Author

frobware commented Jun 1, 2022

/refresh

@openshift-merge-robot openshift-merge-robot merged commit 0c1acbb into openshift:master Jun 1, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 1, 2022

@frobware: All pull requests linked via external trackers have merged:

Bugzilla bug 2080379 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2080379: Group all e2e tests as parallel or serial

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.

@frobware
Copy link
Contributor Author

frobware commented Apr 6, 2023

If you need to iterate and re-run the tests in an interactive iterative manner we have many tests that don't cleanup completely. When I developed this PR I used the following script[1] between successive runs.

Copied inline should the gist disappear.

#!/usr/bin/env bash

for i in $(oc get --no-headers ingresscontrollers.operator.openshift.io -n openshift-ingress-operator | grep -v -w default | awk '{print $1}'); do oc delete -n openshift-ingress-operator ingresscontroller/$i; done
for i in $(oc get --no-headers pods -n openshift-ingress | grep -v -w default | awk '{print $1}'); do oc delete --force -n openshift-ingress pod/$i; done
for i in $(oc get --no-headers services -n openshift-ingress | grep -v -w default | awk '{print $1}'); do oc delete --force -n openshift-ingress service/$i; done
for i in $(oc get --no-headers routes -n openshift-ingress | grep -v -w default | awk '{print $1}'); do oc delete --force -n openshift-ingress routes/$i; done
oc delete configmaps/custom-error-pages -n openshift-config
oc delete configmaps/ca-certificate -n openshift-config
oc delete configmaps/rsyslog-conf -n openshift-ingress
oc delete services/header-name-case-adjustment-echo  -n openshift-ingress
oc delete services/header-name-case-adjustment-echo  -n openshift-ingress
oc delete namespaces routeadmissionpolicytest1
oc delete namespaces routeadmissionpolicytest2

As there have been new additions to the test suite since I last ran this script it is likely incomplete (as of Thu 6 Apr 12:01:58 BST 2023).

[1] https://gist.github.com/frobware/5588673027a05e56eb613e965b9232f6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants