Expose propolis' serial console channel in nexus#1766
Conversation
7516059 to
6a6d5a8
Compare
2d30ac3 to
8e11bff
Compare
luqmana
left a comment
There was a problem hiding this comment.
sweet, thanks lif! a couple small comments, but i'm inclined to land things so we can finish the handmade->generated client migration
| /// Connect to an instance's serial console | ||
| #[channel { | ||
| protocol = WEBSOCKETS, | ||
| path = "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/serial-console/stream", |
There was a problem hiding this comment.
Is the idea we'll have both the streaming and non-streaming endpoint in the limit? I assume the latter for the buffer playback behaviour?
There was a problem hiding this comment.
the streaming endpoint will eventually get params to start from an arbitrary byte offset (within the buffer) as well once i've moved the longer buffering into propolis-server, at which point the main reasons to keep the non-streaming one around would be
- wanting to separate out permission to read it vs. read/write
- wanting to provide an API for accessing it that doesn't require websocket support
| } | ||
| } | ||
| } | ||
| // pass along Close messages |
There was a problem hiding this comment.
Should we also try to gracefully send a close to the other side if one unexpectedly drops?
| use super::sled_agent::SledAgent; | ||
|
|
||
| use propolis_client::api::VolumeConstructionRequest; | ||
| use crucible_client_types::VolumeConstructionRequest; |
There was a problem hiding this comment.
Will the generated client not re-export types it uses?
There was a problem hiding this comment.
kind of a rough edge i think: progenitor re-declares a copy of them based on the openapi spec it's given (which is of course generated using the derive(JsonSchema) on the original crucible_client_types types). unless there's a fancy feature of progenitor i'm missing, this means we end up needing boilerplate to convert between the two
There was a problem hiding this comment.
ah right i know you added the From impls for the generated-migration feature but I wonder if its worth just having in general too
There was a problem hiding this comment.
i get the distinct feeling it might be, yeah.
- Includes updates to progenitor and reqwest 0.11.12 for client support for Dropshot websocket channels. - Literally only passes through the websocket, does not yet include the serial output buffering done by its parent GET endpoint. - Adds support for ensuring a websocket endpoint gets upgraded in nexus integration tests (i.e. test_unauthorized). - Begins migrating sled-agent's propolis-client usage to the new progenitor-generated version (oxidecomputer/propolis#206)
Crucible changes are: update to latest `vergen` (#1770) Update rand dependencies, and fallout from that. (#1764) [crucible-downstairs] migrate to API traits (#1768) [crucible-agent] migrate to API trait (#1766) [crucible-pantry] migrate to API trait (#1767) Add back job delays in the downstairs with the --lossy flag (#1761) Propolis changes are: Crucible update plus a few other dependency changes. (#948) [2/n] [propolis-server] switch to API trait (#946) [1/n] add a temporary indent to propolis server APIs (#945) Handle Intel CPUID leaves 4 and 18h, specialize CPUID for VM shape (#941) Increase viona receive queue length to 2048 (#935) Expand viona header pad to account for options (#937) fix linux p9fs multi message reads (#932) add a D script to report VMs' CPUID queries (#934) Update GH actions Re-enable viona packet data loaning
Crucible changes are: update to latest `vergen` (#1770) Update rand dependencies, and fallout from that. (#1764) [crucible-downstairs] migrate to API traits (#1768) [crucible-agent] migrate to API trait (#1766) [crucible-pantry] migrate to API trait (#1767) Add back job delays in the downstairs with the --lossy flag (#1761) Propolis changes are: Crucible update plus a few other dependency changes. (#948) [2/n] [propolis-server] switch to API trait (#946) [1/n] add a temporary indent to propolis server APIs (#945) Handle Intel CPUID leaves 4 and 18h, specialize CPUID for VM shape (#941) Increase viona receive queue length to 2048 (#935) Expand viona header pad to account for options (#937) fix linux p9fs multi message reads (#932) add a D script to report VMs' CPUID queries (#934) Update GH actions Re-enable viona packet data loaning --------- Co-authored-by: Alan Hanson <alan@oxide.computer> Co-authored-by: Sean Klein <sean@oxide.computer> Co-authored-by: Rain <rain@oxide.computer> Co-authored-by: John Gallagher <john@oxidecomputer.com> Co-authored-by: iliana etaoin <iliana@oxide.computer> Co-authored-by: Sean Klein <sean@oxidecomputer.com> Co-authored-by: Alex Plotnick <alex@oxidecomputer.com> Co-authored-by: David Pacheco <dap@oxidecomputer.com> Co-authored-by: Andrew J. Stone <andrew@oxidecomputer.com>
I'll update how-to-run.adoc after either oxidecomputer/oxide.rs#268 lands (or if we implement it in oxide-sdk-rust instead)