Skip to content

Commit

Permalink
[nexus-db-model] separate out SledUpdate from Sled (#4533)
Browse files Browse the repository at this point in the history
`Sled` consists of several columns that aren't controlled by sled-agent, and we
end up in this weird place where we have `Sled` instances that don't reflect
reality. I'm working on adding a `provision_state` column which is controlled
by the operator, and again for which sled-agent doesn't know.

Clean this up by defining a new struct, `SledUpdate`, which only contains the
columns sled-agent knows about. The other columns get defaults when
`into_insertable` is called.
  • Loading branch information
sunshowers authored Nov 21, 2023
1 parent 08041d6 commit e3e99ee
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 71 deletions.
133 changes: 101 additions & 32 deletions nexus/db-model/src/sled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,38 +62,6 @@ pub struct Sled {
}

impl Sled {
pub fn new(
id: Uuid,
addr: SocketAddrV6,
baseboard: SledBaseboard,
hardware: SledSystemHardware,
rack_id: Uuid,
) -> Self {
let last_used_address = {
let mut segments = addr.ip().segments();
segments[7] += omicron_common::address::RSS_RESERVED_ADDRESSES;
ipv6::Ipv6Addr::from(Ipv6Addr::from(segments))
};
Self {
identity: SledIdentity::new(id),
time_deleted: None,
rcgen: Generation::new(),
rack_id,
is_scrimlet: hardware.is_scrimlet,
serial_number: baseboard.serial_number,
part_number: baseboard.part_number,
revision: baseboard.revision,
usable_hardware_threads: SqlU32::new(
hardware.usable_hardware_threads,
),
usable_physical_ram: hardware.usable_physical_ram,
reservoir_size: hardware.reservoir_size,
ip: ipv6::Ipv6Addr::from(addr.ip()),
port: addr.port().into(),
last_used_address,
}
}

pub fn is_scrimlet(&self) -> bool {
self.is_scrimlet
}
Expand Down Expand Up @@ -153,6 +121,107 @@ impl DatastoreCollectionConfig<super::Service> for Sled {
type CollectionIdColumn = service::dsl::sled_id;
}

/// Form of `Sled` used for updates from sled-agent. This is missing some
/// columns that are present in `Sled` because sled-agent doesn't control them.
#[derive(Debug, Clone)]
pub struct SledUpdate {
id: Uuid,

pub rack_id: Uuid,

is_scrimlet: bool,
serial_number: String,
part_number: String,
revision: i64,

pub usable_hardware_threads: SqlU32,
pub usable_physical_ram: ByteCount,
pub reservoir_size: ByteCount,

// ServiceAddress (Sled Agent).
pub ip: ipv6::Ipv6Addr,
pub port: SqlU16,
}

impl SledUpdate {
pub fn new(
id: Uuid,
addr: SocketAddrV6,
baseboard: SledBaseboard,
hardware: SledSystemHardware,
rack_id: Uuid,
) -> Self {
Self {
id,
rack_id,
is_scrimlet: hardware.is_scrimlet,
serial_number: baseboard.serial_number,
part_number: baseboard.part_number,
revision: baseboard.revision,
usable_hardware_threads: SqlU32::new(
hardware.usable_hardware_threads,
),
usable_physical_ram: hardware.usable_physical_ram,
reservoir_size: hardware.reservoir_size,
ip: addr.ip().into(),
port: addr.port().into(),
}
}

/// Converts self into a form used for inserts of new sleds into the
/// database.
///
/// This form adds default values for fields that are not present in
/// `SledUpdate`.
pub fn into_insertable(self) -> Sled {
let last_used_address = {
let mut segments = self.ip().segments();
segments[7] += omicron_common::address::RSS_RESERVED_ADDRESSES;
ipv6::Ipv6Addr::from(Ipv6Addr::from(segments))
};
Sled {
identity: SledIdentity::new(self.id),
rcgen: Generation::new(),
time_deleted: None,
rack_id: self.rack_id,
is_scrimlet: self.is_scrimlet,
serial_number: self.serial_number,
part_number: self.part_number,
revision: self.revision,
usable_hardware_threads: self.usable_hardware_threads,
usable_physical_ram: self.usable_physical_ram,
reservoir_size: self.reservoir_size,
ip: self.ip,
port: self.port,
last_used_address,
}
}

pub fn id(&self) -> Uuid {
self.id
}

pub fn is_scrimlet(&self) -> bool {
self.is_scrimlet
}

pub fn ip(&self) -> Ipv6Addr {
self.ip.into()
}

pub fn address(&self) -> SocketAddrV6 {
self.address_with_port(self.port.into())
}

pub fn address_with_port(&self, port: u16) -> SocketAddrV6 {
SocketAddrV6::new(self.ip(), port, 0, 0)
}

pub fn serial_number(&self) -> &str {
&self.serial_number
}
}

/// A set of constraints that can be placed on operations that select a sled.
#[derive(Debug)]
pub struct SledReservationConstraints {
Expand Down
12 changes: 6 additions & 6 deletions nexus/db-queries/src/db/datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,8 @@ mod test {
use crate::db::model::{
BlockSize, ComponentUpdate, ComponentUpdateIdentity, ConsoleSession,
Dataset, DatasetKind, ExternalIp, PhysicalDisk, PhysicalDiskKind,
Project, Rack, Region, Service, ServiceKind, SiloUser, Sled,
SledBaseboard, SledSystemHardware, SshKey, SystemUpdate,
Project, Rack, Region, Service, ServiceKind, SiloUser, SledBaseboard,
SledSystemHardware, SledUpdate, SshKey, SystemUpdate,
UpdateableComponentType, VpcSubnet, Zpool,
};
use crate::db::queries::vpc_subnet::FilterConflictingVpcSubnetRangesQuery;
Expand Down Expand Up @@ -599,14 +599,14 @@ mod test {
let rack_id = Uuid::new_v4();
let sled_id = Uuid::new_v4();

let sled = Sled::new(
let sled_update = SledUpdate::new(
sled_id,
bogus_addr,
sled_baseboard_for_test(),
sled_system_hardware_for_test(),
rack_id,
);
datastore.sled_upsert(sled).await.unwrap();
datastore.sled_upsert(sled_update).await.unwrap();
sled_id
}

Expand Down Expand Up @@ -1205,7 +1205,7 @@ mod test {
let rack_id = Uuid::new_v4();
let addr1 = "[fd00:1de::1]:12345".parse().unwrap();
let sled1_id = "0de4b299-e0b4-46f0-d528-85de81a7095f".parse().unwrap();
let sled1 = db::model::Sled::new(
let sled1 = db::model::SledUpdate::new(
sled1_id,
addr1,
sled_baseboard_for_test(),
Expand All @@ -1216,7 +1216,7 @@ mod test {

let addr2 = "[fd00:1df::1]:12345".parse().unwrap();
let sled2_id = "66285c18-0c79-43e0-e54f-95271f271314".parse().unwrap();
let sled2 = db::model::Sled::new(
let sled2 = db::model::SledUpdate::new(
sled2_id,
addr2,
sled_baseboard_for_test(),
Expand Down
6 changes: 3 additions & 3 deletions nexus/db-queries/src/db/datastore/physical_disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ mod test {
use crate::db::datastore::test::{
sled_baseboard_for_test, sled_system_hardware_for_test,
};
use crate::db::model::{PhysicalDiskKind, Sled};
use crate::db::model::{PhysicalDiskKind, Sled, SledUpdate};
use dropshot::PaginationOrder;
use nexus_test_utils::db::test_setup_database;
use nexus_types::identity::Asset;
Expand All @@ -153,14 +153,14 @@ mod test {
let sled_id = Uuid::new_v4();
let addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 0, 0, 0);
let rack_id = Uuid::new_v4();
let sled = Sled::new(
let sled_update = SledUpdate::new(
sled_id,
addr,
sled_baseboard_for_test(),
sled_system_hardware_for_test(),
rack_id,
);
db.sled_upsert(sled)
db.sled_upsert(sled_update)
.await
.expect("Could not upsert sled during test prep")
}
Expand Down
6 changes: 3 additions & 3 deletions nexus/db-queries/src/db/datastore/rack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ mod test {
use crate::db::model::Sled;
use async_bb8_diesel::AsyncSimpleConnection;
use internal_params::DnsRecord;
use nexus_db_model::{DnsGroup, InitialDnsGroup};
use nexus_db_model::{DnsGroup, InitialDnsGroup, SledUpdate};
use nexus_test_utils::db::test_setup_database;
use nexus_types::external_api::shared::SiloIdentityMode;
use nexus_types::identity::Asset;
Expand Down Expand Up @@ -870,14 +870,14 @@ mod test {
async fn create_test_sled(db: &DataStore) -> Sled {
let sled_id = Uuid::new_v4();
let addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 0, 0, 0);
let sled = Sled::new(
let sled_update = SledUpdate::new(
sled_id,
addr,
sled_baseboard_for_test(),
sled_system_hardware_for_test(),
rack_id(),
);
db.sled_upsert(sled)
db.sled_upsert(sled_update)
.await
.expect("Could not upsert sled during test prep")
}
Expand Down
62 changes: 36 additions & 26 deletions nexus/db-queries/src/db/datastore/sled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ use crate::db;
use crate::db::error::public_error_from_diesel;
use crate::db::error::ErrorHandler;
use crate::db::error::TransactionError;
use crate::db::identity::Asset;
use crate::db::model::Sled;
use crate::db::model::SledResource;
use crate::db::model::SledUpdate;
use crate::db::pagination::paginated;
use async_bb8_diesel::AsyncConnection;
use async_bb8_diesel::AsyncRunQueryDsl;
Expand All @@ -29,21 +29,25 @@ use uuid::Uuid;

impl DataStore {
/// Stores a new sled in the database.
pub async fn sled_upsert(&self, sled: Sled) -> CreateResult<Sled> {
pub async fn sled_upsert(
&self,
sled_update: SledUpdate,
) -> CreateResult<Sled> {
use db::schema::sled::dsl;
diesel::insert_into(dsl::sled)
.values(sled.clone())
.values(sled_update.clone().into_insertable())
.on_conflict(dsl::id)
.do_update()
.set((
dsl::time_modified.eq(Utc::now()),
dsl::ip.eq(sled.ip),
dsl::port.eq(sled.port),
dsl::rack_id.eq(sled.rack_id),
dsl::is_scrimlet.eq(sled.is_scrimlet()),
dsl::usable_hardware_threads.eq(sled.usable_hardware_threads),
dsl::usable_physical_ram.eq(sled.usable_physical_ram),
dsl::reservoir_size.eq(sled.reservoir_size),
dsl::ip.eq(sled_update.ip),
dsl::port.eq(sled_update.port),
dsl::rack_id.eq(sled_update.rack_id),
dsl::is_scrimlet.eq(sled_update.is_scrimlet()),
dsl::usable_hardware_threads
.eq(sled_update.usable_hardware_threads),
dsl::usable_physical_ram.eq(sled_update.usable_physical_ram),
dsl::reservoir_size.eq(sled_update.reservoir_size),
))
.returning(Sled::as_returning())
.get_result_async(&*self.pool_connection_unauthorized().await?)
Expand All @@ -53,7 +57,7 @@ impl DataStore {
e,
ErrorHandler::Conflict(
ResourceType::Sled,
&sled.id().to_string(),
&sled_update.id().to_string(),
),
)
})
Expand Down Expand Up @@ -241,52 +245,58 @@ mod test {

let sled_id = Uuid::new_v4();
let addr = SocketAddrV6::new(Ipv6Addr::LOCALHOST, 0, 0, 0);
let mut sled = Sled::new(
let mut sled_update = SledUpdate::new(
sled_id,
addr,
sled_baseboard_for_test(),
sled_system_hardware_for_test(),
rack_id(),
);
let observed_sled = datastore
.sled_upsert(sled.clone())
.sled_upsert(sled_update.clone())
.await
.expect("Could not upsert sled during test prep");
assert_eq!(
observed_sled.usable_hardware_threads,
sled.usable_hardware_threads
sled_update.usable_hardware_threads
);
assert_eq!(
observed_sled.usable_physical_ram,
sled_update.usable_physical_ram
);
assert_eq!(observed_sled.usable_physical_ram, sled.usable_physical_ram);
assert_eq!(observed_sled.reservoir_size, sled.reservoir_size);
assert_eq!(observed_sled.reservoir_size, sled_update.reservoir_size);

// Modify the sizes of hardware
sled.usable_hardware_threads =
SqlU32::new(sled.usable_hardware_threads.0 + 1);
sled_update.usable_hardware_threads =
SqlU32::new(sled_update.usable_hardware_threads.0 + 1);
const MIB: u64 = 1024 * 1024;
sled.usable_physical_ram = ByteCount::from(
sled_update.usable_physical_ram = ByteCount::from(
external::ByteCount::try_from(
sled.usable_physical_ram.0.to_bytes() + MIB,
sled_update.usable_physical_ram.0.to_bytes() + MIB,
)
.unwrap(),
);
sled.reservoir_size = ByteCount::from(
sled_update.reservoir_size = ByteCount::from(
external::ByteCount::try_from(
sled.reservoir_size.0.to_bytes() + MIB,
sled_update.reservoir_size.0.to_bytes() + MIB,
)
.unwrap(),
);

// Test that upserting the sled propagates those changes to the DB.
let observed_sled = datastore
.sled_upsert(sled.clone())
.sled_upsert(sled_update.clone())
.await
.expect("Could not upsert sled during test prep");
assert_eq!(
observed_sled.usable_hardware_threads,
sled.usable_hardware_threads
sled_update.usable_hardware_threads
);
assert_eq!(
observed_sled.usable_physical_ram,
sled_update.usable_physical_ram
);
assert_eq!(observed_sled.usable_physical_ram, sled.usable_physical_ram);
assert_eq!(observed_sled.reservoir_size, sled.reservoir_size);
assert_eq!(observed_sled.reservoir_size, sled_update.reservoir_size);

db.cleanup().await.unwrap();
logctx.cleanup_successful();
Expand Down
2 changes: 1 addition & 1 deletion nexus/src/app/sled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl super::Nexus {
SledRole::Scrimlet => true,
};

let sled = db::model::Sled::new(
let sled = db::model::SledUpdate::new(
id,
info.sa_address,
db::model::SledBaseboard {
Expand Down

0 comments on commit e3e99ee

Please sign in to comment.