Skip to content

Commit ba551b0

Browse files
hawkwhawkw
committed
add InstanceState::SagaUnwound, effective states
I *really* hope I've gotten this right...
1 parent a58c14e commit ba551b0

File tree

13 files changed

+242
-168
lines changed

13 files changed

+242
-168
lines changed

‎nexus/db-model/src/instance_state.rs

Lines changed: 91 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,20 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
44

5-
use super::impl_enum_wrapper;
5+
use super::impl_enum_type;
66
use omicron_common::api::external;
77
use serde::Deserialize;
88
use serde::Serialize;
99
use std::fmt;
10-
use std::io::Write;
1110

12-
impl_enum_wrapper!(
11+
impl_enum_type!(
1312
#[derive(SqlType, Debug)]
1413
#[diesel(postgres_type(name = "instance_state", schema = "public"))]
1514
pub struct InstanceStateEnum;
1615

17-
#[derive(Clone, Debug, PartialEq, AsExpression, FromSqlRow, Serialize, Deserialize)]
16+
#[derive(Copy, Clone, Debug, PartialEq, AsExpression, FromSqlRow, Serialize, Deserialize)]
1817
#[diesel(sql_type = InstanceStateEnum)]
19-
pub struct InstanceState(pub external::InstanceState);
18+
pub enum InstanceState;
2019

2120
// Enum values
2221
Creating => b"creating"
@@ -29,46 +28,116 @@ impl_enum_wrapper!(
2928
Repairing => b"repairing"
3029
Failed => b"failed"
3130
Destroyed => b"destroyed"
31+
SagaUnwound => b"saga_unwound"
3232
);
3333

3434
impl InstanceState {
3535
pub fn new(state: external::InstanceState) -> Self {
36-
Self(state)
36+
Self::from(state)
3737
}
3838

39-
pub fn state(&self) -> &external::InstanceState {
40-
&self.0
39+
pub fn state(&self) -> external::InstanceState {
40+
external::InstanceState::from(*self)
41+
}
42+
43+
pub fn label(&self) -> &'static str {
44+
match self {
45+
InstanceState::Creating => "creating",
46+
InstanceState::Starting => "starting",
47+
InstanceState::Running => "running",
48+
InstanceState::Stopping => "stopping",
49+
InstanceState::Stopped => "stopped",
50+
InstanceState::Rebooting => "rebooting",
51+
InstanceState::Migrating => "migrating",
52+
InstanceState::Repairing => "repairing",
53+
InstanceState::Failed => "failed",
54+
InstanceState::Destroyed => "destroyed",
55+
InstanceState::SagaUnwound => "saga_unwound",
56+
}
4157
}
4258
}
4359

4460
impl fmt::Display for InstanceState {
4561
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
46-
write!(f, "{}", self.0)
62+
write!(f, "{}", self.label())
4763
}
4864
}
4965

5066
impl From<InstanceState> for sled_agent_client::types::InstanceState {
5167
fn from(s: InstanceState) -> Self {
52-
use external::InstanceState::*;
5368
use sled_agent_client::types::InstanceState as Output;
54-
match s.0 {
55-
Creating => Output::Creating,
56-
Starting => Output::Starting,
57-
Running => Output::Running,
58-
Stopping => Output::Stopping,
59-
Stopped => Output::Stopped,
60-
Rebooting => Output::Rebooting,
61-
Migrating => Output::Migrating,
62-
Repairing => Output::Repairing,
63-
Failed => Output::Failed,
64-
Destroyed => Output::Destroyed,
69+
match s {
70+
InstanceState::Creating => Output::Creating,
71+
InstanceState::Starting => Output::Starting,
72+
InstanceState::Running => Output::Running,
73+
InstanceState::Stopping => Output::Stopping,
74+
InstanceState::Stopped => Output::Stopped,
75+
InstanceState::Rebooting => Output::Rebooting,
76+
InstanceState::Migrating => Output::Migrating,
77+
InstanceState::Repairing => Output::Repairing,
78+
InstanceState::Failed => Output::Failed,
79+
InstanceState::Destroyed | InstanceState::SagaUnwound => {
80+
Output::Destroyed
81+
}
6582
}
6683
}
6784
}
6885

6986
impl From<external::InstanceState> for InstanceState {
7087
fn from(state: external::InstanceState) -> Self {
71-
Self::new(state)
88+
match state {
89+
external::InstanceState::Creating => InstanceState::Creating,
90+
external::InstanceState::Starting => InstanceState::Starting,
91+
external::InstanceState::Running => InstanceState::Running,
92+
external::InstanceState::Stopping => InstanceState::Stopping,
93+
external::InstanceState::Stopped => InstanceState::Stopped,
94+
external::InstanceState::Rebooting => InstanceState::Rebooting,
95+
external::InstanceState::Migrating => InstanceState::Migrating,
96+
external::InstanceState::Repairing => InstanceState::Repairing,
97+
external::InstanceState::Failed => InstanceState::Failed,
98+
external::InstanceState::Destroyed => InstanceState::Destroyed,
99+
}
100+
}
101+
}
102+
103+
impl From<InstanceState> for external::InstanceState {
104+
fn from(state: InstanceState) -> Self {
105+
match state {
106+
InstanceState::Creating => external::InstanceState::Creating,
107+
InstanceState::Starting => external::InstanceState::Starting,
108+
InstanceState::Running => external::InstanceState::Running,
109+
InstanceState::Stopping => external::InstanceState::Stopping,
110+
InstanceState::Stopped => external::InstanceState::Stopped,
111+
InstanceState::Rebooting => external::InstanceState::Rebooting,
112+
InstanceState::Migrating => external::InstanceState::Migrating,
113+
InstanceState::Repairing => external::InstanceState::Repairing,
114+
InstanceState::Failed => external::InstanceState::Failed,
115+
InstanceState::Destroyed | InstanceState::SagaUnwound => {
116+
external::InstanceState::Destroyed
117+
}
118+
}
119+
}
120+
}
121+
122+
impl PartialEq<external::InstanceState> for InstanceState {
123+
fn eq(&self, other: &external::InstanceState) -> bool {
124+
match (self, other) {
125+
(InstanceState::Creating, external::InstanceState::Creating)
126+
| (InstanceState::Starting, external::InstanceState::Starting)
127+
| (InstanceState::Running, external::InstanceState::Running)
128+
| (InstanceState::Stopping, external::InstanceState::Stopping)
129+
| (InstanceState::Stopped, external::InstanceState::Stopped)
130+
| (InstanceState::Rebooting, external::InstanceState::Rebooting)
131+
| (InstanceState::Migrating, external::InstanceState::Migrating)
132+
| (InstanceState::Repairing, external::InstanceState::Repairing)
133+
| (InstanceState::Failed, external::InstanceState::Failed)
134+
| (InstanceState::Destroyed, external::InstanceState::Destroyed)
135+
| (
136+
InstanceState::SagaUnwound,
137+
external::InstanceState::Destroyed,
138+
) => true,
139+
_ => false,
140+
}
72141
}
73142
}
74143

‎nexus/db-model/src/sled_instance.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ impl From<SledInstance> for views::SledInstance {
3434
active_sled_id: sled_instance.active_sled_id,
3535
silo_name: sled_instance.silo_name.into(),
3636
project_name: sled_instance.project_name.into(),
37-
state: *sled_instance.state.state(),
37+
state: sled_instance.state.state(),
3838
migration_id: sled_instance.migration_id,
3939
ncpus: sled_instance.ncpus,
4040
memory: sled_instance.memory,

‎nexus/db-queries/src/db/datastore/disk.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ impl DataStore {
183183
//
184184
// We currently only permit attaching disks to stopped instances.
185185
let ok_to_attach_instance_states = vec![
186-
db::model::InstanceState(api::external::InstanceState::Creating),
187-
db::model::InstanceState(api::external::InstanceState::Stopped),
186+
db::model::InstanceState::Creating,
187+
db::model::InstanceState::Stopped,
188188
];
189189

190190
let attach_update = DiskSetClauseForAttach::new(authz_instance.id());
@@ -321,9 +321,9 @@ impl DataStore {
321321
//
322322
// We currently only permit detaching disks from stopped instances.
323323
let ok_to_detach_instance_states = vec![
324-
db::model::InstanceState(api::external::InstanceState::Creating),
325-
db::model::InstanceState(api::external::InstanceState::Stopped),
326-
db::model::InstanceState(api::external::InstanceState::Failed),
324+
db::model::InstanceState::Creating,
325+
db::model::InstanceState::Stopped,
326+
db::model::InstanceState::Failed,
327327
];
328328

329329
let detached_label = api::external::DiskState::Detached.label();

‎nexus/db-queries/src/db/datastore/instance.rs

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use crate::db::lookup::LookupPath;
2121
use crate::db::model::Generation;
2222
use crate::db::model::Instance;
2323
use crate::db::model::InstanceRuntimeState;
24+
use crate::db::model::InstanceState;
2425
use crate::db::model::Name;
2526
use crate::db::model::Project;
2627
use crate::db::model::Sled;
@@ -30,13 +31,14 @@ use crate::db::update_and_check::UpdateAndCheck;
3031
use crate::db::update_and_check::UpdateAndQueryResult;
3132
use crate::db::update_and_check::UpdateStatus;
3233
use async_bb8_diesel::AsyncRunQueryDsl;
33-
use chrono::Utc;
34+
use chrono::{DateTime, Utc};
3435
use diesel::prelude::*;
3536
use nexus_db_model::ApplySledFilterExt;
3637
use nexus_db_model::Disk;
3738
use nexus_db_model::VmmRuntimeState;
3839
use nexus_types::deployment::SledFilter;
3940
use omicron_common::api;
41+
use omicron_common::api::external;
4042
use omicron_common::api::external::http_pagination::PaginatedBy;
4143
use omicron_common::api::external::CreateResult;
4244
use omicron_common::api::external::DataPageParams;
@@ -70,13 +72,39 @@ impl InstanceAndActiveVmm {
7072
self.vmm.as_ref().map(|v| v.sled_id)
7173
}
7274

73-
pub fn effective_state(
74-
&self,
75-
) -> omicron_common::api::external::InstanceState {
76-
if let Some(vmm) = &self.vmm {
77-
vmm.runtime.state.0
78-
} else {
79-
self.instance.runtime().nexus_state.0
75+
pub fn effective_state(&self) -> external::InstanceState {
76+
Self::determine_effective_state(&self.instance, &self.vmm).0
77+
}
78+
79+
pub fn determine_effective_state(
80+
instance: &Instance,
81+
vmm: &Option<Vmm>,
82+
) -> (external::InstanceState, DateTime<Utc>) {
83+
match vmm.as_ref().map(|v| &v.runtime) {
84+
// If an active VMM is `Stopped`, the instance must be recast as
85+
// `Stopping`, as no start saga can proceed until the active VMM has
86+
// been `Destroyed`.
87+
Some(VmmRuntimeState {
88+
state: InstanceState::Stopped,
89+
time_state_updated,
90+
..
91+
}) => (external::InstanceState::Stopping, *time_state_updated),
92+
// If the active VMM is `SagaUnwound`, then the instance should be
93+
// treated as `Stopped`.
94+
Some(VmmRuntimeState {
95+
state: InstanceState::SagaUnwound,
96+
time_state_updated,
97+
..
98+
}) => (external::InstanceState::Stopped, *time_state_updated),
99+
// If the active VMM is `SagaUnwound`, then the instance should be
100+
// treated as `Stopped`.
101+
Some(VmmRuntimeState { state, time_state_updated, .. }) => {
102+
(external::InstanceState::from(*state), *time_state_updated)
103+
}
104+
None => (
105+
external::InstanceState::Stopped,
106+
instance.runtime_state.time_updated,
107+
),
80108
}
81109
}
82110
}
@@ -89,15 +117,11 @@ impl From<(Instance, Option<Vmm>)> for InstanceAndActiveVmm {
89117

90118
impl From<InstanceAndActiveVmm> for omicron_common::api::external::Instance {
91119
fn from(value: InstanceAndActiveVmm) -> Self {
92-
let (run_state, time_run_state_updated) = if let Some(vmm) = value.vmm {
93-
(vmm.runtime.state, vmm.runtime.time_state_updated)
94-
} else {
95-
(
96-
value.instance.runtime_state.nexus_state.clone(),
97-
value.instance.runtime_state.time_updated,
98-
)
99-
};
100-
120+
let (run_state, time_run_state_updated) =
121+
InstanceAndActiveVmm::determine_effective_state(
122+
&value.instance,
123+
&value.vmm,
124+
);
101125
Self {
102126
identity: value.instance.identity(),
103127
project_id: value.instance.project_id,
@@ -109,7 +133,7 @@ impl From<InstanceAndActiveVmm> for omicron_common::api::external::Instance {
109133
.parse()
110134
.expect("found invalid hostname in the database"),
111135
runtime: omicron_common::api::external::InstanceRuntimeState {
112-
run_state: *run_state.state(),
136+
run_state,
113137
time_run_state_updated,
114138
},
115139
}
@@ -208,7 +232,7 @@ impl DataStore {
208232

209233
bail_unless!(
210234
instance.runtime().nexus_state.state()
211-
== &api::external::InstanceState::Creating,
235+
== api::external::InstanceState::Creating,
212236
"newly-created Instance has unexpected state: {:?}",
213237
instance.runtime().nexus_state
214238
);

‎nexus/db-queries/src/db/datastore/network_interface.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,6 @@ impl DataStore {
713713
self.transaction_retry_wrapper("instance_update_network_interface")
714714
.transaction(&conn, |conn| {
715715
let err = err.clone();
716-
let stopped = stopped.clone();
717716
let update_target_query = update_target_query.clone();
718717
async move {
719718
let instance_runtime =
@@ -759,7 +758,6 @@ impl DataStore {
759758
self.transaction_retry_wrapper("instance_update_network_interface")
760759
.transaction(&conn, |conn| {
761760
let err = err.clone();
762-
let stopped = stopped.clone();
763761
let update_target_query = update_target_query.clone();
764762
async move {
765763
let instance_state =

‎nexus/db-queries/src/db/queries/external_ip.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,32 +31,29 @@ use nexus_db_model::IpAttachState;
3131
use nexus_db_model::IpAttachStateEnum;
3232
use omicron_common::address::NUM_SOURCE_NAT_PORTS;
3333
use omicron_common::api::external;
34-
use omicron_common::api::external::InstanceState as ApiInstanceState;
3534
use uuid::Uuid;
3635

3736
// Broadly, we want users to be able to attach/detach at will
3837
// once an instance is created and functional.
3938
pub const SAFE_TO_ATTACH_INSTANCE_STATES_CREATING: [DbInstanceState; 3] = [
40-
DbInstanceState(ApiInstanceState::Stopped),
41-
DbInstanceState(ApiInstanceState::Running),
42-
DbInstanceState(ApiInstanceState::Creating),
43-
];
44-
pub const SAFE_TO_ATTACH_INSTANCE_STATES: [DbInstanceState; 2] = [
45-
DbInstanceState(ApiInstanceState::Stopped),
46-
DbInstanceState(ApiInstanceState::Running),
39+
DbInstanceState::Stopped,
40+
DbInstanceState::Running,
41+
DbInstanceState::Creating,
4742
];
43+
pub const SAFE_TO_ATTACH_INSTANCE_STATES: [DbInstanceState; 2] =
44+
[DbInstanceState::Stopped, DbInstanceState::Running];
4845
// If we're in a state which will naturally resolve to either
4946
// stopped/running, we want users to know that the request can be
5047
// retried safely via Error::unavail.
5148
// TODO: We currently stop if there's a migration or other state change.
5249
// There may be a good case for RPWing
5350
// external_ip_state -> { NAT RPW, sled-agent } in future.
5451
pub const SAFE_TRANSIENT_INSTANCE_STATES: [DbInstanceState; 5] = [
55-
DbInstanceState(ApiInstanceState::Starting),
56-
DbInstanceState(ApiInstanceState::Stopping),
57-
DbInstanceState(ApiInstanceState::Creating),
58-
DbInstanceState(ApiInstanceState::Rebooting),
59-
DbInstanceState(ApiInstanceState::Migrating),
52+
DbInstanceState::Starting,
53+
DbInstanceState::Stopping,
54+
DbInstanceState::Creating,
55+
DbInstanceState::Rebooting,
56+
DbInstanceState::Migrating,
6057
];
6158

6259
/// The maximum number of disks that can be attached to an instance.

‎nexus/db-queries/src/db/queries/network_interface.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,25 +42,25 @@ use uuid::Uuid;
4242
// States an instance must be in to operate on its network interfaces, in
4343
// most situations.
4444
static INSTANCE_STOPPED: Lazy<db::model::InstanceState> =
45-
Lazy::new(|| db::model::InstanceState(external::InstanceState::Stopped));
45+
Lazy::new(|| db::model::InstanceState::Stopped);
4646

4747
static INSTANCE_FAILED: Lazy<db::model::InstanceState> =
48-
Lazy::new(|| db::model::InstanceState(external::InstanceState::Failed));
48+
Lazy::new(|| db::model::InstanceState::Failed);
4949

5050
// An instance can be in the creating state while we manipulate its
5151
// interfaces. The intention is for this only to be the case during sagas.
5252
static INSTANCE_CREATING: Lazy<db::model::InstanceState> =
53-
Lazy::new(|| db::model::InstanceState(external::InstanceState::Creating));
53+
Lazy::new(|| db::model::InstanceState::Creating);
5454

5555
// A sentinel value for the instance state when the instance actually does
5656
// not exist.
5757
static INSTANCE_DESTROYED: Lazy<db::model::InstanceState> =
58-
Lazy::new(|| db::model::InstanceState(external::InstanceState::Destroyed));
58+
Lazy::new(|| db::model::InstanceState::Destroyed);
5959

6060
// A sentinel value for the instance state when the instance has an active
6161
// VMM, irrespective of that VMM's actual state.
6262
static INSTANCE_RUNNING: Lazy<db::model::InstanceState> =
63-
Lazy::new(|| db::model::InstanceState(external::InstanceState::Running));
63+
Lazy::new(|| db::model::InstanceState::Running);
6464

6565
static NO_INSTANCE_SENTINEL_STRING: Lazy<String> =
6666
Lazy::new(|| String::from(NO_INSTANCE_SENTINEL));

0 commit comments

Comments
 (0)