diff --git a/Cargo.lock b/Cargo.lock index ca21716527..b2a65c4c77 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8124,6 +8124,7 @@ dependencies = [ "internal-dns-types", "ipnetwork", "itertools 0.14.0", + "jiff", "lldpd-client", "macaddr", "maplit", @@ -12724,9 +12725,11 @@ dependencies = [ "anyhow", "camino", "cfg-if", + "chrono", "fs-err 3.1.1", "futures", "illumos-utils", + "jiff", "libc", "omicron-common", "omicron-test-utils", @@ -16525,6 +16528,7 @@ dependencies = [ "crc32fast", "flate2", "indexmap 2.11.4", + "jiff", "memchr", "zopfli", "zstd", diff --git a/nexus/Cargo.toml b/nexus/Cargo.toml index 7c752b11cf..b3d97b73da 100644 --- a/nexus/Cargo.toml +++ b/nexus/Cargo.toml @@ -53,6 +53,7 @@ internal-dns-resolver.workspace = true internal-dns-types.workspace = true ipnetwork.workspace = true itertools.workspace = true +jiff.workspace = true lldpd_client.workspace = true macaddr.workspace = true maplit.workspace = true @@ -144,7 +145,7 @@ update-common.workspace = true update-engine.workspace = true omicron-workspace-hack.workspace = true omicron-uuid-kinds.workspace = true -zip.workspace = true +zip = { workspace = true, features = ["jiff-02"] } [dev-dependencies] async-bb8-diesel.workspace = true diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index 8dc13e7ab4..8c35cc0395 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -1241,10 +1241,23 @@ fn recursively_add_directory_to_zipfile( let file_type = entry.file_type()?; if file_type.is_file() { + let src = entry.path(); + + let zip_time = entry + .path() + .metadata() + .and_then(|m| m.modified()) + .ok() + .and_then(|sys_time| jiff::Zoned::try_from(sys_time).ok()) + .and_then(|zoned| { + zip::DateTime::try_from(zoned.datetime()).ok() + }) + .unwrap_or_else(zip::DateTime::default); + let opts = FullFileOptions::default() + .last_modified_time(zip_time) .compression_method(zip::CompressionMethod::Deflated) .large_file(true); - let src = entry.path(); zip.start_file_from_path(dst, opts)?; let mut file = std::fs::File::open(&src)?; diff --git a/sled-diagnostics/Cargo.toml b/sled-diagnostics/Cargo.toml index 12f4387b18..40ebc882ec 100644 --- a/sled-diagnostics/Cargo.toml +++ b/sled-diagnostics/Cargo.toml @@ -10,9 +10,11 @@ workspace = true anyhow.workspace = true camino.workspace = true cfg-if.workspace = true +chrono.workspace = true fs-err = { workspace = true, features = ["tokio"] } futures.workspace = true illumos-utils.workspace = true +jiff.workspace = true libc.workspace = true omicron-workspace-hack.workspace = true once_cell.workspace = true @@ -26,7 +28,7 @@ slog.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["full"] } zfs-test-harness.workspace = true -zip = { workspace = true, features = ["zstd"] } +zip = { workspace = true, features = ["jiff-02","zstd"] } [dev-dependencies] omicron-common.workspace = true diff --git a/sled-diagnostics/src/logs.rs b/sled-diagnostics/src/logs.rs index cc66b16fd6..2ad0970d5a 100644 --- a/sled-diagnostics/src/logs.rs +++ b/sled-diagnostics/src/logs.rs @@ -451,11 +451,11 @@ impl LogsHandle { service: &str, zip: &mut zip::ZipWriter, log_snapshots: &mut LogSnapshots, - logfile: &Utf8Path, + logfile: &LogFile, logtype: LogType, ) -> Result<(), LogError> { let snapshot_logfile = - self.find_log_in_snapshot(log_snapshots, logfile).await?; + self.find_log_in_snapshot(log_snapshots, &logfile.path).await?; if logtype == LogType::Current { // Since we are processing the current log files in a zone we need @@ -501,6 +501,16 @@ impl LogsHandle { .filter(|f| is_log_file(f.path(), filename)) { let logfile = f.path(); + let system_mtime = + f.metadata().and_then(|m| m.modified()).inspect_err(|e| { + warn!(&self.log, "sled-diagnostic failed to get mtime of logfile"; + "error" => %e, + "logfile" => %logfile, + ); + }).ok(); + let mtime = system_mtime + .and_then(|m| jiff::Timestamp::try_from(m).ok()); + if logfile.is_file() { write_log_to_zip( &self.log, @@ -508,6 +518,7 @@ impl LogsHandle { zip, LogType::Current, logfile, + mtime, )?; } } @@ -522,6 +533,7 @@ impl LogsHandle { zip, logtype, &snapshot_logfile, + logfile.modified, )?; } false => { @@ -612,7 +624,7 @@ impl LogsHandle { &service, &mut zip, &mut log_snapshots, - ¤t.path, + ¤t, LogType::Current, ) .await?; @@ -628,13 +640,13 @@ impl LogsHandle { .archived .into_iter() .filter(|log| log.path.as_str().contains("crypt/debug")) - .map(|log| log.path) .collect(); // Since these logs can be spread out across multiple U.2 devices // we need to sort them by timestamp. archived.sort_by_key(|log| { - log.as_str() + log.path + .as_str() .rsplit_once(".") .and_then(|(_, date)| date.parse::().ok()) .unwrap_or(0) @@ -703,6 +715,7 @@ fn write_log_to_zip( zip: &mut zip::ZipWriter, logtype: LogType, snapshot_logfile: &Utf8Path, + mtime: Option, ) -> Result<(), LogError> { let Some(log_name) = snapshot_logfile.file_name() else { warn!( @@ -715,10 +728,19 @@ fn write_log_to_zip( }; let mut src = File::open(&snapshot_logfile)?; + + let zip_mtime = mtime + .and_then(|ts| { + let zoned = ts.in_tz("UTC").ok()?; + zip::DateTime::try_from(zoned.datetime()).ok() + }) + .unwrap_or_else(zip::DateTime::default); + let zip_path = format!("{service}/{logtype}/{log_name}"); zip.start_file_from_path( zip_path, FullFileOptions::default() + .last_modified_time(zip_mtime) .compression_method(zip::CompressionMethod::Zstd) .compression_level(Some(3)) // NB: From the docs @@ -757,8 +779,8 @@ enum ExtraLogKind<'a> { #[derive(Debug, Default, PartialEq)] struct ExtraLogs<'a> { - current: Option<&'a Utf8Path>, - rotated: Vec<&'a Utf8Path>, + current: Option<&'a LogFile>, + rotated: Vec<&'a LogFile>, } fn sort_extra_logs<'a>( @@ -780,15 +802,15 @@ fn sort_extra_logs<'a>( warn!( logger, "found multiple current log files for {name}"; - "old" => %old_path, + "old" => %old_path.path, "new" => %log.path, ); } - entry.current = Some(&log.path); + entry.current = Some(&log); } ExtraLogKind::Rotated { name, log } => { let entry = res.entry(name).or_default(); - entry.rotated.push(&log.path); + entry.rotated.push(&log); } } } @@ -838,9 +860,9 @@ fn sort_cockroach_extra_logs(logs: &[LogFile]) -> HashMap<&str, ExtraLogs<'_>> { let entry = interested.entry(prefix).or_default(); if file_name == format!("{prefix}.log") { - entry.current = Some(log.path.as_path()); + entry.current = Some(log); } else { - entry.rotated.push(log.path.as_path()); + entry.rotated.push(log); } } } @@ -917,52 +939,30 @@ mod test { "bogus.log", "some/dir" ].into_iter().map(|l| { - oxlog::LogFile { path: Utf8PathBuf::from(l), size: None, modified: None } - }).collect(); + oxlog::LogFile { path: Utf8PathBuf::from(l), size: None, modified: None } + }).collect(); + let logs_map: HashMap<_, _> = + logs.iter().map(|l| (l.path.as_str(), l)).collect(); let mut expected: HashMap<&str, ExtraLogs<'_>> = HashMap::new(); // cockroach expected.entry("cockroach").or_default().current = - Some(Utf8Path::new("cockroach.log")); - expected - .entry("cockroach") - .or_default() - .rotated - .push(Utf8Path::new("cockroach.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-01-31T17_11_45Z.011435.log")); - expected - .entry("cockroach") - .or_default() - .rotated - .push(Utf8Path::new("cockroach.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-02-01T01_51_51Z.011486.log")); + Some(&logs_map["cockroach.log"]); + expected.entry("cockroach").or_default().rotated.push(&logs_map["cockroach.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-01-31T17_11_45Z.011435.log"]); + expected.entry("cockroach").or_default().rotated.push(&logs_map["cockroach.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-02-01T01_51_51Z.011486.log"]); // cockroach-health expected.entry("cockroach-health").or_default().current = - Some(Utf8Path::new("cockroach-health.log")); - expected - .entry("cockroach-health") - .or_default() - .rotated - .push(Utf8Path::new("cockroach-health.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-01-31T21_43_26Z.011435.log")); - expected - .entry("cockroach-health") - .or_default() - .rotated - .push(Utf8Path::new("cockroach-health.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-02-01T01_51_53Z.011486.log")); + Some(&logs_map["cockroach-health.log"]); + expected.entry("cockroach-health").or_default().rotated.push(&logs_map["cockroach-health.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-01-31T21_43_26Z.011435.log"]); + expected.entry("cockroach-health").or_default().rotated.push(&logs_map["cockroach-health.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2025-02-01T01_51_53Z.011486.log"]); // cockroach-stderr expected.entry("cockroach-stderr").or_default().current = - Some(Utf8Path::new("cockroach-stderr.log")); - expected - .entry("cockroach-stderr") - .or_default() - .rotated - .push(Utf8Path::new("cockroach-stderr.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2023-08-30T18_56_19Z.011950.log")); - expected - .entry("cockroach-stderr") - .or_default() - .rotated - .push(Utf8Path::new("cockroach-stderr.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2023-08-31T02_59_24Z.010479.log")); + Some(&logs_map["cockroach-stderr.log"]); + expected.entry("cockroach-stderr").or_default().rotated.push(&logs_map["cockroach-stderr.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2023-08-30T18_56_19Z.011950.log"]); + expected.entry("cockroach-stderr").or_default().rotated.push(&logs_map["cockroach-stderr.oxzcockroachdba3628a56-6f85-43b5-be50-71d8f0e04877.root.2023-08-31T02_59_24Z.010479.log"]); let extra = sort_cockroach_extra_logs(logs.as_slice()); assert_eq!( @@ -1177,6 +1177,12 @@ mod illumos_tests { let mut logfile_handle = fs_err::tokio::File::create_new(&logfile).await.unwrap(); logfile_handle.write_all(data.as_bytes()).await.unwrap(); + + // 2025-10-15T00:00:00 + let mtime = std::time::SystemTime::UNIX_EPOCH + + std::time::Duration::from_secs(1760486400); + let std_file = logfile_handle.into_std().await.into_file(); + std_file.set_modified(mtime).unwrap(); } // Populate some file with similar names that should be skipped over @@ -1197,14 +1203,19 @@ mod illumos_tests { let zipfile_path = mountpoint.join("test.zip"); let zipfile = File::create_new(&zipfile_path).unwrap(); let mut zip = ZipWriter::new(zipfile); + let log = LogFile { + path: mountpoint + .join(format!("var/svc/log/{}", logfile_to_data[0].0)), + size: None, + modified: None, + }; loghandle .process_logs( "mg-ddm", &mut zip, &mut log_snapshots, - &mountpoint - .join(format!("var/svc/log/{}", logfile_to_data[0].0)), + &log, LogType::Current, ) .await @@ -1212,12 +1223,20 @@ mod illumos_tests { zip.finish().unwrap(); - // Confirm the zip has our file and data + let expected_zip_mtime = + zip::DateTime::from_date_and_time(2025, 10, 15, 0, 0, 0) + .unwrap(); + + // Confirm the zip has our file and data with the right mtime let mut archive = ZipArchive::new(File::open(zipfile_path).unwrap()).unwrap(); for (name, data) in logfile_to_data { let mut file_in_zip = archive.by_name(&format!("mg-ddm/current/{name}")).unwrap(); + + let mtime = file_in_zip.last_modified().unwrap(); + assert_eq!(mtime, expected_zip_mtime, "file mtime matches"); + let mut contents = String::new(); file_in_zip.read_to_string(&mut contents).unwrap(); @@ -1283,13 +1302,14 @@ mod illumos_tests { let zipfile_path = mountpoint.join("test.zip"); let zipfile = File::create_new(&zipfile_path).unwrap(); let mut zip = ZipWriter::new(zipfile); + let log = LogFile { path: logfile, size: None, modified: None }; loghandle .process_logs( "mg-ddm", &mut zip, &mut log_snapshots, - &logfile, + &log, LogType::Current, ) .await diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index b0d9d2f87d..4017444005 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -150,7 +150,7 @@ x509-cert = { version = "0.2.5" } zerocopy-c38e5c1d305a1b54 = { package = "zerocopy", version = "0.8.27", default-features = false, features = ["derive", "simd"] } zerocopy-ca01ad9e24f5d932 = { package = "zerocopy", version = "0.7.35", features = ["derive", "simd"] } zeroize = { version = "1.8.1", features = ["std", "zeroize_derive"] } -zip-164d15cefe24d7eb = { package = "zip", version = "4.2.0", default-features = false, features = ["bzip2", "deflate", "zstd"] } +zip-164d15cefe24d7eb = { package = "zip", version = "4.2.0", default-features = false, features = ["bzip2", "deflate", "jiff-02", "zstd"] } zip-3b31131e45eafb45 = { package = "zip", version = "0.6.6", default-features = false, features = ["bzip2", "deflate"] } [build-dependencies] @@ -291,7 +291,7 @@ x509-cert = { version = "0.2.5" } zerocopy-c38e5c1d305a1b54 = { package = "zerocopy", version = "0.8.27", default-features = false, features = ["derive", "simd"] } zerocopy-ca01ad9e24f5d932 = { package = "zerocopy", version = "0.7.35", features = ["derive", "simd"] } zeroize = { version = "1.8.1", features = ["std", "zeroize_derive"] } -zip-164d15cefe24d7eb = { package = "zip", version = "4.2.0", default-features = false, features = ["bzip2", "deflate", "zstd"] } +zip-164d15cefe24d7eb = { package = "zip", version = "4.2.0", default-features = false, features = ["bzip2", "deflate", "jiff-02", "zstd"] } zip-3b31131e45eafb45 = { package = "zip", version = "0.6.6", default-features = false, features = ["bzip2", "deflate"] } [target.x86_64-unknown-linux-gnu.dependencies]