Skip to content

Commit

Permalink
add precondition to network configuration updates
Browse files Browse the repository at this point in the history
  • Loading branch information
internet-diglett committed Jan 10, 2025
1 parent 2568a2e commit 57d0efc
Show file tree
Hide file tree
Showing 13 changed files with 499 additions and 308 deletions.
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.

9 changes: 9 additions & 0 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2340,6 +2340,15 @@ pub struct SwitchPortSettings {
pub identity: IdentityMetadata,
}

#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize, PartialEq)]
pub struct SwitchPortSettingsWithChecksum {
/// Value of response
pub value: SwitchPortSettingsView,

/// Checksum of value to use for concurrency control
pub checksum: String,
}

/// This structure contains all port settings information in one place. It's a
/// convenience data structure for getting a complete view of a particular
/// port's settings.
Expand Down
1 change: 1 addition & 0 deletions nexus/db-queries/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ semver.workspace = true
serde.workspace = true
serde_json.workspace = true
serde_with.workspace = true
sha2.workspace = true
sled-agent-client.workspace = true
slog.workspace = true
slog-error-chain.workspace = true
Expand Down
664 changes: 377 additions & 287 deletions nexus/db-queries/src/db/datastore/switch_port.rs

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions nexus/external-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,7 @@ pub trait NexusExternalApi {
async fn networking_switch_port_settings_create(
rqctx: RequestContext<Self::Context>,
new_settings: TypedBody<params::SwitchPortSettingsCreate>,
) -> Result<HttpResponseCreated<SwitchPortSettingsView>, HttpError>;
) -> Result<HttpResponseCreated<SwitchPortSettingsWithChecksum>, HttpError>;

/// Delete switch port settings
#[endpoint {
Expand Down Expand Up @@ -1428,7 +1428,7 @@ pub trait NexusExternalApi {
async fn networking_switch_port_settings_view(
rqctx: RequestContext<Self::Context>,
path_params: Path<params::SwitchPortSettingsInfoSelector>,
) -> Result<HttpResponseOk<SwitchPortSettingsView>, HttpError>;
) -> Result<HttpResponseOk<SwitchPortSettingsWithChecksum>, HttpError>;

/// List switch ports
#[endpoint {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ impl SwitchPortSettingsManager {
changes.push((
location,
port,
PortSettingsChange::Apply(Box::new(settings)),
PortSettingsChange::Apply(Box::new(settings.0)),
));
}
Ok(changes)
Expand Down
1 change: 1 addition & 0 deletions nexus/src/app/rack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ impl super::Nexus {
};

let mut port_settings_params = SwitchPortSettingsCreate {
precondition: None,
identity,
port_config,
groups: vec![],
Expand Down
21 changes: 14 additions & 7 deletions nexus/src/app/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl super::Nexus {
self: &Arc<Self>,
opctx: &OpContext,
params: params::SwitchPortSettingsCreate,
) -> CreateResult<SwitchPortSettingsCombinedResult> {
) -> CreateResult<(SwitchPortSettingsCombinedResult, String)> {
opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;
Self::switch_port_settings_validate(&params)?;

Expand Down Expand Up @@ -90,7 +90,7 @@ impl super::Nexus {
opctx: &OpContext,
params: params::SwitchPortSettingsCreate,
id: Option<Uuid>,
) -> CreateResult<SwitchPortSettingsCombinedResult> {
) -> CreateResult<(SwitchPortSettingsCombinedResult, String)> {
self.db_datastore.switch_port_settings_create(opctx, &params, id).await
}

Expand All @@ -99,7 +99,7 @@ impl super::Nexus {
opctx: &OpContext,
switch_port_settings_id: Uuid,
new_settings: params::SwitchPortSettingsCreate,
) -> CreateResult<SwitchPortSettingsCombinedResult> {
) -> CreateResult<(SwitchPortSettingsCombinedResult, String)> {
let result = self
.db_datastore
.switch_port_settings_update(
Expand Down Expand Up @@ -153,7 +153,7 @@ impl super::Nexus {
&self,
opctx: &OpContext,
name_or_id: &NameOrId,
) -> LookupResult<SwitchPortSettingsCombinedResult> {
) -> LookupResult<(SwitchPortSettingsCombinedResult, String)> {
opctx.authorize(authz::Action::Read, &authz::FLEET).await?;
self.db_datastore.switch_port_settings_get(opctx, name_or_id).await
}
Expand Down Expand Up @@ -189,15 +189,17 @@ impl super::Nexus {
opctx: &OpContext,
switch_port_id: Uuid,
port_settings_id: Option<Uuid>,
current_id: UpdatePrecondition<Uuid>,
precondition: UpdatePrecondition<
params::SwitchPortApplySettingsChecksums,
>,
) -> UpdateResult<()> {
opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;
self.db_datastore
.switch_port_set_settings_id(
opctx,
switch_port_id,
port_settings_id,
current_id,
precondition,
)
.await
}
Expand Down Expand Up @@ -229,11 +231,16 @@ impl super::Nexus {
}
};

let precondition = match settings.precondition.clone() {
Some(v) => UpdatePrecondition::Value(v),
None => UpdatePrecondition::Null,
};

self.set_switch_port_settings_id(
&opctx,
switch_port_id,
Some(switch_port_settings_id),
UpdatePrecondition::DontCare,
precondition,
)
.await?;

Expand Down
23 changes: 16 additions & 7 deletions nexus/src/external_api/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ use nexus_types::{
shared::{BfdStatus, ProbeInfo},
},
};
use omicron_common::api::external::http_pagination::data_page_params_for;
use omicron_common::api::external::http_pagination::marker_for_name;
use omicron_common::api::external::http_pagination::marker_for_name_or_id;
use omicron_common::api::external::http_pagination::name_or_id_pagination;
Expand Down Expand Up @@ -88,6 +87,9 @@ use omicron_common::api::external::TufRepoGetResponse;
use omicron_common::api::external::TufRepoInsertResponse;
use omicron_common::api::external::VpcFirewallRuleUpdateParams;
use omicron_common::api::external::VpcFirewallRules;
use omicron_common::api::external::{
http_pagination::data_page_params_for, SwitchPortSettingsWithChecksum,
};
use omicron_common::bail_unless;
use omicron_uuid_kinds::GenericUuid;
use propolis_client::support::tungstenite::protocol::frame::coding::CloseCode;
Expand Down Expand Up @@ -2805,18 +2807,22 @@ impl NexusExternalApi for NexusExternalApiImpl {
async fn networking_switch_port_settings_create(
rqctx: RequestContext<ApiContext>,
new_settings: TypedBody<params::SwitchPortSettingsCreate>,
) -> Result<HttpResponseCreated<SwitchPortSettingsView>, HttpError> {
) -> Result<HttpResponseCreated<SwitchPortSettingsWithChecksum>, HttpError>
{
let apictx = rqctx.context();
let handler = async {
let nexus = &apictx.context.nexus;
let params = new_settings.into_inner();
let opctx =
crate::context::op_context_for_external_api(&rqctx).await?;
let result =
let (result, checksum) =
nexus.switch_port_settings_post(&opctx, params).await?;

let settings: SwitchPortSettingsView = result.into();
Ok(HttpResponseCreated(settings))
Ok(HttpResponseCreated(SwitchPortSettingsWithChecksum {
value: settings,
checksum,
}))
};
apictx
.context
Expand Down Expand Up @@ -2884,16 +2890,19 @@ impl NexusExternalApi for NexusExternalApiImpl {
async fn networking_switch_port_settings_view(
rqctx: RequestContext<ApiContext>,
path_params: Path<params::SwitchPortSettingsInfoSelector>,
) -> Result<HttpResponseOk<SwitchPortSettingsView>, HttpError> {
) -> Result<HttpResponseOk<SwitchPortSettingsWithChecksum>, HttpError> {
let apictx = rqctx.context();
let handler = async {
let nexus = &apictx.context.nexus;
let query = path_params.into_inner().port;
let opctx =
crate::context::op_context_for_external_api(&rqctx).await?;
let settings =
let (settings, checksum) =
nexus.switch_port_settings_get(&opctx, &query).await?;
Ok(HttpResponseOk(settings.into()))
Ok(HttpResponseOk(SwitchPortSettingsWithChecksum {
value: settings.into(),
checksum,
}))
};
apictx
.context
Expand Down
1 change: 1 addition & 0 deletions nexus/tests/integration_tests/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,7 @@ pub static DEMO_SWITCH_PORT_SETTINGS_APPLY_URL: Lazy<String> = Lazy::new(
);
pub static DEMO_SWITCH_PORT_SETTINGS: Lazy<params::SwitchPortApplySettings> =
Lazy::new(|| params::SwitchPortApplySettings {
precondition: None,
port_settings: NameOrId::Name("portofino".parse().unwrap()),
});
/* TODO requires dpd access
Expand Down
1 change: 1 addition & 0 deletions nexus/tests/integration_tests/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
// apply port settings

let apply_settings = SwitchPortApplySettings {
precondition: None,
port_settings: NameOrId::Name("portofino".parse().unwrap()),
};

Expand Down
21 changes: 20 additions & 1 deletion nexus/types/src/external_api/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1671,6 +1671,8 @@ pub struct SwitchPortSettingsCreate {
pub bgp_peers: HashMap<String, BgpPeerConfig>,
/// Addresses indexed by interface name.
pub addresses: HashMap<String, AddressConfig>,
/// Optional checksum to provide for detecting conflicting concurrent updates
pub precondition: Option<String>,
}

impl SwitchPortSettingsCreate {
Expand All @@ -1686,6 +1688,7 @@ impl SwitchPortSettingsCreate {
routes: HashMap::new(),
bgp_peers: HashMap::new(),
addresses: HashMap::new(),
precondition: None,
}
}
}
Expand Down Expand Up @@ -2006,10 +2009,26 @@ pub struct SwitchPortPageSelector {
/// Parameters for applying settings to switch ports.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
pub struct SwitchPortApplySettings {
/// A name or id to use when applying switch port settings.
pub precondition: Option<SwitchPortApplySettingsChecksums>,

/// Name or ID of the desired configuration
pub port_settings: NameOrId,
}

/// Parameters for applying a precondition to SwitchPortApplySettings
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
pub struct SwitchPortApplySettingsChecksums {
/// md5 checksum of current SwitchPortApplySettings body
/// This field protects against concurrent modification of `SwitchPortApplySettings`
/// Set to `None` if you expect there to not be any settings currently applied.
pub current_settings_checksum: Option<String>,

/// md5 checksum of the desired configuration body
/// This field ensures that the port settings you are applying have not been modified
/// since you last viewed them
pub new_settings_checksum: String,
}

// IMAGES

/// The source of the underlying image.
Expand Down
58 changes: 55 additions & 3 deletions openapi/nexus.json
Original file line number Diff line number Diff line change
Expand Up @@ -7797,7 +7797,7 @@
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/SwitchPortSettingsView"
"$ref": "#/components/schemas/SwitchPortSettingsWithChecksum"
}
}
}
Expand Down Expand Up @@ -7863,7 +7863,7 @@
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/SwitchPortSettingsView"
"$ref": "#/components/schemas/SwitchPortSettingsWithChecksum"
}
}
}
Expand Down Expand Up @@ -20366,18 +20366,44 @@
"type": "object",
"properties": {
"port_settings": {
"description": "A name or id to use when applying switch port settings.",
"description": "Name or ID of the desired configuration",
"allOf": [
{
"$ref": "#/components/schemas/NameOrId"
}
]
},
"precondition": {
"nullable": true,
"allOf": [
{
"$ref": "#/components/schemas/SwitchPortApplySettingsChecksums"
}
]
}
},
"required": [
"port_settings"
]
},
"SwitchPortApplySettingsChecksums": {
"description": "Parameters for applying a precondition to SwitchPortApplySettings",
"type": "object",
"properties": {
"current_settings_checksum": {
"nullable": true,
"description": "md5 checksum of current SwitchPortApplySettings body This field protects against concurrent modification of `SwitchPortApplySettings` Set to `None` if you expect there to not be any settings currently applied.",
"type": "string"
},
"new_settings_checksum": {
"description": "md5 checksum of the desired configuration body This field ensures that the port settings you are applying have not been modified since you last viewed them",
"type": "string"
}
},
"required": [
"new_settings_checksum"
]
},
"SwitchPortConfig": {
"description": "A physical port configuration for a port settings object.",
"type": "object",
Expand Down Expand Up @@ -20690,6 +20716,11 @@
"port_config": {
"$ref": "#/components/schemas/SwitchPortConfigCreate"
},
"precondition": {
"nullable": true,
"description": "Optional checksum to provide for detecting conflicting concurrent updates",
"type": "string"
},
"routes": {
"description": "Routes indexed by interface name.",
"type": "object",
Expand Down Expand Up @@ -20854,6 +20885,27 @@
"vlan_interfaces"
]
},
"SwitchPortSettingsWithChecksum": {
"type": "object",
"properties": {
"checksum": {
"description": "Checksum of value to use for concurrency control",
"type": "string"
},
"value": {
"description": "Value of response",
"allOf": [
{
"$ref": "#/components/schemas/SwitchPortSettingsView"
}
]
}
},
"required": [
"checksum",
"value"
]
},
"SwitchResultsPage": {
"description": "A single page of results",
"type": "object",
Expand Down

0 comments on commit 57d0efc

Please sign in to comment.