test: call wait_for_up in Microvm.start() #4798
Merged
Conversation
This file contains hidden or 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
Always block inside `Microvm.start()` until the vm has finished booting (if a network device is present in the microvm). This means that we no longer need `wait_for_up` calls after every `start()` call. Signed-off-by: Patrick Roy <[email protected]>
If we start Firecracker with a config file, then the microvm will boot in `.spawn()` instead of `.start()` (since we are not driving Firecracker via API requests). Thus, if this is the case, put the `wait_for_up` into `Microvm.spawn()`. Signed-off-by: Patrick Roy <[email protected]>
roypat
force-pushed
the
wait-for-up-by-default
branch
from
7fcf5f2 to
eab4731
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4798 +/- ##
=======================================
Coverage 84.32% 84.32%
=======================================
Files 249 249
Lines 27521 27521
=======================================
Hits 23206 23206
Misses 4315 4315
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
roypat
force-pushed
the
wait-for-up-by-default
branch
from
eab4731 to
f1f594c
Compare
After restoring a snapshot, we are not waiting for the VM to finish booting (it needs to have done that before we took a snapshot), but having a `wait_for_up` here is still a good sanity check for ensuring the VM is still responsive. So far our tests have been inconsistent on whether to put a `wait_for_up` here. This makes them consistent by always doing `wait_for_up` if a network device is available. Signed-off-by: Patrick Roy <[email protected]>
`wait_for_up`s were replaced with ``` grep -rl wait_for_up tests/integration_tests/ \ |xargs sed '/wait_for_up/d' -i ``` Signed-off-by: Patrick Roy <[email protected]>
`vm.start()` already asserts `vm.state == "Running"`, so no need to do it again at the callsite. Signed-off-by: Patrick Roy <[email protected]>
roypat
force-pushed
the
wait-for-up-by-default
branch
from
f1f594c to
4af1906
Compare
roypat
added
the
Status: Awaiting review
pb8o
approved these changes
Sep 13, 2024
bchalios
approved these changes
Sep 13, 2024
9 tasks
roypat
added a commit
to roypat/firecracker
that referenced
this pull request
Sep 16, 2024
To indicate that we're really waiting until sshd is up and accepting connections, as a proxy for the actual boot process. Suggested during code review in firecracker-microvm#4798. Signed-off-by: Patrick Roy <[email protected]>
roypat
added a commit
that referenced
this pull request
Sep 16, 2024
To indicate that we're really waiting until sshd is up and accepting connections, as a proxy for the actual boot process. Suggested during code review in #4798. Signed-off-by: Patrick Roy <[email protected]>
RiverPhillips
pushed a commit
to RiverPhillips/firecracker
that referenced
this pull request
Sep 20, 2024
To indicate that we're really waiting until sshd is up and accepting connections, as a proxy for the actual boot process. Suggested during code review in firecracker-microvm#4798. Signed-off-by: Patrick Roy <[email protected]> Signed-off-by: River Phillips <[email protected]>
5 tasks
ShadowCurse
pushed a commit
to ShadowCurse/firecracker
that referenced
this pull request
Nov 5, 2024
To indicate that we're really waiting until sshd is up and accepting connections, as a proxy for the actual boot process. Suggested during code review in firecracker-microvm#4798. Signed-off-by: Patrick Roy <[email protected]>
ShadowCurse
pushed a commit
to ShadowCurse/firecracker
that referenced
this pull request
Nov 5, 2024
To indicate that we're really waiting until sshd is up and accepting connections, as a proxy for the actual boot process. Suggested during code review in firecracker-microvm#4798. Signed-off-by: Patrick Roy <[email protected]>
ShadowCurse
pushed a commit
to ShadowCurse/firecracker
that referenced
this pull request
Nov 5, 2024
To indicate that we're really waiting until sshd is up and accepting connections, as a proxy for the actual boot process. Suggested during code review in firecracker-microvm#4798. Signed-off-by: Patrick Roy <[email protected]>
ShadowCurse
pushed a commit
to ShadowCurse/firecracker
that referenced
this pull request
Nov 5, 2024
To indicate that we're really waiting until sshd is up and accepting connections, as a proxy for the actual boot process. Suggested during code review in firecracker-microvm#4798. Signed-off-by: Patrick Roy <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Always block inside
Microvm.start()until the vm has finished booting(if a network device is present in the microvm). This means that we no
longer need
wait_for_upcalls after everystart()call.Existing
wait_for_ups were replaced withLicense 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
PR.
CHANGELOG.md.TODOs link to an issue.contribution quality standards.
rust-vmm.