[2/n] [nexus] move background task definitions into a shared crate (#8003)

Author: sunshowers

Cargo.lock

@@ -77,6 +77,7 @@ members = [
     "nexus-sled-agent-shared",
     "nexus/authz-macros",
     "nexus/auth",
+    "nexus/background-task-interface",
     "nexus/db-errors",
     "nexus/db-fixed-data",
     "nexus/db-lookup",
@@ -227,6 +228,7 @@ default-members = [
     "nexus-sled-agent-shared",
     "nexus/authz-macros",
     "nexus/auth",
+    "nexus/background-task-interface",
     "nexus/db-errors",
     "nexus/db-fixed-data",
     "nexus/db-lookup",
@@ -500,6 +502,7 @@ mg-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "8
 ddm-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "8452936a53c3b16e53cbbf4e34e5e59899afc965" }
 multimap = "0.10.0"
 nexus-auth = { path = "nexus/auth" }
+nexus-background-task-interface = { path = "nexus/background-task-interface" }
 nexus-client = { path = "clients/nexus-client" }
 nexus-config = { path = "nexus-config" }
 nexus-db-errors = { path = "nexus/db-errors" }

Cargo.toml

@@ -52,6 +52,7 @@ ipnetwork.workspace = true
 itertools.workspace = true
 lldpd_client.workspace = true
 macaddr.workspace = true
+nexus-background-task-interface.workspace = true
 # Not under "dev-dependencies"; these also need to be implemented for
 # integration tests.
 nexus-config.workspace = true

nexus/Cargo.toml

@@ -0,0 +1,13 @@
+[package]
+name = "nexus-background-task-interface"
+version = "0.1.0"
+edition = "2021"
+license = "MPL-2.0"
+
+[lints]
+workspace = true
+
+[dependencies]
+omicron-workspace-hack.workspace = true
+thiserror.workspace = true
+tokio.workspace = true

nexus/background-task-interface/Cargo.toml

@@ -0,0 +1,95 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+use std::sync::{
+    Arc,
+    atomic::{AtomicBool, Ordering},
+};
+
+use thiserror::Error;
+use tokio::sync::Notify;
+
+/// Activates a background task
+///
+/// For more on what this means, see the documentation at
+/// `nexus/src/app/background/mod.rs`.
+///
+/// Activators are created with [`Activator::new()`] and then wired up to
+/// specific background tasks using Nexus's `Driver::register()`.  If you call
+/// `Activator::activate()` before the activator is wired up to a background
+/// task, then once the Activator _is_ wired up to a task, that task will
+/// immediately be activated.
+///
+/// Activators are designed specifically so they can be created before the
+/// corresponding task has been created and then wired up with just an
+/// `&Activator` (not a `&mut Activator`).  See the
+/// `nexus/src/app/background/mod.rs` documentation for more on why.
+#[derive(Clone)]
+pub struct Activator(Arc<ActivatorInner>);
+
+/// Shared state for an `Activator`.
+struct ActivatorInner {
+    pub(super) notify: Notify,
+    pub(super) wired_up: AtomicBool,
+}
+
+impl Activator {
+    /// Create an activator that is not yet wired up to any background task
+    pub fn new() -> Activator {
+        Self(Arc::new(ActivatorInner {
+            notify: Notify::new(),
+            wired_up: AtomicBool::new(false),
+        }))
+    }
+
+    /// Activate the background task that this Activator has been wired up to
+    ///
+    /// If this Activator has not yet been wired up, then whenever it _is_ wired
+    /// up, that task will be immediately activated.
+    pub fn activate(&self) {
+        self.0.notify.notify_one();
+    }
+
+    /// Sets the task as wired up.
+    ///
+    /// Returns an error if the task was already wired up.
+    pub fn mark_wired_up(&self) -> Result<(), AlreadyWiredUpError> {
+        match self.0.wired_up.compare_exchange(
+            false,
+            true,
+            Ordering::SeqCst,
+            Ordering::SeqCst,
+        ) {
+            Ok(false) => Ok(()),
+            Ok(true) => unreachable!(
+                "on success, the return value is always \
+                 the previous value (false)"
+            ),
+            Err(true) => Err(AlreadyWiredUpError {}),
+            Err(false) => unreachable!(
+                "on failure, the return value is always \
+                 the previous and current value (true)"
+            ),
+        }
+    }
+
+    /// Blocks until the background task that this Activator has been wired up
+    /// to is activated.
+    ///
+    /// If this Activator has not yet been wired up, then whenever it _is_ wired
+    /// up, that task will be immediately activated.
+    pub async fn activated(&self) {
+        debug_assert!(
+            self.0.wired_up.load(Ordering::SeqCst),
+            "nothing should await activation from an activator that hasn't \
+             been wired up"
+        );
+        self.0.notify.notified().await
+    }
+}
+
+/// Indicates that an activator was wired up more than once.
+#[derive(Debug, Error)]
+#[error("activator was already wired up")]
+pub struct AlreadyWiredUpError {}

nexus/background-task-interface/src/activator.rs

@@ -0,0 +1,64 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+use crate::Activator;
+
+/// Interface for activating various background tasks and read data that they
+/// expose to Nexus at-large
+pub struct BackgroundTasks {
+    // Handles to activate specific background tasks
+    pub task_internal_dns_config: Activator,
+    pub task_internal_dns_servers: Activator,
+    pub task_external_dns_config: Activator,
+    pub task_external_dns_servers: Activator,
+    pub task_metrics_producer_gc: Activator,
+    pub task_external_endpoints: Activator,
+    pub task_nat_cleanup: Activator,
+    pub task_bfd_manager: Activator,
+    pub task_inventory_collection: Activator,
+    pub task_support_bundle_collector: Activator,
+    pub task_physical_disk_adoption: Activator,
+    pub task_decommissioned_disk_cleaner: Activator,
+    pub task_phantom_disks: Activator,
+    pub task_blueprint_loader: Activator,
+    pub task_blueprint_executor: Activator,
+    pub task_blueprint_rendezvous: Activator,
+    pub task_crdb_node_id_collector: Activator,
+    pub task_service_zone_nat_tracker: Activator,
+    pub task_switch_port_settings_manager: Activator,
+    pub task_v2p_manager: Activator,
+    pub task_region_replacement: Activator,
+    pub task_region_replacement_driver: Activator,
+    pub task_instance_watcher: Activator,
+    pub task_instance_updater: Activator,
+    pub task_instance_reincarnation: Activator,
+    pub task_service_firewall_propagation: Activator,
+    pub task_abandoned_vmm_reaper: Activator,
+    pub task_vpc_route_manager: Activator,
+    pub task_saga_recovery: Activator,
+    pub task_lookup_region_port: Activator,
+    pub task_region_snapshot_replacement_start: Activator,
+    pub task_region_snapshot_replacement_garbage_collection: Activator,
+    pub task_region_snapshot_replacement_step: Activator,
+    pub task_region_snapshot_replacement_finish: Activator,
+    pub task_tuf_artifact_replication: Activator,
+    pub task_read_only_region_replacement_start: Activator,
+
+    // Handles to activate background tasks that do not get used by Nexus
+    // at-large.  These background tasks are implementation details as far as
+    // the rest of Nexus is concerned.  These handles don't even really need to
+    // be here, but it's convenient.
+    pub task_internal_dns_propagation: Activator,
+    pub task_external_dns_propagation: Activator,
+}
+
+impl BackgroundTasks {
+    /// Activate the specified background task
+    ///
+    /// If the task is currently running, it will be activated again when it
+    /// finishes.
+    pub fn activate(&self, task: &Activator) {
+        task.activate();
+    }
+}

nexus/background-task-interface/src/init.rs

@@ -0,0 +1,17 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Common interface for describint and activating Nexus background tasks.
+//!
+//! This crate defines the [`BackgroundTasks`] type, which lists out all of the
+//! background tasks within Nexus, and provides handles to activate them.
+//!
+//! For more about background tasks, see the documentation at
+//! `nexus/src/app/background/mod.rs`.
+
+mod activator;
+mod init;
+
+pub use activator::*;
+pub use init::*;

nexus/background-task-interface/src/lib.rs

@@ -15,6 +15,7 @@ use futures::FutureExt;
 use futures::StreamExt;
 use futures::future::BoxFuture;
 use futures::stream::FuturesUnordered;
+use nexus_background_task_interface::Activator;
 use nexus_db_queries::context::OpContext;
 use nexus_types::internal_api::views::ActivationReason;
 use nexus_types::internal_api::views::CurrentStatus;
@@ -23,12 +24,8 @@ use nexus_types::internal_api::views::LastResult;
 use nexus_types::internal_api::views::LastResultCompleted;
 use nexus_types::internal_api::views::TaskStatus;
 use std::collections::BTreeMap;
-use std::sync::Arc;
-use std::sync::atomic::AtomicBool;
-use std::sync::atomic::Ordering;
 use std::time::Duration;
 use std::time::Instant;
-use tokio::sync::Notify;
 use tokio::sync::watch;
 use tokio::time::MissedTickBehavior;
 
@@ -118,17 +115,10 @@ impl Driver {
         // requested.  The caller provides their own Activator, which just
         // provides a specific Notify for us to use here.
         let activator = taskdef.activator;
-        if let Err(previous) = activator.0.wired_up.compare_exchange(
-            false,
-            true,
-            Ordering::SeqCst,
-            Ordering::SeqCst,
-        ) {
+        if let Err(error) = activator.mark_wired_up() {
             panic!(
-                "attempted to wire up the same background task handle \
-                 twice (previous \"wired_up\" = {}): currently attempting \
-                 to wire it up to task {:?}",
-                previous, name
+                "{error}: currently attempting to wire it up to task {:?}",
+                name
             );
         }
 
@@ -141,7 +131,7 @@ impl Driver {
         let task_exec = TaskExec::new(
             taskdef.period,
             taskdef.task_impl,
-            Arc::clone(&activator.0),
+            activator.clone(),
             opctx,
             status_tx,
         );
@@ -241,59 +231,6 @@ pub struct TaskDefinition<'a, N: ToString, D: ToString> {
     pub activator: &'a Activator,
 }
 
-/// Activates a background task
-///
-/// See [`crate::app::background`] module-level documentation for more on what
-/// that means.
-///
-/// Activators are created with [`Activator::new()`] and then wired up to
-/// specific background tasks using [`Driver::register()`].  If you call
-/// `Activator::activate()` before the activator is wired up to a background
-/// task, then once the Activator _is_ wired up to a task, that task will
-/// immediately be activated.
-///
-/// Activators are designed specifically so they can be created before the
-/// corresponding task has been created and then wired up with just an
-/// `&Activator` (not a `&mut Activator`).  See the [`super::init`] module-level
-/// documentation for more on why.
-#[derive(Clone)]
-pub struct Activator(Arc<ActivatorInner>);
-
-/// Shared state for an `Activator`.
-struct ActivatorInner {
-    pub(super) notify: Notify,
-    pub(super) wired_up: AtomicBool,
-}
-
-impl Activator {
-    /// Create an activator that is not yet wired up to any background task
-    pub fn new() -> Activator {
-        Self(Arc::new(ActivatorInner {
-            notify: Notify::new(),
-            wired_up: AtomicBool::new(false),
-        }))
-    }
-
-    /// Activate the background task that this Activator has been wired up to
-    ///
-    /// If this Activator has not yet been wired up with [`Driver::register()`],
-    /// then whenever it _is_ wired up, that task will be immediately activated.
-    pub fn activate(&self) {
-        self.0.notify.notify_one();
-    }
-}
-
-impl ActivatorInner {
-    async fn activated(&self) {
-        debug_assert!(
-            self.wired_up.load(Ordering::SeqCst),
-            "nothing should await activation from an activator that hasn't \
-             been wired up"
-        );
-        self.notify.notified().await
-    }
-}
-
 /// Encapsulates state needed by the background tokio task to manage activation
 /// of the background task
 struct TaskExec {
@@ -303,7 +240,7 @@ struct TaskExec {
     imp: Box<dyn BackgroundTask>,
     /// used to receive notifications from the Driver that someone has requested
     /// explicit activation
-    activation: Arc<ActivatorInner>,
+    activation: Activator,
     /// passed through to the background task impl when activated
     opctx: OpContext,
     /// used to send current status back to the Driver
@@ -316,7 +253,7 @@ impl TaskExec {
     fn new(
         period: Duration,
         imp: Box<dyn BackgroundTask>,
-        activation: Arc<ActivatorInner>,
+        activation: Activator,
         opctx: OpContext,
         status_tx: watch::Sender<TaskStatus>,
     ) -> TaskExec {