Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 35 additions & 12 deletions bin/propolis-server/src/lib/migrate/destination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use bitvec::prelude as bv;
use futures::{SinkExt, StreamExt};
use hyper::header::HeaderValue;
use propolis::common::{GuestAddr, Lifecycle, PAGE_SIZE};
use propolis::migrate::{
MigrateCtx, MigrateStateError, Migrator, PayloadOffer, PayloadOffers,
Expand All @@ -17,9 +18,11 @@ use std::convert::TryInto;
use std::io;
use std::net::SocketAddr;
use std::sync::Arc;
use tokio::net::TcpStream;
use tokio_tungstenite::tungstenite::client::IntoClientRequest;
use tokio_tungstenite::tungstenite::protocol::frame::coding::CloseCode;
use tokio_tungstenite::tungstenite::protocol::CloseFrame;
use tokio_tungstenite::{tungstenite, WebSocketStream};
use tokio_tungstenite::{tungstenite, MaybeTlsStream, WebSocketStream};
use uuid::Uuid;

use crate::migrate::codec;
Expand Down Expand Up @@ -80,17 +83,9 @@ pub(crate) async fn initiate(
info!(log, "negotiating migration as destination");

// Build upgrade request to the source instance
// (we do this by hand to avoid a dependency from propolis-server to
// propolis-client)
// TODO(#165): https (wss)
// TODO: We need to make sure the src_addr is a valid target
let src_migrate_url = format!(
"ws://{}/instance/migrate/{}/start",
migrate_info.src_addr, migration_id,
);
info!(log, "Begin migration"; "src_migrate_url" => &src_migrate_url);
let (mut conn, _) =
tokio_tungstenite::connect_async(src_migrate_url).await?;
let mut conn =
migration_start_connect(&log, migrate_info.src_addr, migration_id)
.await?;

// Generate a list of protocols that this target supports, then send them to
// the source and allow it to choose its favorite.
Expand Down Expand Up @@ -153,6 +148,34 @@ pub(crate) async fn initiate(
})
}

async fn migration_start_connect(
log: &slog::Logger,
src_addr: SocketAddr,
migration_id: Uuid,
) -> Result<WebSocketStream<MaybeTlsStream<TcpStream>>, MigrateError> {
// We do this by hand to avoid a dependency from propolis-server to
// propolis-client.
// TODO(#165): https (wss)
// TODO: We need to make sure the src_addr is a valid target
let src_migrate_url =
format!("ws://{}/instance/migrate/{}/start", src_addr, migration_id);
info!(log, "Begin migration"; "src_migrate_url" => &src_migrate_url);
let mut req = src_migrate_url.into_client_request()?;

// Add the api-version header. This assumes the instance_migrate_start API
// hasn't changed. See the note in crates/propolis-server-api/src/lib.rs.
req.headers_mut().insert(
omicron_common::api::VERSION_HEADER,
HeaderValue::from_str(
&propolis_server_api::VERSION_INITIAL.to_string(),
Copy link
Member

Choose a reason for hiding this comment

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

the tension here is that we don't want to always pick the latest version here, because that might be across a change to the HTTP API, but we probably don't want to always use VERSION_INITIAL here so that we can drop support for ancient propolis-server HTTP APIs over time, right?

does it make sense to suggest bumping this header if we've bumped the propolis-server API version without changing this endpoint? (am I off-base in thinking we'll probably want to keep code for the last like.. one or two API versions in-tree, but not beyond that?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does it make sense to suggest bumping this header if we've bumped the propolis-server API version without changing this endpoint?

Ah no, the issue is if a new propolis-server with a newer version hits an old propolis-server without that version, the request will get rejected. So we'll always have to use a version that overlaps with the newest as well as the oldest propolis-server currently deployed.

The exact process of deprecation and removal of old versions is a TBD, but we'll have to consider circumstances like this. Wouldn't worry about it too much for now.

Copy link
Member

Choose a reason for hiding this comment

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

oh, right, so we'd move this version forward as we deprecate the oldest versions of the API. or something along those lines.

)
.expect("VERSION_INITIAL is \"1.0.0\" which is a valid header value"),
);

let (conn, _) = tokio_tungstenite::connect_async(req).await?;
Ok(conn)
}

/// The runner for version 0 of the LM protocol, using RON encoding.
struct RonV0<T: MigrateConn> {
/// The ID for this migration.
Expand Down
1 change: 1 addition & 0 deletions crates/propolis-server-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ edition = "2024"
[dependencies]
crucible-client-types.workspace = true
dropshot.workspace = true
dropshot-api-manager-types.workspace = true
propolis_api_types.workspace = true
34 changes: 34 additions & 0 deletions crates/propolis-server-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use dropshot::{
HttpResponseUpdatedNoContent, Path, Query, RequestContext, TypedBody,
WebsocketChannelResult, WebsocketConnection,
};
use dropshot_api_manager_types::api_versions;
use propolis_api_types::{
InstanceEnsureRequest, InstanceEnsureResponse, InstanceGetResponse,
InstanceMigrateStartRequest, InstanceMigrateStatusResponse,
Expand All @@ -17,6 +18,33 @@ use propolis_api_types::{
VCRRequestPathParams, VolumeStatus, VolumeStatusPathParams,
};

api_versions!([
// WHEN CHANGING THE API (part 1 of 2):
//
// +- Pick a new semver and define it in the list below. The list MUST
// | remain sorted, which generally means that your version should go at
// | the very top.
// |
// | Duplicate this line, uncomment the *second* copy, update that copy for
// | your new API version, and leave the first copy commented out as an
// | example for the next person.
// v
// (next_int, IDENT),
(1, INITIAL),
]);

// WHEN CHANGING THE API (part 2 of 2):
//
// The call to `api_versions!` above defines constants of type
// `semver::Version` that you can use in your Dropshot API definition to specify
// the version when a particular endpoint was added or removed. For example, if
// you used:
//
// (2, ADD_FOOBAR)
//
// Then you could use `VERSION_ADD_FOOBAR` as the version in which endpoints
// were added or removed.

#[dropshot::api_description]
pub trait PropolisServerApi {
type Context;
Expand Down Expand Up @@ -116,6 +144,12 @@ pub trait PropolisServerApi {
//
// Part 1 is verified by the Dropshot API manager. For part 2,
// propolis-server has internal support for protocol negotiation.
//
// Note that we currently bypass Progenitor and always pass in
// VERSION_INITIAL. See `migration_start_connect` in
// propolis-server/src/lib/migrate/destination.rs for where we do it. If we
// introduce a change to this API, we'll have to carefully consider version
// skew between the source and destination servers.
#[channel {
protocol = WEBSOCKETS,
path = "/instance/migrate/{migration_id}/start",
Expand Down
Loading