Skip to content

Commit

Permalink
Begin using typed instance and Propolis UUIDs in sled agent and Nexus (
Browse files Browse the repository at this point in the history
…#5896)

One goal of the ongoing Great Instance State Rework of 2024 is to
clarify that, in broad strokes, Nexus manages instances and sled agents
manage VMMs. Nexus decides whether and when an instance is connected to
a VMM and how to convert VMM states to instance states; sled agents
manage individual Propolis processes and send their state changes to
Nexus so that it has the relevant VMM states available.

Today, sled agent's instance manager tracks VMMs in a map with type
`BTreeMap<Uuid, (Uuid, Instance)>`, where the keys are instance IDs and
the values contain Propolis IDs. In our proposed brave new world, sled
agent can use Propolis IDs as keys and may not need instance IDs at
all--i.e., its map type should be `BTreeMap<PropolisUuid, Vmm>`, where
the `Vmm` may or may not contain an `InstanceUuid`.

To support this future change, change sled agent's instance map to a
`BTreeMap<InstanceUuid, (PropolisUuid, Instance)>` and work through all
the compiler-error fallout. This causes the difference between instance
and Propolis UUIDs to filter all the way up through Nexus's instance
management routines and sagas. This should make it much easier to
refactor sled agent's APIs to take Propolis IDs, since the compiler can
then help catch instance and Propolis ID transpositions.

Nexus's instance management code often deals with instance IDs, Propolis
IDs, and sled IDs together, so also start using `SledUuid` in some of
these paths.

This change draws an arbitrary line at using typed UUIDs in database
model types and in the generated resource objects in the authz layer;
calls into/out of these layers use `GenericUuid` and
`from_untyped_uuid`/`into_untyped_uuid` as needed.(This is not because
these changes are necessarily impossible, merely because the line had to
go somewhere.)

Tests: `cargo nextest`; while this change is large, it should by and
large be a rote refactoring exercise.
  • Loading branch information
gjcolombo authored Jun 17, 2024
1 parent 7d8cb15 commit 50e353f
Show file tree
Hide file tree
Showing 57 changed files with 992 additions and 732 deletions.
1 change: 1 addition & 0 deletions clients/nexus-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ progenitor::generate_api!(
NewPasswordHash = omicron_passwords::NewPasswordHash,
TypedUuidForCollectionKind = omicron_uuid_kinds::CollectionUuid,
TypedUuidForDownstairsKind = omicron_uuid_kinds::TypedUuid<omicron_uuid_kinds::DownstairsKind>,
TypedUuidForPropolisKind = omicron_uuid_kinds::TypedUuid<omicron_uuid_kinds::PropolisKind>,
TypedUuidForSledKind = omicron_uuid_kinds::TypedUuid<omicron_uuid_kinds::SledKind>,
TypedUuidForUpstairsKind = omicron_uuid_kinds::TypedUuid<omicron_uuid_kinds::UpstairsKind>,
TypedUuidForUpstairsRepairKind = omicron_uuid_kinds::TypedUuid<omicron_uuid_kinds::UpstairsRepairKind>,
Expand Down
2 changes: 2 additions & 0 deletions clients/sled-agent-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ progenitor::generate_api!(
PortSpeed = omicron_common::api::internal::shared::PortSpeed,
SourceNatConfig = omicron_common::api::internal::shared::SourceNatConfig,
SwitchLocation = omicron_common::api::external::SwitchLocation,
TypedUuidForInstanceKind = omicron_uuid_kinds::InstanceUuid,
TypedUuidForPropolisKind = omicron_uuid_kinds::PropolisUuid,
TypedUuidForZpoolKind = omicron_uuid_kinds::ZpoolUuid,
Vni = omicron_common::api::external::Vni,
ZpoolKind = omicron_common::zpool_name::ZpoolKind,
Expand Down
7 changes: 4 additions & 3 deletions common/src/api/internal/nexus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::api::external::{
};
use chrono::{DateTime, Utc};
use omicron_uuid_kinds::DownstairsRegionKind;
use omicron_uuid_kinds::PropolisUuid;
use omicron_uuid_kinds::TypedUuid;
use omicron_uuid_kinds::UpstairsRepairKind;
use omicron_uuid_kinds::UpstairsSessionKind;
Expand Down Expand Up @@ -50,9 +51,9 @@ pub struct InstanceProperties {
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
pub struct InstanceRuntimeState {
/// The instance's currently active VMM ID.
pub propolis_id: Option<Uuid>,
pub propolis_id: Option<PropolisUuid>,
/// If a migration is active, the ID of the target VMM.
pub dst_propolis_id: Option<Uuid>,
pub dst_propolis_id: Option<PropolisUuid>,
/// If a migration is active, the ID of that migration.
pub migration_id: Option<Uuid>,
/// Generation number for this state.
Expand Down Expand Up @@ -105,7 +106,7 @@ pub struct SledInstanceState {
pub instance_state: InstanceRuntimeState,

/// The ID of the VMM whose state is being reported.
pub propolis_id: Uuid,
pub propolis_id: PropolisUuid,

/// The most recent state of the sled's VMM process.
pub vmm_state: VmmRuntimeState,
Expand Down
19 changes: 13 additions & 6 deletions dev-tools/omdb/src/bin/omdb/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ use omicron_common::api::external::InstanceState;
use omicron_common::api::external::MacAddr;
use omicron_uuid_kinds::CollectionUuid;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::PropolisUuid;
use omicron_uuid_kinds::SledUuid;
use sled_agent_client::types::VolumeConstructionRequest;
use std::borrow::Cow;
Expand All @@ -129,12 +130,18 @@ use uuid::Uuid;
const NO_ACTIVE_PROPOLIS_MSG: &str = "<no active Propolis>";
const NOT_ON_SLED_MSG: &str = "<not on any sled>";

struct MaybePropolisId(Option<Uuid>);
struct MaybeSledId(Option<Uuid>);
struct MaybePropolisId(Option<PropolisUuid>);
struct MaybeSledId(Option<SledUuid>);

impl From<&InstanceAndActiveVmm> for MaybePropolisId {
fn from(value: &InstanceAndActiveVmm) -> Self {
Self(value.instance().runtime().propolis_id)
Self(
value
.instance()
.runtime()
.propolis_id
.map(PropolisUuid::from_untyped_uuid),
)
}
}

Expand Down Expand Up @@ -1040,7 +1047,7 @@ async fn cmd_db_disk_info(
let my_sled_id = instance.sled_id().unwrap();

let (_, my_sled) = LookupPath::new(opctx, datastore)
.sled_id(my_sled_id)
.sled_id(my_sled_id.into_untyped_uuid())
.fetch()
.await
.context("failed to look up sled")?;
Expand Down Expand Up @@ -1948,15 +1955,15 @@ async fn cmd_db_instances(
check_limit(&instances, limit, ctx);

let mut rows = Vec::new();
let mut h_to_s: HashMap<Uuid, String> = HashMap::new();
let mut h_to_s: HashMap<SledUuid, String> = HashMap::new();

for i in instances {
let host_serial = if i.vmm().is_some() {
if let std::collections::hash_map::Entry::Vacant(e) =
h_to_s.entry(i.sled_id().unwrap())
{
let (_, my_sled) = LookupPath::new(opctx, datastore)
.sled_id(i.sled_id().unwrap())
.sled_id(i.sled_id().unwrap().into_untyped_uuid())
.fetch()
.await
.context("failed to look up sled")?;
Expand Down
21 changes: 14 additions & 7 deletions nexus/db-model/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::schema::{disk, external_ip, instance};
use chrono::{DateTime, Utc};
use db_macros::Resource;
use nexus_types::external_api::params;
use omicron_uuid_kinds::{GenericUuid, InstanceUuid, PropolisUuid};
use serde::Deserialize;
use serde::Serialize;
use uuid::Uuid;
Expand Down Expand Up @@ -82,12 +83,14 @@ impl Instance {
/// Constructs a new instance record with no VMM that will initially appear
/// to be in the Creating state.
pub fn new(
instance_id: Uuid,
instance_id: InstanceUuid,
project_id: Uuid,
params: &params::InstanceCreate,
) -> Self {
let identity =
InstanceIdentity::new(instance_id, params.identity.clone());
let identity = InstanceIdentity::new(
instance_id.into_untyped_uuid(),
params.identity.clone(),
);

let runtime_state = InstanceRuntimeState::new(
InstanceState::Creating,
Expand Down Expand Up @@ -229,8 +232,10 @@ impl From<omicron_common::api::internal::nexus::InstanceRuntimeState>
nexus_state,
time_updated: state.time_updated,
gen: state.gen.into(),
propolis_id: state.propolis_id,
dst_propolis_id: state.dst_propolis_id,
propolis_id: state.propolis_id.map(|id| id.into_untyped_uuid()),
dst_propolis_id: state
.dst_propolis_id
.map(|id| id.into_untyped_uuid()),
migration_id: state.migration_id,
}
}
Expand All @@ -241,10 +246,12 @@ impl From<InstanceRuntimeState>
{
fn from(state: InstanceRuntimeState) -> Self {
Self {
dst_propolis_id: state.dst_propolis_id,
dst_propolis_id: state
.dst_propolis_id
.map(PropolisUuid::from_untyped_uuid),
gen: state.gen.into(),
migration_id: state.migration_id,
propolis_id: state.propolis_id,
propolis_id: state.propolis_id.map(PropolisUuid::from_untyped_uuid),
time_updated: state.time_updated,
}
}
Expand Down
5 changes: 3 additions & 2 deletions nexus/db-model/src/network_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use nexus_types::external_api::params;
use nexus_types::identity::Resource;
use omicron_common::api::{external, internal};
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::InstanceUuid;
use omicron_uuid_kinds::OmicronZoneUuid;
use omicron_uuid_kinds::VnicUuid;
use sled_agent_client::ZoneKind;
Expand Down Expand Up @@ -391,15 +392,15 @@ impl IncompleteNetworkInterface {

pub fn new_instance(
interface_id: Uuid,
instance_id: Uuid,
instance_id: InstanceUuid,
subnet: VpcSubnet,
identity: external::IdentityMetadataCreateParams,
ip: Option<std::net::IpAddr>,
) -> Result<Self, external::Error> {
Self::new(
interface_id,
NetworkInterfaceKind::Instance,
instance_id,
instance_id.into_untyped_uuid(),
subnet,
identity,
ip,
Expand Down
13 changes: 7 additions & 6 deletions nexus/db-model/src/vmm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use super::{Generation, VmmState};
use crate::schema::vmm;
use crate::SqlU16;
use chrono::{DateTime, Utc};
use omicron_uuid_kinds::{GenericUuid, InstanceUuid, PropolisUuid, SledUuid};
use serde::{Deserialize, Serialize};
use uuid::Uuid;

Expand Down Expand Up @@ -68,9 +69,9 @@ pub enum VmmInitialState {
impl Vmm {
/// Creates a new VMM record.
pub fn new(
id: Uuid,
instance_id: Uuid,
sled_id: Uuid,
id: PropolisUuid,
instance_id: InstanceUuid,
sled_id: SledUuid,
propolis_ip: ipnetwork::IpNetwork,
propolis_port: u16,
initial_state: VmmInitialState,
Expand All @@ -82,11 +83,11 @@ impl Vmm {
};

Self {
id,
id: id.into_untyped_uuid(),
time_created: now,
time_deleted: None,
instance_id,
sled_id,
instance_id: instance_id.into_untyped_uuid(),
sled_id: sled_id.into_untyped_uuid(),
propolis_ip,
propolis_port: SqlU16(propolis_port),
runtime: VmmRuntimeState {
Expand Down
Loading

0 comments on commit 50e353f

Please sign in to comment.