fix(microvm): name both causes when a command health check cannot reach the guest - #203
Merged
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
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.
The vsock device backing
commandhealth checks is attached at boot, and only when the deployment already declares such a check. Acommandcheck added to a running deployment therefore cannot reach its VM at all — neither hypervisor exposes a hot-plug path for vhost-vsock.The probe reported that failure as "cannot reach ring-agent in the guest — install it in the image", which is the wrong remedy for this cause and sends the operator debugging a perfectly good image.
The message now names both possible causes and, crucially, states the boot-time constraint: that a restart is what provisions the device. That is the actionable part, and no amount of guest-side debugging would reveal it.
On the detection. The host-side vsock socket exists only when the VM was booted with the device, so it looks like a reliable discriminator. It isn't: a crashed VMM can leave the socket behind until the reconciler reaps it, and a live VM can have its socket unlinked while keeping the device — both states the codebase already handles elsewhere. So the socket check is used only to order the two causes, putting the more likely one first, and never to assert one and hide the other. Both remedies survive either way.
Whether a VM ever restarts on its own depends on the check's
on_failure:restartheals itself once the failure threshold is reached, butalertandstopnever reboot it, so the check stays red until the operator restarts the deployment. Two code comments claimed the VM "eventually restarts" unconditionally; they now say what actually happens.Docs: documented the limitation on the Cloud Hypervisor page and in the runtime comparison, which covers Firecracker too.
Tests
783 unit tests pass (4 new),
cargo fmtandcargo clippy --all-targetsclean. The new tests pin the contract: both causes always named, both remedies always present, and the hint only reordering them. Positions are unwrapped before comparison so a vanished phrase fails the test rather than passing onOptionordering.The probe path itself needs a running VM and is only exercised by the e2e suites, which do not run in CI.