Skip to content

expose instance_migrate_start in propolis-server OpenAPI document #957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 9, 2025
Merged

expose instance_migrate_start in propolis-server OpenAPI document #957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f75935
[spr] initial version
sunshowers Oct 9, 2025
912a610
update comment
sunshowers Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bin/propolis-server/src/lib/migrate/destination.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ pub(crate) async fn initiate(
info!(log, "negotiating migration as destination");

// Build upgrade request to the source instance
// (we do this by hand because it's hidden from the OpenAPI spec)
// (we do this by hand to avoid a dependency from propolis-server to
// propolis-client)
Comment on lines +83 to +84
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah, that's a good point even if this is in the OpenAPI document now. thanks for noting it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually is possible for propolis-server to depend on propolis-client now because of the separation provided by the API trait. But there's a wrinkle here: if propolis-server depended on propolis-client, our tooling (cargo xtask ls-apis in Omicron) would detect that as a circular dependency and as a result would suggest freezing the entire API -- which isn't correct for just this one method that has its own protocol negotiation already built-in.

There's probably a way to work around this within the cargo xtask ls-apis method, but this is an area that generally needs to be handled with care.

// TODO(#165): https (wss)
// TODO: We need to make sure the src_addr is a valid target
let src_migrate_url = format!(
Expand Down
27 changes: 22 additions & 5 deletions crates/propolis-server-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ pub trait PropolisServerApi {
websock: WebsocketConnection,
) -> WebsocketChannelResult;

// See the note on instance_migrate_start below. /instance/vnc is not
// currently used (as of 2025-10), but before it's used we'll want to think
// about versioning considerations for the WebSocket protocol, similar to
// instance_migrate_start.
#[channel {
protocol = WEBSOCKETS,
path = "/instance/vnc",
Expand All @@ -94,14 +98,27 @@ pub trait PropolisServerApi {
websock: WebsocketConnection,
) -> dropshot::WebsocketChannelResult;

// This endpoint is meant to only be called during a migration from the
// destination instance to the source instance as part of the HTTP connection
// upgrade used to establish the migration link. We don't actually want this
// exported via OpenAPI clients.
/// DO NOT USE THIS IF YOU'RE NOT PROPOLIS-SERVER.
///
/// Internal API called during a migration from a destination instance to
/// the source instance as part of the HTTP connection upgrade used to
/// establish the migration link. This API is exported via OpenAPI purely
/// to verify that its shape hasn't changed.
//
// # Versioning notes
//
// This API is expected to work even if the source and destination
// propolis-server instances are on different versions. There are two parts
// to versioning:
//
// 1. The parameters passed into the initial request.
// 2. The protocol used for WebSocket communication.
//
// Part 1 is verified by the Dropshot API manager. For part 2,
// propolis-server has internal support for protocol negotiation.
#[channel {
protocol = WEBSOCKETS,
path = "/instance/migrate/{migration_id}/start",
unpublished = true,
}]
async fn instance_migrate_start(
rqctx: RequestContext<Self::Context>,
Expand Down
29 changes: 29 additions & 0 deletions openapi/propolis-server.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,35 @@
}
}
},
"/instance/migrate/{migration_id}/start": {
"get": {
"summary": "DO NOT USE THIS IF YOU'RE NOT PROPOLIS-SERVER.",
"description": "Internal API called during a migration from a destination instance to the source instance as part of the HTTP connection upgrade used to establish the migration link. This API is exported via OpenAPI purely to verify that its shape hasn't changed.",
"operationId": "instance_migrate_start",
"parameters": [
{
"in": "path",
"name": "migration_id",
"required": true,
"schema": {
"type": "string",
"format": "uuid"
}
}
],
"responses": {
"default": {
"description": "",
"content": {
"*/*": {
"schema": {}
}
}
}
},
"x-dropshot-websocket": {}
}
},
"/instance/migration-status": {
"get": {
"operationId": "instance_migrate_status",
Expand Down
Loading