[nexus] Having children shouldn't prevent reincarnation

See #8178 (comment)

Author: hawkw

nexus/src/app/sagas/instance_update/mod.rs

@@ -342,7 +342,7 @@
 
 use super::{
     ACTION_GENERATE_ID, ActionRegistry, NexusActionContext, NexusSaga,
-    SagaInitError,
+    SagaContext, SagaInitError,
 };
 use crate::app::db::datastore::InstanceGestalt;
 use crate::app::db::datastore::VmmStateUpdateResult;
@@ -1307,37 +1307,7 @@ async fn siu_chain_successor_saga(
                 "instance_id" => %instance_id,
             );
         } else {
-            // If the instance has transitioned to the `Failed` state and no
-            // additional update saga is required, check if the instance's
-            // auto-restart policy allows it to be automatically restarted. If
-            // it does, activate the instance-reincarnation background task to
-            // automatically restart it.
-            let karmic_state = new_state
-                .instance
-                .auto_restart_status(new_state.active_vmm.as_ref());
-            if karmic_state.should_reincarnate() {
-                info!(
-                    log,
-                    "instance update: instance transitioned to Failed, \
-                     but can be automatically restarted; activating \
-                     reincarnation.";
-                    "instance_id" => %instance_id,
-                    "auto_restart_config" => ?new_state.instance.auto_restart,
-                    "runtime_state" => ?new_state.instance.runtime_state,
-                    "intended_state" => %new_state.instance.intended_state,
-                );
-                nexus.background_tasks.task_instance_reincarnation.activate();
-            } else {
-                debug!(
-                    log,
-                    "instance update: instance will not reincarnate";
-                    "instance_id" => %instance_id,
-                    "auto_restart_config" => ?new_state.instance.auto_restart,
-                    "needs_reincarnation" => karmic_state.needs_reincarnation,
-                    "karmic_state" => ?karmic_state.can_reincarnate,
-                    "intended_state" => %new_state.instance.intended_state,
-                )
-            }
+            reincarnate_if_needed(osagactx, &new_state)
         }
 
         Ok::<(), anyhow::Error>(())
@@ -1361,6 +1331,43 @@ async fn siu_chain_successor_saga(
     Ok(())
 }
 
+fn reincarnate_if_needed(osagactx: &SagaContext, state: &InstanceGestalt) {
+    // If the instance has transitioned to the `Failed` state and no
+    // additional update saga is required, check if the instance's
+    // auto-restart policy allows it to be automatically restarted. If
+    // it does, activate the instance-reincarnation background task to
+    // automatically restart it.
+    let karmic_state =
+        state.instance.auto_restart_status(state.active_vmm.as_ref());
+    if karmic_state.should_reincarnate() {
+        info!(
+            &osagactx.log(),
+            "instance update: instance transitioned to Failed, \
+             but can be automatically restarted; activating \
+             reincarnation.";
+            "instance_id" => %state.instance.id(),
+            "auto_restart_config" => ?state.instance.auto_restart,
+            "runtime_state" => ?state.instance.runtime_state,
+            "intended_state" => %state.instance.intended_state,
+        );
+        osagactx
+            .nexus()
+            .background_tasks
+            .task_instance_reincarnation
+            .activate();
+    } else {
+        debug!(
+            &osagactx.log(),
+            "instance update: instance will not reincarnate";
+            "instance_id" => %state.instance.id(),
+            "auto_restart_config" => ?state.instance.auto_restart,
+            "needs_reincarnation" => karmic_state.needs_reincarnation,
+            "karmic_state" => ?karmic_state.can_reincarnate,
+            "intended_state" => %state.instance.intended_state,
+        )
+    }
+}
+
 /// Unlock the instance record while unwinding.
 ///
 /// This is factored out of the actual reverse action, because the `Params` type

nexus/src/app/sagas/instance_update/start.rs

@@ -302,6 +302,18 @@ async fn siu_fetch_state_and_start_real_saga(
             .instance_updater_unlock(&opctx, &authz_instance, &orig_lock)
             .await
             .map_err(ActionError::action_failed)?;
+        // If we're releasing the lock, check if we should activate the
+        // instance reincarnation background task to reincarnate this
+        // instance. A previous activation may have not been able to
+        // reincarnate this instance because we held the lock, so poking the
+        // reincarnation task here should help ensure it sees a failed instance
+        // in a timely manner.
+        //
+        // This doesn't matter a whole lot in production systems, where the task
+        // will activate periodically regardless, but it should make the tests
+        // less flakey, and hopefully also decrease the latency with which
+        // failed instances are reincarnated a bit, maybe?
+        super::reincarnate_if_needed(osagactx, &state);
     }
 
     Ok(())