Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion illumos-utils/src/zfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@ impl Zfs {
.collect::<Result<Vec<&str>, GetValueError>>()?
.join(",");

cmd.args(&[ZFS, "get", "-Ho", "value"]);
cmd.args(&[ZFS, "get", "-Hpo", "value"]);
if let Some(source) = source {
cmd.args(&["-s", &source.to_string()]);
}
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ impl SledAgent {
pub(crate) fn as_support_bundle_logs(&self) -> SupportBundleLogs<'_> {
SupportBundleLogs::new(
&self.log,
self.inner.config_reconciler.internal_disks_rx(),
self.inner.config_reconciler.available_datasets_rx(),
)
}

Expand Down
63 changes: 53 additions & 10 deletions sled-agent/src/support_bundle/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

use camino_tempfile::tempfile_in;
use dropshot::HttpError;
use futures::StreamExt;
use futures::stream::FuturesUnordered;
use range_requests::make_get_response;
use sled_agent_config_reconciler::InternalDisksReceiver;
use sled_agent_config_reconciler::AvailableDatasetsReceiver;
use slog::Logger;
use slog_error_chain::InlineErrorChain;
use tokio::io::AsyncSeekExt;
Expand Down Expand Up @@ -43,15 +45,15 @@ impl From<Error> for HttpError {

pub struct SupportBundleLogs<'a> {
log: &'a Logger,
internal_disks_rx: &'a InternalDisksReceiver,
available_datasets_rx: AvailableDatasetsReceiver,
}

impl<'a> SupportBundleLogs<'a> {
pub fn new(
log: &'a Logger,
internal_disks_rx: &'a InternalDisksReceiver,
available_datasets_rx: AvailableDatasetsReceiver,
) -> Self {
Self { log, internal_disks_rx }
Self { log, available_datasets_rx }
}

/// Get a list of zones on a sled containing logs that we want to include in
Expand All @@ -78,12 +80,53 @@ impl<'a> SupportBundleLogs<'a> {
where
Z: Into<String>,
{
// We are using an M.2 device for temporary storage to assemble a zip
// file made up of all of the discovered zone's logs.
let current_internal_disks = self.internal_disks_rx.current();
let mut m2_debug_datasets = current_internal_disks.all_debug_datasets();
let tempdir = m2_debug_datasets.next().ok_or(Error::MissingStorage)?;
let mut tempfile = tempfile_in(tempdir)?;
// Attempt to find a U.2 device with the most available free space
// for temporary storage to assemble a zip file made up of all of the
// discovered zone's logs.
Comment on lines +83 to +85
Copy link
Contributor

Choose a reason for hiding this comment

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

Style nit I don't feel strongly about - I'd maybe move this chunk of code to a separate method? dataset_for_temporary_storage() or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I had saw the approval on mobile when I stepped away from the laptop and marked it for auto merge, I didn't catch this nit in time so here's the follow up: #8454

let mounted_debug_datasets =
self.available_datasets_rx.all_mounted_debug_datasets();
let storage_paths_to_size: Vec<_> = mounted_debug_datasets
.into_iter()
.map(|dataset_path| {
let path = dataset_path.path;
async move {
match illumos_utils::zfs::Zfs::get_value(
path.as_str(),
"available",
)
.await
{
Ok(size_str) => match size_str.parse::<usize>() {
Ok(size) => Some((path, size)),
Err(e) => {
warn!(
&self.log,
"failed to parse available size for the \
dataset at path {path}: {e}"
);
None
}
},
Err(e) => {
warn!(
&self.log,
"failed to get available size for the dataset \
at path {path}: {e}"
);
None
}
}
}
})
.collect::<FuturesUnordered<_>>()
.collect()
.await;
let (largest_avail_space, _) = storage_paths_to_size
.into_iter()
.flatten()
.max_by_key(|(_, size)| *size)
.ok_or(Error::MissingStorage)?;
let mut tempfile = tempfile_in(largest_avail_space)?;

let log = self.log.clone();
let zone = zone.into();
Expand Down
9 changes: 0 additions & 9 deletions sled-storage/src/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,6 @@ impl AllDisks {
.collect()
}

/// Return the directories that can be used for temporary sled-diagnostics
/// file storage.
pub fn all_sled_diagnostics_directories(&self) -> Vec<Utf8PathBuf> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This reminds me we should really prune a whole bunch of stuff from this crate now that the config reconciler has landed... A lot of it is only used in a couple tests (and I should update those tests to use the config reconciler instead!)

No objection to pruning this method though.

// These directories are currently used for tempfile storage when
// zipping up zone logs before shuffling them off to a nexus collecting
// a support bundle.
self.all_m2_mountpoints(M2_DEBUG_DATASET).into_iter().collect()
}

/// Returns an iterator over all managed disks.
pub fn iter_managed(&self) -> impl Iterator<Item = (&DiskIdentity, &Disk)> {
self.inner.values.iter().filter_map(|(identity, disk)| match disk {
Expand Down
Loading