Skip to content

Include /var/adm/messages in archived zone logs #9448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wfchandler wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Include /var/adm/messages in archived zone logs #9448

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
650fde5
Include /var/adm/messages in archived zone logs
wfchandler Nov 26, 2025
b386fe2
Create separate directories for zones
wfchandler Dec 2, 2025
ddc56e2
Capture /var/adm/messages for all zones
wfchandler Dec 2, 2025
6e5ba44
Comment on archive_logs_from_zone_path params
wfchandler Dec 2, 2025
1a457af
Set zone root properly for non-GZ
wfchandler Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 48 additions & 9 deletions sled-agent/config-reconciler/src/dump_setup.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1099,15 +1099,23 @@ impl DumpSetupWorker {
.as_ref()
.ok_or(ArchiveLogsError::NoDebugDirYet)?;
let oxz_zones = self.zone_invoker.get_zones().await?;

for zone in oxz_zones {
let logdir = if zone.global() {
PathBuf::from("/var/svc/log")
let zone_root = if zone.global() {
zone.path().to_owned()
} else {
zone.path().join("root/var/svc/log")
zone.path().join("root")
};
let logdir = zone_root.join("var/svc/log");
let zone_name = zone.name();
self.archive_logs_from_zone_path(
debug_dir, logdir, zone_name, false,
debug_dir, logdir, "*.log", zone_name, false,
)
.await?;

let adm_logdir = zone_root.join("var/adm");
self.archive_logs_from_zone_path(
debug_dir, adm_logdir, "messages", zone_name, false,
)
.await?;
}
Expand All @@ -1129,6 +1137,7 @@ impl DumpSetupWorker {
.archive_logs_from_zone_path(
debug_dir,
logdir.into(),
"*.log",
zone_name,
true,
)
Expand All @@ -1145,17 +1154,23 @@ impl DumpSetupWorker {
rv
}

// Archives log files found in `logdir` for zone `zone_name` to the destination debug dataset.
//
// `log_name_pattern` should be a glob pattern that matches against file names, e.g., `*.log`, `mylog`.
// If `include_live` is `true`, this will archive all logs, matching on `{log_name_pattern}*`.
// If it is `false`, only rotated logs will be archived, matching on `{log_name_pattern}.[0-9]`.
Comment on lines +1157 to +1161
Copy link
Collaborator

@davepacheco davepacheco Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't notice this before. These lines are over 80 columns. Can you rewrap them? (rustfmt cannot enforce long lines in comments without unstable options, unfortunately.)

async fn archive_logs_from_zone_path(
&self,
debug_dir: &DebugDataset,
logdir: PathBuf,
log_name_pattern: &str,
Copy link
Collaborator

@smklein smklein Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though this is a non-pub function, it's probably worth adding docs for this (e.g., it's a prefix; it interacts with the include_live arguments, it's searching within the logdir path, etc)

Copy link
Contributor Author

@wfchandler wfchandler Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, added in 6e5ba44

zone_name: &str,
include_live: bool,
) -> Result<(), ArchiveLogsError> {
let mut rotated_log_files = Vec::new();
if include_live {
let pattern = logdir
.join("*.log*")
.join(format!("{log_name_pattern}*"))
Copy link
Collaborator

@davepacheco davepacheco Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like kind of a dicey interface to me, but I feel like that's true for a lot of the existing pieces here. I'm hoping to rework the way a bunch of the log archival works to make it more testable anyway so this is probably fine for now.

.to_str()
.ok_or_else(|| ArchiveLogsError::Utf8(zone_name.to_string()))?
.to_string();
Expand All @@ -1166,7 +1181,7 @@ impl DumpSetupWorker {
// any
for n in 1..9 {
let pattern = logdir
.join(format!("*.log.{}", "[0-9]".repeat(n)))
.join(format!("{log_name_pattern}.{}", "[0-9]".repeat(n)))
.to_str()
.ok_or_else(|| {
ArchiveLogsError::Utf8(zone_name.to_string())
Expand Down Expand Up @@ -1793,11 +1808,18 @@ mod tests {
let tempdir = Utf8TempDir::new().unwrap();
let core_dir = tempdir.path().join(CRASH_DATASET);
let debug_dir = tempdir.path().join(DUMP_DATASET);
let zone_logs = tempdir.path().join("root/var/svc/log");
let zone_path = tempdir.path().join("myzone");
let zone_logs = zone_path.join("root/var/svc/log");
let global_path = tempdir.path().join("global");
let adm_logs = global_path.join("var/adm");

let tempdir_path = tempdir.path().as_str().to_string();
let global_zone = Zone::from_str(&format!(
"0:global:running:{global_path}::ipkg:shared"
))
.unwrap();
let zone = Zone::from_str(&format!(
"1:myzone:running:{tempdir_path}::ipkg:shared"
"1:myzone:running:{zone_path}::ipkg:shared"
))
.unwrap();

Expand Down Expand Up @@ -1831,14 +1853,17 @@ mod tests {
.into_iter()
.collect(),
}),
Box::new(FakeZone { zones: vec![zone.clone()] }),
Box::new(FakeZone { zones: vec![global_zone, zone.clone()] }),
logctx.log.clone(),
tokio::sync::mpsc::channel(1).1,
);

tokio::fs::create_dir_all(&zone_path).await.unwrap();
tokio::fs::create_dir_all(&global_path).await.unwrap();
tokio::fs::create_dir_all(&core_dir).await.unwrap();
tokio::fs::create_dir_all(&debug_dir).await.unwrap();
tokio::fs::create_dir_all(&zone_logs).await.unwrap();
tokio::fs::create_dir_all(&adm_logs).await.unwrap();
const LOG_NAME: &'static str = "foo.log.0";
tokio::fs::File::create(zone_logs.join(LOG_NAME))
.await
Expand All @@ -1847,6 +1872,14 @@ mod tests {
.await
.expect("writing fake log");

const ADM_LOG_NAME: &'static str = "messages.0";
tokio::fs::File::create(adm_logs.join(ADM_LOG_NAME))
.await
.expect("creating fake adm log")
.write_all(b"admin stuff")
.await
.expect("writing fake adm log");

const CORE_NAME: &str = "core.myzone.myexe.123.1690540950";
tokio::fs::File::create(core_dir.join(CORE_NAME))
.await
Expand Down Expand Up @@ -1878,6 +1911,12 @@ mod tests {
debug_dir.join(zone.name()).join(LOG_NAME.replace(".0", ".*"));
assert_eq!(glob::glob(log_glob.as_str()).unwrap().count(), 1);
assert!(!zone_logs.join(LOG_NAME).is_file());

let adm_glob =
debug_dir.join("global").join(ADM_LOG_NAME.replace(".0", ".*"));
assert_eq!(glob::glob(adm_glob.as_str()).unwrap().count(), 1);
assert!(!adm_logs.join(ADM_LOG_NAME).is_file());

assert!(debug_dir.join(CORE_NAME).is_file());
assert!(!core_dir.join(CORE_NAME).is_file());
logctx.cleanup_successful();
Expand Down
Loading