From 86b0ae678a9b31d5f0d2b3915ca558b847ee0a0e Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Tue, 3 Sep 2024 16:15:56 -0700 Subject: [PATCH] actually we can just leave it null for now --- nexus/db-model/src/instance.rs | 9 ++++++--- nexus/db-model/src/instance_auto_restart.rs | 6 ------ nexus/db-model/src/schema.rs | 2 +- schema/crdb/dbinit.sql | 2 +- .../turn-boot-on-fault-into-auto-restart/README.adoc | 11 ++++------- .../turn-boot-on-fault-into-auto-restart/up03.sql | 2 +- .../turn-boot-on-fault-into-auto-restart/up04.sql | 2 +- .../turn-boot-on-fault-into-auto-restart/up05.sql | 1 - 8 files changed, 14 insertions(+), 21 deletions(-) delete mode 100644 schema/crdb/turn-boot-on-fault-into-auto-restart/up05.sql diff --git a/nexus/db-model/src/instance.rs b/nexus/db-model/src/instance.rs index 512500d15d..8388611231 100644 --- a/nexus/db-model/src/instance.rs +++ b/nexus/db-model/src/instance.rs @@ -58,9 +58,10 @@ pub struct Instance { /// The auto-restart policy for this instance. /// /// This indicates whether the instance should be automatically restarted by - /// the control plane on failure. + /// the control plane on failure. If this is `NULL`, no auto-restart policy + /// has been configured for this instance by the user. #[diesel(column_name = auto_restart_policy)] - pub auto_restart_policy: InstanceAutoRestart, + pub auto_restart_policy: Option, #[diesel(embed)] pub runtime_state: InstanceRuntimeState, @@ -109,7 +110,9 @@ impl Instance { ncpus: params.ncpus.into(), memory: params.memory.into(), hostname: params.hostname.to_string(), - auto_restart_policy: InstanceAutoRestart::default(), + // TODO(eliza): allow this to be configured via the instance-create + // params... + auto_restart_policy: None, runtime_state, updater_gen: Generation::new(), diff --git a/nexus/db-model/src/instance_auto_restart.rs b/nexus/db-model/src/instance_auto_restart.rs index 5ffb4538f8..ae546a4690 100644 --- a/nexus/db-model/src/instance_auto_restart.rs +++ b/nexus/db-model/src/instance_auto_restart.rs @@ -32,12 +32,6 @@ impl InstanceAutoRestart { } } -impl Default for InstanceAutoRestart { - fn default() -> Self { - Self::Never - } -} - impl fmt::Display for InstanceAutoRestart { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.label()) diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index fa6dd1fec7..733a0657bc 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -407,7 +407,7 @@ table! { ncpus -> Int8, memory -> Int8, hostname -> Text, - auto_restart_policy -> crate::InstanceAutoRestartEnum, + auto_restart_policy -> Nullable, time_state_updated -> Timestamptz, state_generation -> Int8, active_propolis_id -> Nullable, diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 212346c8eb..388bc81993 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -1101,7 +1101,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.instance ( * What failures should result in an instance being automatically restarted * by the control plane. */ - auto_restart_policy omicron.public.instance_auto_restart NOT NULL, + auto_restart_policy omicron.public.instance_auto_restart, CONSTRAINT vmm_iff_active_propolis CHECK ( ((state = 'vmm') AND (active_propolis_id IS NOT NULL)) OR diff --git a/schema/crdb/turn-boot-on-fault-into-auto-restart/README.adoc b/schema/crdb/turn-boot-on-fault-into-auto-restart/README.adoc index bb021c5544..123807ae39 100644 --- a/schema/crdb/turn-boot-on-fault-into-auto-restart/README.adoc +++ b/schema/crdb/turn-boot-on-fault-into-auto-restart/README.adoc @@ -5,8 +5,8 @@ which is a `bool`, with a new `auto_restart_policy` column, which is an enum (`omicron.public.instance_auto_restart`). The new enum type will allow auto-restart policies other than "always" and "never". Existing instance records are backfilled with the `all_failures` variant of -`instance_auto_restart` if `boot_on_fault` is `true`, or `sled_failures_only` if -`boot_on_fault` is `false`. +`instance_auto_restart` if `boot_on_fault` is `true`. Otherwise, the +auto-restart policy is `NULL`. The migration performs the following operations: @@ -14,8 +14,5 @@ The migration performs the following operations: 2. `up02.sql` adds a (nullable) `auto_restart_policy` column to the `instance` table. 3. `up03.sql` updates instance records by setting `auto_restart_policy` to - `all_failures` if `boot_on_fault` is `true`, or `sled_failures_only` if - `boot_on_fault` is `false`. -4. Now that all instance records have a value for `auto_restart_policy`, - `up04.sql` makes the `auto_restart_policy` column non-null. -5. Finally, `up05.sql` drops the now-defunct `boot_on_fault` column. + `all_failures` if `boot_on_fault` is `true`. +4. Finally, `up04.sql` drops the now-defunct `boot_on_fault` column. diff --git a/schema/crdb/turn-boot-on-fault-into-auto-restart/up03.sql b/schema/crdb/turn-boot-on-fault-into-auto-restart/up03.sql index 98a17781a4..629c624663 100644 --- a/schema/crdb/turn-boot-on-fault-into-auto-restart/up03.sql +++ b/schema/crdb/turn-boot-on-fault-into-auto-restart/up03.sql @@ -1,5 +1,5 @@ SET LOCAL disallow_full_table_scans = off; UPDATE omicron.public.instance SET auto_restart_policy = CASE WHEN boot_on_fault = true THEN 'all_failures' - ELSE 'never' + ELSE NULL END; diff --git a/schema/crdb/turn-boot-on-fault-into-auto-restart/up04.sql b/schema/crdb/turn-boot-on-fault-into-auto-restart/up04.sql index 116e16b528..214cab2873 100644 --- a/schema/crdb/turn-boot-on-fault-into-auto-restart/up04.sql +++ b/schema/crdb/turn-boot-on-fault-into-auto-restart/up04.sql @@ -1 +1 @@ -ALTER TABLE omicron.public.instance ALTER COLUMN auto_restart_policy SET NOT NULL; +ALTER TABLE omicron.public.instance DROP COLUMN IF EXISTS boot_on_fault; diff --git a/schema/crdb/turn-boot-on-fault-into-auto-restart/up05.sql b/schema/crdb/turn-boot-on-fault-into-auto-restart/up05.sql deleted file mode 100644 index 214cab2873..0000000000 --- a/schema/crdb/turn-boot-on-fault-into-auto-restart/up05.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE omicron.public.instance DROP COLUMN IF EXISTS boot_on_fault;