Tests: Refactor and clean up JailerContext class #5018
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- make the jailer base class generic - move `screen` bits to a special purpose "jailer" Abstract the idea of the jailer into a Runner. The idea is that we can create different runner modes for different cases. For example we in this commit we have the jailer and screen+jailer, but we could extend it to others, like not using the Firecracker jailer at all. Signed-off-by: Pablo Barbáchano <[email protected]>
Currently the dir structure looks like:
/srv/.../microvm-dir/chroot
and we tend to create files in microvm-dir and then hardlink them into
the chroot dir.
While we are simplifying it, also do away with chroot/path/fsfiles and
convert more strings to `Path`.
Signed-off-by: Pablo Barbáchano <[email protected]>
Now the function always tries to do the right thing. Signed-off-by: Pablo Barbáchano <[email protected]>
pb8o
added
Priority: Low
pb8o
added
the
Status: Awaiting author
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5018 +/- ##
=======================================
Coverage 83.13% 83.13%
=======================================
Files 245 245
Lines 26697 26697
=======================================
Hits 22194 22194
Misses 4503 4503
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
In the integration tests support classes, the Jailer and the Microvm are very coupled, and the logic is spread over the two classes with things like screen support directly in the Microvm class.
This change cleanly separates the Jailer(s) and extracts the screen handling into a new handler,
JailerScreen.There's some other simplifications of the Microvm class for example avoiding creating files in two different places.
Reason
Simplifying the support classes so they can be re-used in other contexts.
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.