Review feedback

- mv zb.rs -> zone-bundle.rs
- Add TOML extension to zone bundle metadata file
- Return 404 on bad zone name
- Typos, safety notes, and link to logadm(8)

Author: bnaecker

illumos-utils/src/running_zone.rs

@@ -880,6 +880,9 @@ impl RunningZone {
         // The rotated log files should have the same prefix as the current, but
         // with an index appended. We'll search the parent directory for
         // matching names, skipping the current file.
+        //
+        // See https://illumos.org/man/8/logadm for details on the naming
+        // conventions around these files.
         let dir = current_log_file.parent().unwrap();
         let mut rotated_files = Vec::new();
         for entry in dir.read_dir_utf8()? {

sled-agent/src/bin/zb.rs renamed to sled-agent/src/bin/zone-bundle.rs

@@ -28,7 +28,7 @@ fn parse_log_level(s: &str) -> anyhow::Result<Level> {
 
 /// Operate on sled agent zone bundles.
 ///
-/// Zoneb bundles are the collected state of a service zone. This includes
+/// Zone bundles are the collected state of a service zone. This includes
 /// information about the processes running in the zone, and the system on which
 /// they're running.
 #[derive(Clone, Debug, Parser)]
@@ -107,7 +107,12 @@ async fn main() -> anyhow::Result<()> {
                 .context("failed to list zone bundles")?
                 .into_inner();
             for bundle in bundles {
-                println!("{}/{}", bundle.id.zone_name, bundle.id.bundle_id);
+                println!(
+                    "{}/{} {}",
+                    bundle.id.zone_name,
+                    bundle.id.bundle_id,
+                    bundle.time_created
+                );
             }
         }
         Cmd::Create { zone_name } => {

sled-agent/src/services.rs

@@ -258,6 +258,9 @@ const BUNDLE_DIRECTORY: &str = "bundle";
 // The directory for zone bundles.
 const ZONE_BUNDLE_DIRECTORY: &str = "zone";
 
+// The name for zone bundle metadata files.
+const ZONE_BUNDLE_METADATA_FILENAME: &str = "metadata.toml";
+
 // A wrapper around `ZoneRequest`, which allows it to be serialized
 // to a toml file.
 #[derive(Clone, serde::Serialize, serde::Deserialize)]
@@ -2005,7 +2008,11 @@ impl ServiceManager {
 
         // Write the metadata file itself, in TOML format.
         let contents = toml::to_string(&zone_metadata)?;
-        insert_data(&mut builder, "metadata", contents.as_bytes())?;
+        insert_data(
+            &mut builder,
+            ZONE_BUNDLE_METADATA_FILENAME,
+            contents.as_bytes(),
+        )?;
         debug!(
             log,
             "wrote zone bundle metadata";
@@ -2113,8 +2120,9 @@ impl ServiceManager {
             // Copy any log files, current and rotated, into the tarball as
             // well.
             //
-            // Saftey: This is a log file, so we're sure it's a single, normal
-            // file and thus has a name.
+            // Safety: This pathbuf was retrieved by locating an existing file
+            // on the filesystem, so we're sure it has a name and the unwrap is
+            // safe.
             debug!(
                 log,
                 "appending current log file to zone bundle";
@@ -2232,7 +2240,7 @@ impl ServiceManager {
             .find(|entry| {
                 entry
                     .path()
-                    .map(|p| p.to_str() == Some("metadata"))
+                    .map(|p| p.to_str() == Some(ZONE_BUNDLE_METADATA_FILENAME))
                     .unwrap_or(false)
             })
         else {

sled-agent/src/sled_agent.rs

@@ -149,7 +149,7 @@ impl From<Error> for dropshot::HttpError {
                     HttpError::for_unavail(None, inner.to_string())
                 }
                 crate::services::BundleError::NoSuchZone { .. } => {
-                    HttpError::for_bad_request(None, inner.to_string())
+                    HttpError::for_not_found(None, inner.to_string())
                 }
                 _ => HttpError::for_internal_error(err.to_string()),
             },