From 95ebbb87174c84c57eb38bdd8bfc698a7c23475b Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 7 Aug 2024 15:35:52 -0700 Subject: [PATCH] review feedback --- illumos-utils/src/zfs.rs | 22 ++++++++++++++++++++-- openapi/sled-agent.json | 2 ++ sled-agent/src/http_entrypoints.rs | 4 +++- sled-agent/src/sled_agent.rs | 4 ++-- sled-storage/src/manager.rs | 12 +++++++----- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/illumos-utils/src/zfs.rs b/illumos-utils/src/zfs.rs index c9fd7fa315..5df1b73c07 100644 --- a/illumos-utils/src/zfs.rs +++ b/illumos-utils/src/zfs.rs @@ -260,9 +260,27 @@ impl Zfs { Ok(()) } - /// Creates a new ZFS filesystem named `name`, unless one already exists. + /// Creates a new ZFS filesystem unless one already exists. /// - /// Applies an optional quota, provided _in bytes_. + /// - `name`: the full path to the zfs dataset + /// - `mountpoint`: The expected mountpoint of this filesystem. + /// If the filesystem already exists, and is not mounted here, and error is + /// returned. + /// - `zoned`: identifies whether or not this filesystem should be + /// used in a zone. Only used when creating a new filesystem - ignored + /// if the filesystem already exists. + /// - `do_format`: if "false", prevents a new filesystem from being created, + /// and returns an error if it is not found. + /// - `encryption_details`: Ensures a filesystem as an encryption root. + /// For new filesystems, this supplies the key, and all datasets within this + /// root are implicitly encrypted. For existing filesystems, ensures that + /// they are mounted (and that keys are loaded), but does not verify the + /// input details. + /// - `size_details`: If supplied, sets size-related information. These + /// values are set on both new filesystem creation as well as when loading + /// existing filesystems. + /// - `additional_options`: Additional ZFS options, which are only set when + /// creating new filesystems. #[allow(clippy::too_many_arguments)] pub fn ensure_filesystem( name: &str, diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index 173a1c8058..c9e5c709e3 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -179,6 +179,7 @@ }, "/datasets": { "get": { + "summary": "Lists the datasets that this sled is configured to use", "operationId": "datasets_get", "responses": { "200": { @@ -200,6 +201,7 @@ } }, "put": { + "summary": "Configures datasets to be used on this sled", "operationId": "datasets_put", "requestBody": { "content": { diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs index c36de9a787..97671b42e6 100644 --- a/sled-agent/src/http_entrypoints.rs +++ b/sled-agent/src/http_entrypoints.rs @@ -352,6 +352,7 @@ async fn omicron_zones_get( Ok(HttpResponseOk(sa.omicron_zones_list().await?)) } +/// Configures datasets to be used on this sled #[endpoint { method = PUT, path = "/datasets", @@ -366,6 +367,7 @@ async fn datasets_put( Ok(HttpResponseOk(result)) } +/// Lists the datasets that this sled is configured to use #[endpoint { method = GET, path = "/datasets", @@ -374,7 +376,7 @@ async fn datasets_get( rqctx: RequestContext, ) -> Result, HttpError> { let sa = rqctx.context(); - Ok(HttpResponseOk(sa.datasets_list().await?)) + Ok(HttpResponseOk(sa.datasets_config_list().await?)) } #[endpoint { diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index 5368539445..29cf4c6de2 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -810,8 +810,8 @@ impl SledAgent { self.inner.zone_bundler.cleanup().await.map_err(Error::from) } - pub async fn datasets_list(&self) -> Result { - Ok(self.storage().datasets_list().await?) + pub async fn datasets_config_list(&self) -> Result { + Ok(self.storage().datasets_config_list().await?) } pub async fn datasets_ensure( diff --git a/sled-storage/src/manager.rs b/sled-storage/src/manager.rs index 3b32b36c09..c086d656d1 100644 --- a/sled-storage/src/manager.rs +++ b/sled-storage/src/manager.rs @@ -273,7 +273,7 @@ impl StorageHandle { /// Reads the last value written to storage by /// [Self::datasets_ensure]. - pub async fn datasets_list(&self) -> Result { + pub async fn datasets_config_list(&self) -> Result { let (tx, rx) = oneshot::channel(); self.tx .send(StorageRequest::DatasetsList { tx: tx.into() }) @@ -479,7 +479,7 @@ impl StorageManager { let _ = tx.0.send(self.datasets_ensure(config).await); } StorageRequest::DatasetsList { tx } => { - let _ = tx.0.send(self.datasets_list().await); + let _ = tx.0.send(self.datasets_config_list().await); } StorageRequest::OmicronPhysicalDisksEnsure { config, tx } => { let _ = @@ -790,8 +790,9 @@ impl StorageManager { status } - async fn datasets_list(&mut self) -> Result { - let log = self.log.new(o!("request" => "datasets_list")); + // Lists datasets that this sled is configured to use. + async fn datasets_config_list(&mut self) -> Result { + let log = self.log.new(o!("request" => "datasets_config_list")); let ledger_paths = self.all_omicron_dataset_ledgers().await; let maybe_ledger = @@ -1637,7 +1638,8 @@ mod tests { assert!(!status.has_error()); // List datasets, expect to see what we just created - let observed_config = harness.handle().datasets_list().await.unwrap(); + let observed_config = + harness.handle().datasets_config_list().await.unwrap(); assert_eq!(config, observed_config); // Calling "datasets_ensure" with the same input should succeed.