Skip to content
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

test: Misc improvements #16064

Merged
merged 6 commits into from May 11, 2021
Merged

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented May 10, 2021

This pull request includes a number of miscellaneous improvements to the end-to-end tests. The last three commits parallelize different tests to spend less time waiting for network requests to timeout.

See commits for details.

@pchaigno pchaigno added area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/ci This PR makes changes to the CI. labels May 10, 2021
@pchaigno pchaigno force-pushed the misc-test-improvements branch 3 times, most recently from 7489a85 to 8766ec8 Compare May 10, 2021 14:06
@pchaigno pchaigno marked this pull request as ready for review May 11, 2021 07:49
@pchaigno pchaigno requested a review from a team May 11, 2021 07:49
@pchaigno pchaigno requested review from a team as code owners May 11, 2021 07:49
@pchaigno pchaigno requested review from a team, kkourt and tklauser May 11, 2021 07:49
Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 🎸 🍕

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from one non-blocking nit.

test/k8sT/DatapathConfiguration.go Outdated Show resolved Hide resolved
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Paul Chaignon <paul@cilium.io>
We use a number of WaitXXX helper function to wait for pods to be
deployed, policies to be enforced, service endpoints to be created, etc.
The default frequency at which these functions check the expected output
is 5s. So for namespace deletions and policy enforcements, we often wait
10s, because the output is not as expected after the first 5s check.

This is unnecessary. We can instead check the output every 1s and shave
off a few seconds every time we wait for something to happen (and we do
that a lot!).

Signed-off-by: Paul Chaignon <paul@cilium.io>
By parallelizing the verification of test cases, we save 5min.

Signed-off-by: Paul Chaignon <paul@cilium.io>
We may spend a lot of time just waiting in these tests because the
requests are sometimes expected to fail (in which case we wait for the
timeout).

Signed-off-by: Paul Chaignon <paul@cilium.io>
For each of the host firewall test cases, we check both an allowed and a
blocked request. We therefore spend a fair amount of time waiting for
the timeout to occur on blocked requests. We can parallelize test cases
to waste less time.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno
Copy link
Member Author

test-me-please

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 11, 2021
@jrajahalme jrajahalme merged commit 9e141aa into cilium:master May 11, 2021
@pchaigno pchaigno deleted the misc-test-improvements branch May 11, 2021 21:57
@pchaigno
Copy link
Member Author

Marking for backports to v1.9 and v1.10 since the impact on CI runtime is non-negligible (~30min saved per PR):

image

@vadorovsky
Copy link
Member

Needs also #15398 to be backported to 1.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants