Background read-only region creation

Cargo.toml

@@ -42,6 +42,7 @@ resolver = "2"
 # crates.io
 aes-gcm-siv = "0.11.1"
 anyhow = "1"
+async-channel = "2.5.0"
 async-recursion = "1.1.1"
 async-trait = "0.1.89"
 atty = "0.2.14"

agent/Cargo.toml

@@ -6,6 +6,7 @@ edition = "2024"
 
 [dependencies]
 anyhow.workspace = true
+async-channel.workspace = true
 chrono.workspace = true
 clap.workspace = true
 crucible-agent-api.workspace = true

agent/src/datafile.rs

@@ -1,6 +1,6 @@
 // Copyright 2021 Oxide Computer Company
 
-use anyhow::{Result, anyhow, bail};
+use anyhow::{Result, bail};
 use crucible_agent_types::region::CreateRegion;
 use crucible_agent_types::region::RegionId;
 use crucible_agent_types::snapshot::CreateRunningSnapshotRequest;
@@ -9,8 +9,9 @@ use crucible_agent_types::snapshot::DeleteSnapshotRequest;
 use crucible_agent_types::snapshot::Snapshot;
 use crucible_common::write_json;
 use crucible_smf::scf_type_t::*;
+use dropshot::HttpError;
 use serde::{Deserialize, Serialize};
-use slog::{Logger, crit, error, info};
+use slog::{Logger, crit, error, info, warn};
 use std::collections::BTreeMap;
 use std::net::SocketAddr;
 use std::path::Path;
@@ -38,6 +39,8 @@ pub struct DataFile {
 #[derive(Debug, PartialEq, Clone)]
 pub enum RegionState {
     Requested,
+    /// Region creation is occuring in the background
+    Creating,
     Created,
     Tombstoned,
     Destroyed,
@@ -50,6 +53,24 @@ impl From<RegionState> for crucible_agent_types::region::State {
             RegionState::Requested => {
                 crucible_agent_types::region::State::Requested
             }
+            RegionState::Creating => {
+                // The Crucible agent will only read the data file on startup,
+                // either after being updated or after crashing. There's a few
+                // considerations here that all conclude that Creating should be
+                // converted here to Requested:
+                //
+                // If the region was in state Creating, then background creation
+                // of that region was occurring when the update or crash
+                // occurred, and the Agent should start again from the
+                // beginning. Starting from the beginning is equivalent by
+                // running the worker thread and starting again from Requested.
+                //
+                // If this From path is being invoked as part of an API
+                // response, then users of the API only care that the Region is
+                // Requested and not yet in state Created, as they have always,
+                // not what the Agent is doing behind the scenes.
+                crucible_agent_types::region::State::Requested
+            }
             RegionState::Created => {
                 crucible_agent_types::region::State::Created
             }
@@ -945,7 +966,17 @@ impl DataFile {
                             return Ok(());
                         }
 
-                        RegionState::Requested | RegionState::Created => {
+                        RegionState::Requested | RegionState::Creating => {
+                            // According to the agent's datafile, the region was
+                            // requested and may exist. According to zfs list,
+                            // it does not yet. How was a snapshot taken?
+                            bail!(
+                                "region {} not found in zfs list! {e}",
+                                request.id.0
+                            );
+                        }
+
+                        RegionState::Created => {
                             // This is a bug: according to the agent's datafile,
                             // the region exists, but according to zfs list, it
                             // does not
@@ -1045,14 +1076,16 @@ impl DataFile {
         let r = inner.regions.get_mut(id).unwrap();
         let nstate = RegionState::Created;
         match &r.state {
-            RegionState::Requested => (),
+            RegionState::Requested | RegionState::Creating => (),
+
             RegionState::Tombstoned => {
                 /*
                  * Nexus requested that we destroy this region before we
                  * finished provisioning it.
                  */
                 return Ok(());
             }
+
             x => bail!("created region in weird state {:?}", x),
         }
 
@@ -1147,8 +1180,10 @@ impl DataFile {
         let r = inner.regions.get_mut(id).unwrap();
         let nstate = RegionState::Destroyed;
         match &r.state {
-            RegionState::Requested => (),
+            RegionState::Requested | RegionState::Creating => (),
+
             RegionState::Tombstoned => (),
+
             x => bail!("region to destroy in weird state {:?}", x),
         }
 
@@ -1200,13 +1235,15 @@ impl DataFile {
     /**
      * Nexus has requested that we destroy this particular region.
      */
-    pub fn destroy(&self, id: &RegionId) -> Result<()> {
+    pub fn destroy(&self, id: &RegionId) -> Result<(), HttpError> {
         let mut inner = self.inner.lock().unwrap();
 
-        let r = inner
-            .regions
-            .get_mut(id)
-            .ok_or_else(|| anyhow!("region {} does not exist", id.0))?;
+        let r = inner.regions.get_mut(id).ok_or_else(|| {
+            HttpError::for_not_found(
+                None,
+                format!("region {} does not exist", id.0),
+            )
+        })?;
 
         match r.state {
             RegionState::Tombstoned | RegionState::Destroyed => {
@@ -1216,6 +1253,24 @@ impl DataFile {
                  * - Already destroyed; no more work to do.
                  */
             }
+
+            RegionState::Creating => {
+                // `Creating` means that some processing is occurring in the
+                // background in a region creation thread, and if we set this
+                // region's state to `Tombstoned` and allow the worker thread to
+                // operate on it, this would conflict and lead to chaos.
+                // Restrict this with a 503.
+
+                let m = format!(
+                    "cannot destroy region {:?} in state {:?}",
+                    r.id.0, r.state,
+                );
+
+                warn!(self.log, "{m}");
+
+                return Err(HttpError::for_unavail(None, m));
+            }
+
             RegionState::Requested
             | RegionState::Created
             | RegionState::Failed => {
@@ -1305,6 +1360,7 @@ impl DataFile {
 
         match region.state {
             RegionState::Requested
+            | RegionState::Creating
             | RegionState::Destroyed
             | RegionState::Tombstoned
             | RegionState::Failed => {
@@ -1336,6 +1392,27 @@ impl DataFile {
 
         Ok(results)
     }
+
+    /**
+     * Mark a particular region as provisioning in the background.
+     */
+    pub fn creating(&self, id: &RegionId) {
+        let mut inner = self.inner.lock().unwrap();
+
+        let r = inner.regions.get_mut(id).unwrap();
+        let nstate = RegionState::Creating;
+        if r.state == nstate {
+            return;
+        }
+
+        info!(
+            self.log,
+            "region {} state: {:?} -> {:?}", r.id.0, r.state, nstate,
+        );
+        r.state = nstate;
+
+        self.store(inner);
+    }
 }
 
 #[cfg(test)]