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

Add end-to-end tests for local cache #292

Merged
merged 7 commits into from
Nov 18, 2024
Merged

Conversation

unexge
Copy link
Contributor

@unexge unexge commented Nov 13, 2024

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@muddyfish
Copy link
Contributor

Are the thoughts here that MP should work as a filesystem, or that we want to test it actually uses a cache?

checkWriteToPath(f, pod, first, testWriteSize, seed)
checkReadFromPath(f, pod, first, testWriteSize, seed)
e2evolume.VerifyExecInPodSucceed(f, pod, fmt.Sprintf("mkdir %s && cd %s && touch %s", dir, dir, second))
checkListingPath(f, pod, dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth verifying what the actual response is here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed with fb140b9

tests/e2e-kubernetes/testsuites/cache.go Show resolved Hide resolved
l.cleanup = append(l.cleanup, f)
}

cleanup := func(ctx context.Context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not blocking: Don't we have this setup in a load of other tests? At some point, maybe we should try and de-duplicate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's true. It's a bit tricky because we create and use a local variable in the function scope and reuse it with each test run via BeforeEach lifecycle events, see local struct in each test suite. Might need to change that pattern a bit to have something reusable.

tests/e2e-kubernetes/testsuites/cache.go Show resolved Hide resolved
Signed-off-by: Burak Varlı <burakvar@amazon.co.uk>
…eged

Signed-off-by: Burak Varlı <burakvar@amazon.co.uk>
Signed-off-by: Burak Varlı <burakvar@amazon.co.uk>
@unexge unexge force-pushed the unexge/e2e-tests-for-local-cache branch from 2f86183 to fb140b9 Compare November 15, 2024 11:08
@unexge
Copy link
Contributor Author

unexge commented Nov 15, 2024

Are the thoughts here that MP should work as a filesystem, or that we want to test it actually uses a cache?

Former - I wanted to have a smoke test mainly, I expect Mountpoint to have more extensive tests to ensure cache works as expected. Here just wanted to ensure the filesystem still works when cache is enabled with multi-container, non-root etc setups.

Signed-off-by: Burak Varlı <burakvar@amazon.co.uk>
testWriteSize := 1024 // 1KB

checkWriteToPath(f, pod, first, testWriteSize, seed)
checkReadFromPath(f, pod, first, testWriteSize, seed)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do multiple read checks for verifying it works a second time (cache works)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed with 7b15d44

Signed-off-by: Burak Varlı <burakvar@amazon.co.uk>
Signed-off-by: Burak Varlı <burakvar@amazon.co.uk>
Signed-off-by: Burak Varlı <burakvar@amazon.co.uk>
@unexge unexge requested a review from muddyfish November 15, 2024 16:19
"allow-delete",
"allow-other",
fmt.Sprintf("uid=%d", *e2epod.GetDefaultNonRootUser()),
fmt.Sprintf("gid=%d", defaultNonRootGroup),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Would these constants be useful as functions/moved to e2epod?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e2epod is a Kubernetes package, but I can move these constants to util.go in a follow-up PR.

@unexge unexge merged commit 5cf881c into main Nov 18, 2024
1 check passed
@unexge unexge deleted the unexge/e2e-tests-for-local-cache branch November 18, 2024 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants