Skip to content

Commit

Permalink
Split instance state enum into instance/VMM state enums (#5854)
Browse files Browse the repository at this point in the history 
Split the existing `instance_state` enum in the CRDB schema into
separate instance state and VMM state enums and remove unused enum
variants. Instances now have five states: Creating, NoVmm, Vmm, Failed,
and Destroyed. VMMs have most of the states they had before, except that
the unused Creating and Repairing states have been removed. This change
makes it easier to add new states (e.g. SagaUnwound, see #5848) that
apply only to instances/VMMs without having to update code that's
working with the other type of DB record.

Update the routines the IP attach/detach sagas use to decide where (if
anywhere) to dispatch IP change messages. These routines were looking
for instance states that weren't actually being used (the states of
interest appear in the instances' active VMMs instead).

Add a data migration test to make sure that the Stopped and Running
instance states are converted as expected.

Tests: `cargo nextest`.
  • Loading branch information
@gjcolombo
gjcolombo authored Jun 5, 2024
1 parent dbcc754 commit e488d57
Show file tree
Hide file tree
Showing 60 changed files with 1,043 additions and 607 deletions.
80 changes: 27 additions & 53 deletions clients/nexus-client/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,34 @@ impl From<types::DiskState> for omicron_common::api::external::DiskState {
}
}

impl From<types::InstanceState>
for omicron_common::api::external::InstanceState
{
fn from(s: types::InstanceState) -> Self {
impl From<omicron_common::api::internal::nexus::VmmState> for types::VmmState {
fn from(s: omicron_common::api::internal::nexus::VmmState) -> Self {
use omicron_common::api::internal::nexus::VmmState as Input;
match s {
types::InstanceState::Creating => Self::Creating,
types::InstanceState::Starting => Self::Starting,
types::InstanceState::Running => Self::Running,
types::InstanceState::Stopping => Self::Stopping,
types::InstanceState::Stopped => Self::Stopped,
types::InstanceState::Rebooting => Self::Rebooting,
types::InstanceState::Migrating => Self::Migrating,
types::InstanceState::Repairing => Self::Repairing,
types::InstanceState::Failed => Self::Failed,
types::InstanceState::Destroyed => Self::Destroyed,
Input::Starting => types::VmmState::Starting,
Input::Running => types::VmmState::Running,
Input::Stopping => types::VmmState::Stopping,
Input::Stopped => types::VmmState::Stopped,
Input::Rebooting => types::VmmState::Rebooting,
Input::Migrating => types::VmmState::Migrating,
Input::Failed => types::VmmState::Failed,
Input::Destroyed => types::VmmState::Destroyed,
}
}
}

impl From<types::VmmState> for omicron_common::api::internal::nexus::VmmState {
fn from(s: types::VmmState) -> Self {
use omicron_common::api::internal::nexus::VmmState as Output;
match s {
types::VmmState::Starting => Output::Starting,
types::VmmState::Running => Output::Running,
types::VmmState::Stopping => Output::Stopping,
types::VmmState::Stopped => Output::Stopped,
types::VmmState::Rebooting => Output::Rebooting,
types::VmmState::Migrating => Output::Migrating,
types::VmmState::Failed => Output::Failed,
types::VmmState::Destroyed => Output::Destroyed,
}
}
}
Expand Down Expand Up @@ -140,26 +153,6 @@ impl From<omicron_common::api::internal::nexus::SledInstanceState>
}
}

impl From<omicron_common::api::external::InstanceState>
for types::InstanceState
{
fn from(s: omicron_common::api::external::InstanceState) -> Self {
use omicron_common::api::external::InstanceState;
match s {
InstanceState::Creating => Self::Creating,
InstanceState::Starting => Self::Starting,
InstanceState::Running => Self::Running,
InstanceState::Stopping => Self::Stopping,
InstanceState::Stopped => Self::Stopped,
InstanceState::Rebooting => Self::Rebooting,
InstanceState::Migrating => Self::Migrating,
InstanceState::Repairing => Self::Repairing,
InstanceState::Failed => Self::Failed,
InstanceState::Destroyed => Self::Destroyed,
}
}
}

impl From<omicron_common::api::internal::nexus::DiskRuntimeState>
for types::DiskRuntimeState
{
Expand Down Expand Up @@ -192,25 +185,6 @@ impl From<omicron_common::api::external::DiskState> for types::DiskState {
}
}

impl From<&types::InstanceState>
for omicron_common::api::external::InstanceState
{
fn from(state: &types::InstanceState) -> Self {
match state {
types::InstanceState::Creating => Self::Creating,
types::InstanceState::Starting => Self::Starting,
types::InstanceState::Running => Self::Running,
types::InstanceState::Stopping => Self::Stopping,
types::InstanceState::Stopped => Self::Stopped,
types::InstanceState::Rebooting => Self::Rebooting,
types::InstanceState::Migrating => Self::Migrating,
types::InstanceState::Repairing => Self::Repairing,
types::InstanceState::Failed => Self::Failed,
types::InstanceState::Destroyed => Self::Destroyed,
}
}
}

impl From<omicron_common::api::internal::nexus::ProducerKind>
for types::ProducerKind
{
Expand Down
62 changes: 27 additions & 35 deletions clients/sled-agent-client/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,22 +257,18 @@ impl From<omicron_common::api::internal::nexus::InstanceRuntimeState>
}
}

impl From<omicron_common::api::external::InstanceState>
for types::InstanceState
{
fn from(s: omicron_common::api::external::InstanceState) -> Self {
use omicron_common::api::external::InstanceState::*;
impl From<omicron_common::api::internal::nexus::VmmState> for types::VmmState {
fn from(s: omicron_common::api::internal::nexus::VmmState) -> Self {
use omicron_common::api::internal::nexus::VmmState as Input;
match s {
Creating => Self::Creating,
Starting => Self::Starting,
Running => Self::Running,
Stopping => Self::Stopping,
Stopped => Self::Stopped,
Rebooting => Self::Rebooting,
Migrating => Self::Migrating,
Repairing => Self::Repairing,
Failed => Self::Failed,
Destroyed => Self::Destroyed,
Input::Starting => types::VmmState::Starting,
Input::Running => types::VmmState::Running,
Input::Stopping => types::VmmState::Stopping,
Input::Stopped => types::VmmState::Stopped,
Input::Rebooting => types::VmmState::Rebooting,
Input::Migrating => types::VmmState::Migrating,
Input::Failed => types::VmmState::Failed,
Input::Destroyed => types::VmmState::Destroyed,
}
}
}
Expand All @@ -299,6 +295,22 @@ impl From<types::InstanceRuntimeState>
}
}

impl From<types::VmmState> for omicron_common::api::internal::nexus::VmmState {
fn from(s: types::VmmState) -> Self {
use omicron_common::api::internal::nexus::VmmState as Output;
match s {
types::VmmState::Starting => Output::Starting,
types::VmmState::Running => Output::Running,
types::VmmState::Stopping => Output::Stopping,
types::VmmState::Stopped => Output::Stopped,
types::VmmState::Rebooting => Output::Rebooting,
types::VmmState::Migrating => Output::Migrating,
types::VmmState::Failed => Output::Failed,
types::VmmState::Destroyed => Output::Destroyed,
}
}
}

impl From<types::VmmRuntimeState>
for omicron_common::api::internal::nexus::VmmRuntimeState
{
Expand All @@ -319,26 +331,6 @@ impl From<types::SledInstanceState>
}
}

impl From<types::InstanceState>
for omicron_common::api::external::InstanceState
{
fn from(s: types::InstanceState) -> Self {
use types::InstanceState::*;
match s {
Creating => Self::Creating,
Starting => Self::Starting,
Running => Self::Running,
Stopping => Self::Stopping,
Stopped => Self::Stopped,
Rebooting => Self::Rebooting,
Migrating => Self::Migrating,
Repairing => Self::Repairing,
Failed => Self::Failed,
Destroyed => Self::Destroyed,
}
}
}

impl From<types::InstanceCpuCount>
for omicron_common::api::external::InstanceCpuCount
{
Expand Down
16 changes: 16 additions & 0 deletions common/src/api/external/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,22 @@ pub enum InstanceState {
Destroyed,
}

impl From<crate::api::internal::nexus::VmmState> for InstanceState {
fn from(state: crate::api::internal::nexus::VmmState) -> Self {
use crate::api::internal::nexus::VmmState as InternalVmmState;
match state {
InternalVmmState::Starting => Self::Starting,
InternalVmmState::Running => Self::Running,
InternalVmmState::Stopping => Self::Stopping,
InternalVmmState::Stopped => Self::Stopped,
InternalVmmState::Rebooting => Self::Rebooting,
InternalVmmState::Migrating => Self::Migrating,
InternalVmmState::Failed => Self::Failed,
InternalVmmState::Destroyed => Self::Destroyed,
}
}
}

impl Display for InstanceState {
fn fmt(&self, f: &mut Formatter) -> FormatResult {
write!(f, "{}", self.label())
Expand Down
29 changes: 27 additions & 2 deletions common/src/api/internal/nexus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use crate::api::external::{
ByteCount, DiskState, Generation, Hostname, InstanceCpuCount,
InstanceState, SemverVersion, Vni,
SemverVersion, Vni,
};
use chrono::{DateTime, Utc};
use omicron_uuid_kinds::DownstairsRegionKind;
Expand Down Expand Up @@ -60,11 +60,36 @@ pub struct InstanceRuntimeState {
pub time_updated: DateTime<Utc>,
}

/// One of the states that a VMM can be in.
#[derive(
Copy, Clone, Debug, Deserialize, Serialize, JsonSchema, Eq, PartialEq,
)]
#[serde(rename_all = "snake_case")]
pub enum VmmState {
/// The VMM is initializing and has not started running guest CPUs yet.
Starting,
/// The VMM has finished initializing and may be running guest CPUs.
Running,
/// The VMM is shutting down.
Stopping,
/// The VMM's guest has stopped, and the guest will not run again, but the
/// VMM process may not have released all of its resources yet.
Stopped,
/// The VMM is being restarted or its guest OS is rebooting.
Rebooting,
/// The VMM is part of a live migration.
Migrating,
/// The VMM process reported an internal failure.
Failed,
/// The VMM process has been destroyed and its resources have been released.
Destroyed,
}

/// The dynamic runtime properties of an individual VMM process.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
pub struct VmmRuntimeState {
/// The last state reported by this VMM.
pub state: InstanceState,
pub state: VmmState,
/// The generation number for this VMM's state.
pub gen: Generation,
/// Timestamp for the VMM's state.
Expand Down
24 changes: 11 additions & 13 deletions nexus/db-model/src/instance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ impl Instance {
InstanceIdentity::new(instance_id, params.identity.clone());

let runtime_state = InstanceRuntimeState::new(
InstanceState::new(
omicron_common::api::external::InstanceState::Creating,
),
InstanceState::Creating,
identity.time_modified,
);

Expand Down Expand Up @@ -138,13 +136,6 @@ impl DatastoreAttachTargetConfig<ExternalIp> for Instance {
// `diesel::prelude::AsChangeset`.
#[diesel(table_name = instance, treat_none_as_null = true)]
pub struct InstanceRuntimeState {
/// The instance state to fall back on if asked to compute this instance's
/// state while it has no active VMM.
///
/// This field is guarded by the instance's `gen` field.
#[diesel(column_name = state)]
pub nexus_state: InstanceState,

/// The time at which the runtime state was last updated. This is distinct
/// from the time the record was last modified, because some updates don't
/// modify the runtime state.
Expand Down Expand Up @@ -197,6 +188,13 @@ pub struct InstanceRuntimeState {
/// lock was not held is still valid when setting the lock ID.
#[diesel(column_name = updater_gen)]
pub updater_gen: Generation,

/// The "internal" state of this instance. The instance's externally-visible
/// state may be delegated to the instance's active VMM, if it has one.
///
/// This field is guarded by the instance's `gen` field.
#[diesel(column_name = state)]
pub nexus_state: InstanceState,
}

impl InstanceRuntimeState {
Expand All @@ -221,13 +219,13 @@ impl From<omicron_common::api::internal::nexus::InstanceRuntimeState>
state: omicron_common::api::internal::nexus::InstanceRuntimeState,
) -> Self {
let nexus_state = if state.propolis_id.is_some() {
omicron_common::api::external::InstanceState::Running
InstanceState::Vmm
} else {
omicron_common::api::external::InstanceState::Stopped
InstanceState::NoVmm
};

Self {
nexus_state: InstanceState::new(nexus_state),
nexus_state,
time_updated: state.time_updated,
gen: state.gen.into(),
propolis_id: state.propolis_id,
Expand Down
Loading

0 comments on commit e488d57

Please sign in to comment.