Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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.

16 changes: 5 additions & 11 deletions nexus/mgs-updates/src/test_util/host_phase_2_test_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ mod api_impl {
use sled_agent_types::support_bundle::SupportBundleMetadata;
use sled_agent_types::support_bundle::SupportBundlePathParam;
use sled_agent_types::support_bundle::SupportBundleTransferQueryParams;
use sled_agent_types::trust_quorum::TrustQuorumCoordinatorStatusResponse;
use sled_agent_types::trust_quorum::TrustQuorumNetworkConfigResponse;
use sled_agent_types::zone_bundle::BundleUtilization;
use sled_agent_types::zone_bundle::CleanupContext;
use sled_agent_types::zone_bundle::CleanupContextUpdate;
Expand Down Expand Up @@ -1003,9 +1005,7 @@ mod api_impl {
async fn trust_quorum_coordinator_status(
_request_context: RequestContext<Self::Context>,
) -> Result<
HttpResponseOk<
Option<trust_quorum_types::status::CoordinatorStatus>,
>,
HttpResponseOk<TrustQuorumCoordinatorStatusResponse>,
HttpError,
> {
unimplemented!()
Expand Down Expand Up @@ -1065,14 +1065,8 @@ mod api_impl {

async fn trust_quorum_network_config_get(
_request_context: RequestContext<Self::Context>,
) -> Result<
HttpResponseOk<
Option<
sled_agent_types::trust_quorum::TrustQuorumNetworkConfig,
>,
>,
HttpError,
> {
) -> Result<HttpResponseOk<TrustQuorumNetworkConfigResponse>, HttpError>
{
unimplemented!()
}

Expand Down
10 changes: 9 additions & 1 deletion nexus/src/app/background/tasks/trust_quorum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,15 @@ async fn prepare(
config.coordinator
)
})?
.into_inner();
.into_inner()
.status
.ok_or_else(|| {
anyhow!(
"Coordinator {} reported no coordinator status: it is not \
currently coordinating a reconfiguration.",
config.coordinator
)
})?;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@andrewjstone how should we handle this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should abort the configuration. The user will have to find out about it by querying the status, but that's unavoidable at this point.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can I do this in a followup? This change is currently behavior-preserving (just makes the error messages better and closes a footgun) and I would want to introduce such a behavior change in a followup.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes. Totally. I expected as much. It's a major change, although a nice fix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What I'm going to do in this commit is to also include the epoch in the response, so that Nexus can use that as part of its abort decision-making.


// Write state back to DB
let state = datastore
Expand Down
1 change: 1 addition & 0 deletions openapi/sled-agent/sled-agent-60.0.0-f9d5fe.json.gitstub
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
d31af1e7ce811874957823cb8f731b6f10e35a1b:openapi/sled-agent/sled-agent-60.0.0-f9d5fe.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://oxide.computer",
"email": "api@oxide.computer"
},
"version": "60.0.0"
"version": "61.0.0"
},
"paths": {
"/artifacts": {
Expand Down Expand Up @@ -1691,7 +1691,7 @@
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/CoordinatorStatus"
"$ref": "#/components/schemas/TrustQuorumCoordinatorStatusResponse"
}
}
}
Expand All @@ -1715,7 +1715,7 @@
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/TrustQuorumNetworkConfig"
"$ref": "#/components/schemas/TrustQuorumNetworkConfigResponse"
}
}
}
Expand Down Expand Up @@ -10422,6 +10422,21 @@
"rack_network_config"
]
},
"TrustQuorumCoordinatorStatusResponse": {
"description": "Response type for `trust_quorum_coordinator_status`.",
"type": "object",
"properties": {
"status": {
"nullable": true,
"description": "The coordinator status, or `None` if this node is not currently coordinating a reconfiguration.",
"allOf": [
{
"$ref": "#/components/schemas/CoordinatorStatus"
}
]
}
}
},
"TrustQuorumNetworkConfig": {
"description": "Network configuration used to bring up the control plane.\n\nThis type mirrors `bootstore::schemes::v0::NetworkConfig` but adds `JsonSchema` for API compatibility.",
"type": "object",
Expand All @@ -10441,6 +10456,21 @@
"generation"
]
},
"TrustQuorumNetworkConfigResponse": {
"description": "Response type for `trust_quorum_network_config`.",
"type": "object",
"properties": {
"config": {
"nullable": true,
"description": "The trust quorum network config, or `None` if a config has not been loaded or received since Sled Agent started.",
"allOf": [
{
"$ref": "#/components/schemas/TrustQuorumNetworkConfig"
}
]
}
}
},
"TxEqConfig": {
"description": "Per-port tx-eq overrides. This can be used to fine-tune the transceiver equalization settings to improve signal integrity.",
"type": "object",
Expand Down
2 changes: 1 addition & 1 deletion openapi/sled-agent/sled-agent-latest.json
1 change: 1 addition & 0 deletions sled-agent/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ sled-hardware-types.workspace = true
slog-error-chain.workspace = true
static_assertions.workspace = true
trust-quorum-types.workspace = true
trust-quorum-types-versions.workspace = true
uuid.workspace = true
x509-cert.workspace = true
90 changes: 81 additions & 9 deletions sled-agent/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@ use std::collections::BTreeMap;

use camino::Utf8PathBuf;
use dropshot::{
Body, FreeformBody, Header, HttpError, HttpResponseAccepted,
HttpResponseCreated, HttpResponseDeleted, HttpResponseHeaders,
HttpResponseOk, HttpResponseUpdatedNoContent, Path, Query, RequestContext,
StreamingBody, TypedBody,
Body, ClientErrorStatusCode, FreeformBody, Header, HttpError,
HttpResponseAccepted, HttpResponseCreated, HttpResponseDeleted,
HttpResponseHeaders, HttpResponseOk, HttpResponseUpdatedNoContent, Path,
Query, RequestContext, StreamingBody, TypedBody,
};
use dropshot_api_manager_types::api_versions;
use omicron_common::api::internal::shared::{
ExternalIpGatewayMap, ResolvedVpcRouteSet, ResolvedVpcRouteState,
SledIdentifiers, VirtualNetworkInterfaceHost,
};
use sled_agent_types_versions::{
latest, v1, v11, v14, v18, v20, v22, v24, v25, v26, v28, v29, v30, v31,
v32, v33, v34, v37, v39, v40, v41, v42, v43, v44, v46, v47, v48, v49, v50,
v51, v52, v53, v55,
latest, v1, v11, v14, v15, v18, v20, v22, v24, v25, v26, v28, v29, v30,
v31, v32, v33, v34, v37, v39, v40, v41, v42, v43, v44, v46, v47, v48, v49,
v50, v51, v52, v53, v55,
};
use sled_diagnostics::SledDiagnosticsQueryOutput;
use slog_error_chain::InlineErrorChain;
Expand All @@ -36,6 +36,7 @@ api_versions!([
// | example for the next person.
// v
// (next_int, IDENT),
(61, TRUST_QUORUM_NULLABLE_RESPONSES),
(60, ADD_MULTIRACK_JOIN_SERVICE_STATUS),
(59, INSTANCE_ATTACHED_SUBNETS_MAP),
(58, BYTE_WRAPPER_0_2_BUMP),
Expand Down Expand Up @@ -1605,14 +1606,55 @@ pub trait SledAgentApi {
#[endpoint {
method = GET,
path = "/trust-quorum/coordinator-status",
versions = VERSION_TRUST_QUORUM_NULLABLE_RESPONSES..,
}]
async fn trust_quorum_coordinator_status(
request_context: RequestContext<Self::Context>,
) -> Result<
HttpResponseOk<Option<trust_quorum_types::status::CoordinatorStatus>>,
HttpResponseOk<
latest::trust_quorum::TrustQuorumCoordinatorStatusResponse,
>,
HttpError,
>;

/// Get the coordinator status if this node is coordinating a reconfiguration
#[endpoint {
operation_id = "trust_quorum_coordinator_status",
method = GET,
path = "/trust-quorum/coordinator-status",
versions = ..VERSION_TRUST_QUORUM_NULLABLE_RESPONSES,
}]
async fn trust_quorum_coordinator_status_v13(
request_context: RequestContext<Self::Context>,
) -> Result<
HttpResponseOk<
trust_quorum_types_versions::v1::status::CoordinatorStatus,
>,
HttpError,
> {
// These versions were declared as returning an
// Option<CoordinatorStatus>, but their corresponding OpenAPI documents
// indicated that the CoordinatorStatus response was non-nullable. (This
// is a Dropshot bug; see omicron#11623). We've since changed this API
// to return a non-nullable CoordinatorStatus, making the API match the
// immutable blessed documents. But that leaves open the question of how
// None can be represented on the wire. We return a 404 -- old clients
// will receive it as an error and will handle it reasonably well.
Self::trust_quorum_coordinator_status(request_context).await?.try_map(
|response| {
response.status.ok_or_else(|| {
HttpError::for_client_error(
None,
ClientErrorStatusCode::NOT_FOUND,
"this node is not currently coordinating a trust \
quorum reconfiguration"
.to_string(),
)
})
},
)
}

/// Attempt to prepare and commit a trust quorum configuration
#[endpoint {
method = PUT,
Expand Down Expand Up @@ -1672,14 +1714,44 @@ pub trait SledAgentApi {
#[endpoint {
method = GET,
path = "/trust-quorum/network-config",
versions = VERSION_TRUST_QUORUM_NULLABLE_RESPONSES..,
}]
async fn trust_quorum_network_config_get(
request_context: RequestContext<Self::Context>,
) -> Result<
HttpResponseOk<Option<latest::trust_quorum::TrustQuorumNetworkConfig>>,
HttpResponseOk<latest::trust_quorum::TrustQuorumNetworkConfigResponse>,
HttpError,
>;

/// Get the current network config from trust quorum
#[endpoint {
operation_id = "trust_quorum_network_config_get",
method = GET,
path = "/trust-quorum/network-config",
versions = ..VERSION_TRUST_QUORUM_NULLABLE_RESPONSES,
}]
async fn trust_quorum_network_config_get_v15(
request_context: RequestContext<Self::Context>,
) -> Result<
HttpResponseOk<v15::trust_quorum::TrustQuorumNetworkConfig>,
HttpError,
> {
// See the comment in trust_quorum_coordinator_status_v13.
Self::trust_quorum_network_config_get(request_context).await?.try_map(
|response| {
response.config.ok_or_else(|| {
HttpError::for_client_error(
None,
ClientErrorStatusCode::NOT_FOUND,
"this node does not have a trust quorum network \
config"
.to_string(),
)
})
},
)
}

/// Update the network config in trust quorum
#[endpoint {
method = PUT,
Expand Down
19 changes: 13 additions & 6 deletions sled-agent/src/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ use sled_agent_types::support_bundle::{
SupportBundleTransferQueryParams,
};
use sled_agent_types::trust_quorum::{
ProxyCommitRequest, ProxyPrepareAndCommitRequest, TrustQuorumNetworkConfig,
ProxyCommitRequest, ProxyPrepareAndCommitRequest,
TrustQuorumCoordinatorStatusResponse, TrustQuorumNetworkConfig,
TrustQuorumNetworkConfigResponse,
};
use sled_agent_types::zone_bundle::{
BundleUtilization, CleanupContext, CleanupContextUpdate, CleanupCount,
Expand All @@ -75,7 +77,7 @@ use slog_error_chain::InlineErrorChain;
use trust_quorum_types::messages::{
CommitRequest, LrtqUpgradeMsg, PrepareAndCommitRequest, ReconfigureMsg,
};
use trust_quorum_types::status::{CommitStatus, CoordinatorStatus, NodeStatus};
use trust_quorum_types::status::{CommitStatus, NodeStatus};

// Fixed identifiers for prior versions only
use sled_agent_types_versions::{v20, v25, v26, v30, v33, v39, v42, v47, v48};
Expand Down Expand Up @@ -1550,7 +1552,8 @@ impl SledAgentApi for SledAgentImpl {

async fn trust_quorum_coordinator_status(
request_context: RequestContext<Self::Context>,
) -> Result<HttpResponseOk<Option<CoordinatorStatus>>, HttpError> {
) -> Result<HttpResponseOk<TrustQuorumCoordinatorStatusResponse>, HttpError>
{
let sa = request_context.context();
sa.latencies()
.instrument_dropshot_handler(&request_context, async {
Expand All @@ -1562,7 +1565,9 @@ impl SledAgentApi for SledAgentImpl {
)
},
)?;
Ok(HttpResponseOk(status))
Ok(HttpResponseOk(TrustQuorumCoordinatorStatusResponse {
status,
}))
})
.await
}
Expand Down Expand Up @@ -1689,7 +1694,7 @@ impl SledAgentApi for SledAgentImpl {

async fn trust_quorum_network_config_get(
request_context: RequestContext<Self::Context>,
) -> Result<HttpResponseOk<Option<TrustQuorumNetworkConfig>>, HttpError>
) -> Result<HttpResponseOk<TrustQuorumNetworkConfigResponse>, HttpError>
{
let sa = request_context.context();
sa.latencies()
Expand All @@ -1700,7 +1705,9 @@ impl SledAgentApi for SledAgentImpl {
InlineErrorChain::new(&e).to_string(),
)
})?;
Ok(HttpResponseOk(config.map(TrustQuorumNetworkConfig::from)))
Ok(HttpResponseOk(TrustQuorumNetworkConfigResponse {
config: config.map(TrustQuorumNetworkConfig::from),
}))
})
.await
}
Expand Down
10 changes: 6 additions & 4 deletions sled-agent/src/sim/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ use sled_agent_types::support_bundle::{
SupportBundleTransferQueryParams,
};
use sled_agent_types::trust_quorum::{
ProxyCommitRequest, ProxyPrepareAndCommitRequest, TrustQuorumNetworkConfig,
ProxyCommitRequest, ProxyPrepareAndCommitRequest,
TrustQuorumCoordinatorStatusResponse, TrustQuorumNetworkConfig,
TrustQuorumNetworkConfigResponse,
};
use sled_agent_types::zone_bundle::{
BundleUtilization, CleanupContext, CleanupContextUpdate, CleanupCount,
Expand All @@ -96,7 +98,6 @@ use trust_quorum_types::messages::{
CommitRequest, LrtqUpgradeMsg, PrepareAndCommitRequest, ReconfigureMsg,
};
use trust_quorum_types::status::CommitStatus;
use trust_quorum_types::status::CoordinatorStatus;
use trust_quorum_types::status::NodeStatus;

use super::sled_agent::SledAgent;
Expand Down Expand Up @@ -1035,7 +1036,8 @@ impl SledAgentApi for SledAgentSimImpl {

async fn trust_quorum_coordinator_status(
_request_context: RequestContext<Self::Context>,
) -> Result<HttpResponseOk<Option<CoordinatorStatus>>, HttpError> {
) -> Result<HttpResponseOk<TrustQuorumCoordinatorStatusResponse>, HttpError>
{
method_unimplemented()
}

Expand Down Expand Up @@ -1075,7 +1077,7 @@ impl SledAgentApi for SledAgentSimImpl {

async fn trust_quorum_network_config_get(
_request_context: RequestContext<Self::Context>,
) -> Result<HttpResponseOk<Option<TrustQuorumNetworkConfig>>, HttpError>
) -> Result<HttpResponseOk<TrustQuorumNetworkConfigResponse>, HttpError>
{
method_unimplemented()
}
Expand Down
3 changes: 3 additions & 0 deletions sled-agent/types/versions/src/latest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,9 @@ pub mod trust_quorum {
pub use crate::v13::trust_quorum::ProxyPrepareAndCommitRequest;

pub use crate::v15::trust_quorum::TrustQuorumNetworkConfig;

pub use crate::v61::trust_quorum::TrustQuorumCoordinatorStatusResponse;
pub use crate::v61::trust_quorum::TrustQuorumNetworkConfigResponse;
}

pub mod uplink {
Expand Down
Loading
Loading