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
3 changes: 2 additions & 1 deletion nexus/db-model/src/schema_versions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
///
/// This must be updated when you change the database schema. Refer to
/// schema/crdb/README.adoc in the root of this repository for details.
pub const SCHEMA_VERSION: Version = Version::new(291, 0, 0);
pub const SCHEMA_VERSION: Version = Version::new(292, 0, 0);

/// List of all past database schema versions, in *reverse* order
///
Expand All @@ -28,6 +28,7 @@ pub static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
// | leaving the first copy as an example for the next person.
// v
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
KnownVersion::new(292, "allow-ddm-traffic"),
KnownVersion::new(291, "bgp-peer-src-addr"),
KnownVersion::new(290, "saga-abandon-reason-orphaned"),
KnownVersion::new(289, "normalize-service-external-ips"),
Expand Down
10 changes: 8 additions & 2 deletions nexus/db-model/src/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,17 @@ impl Into<networking_types::SwitchPortSettingsGroup>
pub struct SwitchPortConfig {
pub port_settings_id: Uuid,
Comment thread
andrewjstone marked this conversation as resolved.
pub geometry: SwitchPortGeometry,
/// Whether DDM traffic is permitted on this port.
pub allow_ddm_traffic: bool,
}

impl SwitchPortConfig {
pub fn new(port_settings_id: Uuid, geometry: SwitchPortGeometry) -> Self {
Self { port_settings_id, geometry }
pub fn new(
port_settings_id: Uuid,
geometry: SwitchPortGeometry,
allow_ddm_traffic: bool,
) -> Self {
Self { port_settings_id, geometry, allow_ddm_traffic }
}
}

Expand Down
73 changes: 66 additions & 7 deletions nexus/db-queries/src/db/datastore/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,11 +360,18 @@ impl DataStore {
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
}

// Until the rest of multirack is implemented, we don't want to expose
// `allow_ddm_traffic` in the external API. That's why it's passed
// separately from `params`.
//
// There is a more detailed comment in `switch_port_settings_update` where
// this value gets used.
pub async fn switch_port_settings_create(
&self,
opctx: &OpContext,
params: &networking::SwitchPortSettingsCreate,
id: Option<Uuid>,
allow_ddm_traffic: bool,
Comment thread
andrewjstone marked this conversation as resolved.
) -> CreateResult<SwitchPortSettingsCombinedResult> {
let err = OptionalError::new();
let conn = self.pool_connection_authorized(opctx).await?;
Expand All @@ -375,7 +382,14 @@ impl DataStore {
.transaction(&conn, |conn| {
let err = err.clone();
async move {
do_switch_port_settings_create(&conn, id, params, err).await
do_switch_port_settings_create(
&conn,
id,
params,
allow_ddm_traffic,
err,
)
.await
}
})
.await
Expand Down Expand Up @@ -459,9 +473,32 @@ impl DataStore {
let create_err = create_err.clone();
let selector = NameOrId::Id(id);
async move {
use nexus_db_schema::schema::switch_port_settings_port_config::dsl as port_config_dsl;

// Until the rest of multirack is implemented, we don't
// want to expose `allow_ddm_traffic` in the external API.
// It's very possible that the actual external configuration
// will look different. For now, we must just ensure that
// a customer doesn't set this flag to true at RSS time
// as we'll maintain whatever actual value was set during
// RSS below. This allows us to test multirack during
// implementation without making it customer visible.
Comment thread
andrewjstone marked this conversation as resolved.
//
// Since `allow_ddm_traffic` isn't part of the external API
// it's absent from `params`. Read it back before the delete
// and carry it into the recreate. Otherwise an operator
// editing unrelated port settings would silently clear what
// RSS configured.
let allow_ddm_traffic: bool =
port_config_dsl::switch_port_settings_port_config
.filter(port_config_dsl::port_settings_id.eq(id))
.select(port_config_dsl::allow_ddm_traffic)
.get_result_async(&conn)
.await?;

do_switch_port_settings_delete(&conn, &selector, delete_err).await?;
do_switch_port_settings_create(
&conn, Some(id), params, create_err,
&conn, Some(id), params, allow_ddm_traffic, create_err,
).await
}
})
Expand Down Expand Up @@ -1566,6 +1603,7 @@ async fn do_switch_port_settings_create(
conn: &Connection<DTraceConnection<PgConnection>>,
id: Option<Uuid>,
params: &networking::SwitchPortSettingsCreate,
allow_ddm_traffic: bool,
err: OptionalError<SwitchPortSettingsCreateError>,
) -> Result<SwitchPortSettingsCombinedResult, diesel::result::Error> {
use nexus_db_schema::schema::{
Expand Down Expand Up @@ -1599,8 +1637,11 @@ async fn do_switch_port_settings_create(
let psid = db_port_settings.identity.id;

// add the port config
let port_config =
SwitchPortConfig::new(psid, params.port_config.geometry.into());
let port_config = SwitchPortConfig::new(
psid,
params.port_config.geometry.into(),
allow_ddm_traffic,
);

let db_port_config: SwitchPortConfig =
diesel::insert_into(port_config_dsl::switch_port_settings_port_config)
Expand Down Expand Up @@ -2462,8 +2503,14 @@ mod test {
addresses: vec![],
};

let allow_ddm_traffic = false;
let settings_result = datastore
.switch_port_settings_create(&opctx, &settings, None)
.switch_port_settings_create(
&opctx,
&settings,
None,
allow_ddm_traffic,
)
.await
.unwrap();

Expand Down Expand Up @@ -2833,8 +2880,14 @@ mod test {
}],
addresses: vec![],
};
let allow_ddm_traffic = false;
let settings_result = datastore
.switch_port_settings_create(&opctx, &settings, None)
.switch_port_settings_create(
&opctx,
&settings,
None,
allow_ddm_traffic,
)
.await
.unwrap();
datastore
Expand Down Expand Up @@ -3045,8 +3098,14 @@ mod test {
addresses: vec![],
};

let allow_ddm_traffic = false;
let result = datastore
.switch_port_settings_create(&opctx, &settings, None)
.switch_port_settings_create(
&opctx,
&settings,
None,
allow_ddm_traffic,
)
.await
.expect("created settings");

Expand Down
1 change: 1 addition & 0 deletions nexus/db-schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ table! {
switch_port_settings_port_config (port_settings_id) {
port_settings_id -> Uuid,
geometry -> crate::enums::SwitchPortGeometryEnum,
allow_ddm_traffic -> Bool,
}
}

Expand Down
7 changes: 6 additions & 1 deletion nexus/src/app/rack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,12 @@ impl super::Nexus {

match self
.db_datastore
.switch_port_settings_create(opctx, &port_settings_params, None)
.switch_port_settings_create(

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.

This is where the value from RSS is actually stored in the DB.

opctx,
&port_settings_params,
None,
uplink_config.allow_ddm_traffic,
)
.await
{
Ok(_) | Err(Error::ObjectAlreadyExists { .. }) => Ok(()),
Expand Down
15 changes: 13 additions & 2 deletions nexus/src/app/sagas/instance_start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,13 +1337,24 @@ mod test {
}],
}];

let allow_ddm_traffic = false;
let uplink0_settings = datastore
.switch_port_settings_create(&opctx, &uplink0_params, None)
.switch_port_settings_create(
&opctx,
&uplink0_params,
None,
allow_ddm_traffic,
)
.await
.expect("should be able to create configuration for uplink0");

let uplink1_settings = datastore
.switch_port_settings_create(&opctx, &uplink1_params, None)
.switch_port_settings_create(
&opctx,
&uplink1_params,
None,
allow_ddm_traffic,
)
.await
.expect("should be able to create configuration for uplink1");

Expand Down
15 changes: 13 additions & 2 deletions nexus/src/app/sagas/instance_update/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2928,13 +2928,24 @@ mod test {
}],
}];

let allow_ddm_traffic = false;
let uplink0_settings = datastore
.switch_port_settings_create(&opctx, &uplink0_params, None)
.switch_port_settings_create(
&opctx,
&uplink0_params,
None,
allow_ddm_traffic,
)
.await
.expect("should be able to create configuration for uplink0");

let uplink1_settings = datastore
.switch_port_settings_create(&opctx, &uplink1_params, None)
.switch_port_settings_create(
&opctx,
&uplink1_params,
None,
allow_ddm_traffic,
)
.await
.expect("should be able to create configuration for uplink1");

Expand Down
8 changes: 7 additions & 1 deletion nexus/src/app/switch_port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,15 @@ impl super::Nexus {
params: networking::SwitchPortSettingsCreate,
id: Option<Uuid>,
) -> CreateResult<SwitchPortSettingsCombinedResult> {
// We explicitly do not expose `allow_ddm_traffic` through the external
// API. We want to wait until multirack is further along to determine
// the shape of exposure. The flag is only set by RSS for testing
// purposes. Setting it to false here won't restrict our testing right
// now.
let allow_ddm_traffic = false;
let result = self
.db_datastore
.switch_port_settings_create(opctx, &params, id)
.switch_port_settings_create(opctx, &params, id, allow_ddm_traffic)
.await?;

// eagerly propagate changes via rpw
Expand Down
1 change: 1 addition & 0 deletions nexus/switch-config/preparation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,5 +229,6 @@ fn port_input_from_db(
post1: c.post1,
})
.collect(),
allow_ddm_traffic: info.port.allow_ddm_traffic,
}
}
5 changes: 4 additions & 1 deletion nexus/switch-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ pub struct PortInput {
pub lldp: Vec<LldpInput>,
/// The port's TX-EQ overrides. Only the first entry is used today.
pub tx_eq: Vec<TxEqConfig>,
/// Whether DDM traffic is permitted on this port.
pub allow_ddm_traffic: bool,
}

/// An IP address assigned to a port.
Expand Down Expand Up @@ -460,7 +462,7 @@ pub fn build_rack_network_config(
management_addrs: c.management_ip.map(|ip| vec![ip]),
}),
tx_eq,
allow_ddm_traffic: false,
allow_ddm_traffic: port.allow_ddm_traffic,
};

ports.push(port_config);
Expand Down Expand Up @@ -540,6 +542,7 @@ mod tests {
routes: vec![],
lldp: vec![],
tx_eq: vec![],
allow_ddm_traffic: false,
}
}

Expand Down
2 changes: 2 additions & 0 deletions schema/crdb/allow-ddm-traffic/up01.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE omicron.public.switch_port_settings_port_config
ADD COLUMN IF NOT EXISTS allow_ddm_traffic BOOL NOT NULL DEFAULT FALSE;
2 changes: 2 additions & 0 deletions schema/crdb/allow-ddm-traffic/up02.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE omicron.public.switch_port_settings_port_config
ALTER COLUMN allow_ddm_traffic DROP DEFAULT;
5 changes: 3 additions & 2 deletions schema/crdb/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3752,7 +3752,8 @@ CREATE TYPE IF NOT EXISTS omicron.public.switch_port_geometry AS ENUM (

CREATE TABLE IF NOT EXISTS omicron.public.switch_port_settings_port_config (
port_settings_id UUID PRIMARY KEY,
geometry omicron.public.switch_port_geometry
geometry omicron.public.switch_port_geometry,
allow_ddm_traffic BOOL NOT NULL
);

CREATE TYPE IF NOT EXISTS omicron.public.switch_link_fec AS ENUM (
Expand Down Expand Up @@ -9261,7 +9262,7 @@ INSERT INTO omicron.public.db_metadata (
version,
target_version
) VALUES
(TRUE, NOW(), NOW(), '291.0.0', NULL)
(TRUE, NOW(), NOW(), '292.0.0', NULL)
ON CONFLICT DO NOTHING;

COMMIT;
Loading