PHD: add test coverage for instance_ensure API endpoint
#753
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
hawkw
approved these changes
Aug 28, 2024
Member
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
Overall, this looks good to me! I think there's an easier way out that doesn't require doing the whole type erasure dance, though.
hawkw
approved these changes
Aug 28, 2024
Comment on lines
337
to
346
| let send_fut: Pin< | ||
| Box< | ||
| dyn Future< | ||
| Output = Result< | ||
| ResponseValue<InstanceEnsureResponse>, | ||
| _, | ||
| >, | ||
| > + Send, | ||
| >, | ||
| > = match api { |
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.
PHD uses the propolis-server
instance_spec_ensureendpoint when creating VMs. This maximizes the framework's control over what virtual devices and backends get created and its knowledge of how devices should manifest to guests.When sled agent creates a VM, it uses the
instance_ensureendpoint, which takes anInstanceEnsureRequestthat specifies components at a slightly higher level of abstraction thaninstance_spec_ensure. The server handles calls to the former endpoint by shimming theInstanceEnsureRequestinto an instance spec and then pretending the caller calledinstance_spec_ensure. The shim is relatively straightforward, but because PHD doesn't use it (and as #750 shows) it can be a great place for bugs to hide...Although the medium-term plan is to try to switch sled agent over to using the spec endpoint (see RFD 505), for now add an affordance to PHD to allow it to use the
instance_ensureendpoint if a test requests it.Tested by running the new test against master before and after #751 merged and verifying that the test fails without that change and passes with it.