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

[e2e] Simplify pre-funded key usage #3011

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

marun
Copy link
Contributor

@marun marun commented May 10, 2024

Why this should be merged

As part of refactoring the e2e tests for reuse in antithesis and load testing, this change simplifies key usage so that tests can be supplied a specific pre-funded key rather than retrieving their own key at runtime. Fund usage is also minimized . It should be possible to run most tests repeatedly for extended periods of time without running out of keys or funds.

How this works

Previously, pre-funded keys were distributed via an http server to ensure a given key was used by at most a single test. This was intended to ensure compatibility with parallel test execution by guaranteeing that no pre-funded key would ever be used concurrently.

Since ginkgo uses process-based parallization, it's sufficient to ensure that each process is allocated a unique key. The tests assigned to run in a given test process are executed sequentially so a key won't be in danger of concurrent usage.

As part of this change, the virtuous test was updated to use the one funded key to fund a new set of keys. Previously 10 pre-funded keys were required.

How this was tested

CI, local parallel execution

@marun marun added the testing This primarily focuses on testing label May 10, 2024
@marun marun self-assigned this May 10, 2024
@marun marun requested review from abi87 and gyuho as code owners May 10, 2024 04:07
@marun marun force-pushed the e2e-simplify-key-usage branch 2 times, most recently from d0d2262 to de1e0c0 Compare May 10, 2024 04:13
Copy link
Contributor

@abi87 abi87 left a comment

Choose a reason for hiding this comment

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

lgtm

marun added 4 commits May 15, 2024 10:39
As part of refactoring the e2e tests for reuse in antithesis and load
testing, this change simplifies key usage so that tests can be
supplied a specific pre-funded key rather than retrieving their own
key at runtime. Fund usage is also minimized. It should be possible to
run most tests repeatedly for extended periods of time without running
out of keys or funds.

Previously, pre-funded keys were distributed via an http server to
ensure a given key was used by at most a single test. This was
intended to ensure compatibility with parallel test execution by
guaranteeing that no pre-funded key would ever be used concurrently.

Since ginkgo uses process-based parallization, it's sufficient to
ensure that each process is allocated a single unique key. The tests
assigned to run in a given test process are executed sequentially so a
key won't be in danger of concurrent usage.

As part of this change, the virtuous test was updated to use one
pre-funded key to fund a new set of keys. Previously 10 pre-funded
keys were required.
@marun
Copy link
Contributor Author

marun commented May 15, 2024

Updated to fix an unrelated latent bug that cause the e2e job to fail.

@marun marun marked this pull request as draft May 20, 2024 17:19
@marun
Copy link
Contributor Author

marun commented May 20, 2024

I need to fix the periodic e2e failures introduced by this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing This primarily focuses on testing
Projects
Status: In Review 👀
Development

Successfully merging this pull request may close these issues.

None yet

2 participants