From 07dd4738f44c6de8d555082e05d934fe5d8213de Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 11 Apr 2025 10:33:22 -0700 Subject: [PATCH 01/31] [nexus] Put support bundles in internal API too --- Cargo.lock | 1 + nexus/internal-api/Cargo.toml | 1 + nexus/internal-api/src/lib.rs | 101 +++++- nexus/src/internal_api/http_entrypoints.rs | 284 +++++++++++++++ openapi/nexus-internal.json | 381 +++++++++++++++++++++ 5 files changed, 765 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 41b1d6ff789..511f7b87f9d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6100,6 +6100,7 @@ name = "nexus-internal-api" version = "0.1.0" dependencies = [ "dropshot 0.16.0", + "http", "nexus-types", "omicron-common", "omicron-uuid-kinds", diff --git a/nexus/internal-api/Cargo.toml b/nexus/internal-api/Cargo.toml index 76fa6bd59aa..ef6a9c6e464 100644 --- a/nexus/internal-api/Cargo.toml +++ b/nexus/internal-api/Cargo.toml @@ -9,6 +9,7 @@ workspace = true [dependencies] dropshot.workspace = true +http.workspace = true nexus-types.workspace = true omicron-common.workspace = true omicron-uuid-kinds.workspace = true diff --git a/nexus/internal-api/src/lib.rs b/nexus/internal-api/src/lib.rs index a7ac6b22a29..3cfbaf55a09 100644 --- a/nexus/internal-api/src/lib.rs +++ b/nexus/internal-api/src/lib.rs @@ -5,18 +5,19 @@ use std::collections::{BTreeMap, BTreeSet}; use dropshot::{ - HttpError, HttpResponseCreated, HttpResponseDeleted, HttpResponseOk, + Body, HttpError, HttpResponseCreated, HttpResponseDeleted, HttpResponseOk, HttpResponseUpdatedNoContent, Path, Query, RequestContext, ResultsPage, TypedBody, }; +use http::Response; use nexus_types::{ deployment::{ Blueprint, BlueprintMetadata, BlueprintTarget, BlueprintTargetSet, ClickhousePolicy, }, external_api::{ - params::{PhysicalDiskPath, SledSelector, UninitializedSledId}, - shared::{ProbeInfo, UninitializedSled}, + params::{self, PhysicalDiskPath, SledSelector, UninitializedSledId}, + shared::{self, ProbeInfo, UninitializedSled}, views::Ping, views::PingStatus, views::SledPolicy, @@ -525,6 +526,100 @@ pub trait NexusInternalApi { disk: TypedBody, ) -> Result; + // Support bundles (experimental) + + /// List all support bundles + #[endpoint { + method = GET, + path = "/experimental/v1/system/support-bundles", + }] + async fn support_bundle_list( + rqctx: RequestContext, + query_params: Query, + ) -> Result>, HttpError>; + + /// View a support bundle + #[endpoint { + method = GET, + path = "/experimental/v1/system/support-bundles/{support_bundle}", + }] + async fn support_bundle_view( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError>; + + /// Download the index of a support bundle + #[endpoint { + method = GET, + path = "/experimental/v1/system/support-bundles/{support_bundle}/index", + }] + async fn support_bundle_index( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError>; + + /// Download the contents of a support bundle + #[endpoint { + method = GET, + path = "/experimental/v1/system/support-bundles/{support_bundle}/download", + }] + async fn support_bundle_download( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError>; + + /// Download a file within a support bundle + #[endpoint { + method = GET, + path = "/experimental/v1/system/support-bundles/{support_bundle}/download/{file}", + }] + async fn support_bundle_download_file( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError>; + + /// Download the metadata of a support bundle + #[endpoint { + method = HEAD, + path = "/experimental/v1/system/support-bundles/{support_bundle}/download", + }] + async fn support_bundle_head( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError>; + + /// Download the metadata of a file within the support bundle + #[endpoint { + method = HEAD, + path = "/experimental/v1/system/support-bundles/{support_bundle}/download/{file}", + }] + async fn support_bundle_head_file( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError>; + + /// Create a new support bundle + #[endpoint { + method = POST, + path = "/experimental/v1/system/support-bundles", + }] + async fn support_bundle_create( + rqctx: RequestContext, + ) -> Result, HttpError>; + + /// Delete an existing support bundle + /// + /// May also be used to cancel a support bundle which is currently being + /// collected, or to remove metadata for a support bundle that has failed. + #[endpoint { + method = DELETE, + path = "/experimental/v1/system/support-bundles/{support_bundle}", + }] + async fn support_bundle_delete( + rqctx: RequestContext, + path_params: Path, + ) -> Result; + /// Get all the probes associated with a given sled. #[endpoint { method = GET, diff --git a/nexus/src/internal_api/http_entrypoints.rs b/nexus/src/internal_api/http_entrypoints.rs index 5cabe56a984..afbe0a41ce8 100644 --- a/nexus/src/internal_api/http_entrypoints.rs +++ b/nexus/src/internal_api/http_entrypoints.rs @@ -5,8 +5,11 @@ //! Handler functions (entrypoints) for HTTP APIs internal to the control plane use super::params::{OximeterInfo, RackInitializationRequest}; +use crate::app::support_bundles::SupportBundleQueryType; use crate::context::ApiContext; +use crate::external_api::shared; use dropshot::ApiDescription; +use dropshot::Body; use dropshot::HttpError; use dropshot::HttpResponseCreated; use dropshot::HttpResponseDeleted; @@ -17,6 +20,7 @@ use dropshot::Query; use dropshot::RequestContext; use dropshot::ResultsPage; use dropshot::TypedBody; +use http::Response; use nexus_internal_api::*; use nexus_types::deployment::Blueprint; use nexus_types::deployment::BlueprintMetadata; @@ -25,6 +29,8 @@ use nexus_types::deployment::BlueprintTargetSet; use nexus_types::deployment::ClickhousePolicy; use nexus_types::external_api::params::PhysicalDiskPath; use nexus_types::external_api::params::SledSelector; +use nexus_types::external_api::params::SupportBundleFilePath; +use nexus_types::external_api::params::SupportBundlePath; use nexus_types::external_api::params::UninitializedSledId; use nexus_types::external_api::shared::ProbeInfo; use nexus_types::external_api::shared::UninitializedSled; @@ -54,6 +60,8 @@ use omicron_common::api::internal::nexus::RepairStartInfo; use omicron_common::api::internal::nexus::SledVmmState; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::InstanceUuid; +use omicron_uuid_kinds::SupportBundleUuid; +use range_requests::RequestContextEx; use std::collections::BTreeMap; type NexusApiDescription = ApiDescription; @@ -899,6 +907,282 @@ impl NexusInternalApi for NexusInternalApiImpl { .await } + async fn support_bundle_list( + rqctx: RequestContext, + query_params: Query, + ) -> Result>, HttpError> + { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + + let query = query_params.into_inner(); + let pagparams = data_page_params_for(&rqctx, &query)?; + + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + + let bundles = nexus + .support_bundle_list(&opctx, &pagparams) + .await? + .into_iter() + .map(|p| p.into()) + .collect(); + + Ok(HttpResponseOk(ScanById::results_page( + &query, + bundles, + &|_, bundle: &shared::SupportBundleInfo| { + bundle.id.into_untyped_uuid() + }, + )?)) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn support_bundle_view( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let path = path_params.into_inner(); + + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + + let bundle = nexus + .support_bundle_view( + &opctx, + SupportBundleUuid::from_untyped_uuid(path.support_bundle), + ) + .await?; + + Ok(HttpResponseOk(bundle.into())) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn support_bundle_index( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let path = path_params.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + + let head = false; + let range = rqctx.range(); + + let body = nexus + .support_bundle_download( + &opctx, + SupportBundleUuid::from_untyped_uuid(path.support_bundle), + SupportBundleQueryType::Index, + head, + range, + ) + .await?; + Ok(body) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn support_bundle_download( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let path = path_params.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + + let head = false; + let range = rqctx.range(); + + let body = nexus + .support_bundle_download( + &opctx, + SupportBundleUuid::from_untyped_uuid(path.support_bundle), + SupportBundleQueryType::Whole, + head, + range, + ) + .await?; + Ok(body) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn support_bundle_download_file( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let path = path_params.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let head = false; + let range = rqctx.range(); + + let body = nexus + .support_bundle_download( + &opctx, + SupportBundleUuid::from_untyped_uuid( + path.bundle.support_bundle, + ), + SupportBundleQueryType::Path { file_path: path.file }, + head, + range, + ) + .await?; + Ok(body) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn support_bundle_head( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let path = path_params.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let head = true; + let range = rqctx.range(); + + let body = nexus + .support_bundle_download( + &opctx, + SupportBundleUuid::from_untyped_uuid(path.support_bundle), + SupportBundleQueryType::Whole, + head, + range, + ) + .await?; + Ok(body) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn support_bundle_head_file( + rqctx: RequestContext, + path_params: Path, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let path = path_params.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let head = true; + let range = rqctx.range(); + + let body = nexus + .support_bundle_download( + &opctx, + SupportBundleUuid::from_untyped_uuid( + path.bundle.support_bundle, + ), + SupportBundleQueryType::Path { file_path: path.file }, + head, + range, + ) + .await?; + Ok(body) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn support_bundle_create( + rqctx: RequestContext, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + + let bundle = nexus + .support_bundle_create(&opctx, "Created by external API") + .await?; + Ok(HttpResponseCreated(bundle.into())) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn support_bundle_delete( + rqctx: RequestContext, + path_params: Path, + ) -> Result { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let path = path_params.into_inner(); + + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + + nexus + .support_bundle_delete( + &opctx, + SupportBundleUuid::from_untyped_uuid(path.support_bundle), + ) + .await?; + + Ok(HttpResponseDeleted()) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + async fn probes_get( rqctx: RequestContext, path_params: Path, diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index 40d2c88c942..6c5ee7cc011 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -744,6 +744,302 @@ } } }, + "/experimental/v1/system/support-bundles": { + "get": { + "summary": "List all support bundles", + "operationId": "support_bundle_list", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/IdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SupportBundleInfoResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [] + } + }, + "post": { + "summary": "Create a new support bundle", + "operationId": "support_bundle_create", + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SupportBundleInfo" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/experimental/v1/system/support-bundles/{support_bundle}": { + "get": { + "summary": "View a support bundle", + "operationId": "support_bundle_view", + "parameters": [ + { + "in": "path", + "name": "support_bundle", + "description": "ID of the support bundle", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SupportBundleInfo" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "delete": { + "summary": "Delete an existing support bundle", + "description": "May also be used to cancel a support bundle which is currently being collected, or to remove metadata for a support bundle that has failed.", + "operationId": "support_bundle_delete", + "parameters": [ + { + "in": "path", + "name": "support_bundle", + "description": "ID of the support bundle", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "204": { + "description": "successful deletion" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/experimental/v1/system/support-bundles/{support_bundle}/download": { + "get": { + "summary": "Download the contents of a support bundle", + "operationId": "support_bundle_download", + "parameters": [ + { + "in": "path", + "name": "support_bundle", + "description": "ID of the support bundle", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "default": { + "description": "", + "content": { + "*/*": { + "schema": {} + } + } + } + } + }, + "head": { + "summary": "Download the metadata of a support bundle", + "operationId": "support_bundle_head", + "parameters": [ + { + "in": "path", + "name": "support_bundle", + "description": "ID of the support bundle", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "default": { + "description": "", + "content": { + "*/*": { + "schema": {} + } + } + } + } + } + }, + "/experimental/v1/system/support-bundles/{support_bundle}/download/{file}": { + "get": { + "summary": "Download a file within a support bundle", + "operationId": "support_bundle_download_file", + "parameters": [ + { + "in": "path", + "name": "file", + "description": "The file within the bundle to download", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "path", + "name": "support_bundle", + "description": "ID of the support bundle", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "default": { + "description": "", + "content": { + "*/*": { + "schema": {} + } + } + } + } + }, + "head": { + "summary": "Download the metadata of a file within the support bundle", + "operationId": "support_bundle_head_file", + "parameters": [ + { + "in": "path", + "name": "file", + "description": "The file within the bundle to download", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "path", + "name": "support_bundle", + "description": "ID of the support bundle", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "default": { + "description": "", + "content": { + "*/*": { + "schema": {} + } + } + } + } + } + }, + "/experimental/v1/system/support-bundles/{support_bundle}/index": { + "get": { + "summary": "Download the index of a support bundle", + "operationId": "support_bundle_index", + "parameters": [ + { + "in": "path", + "name": "support_bundle", + "description": "ID of the support bundle", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "default": { + "description": "", + "content": { + "*/*": { + "schema": {} + } + } + } + } + } + }, "/instances/{instance_id}/migrate": { "post": { "operationId": "instance_migrate", @@ -5836,6 +6132,87 @@ "weight" ] }, + "SupportBundleInfo": { + "type": "object", + "properties": { + "id": { + "$ref": "#/components/schemas/TypedUuidForSupportBundleKind" + }, + "reason_for_creation": { + "type": "string" + }, + "reason_for_failure": { + "nullable": true, + "type": "string" + }, + "state": { + "$ref": "#/components/schemas/SupportBundleState" + }, + "time_created": { + "type": "string", + "format": "date-time" + } + }, + "required": [ + "id", + "reason_for_creation", + "state", + "time_created" + ] + }, + "SupportBundleInfoResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/SupportBundleInfo" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, + "SupportBundleState": { + "oneOf": [ + { + "description": "Support Bundle still actively being collected.\n\nThis is the initial state for a Support Bundle, and it will automatically transition to either \"Failing\" or \"Active\".\n\nIf a user no longer wants to access a Support Bundle, they can request cancellation, which will transition to the \"Destroying\" state.", + "type": "string", + "enum": [ + "collecting" + ] + }, + { + "description": "Support Bundle is being destroyed.\n\nOnce backing storage has been freed, this bundle is destroyed.", + "type": "string", + "enum": [ + "destroying" + ] + }, + { + "description": "Support Bundle was not created successfully, or was created and has lost backing storage.\n\nThe record of the bundle still exists for readability, but the only valid operation on these bundles is to destroy them.", + "type": "string", + "enum": [ + "failed" + ] + }, + { + "description": "Support Bundle has been processed, and is ready for usage.", + "type": "string", + "enum": [ + "active" + ] + } + ] + }, "SwitchLocation": { "description": "Identifies switch physical location", "oneOf": [ @@ -5942,6 +6319,10 @@ "type": "string", "format": "uuid" }, + "TypedUuidForSupportBundleKind": { + "type": "string", + "format": "uuid" + }, "TypedUuidForUpstairsRepairKind": { "type": "string", "format": "uuid" From c079c3f66a3f727b87cc13818cc59c0a1a888e22 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 14 Apr 2025 13:06:49 -0700 Subject: [PATCH 02/31] [omdb] Basic commands to access support bundles --- Cargo.lock | 2 + dev-tools/omdb/Cargo.toml | 2 + dev-tools/omdb/src/bin/omdb/nexus.rs | 196 +++++++++++++++++++++++++++ 3 files changed, 200 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 511f7b87f9d..9a105fa34ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7336,6 +7336,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-bb8-diesel", + "bytes", "camino", "camino-tempfile", "chrono", @@ -7384,6 +7385,7 @@ dependencies = [ "pq-sys", "ratatui", "reedline", + "reqwest", "serde", "serde_json", "sled-agent-client", diff --git a/dev-tools/omdb/Cargo.toml b/dev-tools/omdb/Cargo.toml index e605bd58300..ca4898549b8 100644 --- a/dev-tools/omdb/Cargo.toml +++ b/dev-tools/omdb/Cargo.toml @@ -13,6 +13,7 @@ omicron-rpaths.workspace = true [dependencies] anyhow.workspace = true async-bb8-diesel.workspace = true +bytes.workspace = true camino.workspace = true chrono.workspace = true clap.workspace = true @@ -48,6 +49,7 @@ oximeter-db = { workspace = true, default-features = false, features = [ "oxql" pq-sys = "*" ratatui.workspace = true reedline.workspace = true +reqwest.workspace = true serde.workspace = true serde_json.workspace = true sled-agent-client.workspace = true diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index feef78cd77b..0093b77c83e 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -21,6 +21,7 @@ use clap::Args; use clap::ColorChoice; use clap::Subcommand; use clap::ValueEnum; +use futures::StreamExt; use futures::TryStreamExt; use futures::future::try_join; use http::StatusCode; @@ -67,6 +68,7 @@ use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::ParseError; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledUuid; +use omicron_uuid_kinds::SupportBundleUuid; use serde::Deserialize; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; @@ -120,6 +122,8 @@ enum NexusCommands { Sagas(SagasArgs), /// interact with sleds Sleds(SledsArgs), + /// interact with support bundles + SupportBundles(SupportBundleArgs), } #[derive(Debug, Args)] @@ -441,6 +445,45 @@ struct DiskExpungeArgs { physical_disk_id: PhysicalDiskUuid, } +#[derive(Debug, Args)] +struct SupportBundleArgs { + #[command(subcommand)] + command: SupportBundleCommands, +} + +#[derive(Debug, Subcommand)] +#[allow(clippy::large_enum_variant)] +enum SupportBundleCommands { + /// List all support bundles + List, + /// Create a new support bundle + Create, + /// Delete a support bundle + Delete(SupportBundleDeleteArgs), + /// Download the index of a support bundle + /// + /// This is a "list of files", from which individual files can be accessed + GetIndex(SupportBundleIndexArgs), + /// View a file within a support bundle + GetFile(SupportBundleFileArgs), +} + +#[derive(Debug, Args)] +struct SupportBundleDeleteArgs { + id: SupportBundleUuid, +} + +#[derive(Debug, Args)] +struct SupportBundleIndexArgs { + id: SupportBundleUuid, +} + +#[derive(Debug, Args)] +struct SupportBundleFileArgs { + id: SupportBundleUuid, + path: Utf8PathBuf, +} + impl NexusArgs { /// Run a `omdb nexus` subcommand. pub(crate) async fn run_cmd( @@ -619,6 +662,27 @@ impl NexusArgs { cmd_nexus_sled_expunge_disk(&client, args, omdb, log, token) .await } + NexusCommands::SupportBundles(SupportBundleArgs { + command: SupportBundleCommands::List, + }) => cmd_nexus_support_bundles_list(&client).await, + NexusCommands::SupportBundles(SupportBundleArgs { + command: SupportBundleCommands::Create, + }) => { + let token = omdb.check_allow_destructive()?; + cmd_nexus_support_bundles_create(&client, token).await + } + NexusCommands::SupportBundles(SupportBundleArgs { + command: SupportBundleCommands::Delete(args), + }) => { + let token = omdb.check_allow_destructive()?; + cmd_nexus_support_bundles_delete(&client, args, token).await + } + NexusCommands::SupportBundles(SupportBundleArgs { + command: SupportBundleCommands::GetIndex(args), + }) => cmd_nexus_support_bundles_get_index(&client, args).await, + NexusCommands::SupportBundles(SupportBundleArgs { + command: SupportBundleCommands::GetFile(args), + }) => cmd_nexus_support_bundles_get_file(&client, args).await, } } } @@ -3248,3 +3312,135 @@ async fn cmd_nexus_sled_expunge_disk_with_datastore( eprintln!("expunged disk {}", args.physical_disk_id); Ok(()) } + +/// Runs `omdb nexus support-bundles list` +async fn cmd_nexus_support_bundles_list( + client: &nexus_client::Client, +) -> Result<(), anyhow::Error> { + let support_bundle_stream = client.support_bundle_list_stream(None, None); + + let support_bundles = support_bundle_stream + .try_collect::>() + .await + .context("listing support bundles")?; + + #[derive(Tabled)] + #[tabled(rename_all = "SCREAMING_SNAKE_CASE")] + struct SupportBundleInfo { + id: Uuid, + time_created: DateTime, + reason_for_creation: String, + reason_for_failure: String, + state: String, + } + let rows = support_bundles.into_iter().map(|sb| SupportBundleInfo { + id: *sb.id, + time_created: sb.time_created, + reason_for_creation: sb.reason_for_creation, + reason_for_failure: sb + .reason_for_failure + .unwrap_or_else(|| "-".to_string()), + state: format!("{:?}", sb.state), + }); + let table = tabled::Table::new(rows) + .with(tabled::settings::Style::empty()) + .with(tabled::settings::Padding::new(0, 1, 0, 0)) + .to_string(); + println!("{}", table); + Ok(()) +} + +/// Runs `omdb nexus support-bundles create` +async fn cmd_nexus_support_bundles_create( + client: &nexus_client::Client, + _destruction_token: DestructiveOperationToken, +) -> Result<(), anyhow::Error> { + let support_bundle_id = client + .support_bundle_create() + .await + .context("creating support bundle")? + .into_inner() + .id; + println!("created support bundle: {support_bundle_id}"); + Ok(()) +} + +/// Runs `omdb nexus support-bundles delete` +async fn cmd_nexus_support_bundles_delete( + client: &nexus_client::Client, + args: &SupportBundleDeleteArgs, + _destruction_token: DestructiveOperationToken, +) -> Result<(), anyhow::Error> { + let _ = client + .support_bundle_delete(args.id.as_untyped_uuid()) + .await + .with_context(|| format!("deleting support bundle {}", args.id))?; + println!("support bundle {} deleted", args.id); + Ok(()) +} + +async fn print_utf8_stream_to_stdout( + mut stream: impl futures::Stream> + + std::marker::Unpin, +) -> Result<(), anyhow::Error> { + let mut leftover = None; + while let Some(data) = stream.next().await { + match data { + Err(err) => return Err(anyhow::anyhow!(err)), + Ok(data) => { + let combined = match leftover.take() { + Some(old) => [old, data].concat(), + None => data.to_vec(), + }; + + match std::str::from_utf8(&combined) { + Ok(data) => println!("{data}"), + Err(_) => leftover = Some(combined.into()), + } + } + } + } + Ok(()) +} + +/// Runs `omdb nexus support-bundles get-index` +async fn cmd_nexus_support_bundles_get_index( + client: &nexus_client::Client, + args: &SupportBundleIndexArgs, +) -> Result<(), anyhow::Error> { + let stream = client + .support_bundle_index(args.id.as_untyped_uuid()) + .await + .with_context(|| { + format!("downloading support bundle index {}", args.id) + })? + .into_inner_stream(); + print_utf8_stream_to_stdout(stream).await.with_context(|| { + format!("streaming support bundle index {}", args.id) + })?; + Ok(()) +} + +/// Runs `omdb nexus support-bundles get-file` +async fn cmd_nexus_support_bundles_get_file( + client: &nexus_client::Client, + args: &SupportBundleFileArgs, +) -> Result<(), anyhow::Error> { + let stream = client + .support_bundle_download_file( + args.id.as_untyped_uuid(), + args.path.as_str(), + ) + .await + .with_context(|| { + format!( + "downloading support bundle file {}: {}", + args.id, args.path + ) + })? + .into_inner_stream(); + print_utf8_stream_to_stdout(stream).await.with_context(|| { + format!("streaming support bundle file {}: {}", args.id, args.path) + })?; + Ok(()) +} From df47341ab136f3398832054e26204a785bbe33f5 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 14 Apr 2025 14:33:11 -0700 Subject: [PATCH 03/31] Updated output --- dev-tools/omdb/tests/usage_errors.out | 1 + 1 file changed, 1 insertion(+) diff --git a/dev-tools/omdb/tests/usage_errors.out b/dev-tools/omdb/tests/usage_errors.out index 4e046417993..78e21b87911 100644 --- a/dev-tools/omdb/tests/usage_errors.out +++ b/dev-tools/omdb/tests/usage_errors.out @@ -703,6 +703,7 @@ Commands: clickhouse-policy Interact with clickhouse policy sagas view sagas, create and complete demo sagas sleds interact with sleds + support-bundles interact with support bundles help Print this message or the help of the given subcommand(s) Options: From 12f461b081cc81999cdfc6a9704640fa647b4138 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 15 Apr 2025 13:32:42 -0700 Subject: [PATCH 04/31] [nexus] Make it 'more default' for Debug datasets to exist in test environment --- dev-tools/omicron-dev/src/main.rs | 13 +++++++ .../read_only_region_replacement_start.rs | 7 +++- .../src/app/sagas/region_replacement_start.rs | 14 ++++--- .../region_snapshot_replacement_start.rs | 4 +- nexus/test-utils/src/resource_helpers.rs | 39 ++++++++++++++----- nexus/tests/integration_tests/disks.rs | 27 +++++++++---- nexus/tests/integration_tests/snapshots.rs | 9 ++++- .../integration_tests/support_bundles.rs | 34 ++++------------ nexus/tests/integration_tests/unauthorized.rs | 26 +------------ 9 files changed, 92 insertions(+), 81 deletions(-) diff --git a/dev-tools/omicron-dev/src/main.rs b/dev-tools/omicron-dev/src/main.rs index fafd16c2b0c..f4e827b08d7 100644 --- a/dev-tools/omicron-dev/src/main.rs +++ b/dev-tools/omicron-dev/src/main.rs @@ -8,6 +8,7 @@ use futures::StreamExt; use libc::SIGINT; use nexus_config::NexusConfig; use nexus_test_interface::NexusServer; +use nexus_test_utils::resource_helpers::DiskTest; use signal_hook_tokio::Signals; #[tokio::main] @@ -78,6 +79,18 @@ impl RunAllArgs { >(&mut config, 0) .await .context("error setting up services")?; + + println!("omicron-dev: Adding disks to first sled agent"); + + // This is how our integration tests are identifying that "disks exist" + // within the database. + // + // This inserts: + // - DEFAULT_ZPOOL_COUNT zpools, each of which contains: + // - A crucible dataset + // - A debug dataset + DiskTest::new(&cptestctx).await; + println!("omicron-dev: services are running."); // Print out basic information about what was started. diff --git a/nexus/src/app/background/tasks/read_only_region_replacement_start.rs b/nexus/src/app/background/tasks/read_only_region_replacement_start.rs index e6080e68c96..a1b242b586a 100644 --- a/nexus/src/app/background/tasks/read_only_region_replacement_start.rs +++ b/nexus/src/app/background/tasks/read_only_region_replacement_start.rs @@ -179,6 +179,7 @@ mod test { use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external; + use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::VolumeUuid; @@ -209,14 +210,16 @@ mod test { datastore.clone(), ); - // Record which datasets map to which zpools for later + // Record which crucible datasets map to which zpools for later let mut dataset_to_zpool: BTreeMap = BTreeMap::default(); for zpool in disk_test.zpools() { for dataset in &zpool.datasets { - dataset_to_zpool.insert(zpool.id, dataset.id); + if matches!(dataset.kind, DatasetKind::Crucible) { + dataset_to_zpool.insert(zpool.id, dataset.id); + } } } diff --git a/nexus/src/app/sagas/region_replacement_start.rs b/nexus/src/app/sagas/region_replacement_start.rs index b1fe3c5a27b..ba0bce68e1f 100644 --- a/nexus/src/app/sagas/region_replacement_start.rs +++ b/nexus/src/app/sagas/region_replacement_start.rs @@ -791,6 +791,8 @@ pub(crate) mod test { nexus_test_utils::ControlPlaneTestContext; type DiskTest<'a> = nexus_test_utils::resource_helpers::DiskTest<'a, crate::Server>; + type DiskTestBuilder<'a> = + nexus_test_utils::resource_helpers::DiskTestBuilder<'a, crate::Server>; const DISK_NAME: &str = "my-disk"; const PROJECT_NAME: &str = "springfield-squidport"; @@ -799,8 +801,8 @@ pub(crate) mod test { async fn test_region_replacement_start_saga( cptestctx: &ControlPlaneTestContext, ) { - let mut disk_test = DiskTest::new(cptestctx).await; - disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await; + let _disk_test = + DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await; let client = &cptestctx.external_client; let nexus = &cptestctx.server.server_context().nexus; @@ -1118,8 +1120,8 @@ pub(crate) mod test { async fn test_action_failure_can_unwind_idempotently( cptestctx: &ControlPlaneTestContext, ) { - let mut disk_test = DiskTest::new(cptestctx).await; - disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await; + let disk_test = + DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await; let log = &cptestctx.logctx.log; @@ -1195,8 +1197,8 @@ pub(crate) mod test { async fn test_actions_succeed_idempotently( cptestctx: &ControlPlaneTestContext, ) { - let mut disk_test = DiskTest::new(cptestctx).await; - disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await; + let _disk_test = + DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await; let client = &cptestctx.external_client; let nexus = &cptestctx.server.server_context().nexus; diff --git a/nexus/src/app/sagas/region_snapshot_replacement_start.rs b/nexus/src/app/sagas/region_snapshot_replacement_start.rs index def3d6ac919..f7be2925dd2 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_start.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_start.rs @@ -1258,8 +1258,8 @@ pub(crate) mod test { assert_eq!(region_allocations(&datastore).await, 0); - let mut disk_test = DiskTest::new(cptestctx).await; - disk_test.add_zpool_with_dataset(cptestctx.first_sled_id()).await; + let disk_test = + DiskTestBuilder::new(cptestctx).with_zpool_count(4).build().await; assert_eq!(region_allocations(&datastore).await, 0); diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 8be043655dd..e2c0e62ea1f 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -1225,9 +1225,10 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { disk_test.sleds.keys().cloned().collect::>() { for _ in 0..zpool_count { - disk_test.add_zpool_with_dataset(sled_id).await; + disk_test.add_zpool_with_datasets(sled_id).await; } } + disk_test.propagate_datasets_to_sleds().await; disk_test } @@ -1235,7 +1236,7 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { pub async fn add_blueprint_disks(&mut self, blueprint: &Blueprint) { for (sled_id, sled_config) in blueprint.sleds.iter() { for disk in &sled_config.disks { - self.add_zpool_with_dataset_ext( + self.add_zpool_with_datasets_ext( *sled_id, disk.id, disk.pool_id, @@ -1250,15 +1251,21 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { } } - pub async fn add_zpool_with_dataset(&mut self, sled_id: SledUuid) { - self.add_zpool_with_dataset_ext( + pub async fn add_zpool_with_datasets(&mut self, sled_id: SledUuid) { + self.add_zpool_with_datasets_ext( sled_id, PhysicalDiskUuid::new_v4(), ZpoolUuid::new_v4(), - vec![TestDataset { - id: DatasetUuid::new_v4(), - kind: DatasetKind::Crucible, - }], + vec![ + TestDataset { + id: DatasetUuid::new_v4(), + kind: DatasetKind::Crucible, + }, + TestDataset { + id: DatasetUuid::new_v4(), + kind: DatasetKind::Debug, + }, + ], Self::DEFAULT_ZPOOL_SIZE_GIB, ) .await @@ -1279,7 +1286,7 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { // 2. Re-work the DiskTestBuilder API to automatically deploy the "disks + datasets" config // to sled agents exactly once. Adding new zpools / datasets after the DiskTest has been // started will need to also make a decision about re-deploying this configuration. - pub async fn propagate_datasets_to_sleds(&mut self) { + async fn propagate_datasets_to_sleds(&mut self) { let cptestctx = self.cptestctx; for (sled_id, PerSledDiskState { zpools }) in &self.sleds { @@ -1347,7 +1354,7 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { } } - pub async fn add_zpool_with_dataset_ext( + pub async fn add_zpool_with_datasets_ext( &mut self, sled_id: SledUuid, physical_disk_id: PhysicalDiskUuid, @@ -1508,10 +1515,15 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { zpools.push(zpool); } + /// Configures all region requests within Crucible datasets to return + /// "Requested", then "Created". pub async fn set_requested_then_created_callback(&self) { for (sled_id, state) in &self.sleds { for zpool in &state.zpools { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } let crucible = self .get_sled(*sled_id) .get_crucible_dataset(zpool.id, dataset.id); @@ -1532,10 +1544,14 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { } } + /// Configures all region requests within Crucible datasets to fail pub async fn set_always_fail_callback(&self) { for (sled_id, state) in &self.sleds { for zpool in &state.zpools { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } let crucible = self .get_sled(*sled_id) .get_crucible_dataset(zpool.id, dataset.id); @@ -1555,6 +1571,9 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { for (sled_id, state) in &self.sleds { for zpool in &state.zpools { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } let crucible = self .get_sled(*sled_id) .get_crucible_dataset(zpool.id, dataset.id); diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs index b38bd059007..b3f70cb5cf0 100644 --- a/nexus/tests/integration_tests/disks.rs +++ b/nexus/tests/integration_tests/disks.rs @@ -45,6 +45,7 @@ use omicron_common::api::external::InstanceState; use omicron_common::api::external::Name; use omicron_common::api::external::NameOrId; use omicron_common::api::external::{ByteCount, SimpleIdentityOrName as _}; +use omicron_common::disk::DatasetKind; use omicron_nexus::Nexus; use omicron_nexus::TestInterfaces as _; use omicron_nexus::app::{MAX_DISK_SIZE_BYTES, MIN_DISK_SIZE_BYTES}; @@ -999,17 +1000,13 @@ async fn test_disk_backed_by_multiple_region_sets( ) { let client = &cptestctx.external_client; - // Create three zpools, each with one dataset - let mut test = DiskTest::new(&cptestctx).await; + // Create six zpools, each with one crucible dataset + let _test = + DiskTestBuilder::new(&cptestctx).with_zpool_count(6).build().await; // Assert default is still 16 GiB assert_eq!(16, DiskTest::DEFAULT_ZPOOL_SIZE_GIB); - // Create another three zpools, each with one dataset - test.add_zpool_with_dataset(cptestctx.first_sled_id()).await; - test.add_zpool_with_dataset(cptestctx.first_sled_id()).await; - test.add_zpool_with_dataset(cptestctx.first_sled_id()).await; - create_project_and_pool(client).await; // Ask for a 20 gibibyte disk. @@ -1558,6 +1555,10 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // Total occupied size should start at 0 for zpool in test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } + assert_eq!( datastore .regions_total_reserved_size(dataset.id) @@ -1607,6 +1608,9 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // plus reservation overhead for zpool in test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } assert_eq!( datastore .regions_total_reserved_size(dataset.id) @@ -1644,6 +1648,9 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // Total occupied size is still 7 GiB * 3 (plus overhead) for zpool in test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } assert_eq!( datastore .regions_total_reserved_size(dataset.id) @@ -1668,6 +1675,9 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // Total occupied size should be 0 for zpool in test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } assert_eq!( datastore .regions_total_reserved_size(dataset.id) @@ -1704,6 +1714,9 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // Total occupied size should be 10 GiB * 3 plus overhead for zpool in test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } assert_eq!( datastore .regions_total_reserved_size(dataset.id) diff --git a/nexus/tests/integration_tests/snapshots.rs b/nexus/tests/integration_tests/snapshots.rs index e4ccb18aed6..17b0a58c80e 100644 --- a/nexus/tests/integration_tests/snapshots.rs +++ b/nexus/tests/integration_tests/snapshots.rs @@ -38,6 +38,7 @@ use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; use omicron_common::api::external::Name; +use omicron_common::disk::DatasetKind; use omicron_nexus::app::MIN_DISK_SIZE_BYTES; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; @@ -978,7 +979,11 @@ async fn test_snapshot_unwind(cptestctx: &ControlPlaneTestContext) { // Set the third region's running snapshot callback so it fails let zpool = disk_test.zpools().nth(2).expect("Not enough zpools"); - let dataset = &zpool.datasets[0]; + let dataset = zpool + .datasets + .iter() + .find(|dataset| matches!(dataset.kind, DatasetKind::Crucible)) + .expect("No crucible dataset found on this zpool"); cptestctx .first_sled_agent() .get_crucible_dataset(zpool.id, dataset.id) @@ -1616,7 +1621,7 @@ async fn test_region_allocation_for_snapshot( assert_eq!(allocated_regions.len(), 1); // If an additional region is required, make sure that works too. - disk_test.add_zpool_with_dataset(sled_id).await; + disk_test.add_zpool_with_datasets(sled_id).await; datastore .arbitrary_region_allocate( diff --git a/nexus/tests/integration_tests/support_bundles.rs b/nexus/tests/integration_tests/support_bundles.rs index d542d13d825..0e4f03c78d9 100644 --- a/nexus/tests/integration_tests/support_bundles.rs +++ b/nexus/tests/integration_tests/support_bundles.rs @@ -14,22 +14,23 @@ use nexus_client::types::LastResult; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; -use nexus_test_utils::resource_helpers::TestDataset; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::shared::SupportBundleInfo; use nexus_types::external_api::shared::SupportBundleState; use nexus_types::internal_api::background::SupportBundleCleanupReport; use nexus_types::internal_api::background::SupportBundleCollectionReport; use omicron_common::api::internal::shared::DatasetKind; -use omicron_uuid_kinds::{DatasetUuid, SupportBundleUuid, ZpoolUuid}; +use omicron_uuid_kinds::SupportBundleUuid; use serde::Deserialize; use std::io::Cursor; use zip::read::ZipArchive; type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; -type DiskTest<'a> = - nexus_test_utils::resource_helpers::DiskTest<'a, omicron_nexus::Server>; +type DiskTestBuilder<'a> = nexus_test_utils::resource_helpers::DiskTestBuilder< + 'a, + omicron_nexus::Server, +>; // -- HTTP methods -- // @@ -340,29 +341,8 @@ async fn test_support_bundle_not_found(cptestctx: &ControlPlaneTestContext) { async fn test_support_bundle_lifecycle(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; - let mut disk_test = DiskTest::new(&cptestctx).await; - - // We really care about just "getting a debug dataset" here. - let sled_id = cptestctx.first_sled_id(); - disk_test - .add_zpool_with_dataset_ext( - sled_id, - nexus_test_utils::PHYSICAL_DISK_UUID.parse().unwrap(), - ZpoolUuid::new_v4(), - vec![ - TestDataset { - id: DatasetUuid::new_v4(), - kind: DatasetKind::Crucible, - }, - TestDataset { - id: DatasetUuid::new_v4(), - kind: DatasetKind::Debug, - }, - ], - DiskTest::DEFAULT_ZPOOL_SIZE_GIB, - ) - .await; - disk_test.propagate_datasets_to_sleds().await; + let disk_test = + DiskTestBuilder::new(&cptestctx).with_zpool_count(1).build().await; // Validate our test setup: We should see a single Debug dataset // in our disk test. diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 5ccb0b2ceb1..c8dc230847c 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -18,11 +18,7 @@ use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::http_testing::TestResponse; -use nexus_test_utils::resource_helpers::TestDataset; use nexus_test_utils_macros::nexus_test; -use omicron_common::disk::DatasetKind; -use omicron_uuid_kinds::DatasetUuid; -use omicron_uuid_kinds::ZpoolUuid; use std::sync::LazyLock; type ControlPlaneTestContext = @@ -59,27 +55,7 @@ type DiskTest<'a> = // 403). #[nexus_test] async fn test_unauthorized(cptestctx: &ControlPlaneTestContext) { - let mut disk_test = DiskTest::new(cptestctx).await; - let sled_id = cptestctx.first_sled_id(); - disk_test - .add_zpool_with_dataset_ext( - sled_id, - nexus_test_utils::PHYSICAL_DISK_UUID.parse().unwrap(), - ZpoolUuid::new_v4(), - vec![ - TestDataset { - id: DatasetUuid::new_v4(), - kind: DatasetKind::Crucible, - }, - TestDataset { - id: DatasetUuid::new_v4(), - kind: DatasetKind::Debug, - }, - ], - DiskTest::DEFAULT_ZPOOL_SIZE_GIB, - ) - .await; - disk_test.propagate_datasets_to_sleds().await; + let _disk_test = DiskTest::new(cptestctx).await; let client = &cptestctx.external_client; let log = &cptestctx.logctx.log; From 1af91c63f0c2d28b66a8484d30c71abdba769029 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 15 Apr 2025 13:40:25 -0700 Subject: [PATCH 05/31] test patching --- nexus/src/app/sagas/disk_create.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nexus/src/app/sagas/disk_create.rs b/nexus/src/app/sagas/disk_create.rs index 701fdae63b7..2905218c42e 100644 --- a/nexus/src/app/sagas/disk_create.rs +++ b/nexus/src/app/sagas/disk_create.rs @@ -16,6 +16,7 @@ use nexus_db_queries::db::identity::{Asset, Resource}; use nexus_db_queries::db::lookup::LookupPath; use omicron_common::api::external::DiskState; use omicron_common::api::external::Error; +use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::VolumeUuid; use rand::{RngCore, SeedableRng, rngs::StdRng}; use serde::Deserialize; @@ -1013,6 +1014,9 @@ pub(crate) mod test { ) -> bool { for zpool in test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } if datastore .regions_total_reserved_size(dataset.id) .await @@ -1029,6 +1033,9 @@ pub(crate) mod test { fn no_regions_ensured(sled_agent: &SledAgent, test: &DiskTest<'_>) -> bool { for zpool in test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } let crucible_dataset = sled_agent.get_crucible_dataset(zpool.id, dataset.id); if !crucible_dataset.is_empty() { From 73b497524816584abe42d69ff609772038dc04be Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 15 Apr 2025 13:58:19 -0700 Subject: [PATCH 06/31] Try compiling --- nexus/src/app/sagas/disk_create.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/app/sagas/disk_create.rs b/nexus/src/app/sagas/disk_create.rs index 2905218c42e..5ea0b8bdbf4 100644 --- a/nexus/src/app/sagas/disk_create.rs +++ b/nexus/src/app/sagas/disk_create.rs @@ -16,7 +16,6 @@ use nexus_db_queries::db::identity::{Asset, Resource}; use nexus_db_queries::db::lookup::LookupPath; use omicron_common::api::external::DiskState; use omicron_common::api::external::Error; -use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::VolumeUuid; use rand::{RngCore, SeedableRng, rngs::StdRng}; use serde::Deserialize; @@ -837,6 +836,7 @@ pub(crate) mod test { use omicron_common::api::external::ByteCount; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Name; + use omicron_common::disk::DatasetKind; use omicron_sled_agent::sim::SledAgent; use uuid::Uuid; From 8022e12720f996faa94d6a7379c2e7745d75b52d Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 15 Apr 2025 15:04:39 -0700 Subject: [PATCH 07/31] Use internal opctx --- nexus/src/internal_api/http_entrypoints.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/nexus/src/internal_api/http_entrypoints.rs b/nexus/src/internal_api/http_entrypoints.rs index afbe0a41ce8..5c7f2a42da8 100644 --- a/nexus/src/internal_api/http_entrypoints.rs +++ b/nexus/src/internal_api/http_entrypoints.rs @@ -920,7 +920,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let pagparams = data_page_params_for(&rqctx, &query)?; let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; let bundles = nexus .support_bundle_list(&opctx, &pagparams) @@ -954,7 +954,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let path = path_params.into_inner(); let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; let bundle = nexus .support_bundle_view( @@ -981,7 +981,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let nexus = &apictx.context.nexus; let path = path_params.into_inner(); let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; let head = false; let range = rqctx.range(); @@ -1013,7 +1013,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let nexus = &apictx.context.nexus; let path = path_params.into_inner(); let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; let head = false; let range = rqctx.range(); @@ -1045,7 +1045,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let nexus = &apictx.context.nexus; let path = path_params.into_inner(); let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; let head = false; let range = rqctx.range(); @@ -1078,7 +1078,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let nexus = &apictx.context.nexus; let path = path_params.into_inner(); let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; let head = true; let range = rqctx.range(); @@ -1109,7 +1109,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let nexus = &apictx.context.nexus; let path = path_params.into_inner(); let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; let head = true; let range = rqctx.range(); @@ -1141,7 +1141,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let nexus = &apictx.context.nexus; let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; let bundle = nexus .support_bundle_create(&opctx, "Created by external API") @@ -1165,7 +1165,7 @@ impl NexusInternalApi for NexusInternalApiImpl { let path = path_params.into_inner(); let opctx = - crate::context::op_context_for_external_api(&rqctx).await?; + crate::context::op_context_for_internal_api(&rqctx).await; nexus .support_bundle_delete( From 47819c0e526b7c75809a4c85b18bf38f56b0e0dc Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 15 Apr 2025 15:50:42 -0700 Subject: [PATCH 08/31] Patching tests more --- .../region_snapshot_replacement_start.rs | 5 +++ .../region_snapshot_replacement_start.rs | 5 +++ nexus/test-utils/src/resource_helpers.rs | 41 +++++++++++-------- nexus/tests/integration_tests/unauthorized.rs | 26 +++++++++++- 4 files changed, 59 insertions(+), 18 deletions(-) diff --git a/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs b/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs index 362f5abf3e7..0fe399f125b 100644 --- a/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs +++ b/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs @@ -325,6 +325,7 @@ mod test { use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external; + use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::VolumeUuid; @@ -465,6 +466,10 @@ mod test { for zpool in disk_test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } + dataset_to_zpool .insert(zpool.id.to_string(), dataset.id.to_string()); diff --git a/nexus/src/app/sagas/region_snapshot_replacement_start.rs b/nexus/src/app/sagas/region_snapshot_replacement_start.rs index f7be2925dd2..1d208bdb6bb 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_start.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_start.rs @@ -1237,6 +1237,7 @@ pub(crate) mod test { use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views; use nexus_types::identity::Asset; + use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::GenericUuid; use sled_agent_client::VolumeConstructionRequest; @@ -1500,6 +1501,10 @@ pub(crate) mod test { for zpool in test.zpools() { for dataset in &zpool.datasets { + if !matches!(dataset.kind, DatasetKind::Crucible) { + continue; + } + let crucible_dataset = sled_agent.get_crucible_dataset(zpool.id, dataset.id); for region in crucible_dataset.list() { diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index e2c0e62ea1f..0add8728cb9 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -1182,6 +1182,7 @@ impl<'a> Iterator for ZpoolIterator<'a> { pub struct DiskTest<'a, N: NexusServer> { cptestctx: &'a ControlPlaneTestContext, sleds: BTreeMap, + generation: Generation, } impl<'a, N: NexusServer> DiskTest<'a, N> { @@ -1219,7 +1220,8 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { sleds.insert(sled_id, PerSledDiskState { zpools: vec![] }); } - let mut disk_test = Self { cptestctx, sleds }; + let mut disk_test = + Self { cptestctx, sleds, generation: Generation::new() }; for sled_id in disk_test.sleds.keys().cloned().collect::>() @@ -1251,6 +1253,12 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { } } + /// Adds the zpool and datasets into the database. + /// + /// Does not inform sled agents to use these pools. + /// + /// See: [Self::propagate_datasets_to_sleds] if you want to send + /// this configuration to a simulated sled agent. pub async fn add_zpool_with_datasets(&mut self, sled_id: SledUuid) { self.add_zpool_with_datasets_ext( sled_id, @@ -1271,22 +1279,13 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { .await } - // Propagate the dataset configuration to all Sled Agents. - // - // # Panics - // - // This function will panic if any of the Sled Agents have already - // applied dataset configuration. - // + /// Propagate the dataset configuration to all Sled Agents. // TODO: Ideally, we should do the following: - // 1. Also call a similar method to invoke the "omicron_physical_disks_ensure" API. Right now, - // we aren't calling this at all for the simulated sled agent, which only works because - // the simulated sled agent simply treats this as a stored config, rather than processing it - // to actually provide a different view of storage. - // 2. Re-work the DiskTestBuilder API to automatically deploy the "disks + datasets" config - // to sled agents exactly once. Adding new zpools / datasets after the DiskTest has been - // started will need to also make a decision about re-deploying this configuration. - async fn propagate_datasets_to_sleds(&mut self) { + // Also call a similar method to invoke the "omicron_physical_disks_ensure" API. Right now, + // we aren't calling this at all for the simulated sled agent, which only works because + // the simulated sled agent simply treats this as a stored config, rather than processing it + // to actually provide a different view of storage. + pub async fn propagate_datasets_to_sleds(&mut self) { let cptestctx = self.cptestctx; for (sled_id, PerSledDiskState { zpools }) in &self.sleds { @@ -1322,7 +1321,8 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { }) .collect(); - let generation = Generation::new().next(); + self.generation = self.generation.next(); + let generation = self.generation; let dataset_config = DatasetsConfig { generation, datasets }; let res = sled_agent.datasets_ensure(dataset_config).expect( "Should have been able to ensure datasets, but could not. @@ -1354,6 +1354,13 @@ impl<'a, N: NexusServer> DiskTest<'a, N> { } } + /// Adds the zpool and datasets into the database, with additional + /// configuration. + /// + /// Does not inform sled agents to use these pools. + /// + /// See: [Self::propagate_datasets_to_sleds] if you want to send + /// this configuration to a simulated sled agent. pub async fn add_zpool_with_datasets_ext( &mut self, sled_id: SledUuid, diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index c8dc230847c..6cef42c17c2 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -18,7 +18,11 @@ use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::http_testing::TestResponse; +use nexus_test_utils::resource_helpers::TestDataset; use nexus_test_utils_macros::nexus_test; +use omicron_common::disk::DatasetKind; +use omicron_uuid_kinds::DatasetUuid; +use omicron_uuid_kinds::ZpoolUuid; use std::sync::LazyLock; type ControlPlaneTestContext = @@ -55,7 +59,27 @@ type DiskTest<'a> = // 403). #[nexus_test] async fn test_unauthorized(cptestctx: &ControlPlaneTestContext) { - let _disk_test = DiskTest::new(cptestctx).await; + let mut disk_test = DiskTest::new(cptestctx).await; + let sled_id = cptestctx.first_sled_id(); + disk_test + .add_zpool_with_datasets_ext( + sled_id, + nexus_test_utils::PHYSICAL_DISK_UUID.parse().unwrap(), + ZpoolUuid::new_v4(), + vec![ + TestDataset { + id: DatasetUuid::new_v4(), + kind: DatasetKind::Crucible, + }, + TestDataset { + id: DatasetUuid::new_v4(), + kind: DatasetKind::Debug, + }, + ], + DiskTest::DEFAULT_ZPOOL_SIZE_GIB, + ) + .await; + disk_test.propagate_datasets_to_sleds().await; let client = &cptestctx.external_client; let log = &cptestctx.logctx.log; From 6374a7bab27d06af2c554c500c0311ee82886e6c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 16 Apr 2025 10:38:38 -0700 Subject: [PATCH 09/31] Don't inject newlines --- dev-tools/omdb/src/bin/omdb/nexus.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 0093b77c83e..5f62c79ba3a 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -3394,7 +3394,7 @@ async fn print_utf8_stream_to_stdout( }; match std::str::from_utf8(&combined) { - Ok(data) => println!("{data}"), + Ok(data) => print!("{data}"), Err(_) => leftover = Some(combined.into()), } } From c28a398d6b8ba1b4d60bbfa26c6feedaf146b0f3 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 23 Apr 2025 13:25:11 -0700 Subject: [PATCH 10/31] Continuing to iterate on TUI --- Cargo.lock | 17 ++ Cargo.toml | 3 + dev-tools/omdb/Cargo.toml | 1 + dev-tools/omdb/src/bin/omdb/nexus.rs | 234 ++++++++++++++++++ .../support-bundle-reader-lib/Cargo.toml | 20 ++ .../src/bundle_accessor.rs | 174 +++++++++++++ .../support-bundle-reader-lib/src/index.rs | 23 ++ .../support-bundle-reader-lib/src/lib.rs | 142 +++++++++++ 8 files changed, 614 insertions(+) create mode 100644 dev-tools/support-bundle-reader-lib/Cargo.toml create mode 100644 dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs create mode 100644 dev-tools/support-bundle-reader-lib/src/index.rs create mode 100644 dev-tools/support-bundle-reader-lib/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 1fec26360c4..583058cc833 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7397,6 +7397,7 @@ dependencies = [ "steno", "strum", "subprocess", + "support-bundle-reader-lib", "supports-color", "tabled 0.15.0", "textwrap", @@ -11994,6 +11995,22 @@ version = "2.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" +[[package]] +name = "support-bundle-reader-lib" +version = "0.1.0" +dependencies = [ + "anyhow", + "async-trait", + "bytes", + "camino", + "futures", + "nexus-client", + "omicron-uuid-kinds", + "omicron-workspace-hack", + "reqwest", + "tokio", +] + [[package]] name = "supports-color" version = "3.0.2" diff --git a/Cargo.toml b/Cargo.toml index 6c67ccc345b..88ce8daa286 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ members = [ "dev-tools/releng", "dev-tools/repl-utils", "dev-tools/repo-depot-standalone", + "dev-tools/support-bundle-reader-lib", "dev-tools/xtask", "dns-server", "dns-server-api", @@ -191,6 +192,7 @@ default-members = [ "dev-tools/releng", "dev-tools/repl-utils", "dev-tools/repo-depot-standalone", + "dev-tools/support-bundle-reader-lib", # Do not include xtask in the list of default members, because this causes # hakari to not work as well and build times to be longer. # See omicron#4392. @@ -670,6 +672,7 @@ steno = "0.4.1" strum = { version = "0.26", features = [ "derive" ] } subprocess = "0.2.9" supports-color = "3.0.2" +support-bundle-reader-lib = { path = "dev-tools/support-bundle-reader-lib" } swrite = "0.1.0" sync-ptr = "0.1.1" libsw = { version = "3.4.0", features = ["tokio"] } diff --git a/dev-tools/omdb/Cargo.toml b/dev-tools/omdb/Cargo.toml index ca4898549b8..41a2acd6b0a 100644 --- a/dev-tools/omdb/Cargo.toml +++ b/dev-tools/omdb/Cargo.toml @@ -57,6 +57,7 @@ slog.workspace = true slog-error-chain.workspace = true steno.workspace = true strum.workspace = true +support-bundle-reader-lib.workspace = true supports-color.workspace = true tabled.workspace = true textwrap.workspace = true diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 5f62c79ba3a..2a5a4ac3944 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -21,6 +21,16 @@ use clap::Args; use clap::ColorChoice; use clap::Subcommand; use clap::ValueEnum; +use crossterm::{ + event::{ + self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode, + }, + execute, + terminal::{ + EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, + enable_raw_mode, + }, +}; use futures::StreamExt; use futures::TryStreamExt; use futures::future::try_join; @@ -69,12 +79,28 @@ use omicron_uuid_kinds::ParseError; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::SupportBundleUuid; +use ratatui::Frame; +use ratatui::Terminal; +use ratatui::backend::Backend; +use ratatui::backend::CrosstermBackend; +use ratatui::layout::Constraint; +use ratatui::layout::Layout; +use ratatui::style::Modifier; +use ratatui::style::Style; +use ratatui::widgets::Block; +use ratatui::widgets::Borders; +use ratatui::widgets::List; +use ratatui::widgets::ListState; +use ratatui::widgets::Paragraph; use serde::Deserialize; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; use std::collections::BTreeSet; use std::str::FromStr; use std::sync::Arc; +use std::time::Duration; +use support_bundle_reader_lib::SupportBundleAccessor; +use support_bundle_reader_lib::SupportBundleDashboard; use tabled::Tabled; use tabled::settings::Padding; use tabled::settings::object::Columns; @@ -466,6 +492,8 @@ enum SupportBundleCommands { GetIndex(SupportBundleIndexArgs), /// View a file within a support bundle GetFile(SupportBundleFileArgs), + /// Creates dashboard for viewing the contents of a support bundle + Inspect(SupportBundleInspectArgs), } #[derive(Debug, Args)] @@ -484,6 +512,15 @@ struct SupportBundleFileArgs { path: Utf8PathBuf, } +#[derive(Debug, Args)] +struct SupportBundleInspectArgs { + /// A specific bundle to inspect. If none is supplied, the latest active + /// bundle is used. + id: Option, + + // TODO: Option to view a local file? +} + impl NexusArgs { /// Run a `omdb nexus` subcommand. pub(crate) async fn run_cmd( @@ -683,6 +720,10 @@ impl NexusArgs { NexusCommands::SupportBundles(SupportBundleArgs { command: SupportBundleCommands::GetFile(args), }) => cmd_nexus_support_bundles_get_file(&client, args).await, + NexusCommands::SupportBundles(SupportBundleArgs { + command: SupportBundleCommands::Inspect(args), + }) => cmd_nexus_support_bundles_inspect(&client, args).await, + } } } @@ -3444,3 +3485,196 @@ async fn cmd_nexus_support_bundles_get_file( })?; Ok(()) } + +enum InspectRunStep { + // Keep running the dashboard + Continue, + // Exit the dashboard + Exit, + // Exit the dashboard GUI, but pipe a selected file to an output stream + PipeFile, +} + +/// Runs `omdb nexus support-bundles inspect` +async fn cmd_nexus_support_bundles_inspect( + client: &nexus_client::Client, + args: &SupportBundleInspectArgs, +) -> Result<(), anyhow::Error> { + let id = match args.id { + Some(id) => id, + None => { + // Grab the latest if one isn't supplied + let support_bundle_stream = client.support_bundle_list_stream(None, None); + let mut support_bundles = support_bundle_stream + .try_collect::>() + .await + .context("listing support bundles")?; + support_bundles.sort_by_key(|k| k.time_created); + + let sb = support_bundles.into_iter().find(|sb| { + matches!(sb.state, nexus_client::types::SupportBundleState::Active) + }).ok_or(anyhow::anyhow!("Cannot find active support bundle. Try creating one"))?; + + eprintln!("Inspecting bundle {} from {}", sb.id, sb.time_created); + + SupportBundleUuid::from_untyped_uuid(sb.id.into_untyped_uuid()) + } + }; + + let accessor = support_bundle_reader_lib::InternalApiAccess::new( + client, + id, + ); + let mut dashboard = SupportBundleDashboard::new( + &accessor, + ).await?; + + enable_raw_mode()?; + + // TODO: It should probably be a flag whether or not this is stderr or + // stdout. + let mut stderr = std::io::stderr(); + execute!(stderr, EnterAlternateScreen, EnableMouseCapture)?; + let backend = CrosstermBackend::new(stderr); + let mut terminal = Terminal::new(backend)?; + + let mut force_update = true; + let pipe_selected_file = loop { + match run_support_bundle_dashboard( + &mut terminal, + &mut dashboard, + force_update, + ).await { + Err(err) => break Err(err), + Ok(InspectRunStep::Exit) => break Ok(false), + Ok(InspectRunStep::Continue) => (), + Ok(InspectRunStep::PipeFile) => break Ok(true), + }; + + force_update = false; + tokio::time::sleep(Duration::from_millis(10)).await; + }; + + // restore terminal + disable_raw_mode()?; + execute!( + terminal.backend_mut(), + LeaveAlternateScreen, + DisableMouseCapture + )?; + terminal.show_cursor()?; + + match pipe_selected_file { + Ok(true) => { + if let Some(contents) = dashboard.buffered_file_contents() { + std::io::copy(&mut std::io::Cursor::new(contents.as_bytes()), &mut std::io::stdout())?; + } + }, + Ok(false) => (), + Err(err) => eprintln!("{err:?}"), + } + Ok(()) +} + +async fn run_support_bundle_dashboard<'a, B: Backend, S: SupportBundleAccessor>( + terminal: &mut Terminal, + dashboard: &mut SupportBundleDashboard<'a, S>, + force_update: bool, +) -> anyhow::Result { + let update = if crossterm::event::poll(Duration::from_secs(0))? { + if let Event::Key(key) = event::read()? { + match key.code { + KeyCode::Char('q') => return Ok(InspectRunStep::Exit), + KeyCode::Up | KeyCode::Char('k') => dashboard.select_up(), + KeyCode::Down | KeyCode::Char('j') => dashboard.select_down(), + KeyCode::Char(' ') => { + dashboard.open_and_buffer().await?; + return Ok(InspectRunStep::PipeFile); + } + // TODO: file text seems to not wrap around; it probably should? + KeyCode::Enter => dashboard.toggle_file_open().await?, + _ => {} + } + } + true + } else { + force_update + }; + + if force_update { + terminal.clear()?; + } + + if update { + terminal.draw(|f| draw(f, dashboard))?; + } + + Ok(InspectRunStep::Continue) +} + +fn create_file_list<'a, S: SupportBundleAccessor>(dashboard: &'a SupportBundleDashboard<'_, S>) -> List<'a> { + let files = dashboard.index() + .files() + .iter() + .map(|f| f.as_str()); + List::new(files) + .highlight_symbol("> ") + .highlight_style(Style::new().add_modifier(Modifier::BOLD)) + .block( + Block::new().title("Files").borders(Borders::ALL) + ) +} + +fn create_file_contents<'a, S: SupportBundleAccessor>(dashboard: &'a SupportBundleDashboard<'_, S>) -> Option> { + dashboard.buffered_file_contents().map(|c| { + Paragraph::new(c) + .block( + Block::new().title(dashboard.selected_file_name().as_str()) + .borders(Borders::ALL) + ) + }) +} + +const FILE_PICKER_USAGE: [&'static str; 3] = [ + "Press UP or DOWN to select a file", + "Press ENTER to view a file, or SPACE to exit the terminal and dump the file to stdout", + "Press 'q' to quit", +]; + +const FILE_VIEWER_USAGE: [&'static str; 3] = [ + "Press ENTER to stop viewing file", + "", + "Press 'q' to quit", +]; + +fn draw<'a, S: SupportBundleAccessor>( + f: &mut Frame, dashboard: &mut SupportBundleDashboard<'a, S> +) { + let file_list = create_file_list(dashboard); + let file_contents = create_file_contents(dashboard); + + let mut file_state = ListState::default() + .with_offset(0) + .with_selected(Some(dashboard.selected_file_index())); + + let layout = Layout::vertical([ + Constraint::Min(0), + Constraint::Length(5), + ]); + + + let [main_display_rect, usage_rect] = layout.areas(f.area()); + + if let Some(file_contents) = file_contents { + let usage_list = List::new(FILE_VIEWER_USAGE) + .block(Block::new().title("Usage").borders(Borders::ALL)); + + f.render_widget(file_contents, main_display_rect); + f.render_widget(usage_list, usage_rect); + } else { + let usage_list = List::new(FILE_PICKER_USAGE) + .block(Block::new().title("Usage").borders(Borders::ALL)); + f.render_stateful_widget(file_list, main_display_rect, &mut file_state); + f.render_widget(usage_list, usage_rect); + } +} diff --git a/dev-tools/support-bundle-reader-lib/Cargo.toml b/dev-tools/support-bundle-reader-lib/Cargo.toml new file mode 100644 index 00000000000..0e5c4bc5e55 --- /dev/null +++ b/dev-tools/support-bundle-reader-lib/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "support-bundle-reader-lib" +version = "0.1.0" +edition = "2021" +license = "MPL-2.0" + +[lints] +workspace = true + +[dependencies] +anyhow.workspace = true +async-trait.workspace = true +bytes.workspace = true +camino.workspace = true +futures.workspace = true +nexus-client.workspace = true +omicron-workspace-hack.workspace = true +omicron-uuid-kinds.workspace = true +reqwest.workspace = true +tokio.workspace = true diff --git a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs new file mode 100644 index 00000000000..b55660ad44a --- /dev/null +++ b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs @@ -0,0 +1,174 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Utilities to help insepct support bundles + +use anyhow::Context as _; +use anyhow::Result; +use async_trait::async_trait; +use bytes::Buf; +use bytes::Bytes; +use camino::Utf8Path; +use camino::Utf8PathBuf; +use futures::Future; +use futures::Stream; +use omicron_uuid_kinds::GenericUuid; +use omicron_uuid_kinds::SupportBundleUuid; +use std::io; +use std::pin::Pin; +use std::task::Context; +use std::task::Poll; +use tokio::io::AsyncRead; +use tokio::io::ReadBuf; + +use crate::SupportBundleIndex; +use crate::utf8_stream_to_string; + +/// An I/O source which can read to a buffer +/// +/// This describes access to individual files within the bundle. +pub trait FileAccessor: AsyncRead {} +impl FileAccessor for T {} + +/// Describes how the support bundle's data and metadata are accessed. +#[async_trait] +pub trait SupportBundleAccessor { + type FileAccessor<'a>: FileAccessor where Self: 'a; + + /// Access the index of a support bundle + async fn get_index(&self) -> Result; + + /// Access a file within the support bundle + async fn get_file(&self, path: &Utf8Path) -> Result>; +} + +pub struct StreamedFile<'a> { + client: &'a nexus_client::Client, + id: SupportBundleUuid, + path: Utf8PathBuf, + stream: Option> + Send>>>, + buffer: Bytes, +} + +impl<'a> StreamedFile<'a> { + fn new(client: &'a nexus_client::Client, id: SupportBundleUuid, path: Utf8PathBuf) -> Self { + Self { + client, + id, + path, + stream: None, + buffer: Bytes::new(), + } + } + + async fn start_stream(&mut self) -> Result<()> { + // TODO: Add range headers? + let stream = self.client.support_bundle_download_file( + self.id.as_untyped_uuid(), + self.path.as_str() + ) + .await + .with_context(|| { + format!("downloading support bundle file {}: {}", self.id, self.path) + })? + .into_inner_stream(); + + self.stream = Some(Box::pin(stream)); + Ok(()) + } +} + +impl AsyncRead for StreamedFile<'_> { + fn poll_read( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + while self.buffer.is_empty() { + if self.stream.is_none() { + + // NOTE: this is broken? + let fut = self.start_stream(); + let mut fut = Box::pin(fut); + + match futures::ready!(fut.as_mut().poll(cx)) { + Ok(()) => {} + Err(e) => return Poll::Ready(Err(io::Error::new(io::ErrorKind::Other, e))), + } + } + + match futures::ready!(self.stream.as_mut().unwrap().as_mut().poll_next(cx)) { + Some(Ok(bytes)) => { + self.buffer = bytes; + } + Some(Err(e)) => return Poll::Ready(Err(io::Error::new(io::ErrorKind::Other, e))), + None => return Poll::Ready(Ok(())), // EOF + } + } + + let to_copy = std::cmp::min(self.buffer.len(), buf.remaining()); + buf.put_slice(&self.buffer[..to_copy]); + self.buffer.advance(to_copy); + + Poll::Ready(Ok(())) + } +} + +/// Access to a support bundle from the internal API +pub struct InternalApiAccess<'a> { + client: &'a nexus_client::Client, + id: SupportBundleUuid, +} + +impl<'a> InternalApiAccess<'a> { + pub fn new(client: &'a nexus_client::Client, id: SupportBundleUuid) -> Self { + Self { + client, + id, + } + } +} + +// Access for: The nexus internal API +#[async_trait] +impl<'c> SupportBundleAccessor for InternalApiAccess<'c> { + type FileAccessor<'a> = StreamedFile<'a> where Self: 'a; + + async fn get_index(&self) -> Result { + let stream = self + .client + .support_bundle_index(self.id.as_untyped_uuid()) + .await + .with_context(|| { + format!("downloading support bundle index {}", self.id) + })? + .into_inner_stream(); + let s = utf8_stream_to_string(stream).await?; + + Ok(SupportBundleIndex::new(&s)) + } + + async fn get_file(&self, path: &Utf8Path) -> Result> { + let mut file = StreamedFile::new(self.client, self.id, path.to_path_buf()); + file.start_stream().await?; + Ok(file) + } +} + +// TODO: Probably want to impl this on a new struct that contains a "ZipReader" + +// Access for: Local zip files +#[async_trait] +impl SupportBundleAccessor for tokio::fs::File { + type FileAccessor<'a> = tokio::fs::File; + + async fn get_index(&self) -> Result { + todo!(); + } + + async fn get_file(&self, _path: &Utf8Path) -> Result> { + todo!(); + } +} + diff --git a/dev-tools/support-bundle-reader-lib/src/index.rs b/dev-tools/support-bundle-reader-lib/src/index.rs new file mode 100644 index 00000000000..c0bba2ef0f8 --- /dev/null +++ b/dev-tools/support-bundle-reader-lib/src/index.rs @@ -0,0 +1,23 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use camino::Utf8PathBuf; + +/// The index, or list of files, within a support bundle. +pub struct SupportBundleIndex(Vec); + +impl SupportBundleIndex { + pub fn new(s: &str) -> Self { + let mut all_file_names: Vec<_> = s.lines().map(|line| { + Utf8PathBuf::from(line) + }).collect(); + all_file_names.sort(); + SupportBundleIndex(all_file_names) + } + + /// Returns all files in the index, sorted by path + pub fn files(&self) -> &Vec { + &self.0 + } +} diff --git a/dev-tools/support-bundle-reader-lib/src/lib.rs b/dev-tools/support-bundle-reader-lib/src/lib.rs new file mode 100644 index 00000000000..49d68d1cab3 --- /dev/null +++ b/dev-tools/support-bundle-reader-lib/src/lib.rs @@ -0,0 +1,142 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Utilities to help insepct support bundles + +use anyhow::bail; +use anyhow::Result; +use camino::Utf8Path; +use futures::StreamExt; +use std::pin::Pin; +use tokio::io::AsyncReadExt; + +mod bundle_accessor; +mod index; + +pub use bundle_accessor::FileAccessor; +pub use bundle_accessor::InternalApiAccess; +pub use bundle_accessor::SupportBundleAccessor; +pub use index::SupportBundleIndex; + +/// A dashboard for inspecting a support bundle contents +pub struct SupportBundleDashboard<'a, S> { + access: &'a S, + index: SupportBundleIndex, + selected: usize, + opened: Option>>, + buffered: String, +} + +impl<'a, S: SupportBundleAccessor> SupportBundleDashboard<'a, S> { + pub async fn new(access: &'a S) -> Result { + let index = access.get_index().await?; + if index.files().is_empty() { + bail!("No files found in support bundle"); + } + Ok(Self { + access, + index, + selected: 0, + opened: None, + buffered: String::new(), + }) + } + + pub fn index(&self) -> &SupportBundleIndex { + &self.index + } + + pub fn select_up(&mut self) { + if self.buffered_file_contents().is_none() { + self.selected = self.selected.saturating_sub(1); + } + } + + pub fn select_down(&mut self) { + if self.buffered_file_contents().is_none() { + self.selected = std::cmp::min(self.selected + 1, self.index.files().len() - 1); + } + } + + pub async fn toggle_file_open(&mut self) -> Result<()> { + if self.buffered_file_contents().is_none() { + self.open_and_buffer().await?; + } else { + self.close_file(); + } + Ok(()) + } + + pub async fn open_and_buffer(&mut self) -> Result<()> { + self.close_file(); + self.open_file().await?; + self.read_to_buffer().await?; + Ok(()) + } + + async fn open_file(&mut self) -> Result<()> { + let path = &self.index.files()[self.selected]; + let file = self.access.get_file(&path).await?; + self.opened = Some(Box::pin(file)); + self.buffered = String::new(); + Ok(()) + } + + fn close_file(&mut self) { + self.opened = None; + self.buffered = String::new(); + } + + async fn read_to_buffer(&mut self) -> Result<()> { + let Some(file) = self.opened.as_mut() else { + return Ok(()); + }; + file.read_to_string(&mut self.buffered).await?; + Ok(()) + } + + pub fn buffered_file_contents(&self) -> Option<&str> { + if self.opened.is_some() { + return Some(&self.buffered); + } + None + } + + pub fn selected_file_index(&self) -> usize { + self.selected + } + + pub fn selected_file_name(&self) -> &Utf8Path { + &self.index.files()[self.selected_file_index()] + } +} + +pub(crate) async fn utf8_stream_to_string( + mut stream: impl futures::Stream> + + std::marker::Unpin, +) -> Result { + let mut result = String::new(); + + // When we read from the string, we might not read a whole UTF-8 sequence. + // Keep this "leftover" type here to concatenate this partially-read data + // when we read the next sequence. + let mut leftover: Option = None; + while let Some(data) = stream.next().await { + match data { + Err(err) => return Err(anyhow::anyhow!(err)), + Ok(data) => { + let combined = match leftover.take() { + Some(old) => [old, data].concat(), + None => data.to_vec(), + }; + + match std::str::from_utf8(&combined) { + Ok(data) => result += data, + Err(_) => leftover = Some(combined.into()), + } + } + } + } + Ok(result) +} From 820556fd91dd29cf182d45811c20edfd9da14a0c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 23 Apr 2025 16:09:30 -0700 Subject: [PATCH 11/31] Shift to move faster, fix dirs, wrapping --- dev-tools/omdb/src/bin/omdb/nexus.rs | 111 ++++++++++-------- .../src/bundle_accessor.rs | 84 ++++++++----- .../support-bundle-reader-lib/src/index.rs | 5 +- .../support-bundle-reader-lib/src/lib.rs | 74 +++++++----- 4 files changed, 167 insertions(+), 107 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 5a593cc2ee5..68477960d24 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -22,9 +22,7 @@ use clap::ColorChoice; use clap::Subcommand; use clap::ValueEnum; use crossterm::{ - event::{ - self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode, - }, + event::{self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode}, execute, terminal::{ EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, @@ -94,6 +92,7 @@ use ratatui::widgets::Borders; use ratatui::widgets::List; use ratatui::widgets::ListState; use ratatui::widgets::Paragraph; +use ratatui::widgets::Wrap; use serde::Deserialize; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; @@ -548,7 +547,6 @@ struct SupportBundleInspectArgs { /// A specific bundle to inspect. If none is supplied, the latest active /// bundle is used. id: Option, - // TODO: Option to view a local file? } @@ -767,7 +765,6 @@ impl NexusArgs { NexusCommands::SupportBundles(SupportBundleArgs { command: SupportBundleCommands::Inspect(args), }) => cmd_nexus_support_bundles_inspect(&client, args).await, - } } } @@ -3625,16 +3622,25 @@ async fn cmd_nexus_support_bundles_inspect( Some(id) => id, None => { // Grab the latest if one isn't supplied - let support_bundle_stream = client.support_bundle_list_stream(None, None); + let support_bundle_stream = + client.support_bundle_list_stream(None, None); let mut support_bundles = support_bundle_stream .try_collect::>() .await .context("listing support bundles")?; support_bundles.sort_by_key(|k| k.time_created); - let sb = support_bundles.into_iter().find(|sb| { - matches!(sb.state, nexus_client::types::SupportBundleState::Active) - }).ok_or(anyhow::anyhow!("Cannot find active support bundle. Try creating one"))?; + let sb = support_bundles + .into_iter() + .find(|sb| { + matches!( + sb.state, + nexus_client::types::SupportBundleState::Active + ) + }) + .ok_or(anyhow::anyhow!( + "Cannot find active support bundle. Try creating one" + ))?; eprintln!("Inspecting bundle {} from {}", sb.id, sb.time_created); @@ -3642,13 +3648,9 @@ async fn cmd_nexus_support_bundles_inspect( } }; - let accessor = support_bundle_reader_lib::InternalApiAccess::new( - client, - id, - ); - let mut dashboard = SupportBundleDashboard::new( - &accessor, - ).await?; + let accessor = + support_bundle_reader_lib::InternalApiAccess::new(client, id); + let mut dashboard = SupportBundleDashboard::new(&accessor).await?; enable_raw_mode()?; @@ -3665,7 +3667,9 @@ async fn cmd_nexus_support_bundles_inspect( &mut terminal, &mut dashboard, force_update, - ).await { + ) + .await + { Err(err) => break Err(err), Ok(InspectRunStep::Exit) => break Ok(false), Ok(InspectRunStep::Continue) => (), @@ -3688,31 +3692,44 @@ async fn cmd_nexus_support_bundles_inspect( match pipe_selected_file { Ok(true) => { if let Some(contents) = dashboard.buffered_file_contents() { - std::io::copy(&mut std::io::Cursor::new(contents.as_bytes()), &mut std::io::stdout())?; + std::io::copy( + &mut std::io::Cursor::new(contents.as_bytes()), + &mut std::io::stdout(), + )?; } - }, + } Ok(false) => (), Err(err) => eprintln!("{err:?}"), } Ok(()) } -async fn run_support_bundle_dashboard<'a, B: Backend, S: SupportBundleAccessor>( +async fn run_support_bundle_dashboard< + 'a, + B: Backend, + S: SupportBundleAccessor, +>( terminal: &mut Terminal, dashboard: &mut SupportBundleDashboard<'a, S>, force_update: bool, ) -> anyhow::Result { let update = if crossterm::event::poll(Duration::from_secs(0))? { if let Event::Key(key) = event::read()? { + let shifted = key.modifiers.contains(event::KeyModifiers::SHIFT); match key.code { KeyCode::Char('q') => return Ok(InspectRunStep::Exit), - KeyCode::Up | KeyCode::Char('k') => dashboard.select_up(), - KeyCode::Down | KeyCode::Char('j') => dashboard.select_down(), + KeyCode::Up | KeyCode::Char('k') => { + let count = if shifted { 5 } else { 1 }; + dashboard.select_up(count).await?; + } + KeyCode::Down | KeyCode::Char('j') => { + let count = if shifted { 5 } else { 1 }; + dashboard.select_down(count).await?; + } KeyCode::Char(' ') => { dashboard.open_and_buffer().await?; return Ok(InspectRunStep::PipeFile); } - // TODO: file text seems to not wrap around; it probably should? KeyCode::Enter => dashboard.toggle_file_open().await?, _ => {} } @@ -3733,43 +3750,45 @@ async fn run_support_bundle_dashboard<'a, B: Backend, S: SupportBundleAccessor>( Ok(InspectRunStep::Continue) } -fn create_file_list<'a, S: SupportBundleAccessor>(dashboard: &'a SupportBundleDashboard<'_, S>) -> List<'a> { - let files = dashboard.index() - .files() - .iter() - .map(|f| f.as_str()); +fn create_file_list<'a, S: SupportBundleAccessor>( + dashboard: &'a SupportBundleDashboard<'_, S>, +) -> List<'a> { + let files = dashboard.index().files().iter().map(|f| f.as_str()); List::new(files) .highlight_symbol("> ") .highlight_style(Style::new().add_modifier(Modifier::BOLD)) - .block( - Block::new().title("Files").borders(Borders::ALL) - ) + .block(Block::new().title("Files").borders(Borders::ALL)) } -fn create_file_contents<'a, S: SupportBundleAccessor>(dashboard: &'a SupportBundleDashboard<'_, S>) -> Option> { +fn create_file_contents<'a, S: SupportBundleAccessor>( + dashboard: &'a SupportBundleDashboard<'_, S>, +) -> Option> { dashboard.buffered_file_contents().map(|c| { - Paragraph::new(c) - .block( - Block::new().title(dashboard.selected_file_name().as_str()) - .borders(Borders::ALL) - ) + Paragraph::new(c).wrap(Wrap { trim: false }).block( + Block::new() + .title(dashboard.selected_file_name().as_str()) + .borders(Borders::ALL), + ) }) } -const FILE_PICKER_USAGE: [&'static str; 3] = [ - "Press UP or DOWN to select a file", - "Press ENTER to view a file, or SPACE to exit the terminal and dump the file to stdout", +const FILE_PICKER_USAGE: [&'static str; 4] = [ + "Press UP or DOWN to select a file. Hold SHIFT to move faster", + "Press ENTER to view a file", + "Press SPACE to exit the terminal and dump the file to stdout", "Press 'q' to quit", ]; -const FILE_VIEWER_USAGE: [&'static str; 3] = [ +const FILE_VIEWER_USAGE: [&'static str; 4] = [ + "Press UP or DOWN to select a file. Hold SHIFT to move faster", "Press ENTER to stop viewing file", - "", + "Press SPACE to exit the terminal and dump the file to stdout", "Press 'q' to quit", ]; fn draw<'a, S: SupportBundleAccessor>( - f: &mut Frame, dashboard: &mut SupportBundleDashboard<'a, S> + f: &mut Frame, + dashboard: &mut SupportBundleDashboard<'a, S>, ) { let file_list = create_file_list(dashboard); let file_contents = create_file_contents(dashboard); @@ -3778,11 +3797,7 @@ fn draw<'a, S: SupportBundleAccessor>( .with_offset(0) .with_selected(Some(dashboard.selected_file_index())); - let layout = Layout::vertical([ - Constraint::Min(0), - Constraint::Length(5), - ]); - + let layout = Layout::vertical([Constraint::Min(0), Constraint::Length(6)]); let [main_display_rect, usage_rect] = layout.areas(f.area()); diff --git a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs index b55660ad44a..f73a52ea753 100644 --- a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs +++ b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs @@ -34,13 +34,16 @@ impl FileAccessor for T {} /// Describes how the support bundle's data and metadata are accessed. #[async_trait] pub trait SupportBundleAccessor { - type FileAccessor<'a>: FileAccessor where Self: 'a; + type FileAccessor<'a>: FileAccessor + where + Self: 'a; /// Access the index of a support bundle async fn get_index(&self) -> Result; /// Access a file within the support bundle - async fn get_file(&self, path: &Utf8Path) -> Result>; + async fn get_file(&self, path: &Utf8Path) + -> Result>; } pub struct StreamedFile<'a> { @@ -52,25 +55,28 @@ pub struct StreamedFile<'a> { } impl<'a> StreamedFile<'a> { - fn new(client: &'a nexus_client::Client, id: SupportBundleUuid, path: Utf8PathBuf) -> Self { - Self { - client, - id, - path, - stream: None, - buffer: Bytes::new(), - } + fn new( + client: &'a nexus_client::Client, + id: SupportBundleUuid, + path: Utf8PathBuf, + ) -> Self { + Self { client, id, path, stream: None, buffer: Bytes::new() } } async fn start_stream(&mut self) -> Result<()> { // TODO: Add range headers? - let stream = self.client.support_bundle_download_file( + let stream = self + .client + .support_bundle_download_file( self.id.as_untyped_uuid(), - self.path.as_str() + self.path.as_str(), ) .await .with_context(|| { - format!("downloading support bundle file {}: {}", self.id, self.path) + format!( + "downloading support bundle file {}: {}", + self.id, self.path + ) })? .into_inner_stream(); @@ -87,22 +93,33 @@ impl AsyncRead for StreamedFile<'_> { ) -> Poll> { while self.buffer.is_empty() { if self.stream.is_none() { - // NOTE: this is broken? let fut = self.start_stream(); let mut fut = Box::pin(fut); match futures::ready!(fut.as_mut().poll(cx)) { Ok(()) => {} - Err(e) => return Poll::Ready(Err(io::Error::new(io::ErrorKind::Other, e))), + Err(e) => { + return Poll::Ready(Err(io::Error::new( + io::ErrorKind::Other, + e, + ))); + } } } - match futures::ready!(self.stream.as_mut().unwrap().as_mut().poll_next(cx)) { + match futures::ready!( + self.stream.as_mut().unwrap().as_mut().poll_next(cx) + ) { Some(Ok(bytes)) => { self.buffer = bytes; } - Some(Err(e)) => return Poll::Ready(Err(io::Error::new(io::ErrorKind::Other, e))), + Some(Err(e)) => { + return Poll::Ready(Err(io::Error::new( + io::ErrorKind::Other, + e, + ))); + } None => return Poll::Ready(Ok(())), // EOF } } @@ -122,18 +139,21 @@ pub struct InternalApiAccess<'a> { } impl<'a> InternalApiAccess<'a> { - pub fn new(client: &'a nexus_client::Client, id: SupportBundleUuid) -> Self { - Self { - client, - id, - } + pub fn new( + client: &'a nexus_client::Client, + id: SupportBundleUuid, + ) -> Self { + Self { client, id } } } // Access for: The nexus internal API #[async_trait] impl<'c> SupportBundleAccessor for InternalApiAccess<'c> { - type FileAccessor<'a> = StreamedFile<'a> where Self: 'a; + type FileAccessor<'a> + = StreamedFile<'a> + where + Self: 'a; async fn get_index(&self) -> Result { let stream = self @@ -149,9 +169,15 @@ impl<'c> SupportBundleAccessor for InternalApiAccess<'c> { Ok(SupportBundleIndex::new(&s)) } - async fn get_file(&self, path: &Utf8Path) -> Result> { - let mut file = StreamedFile::new(self.client, self.id, path.to_path_buf()); - file.start_stream().await?; + async fn get_file( + &self, + path: &Utf8Path, + ) -> Result> { + let mut file = + StreamedFile::new(self.client, self.id, path.to_path_buf()); + file.start_stream() + .await + .with_context(|| "failed to start stream in get_file")?; Ok(file) } } @@ -167,8 +193,10 @@ impl SupportBundleAccessor for tokio::fs::File { todo!(); } - async fn get_file(&self, _path: &Utf8Path) -> Result> { + async fn get_file( + &self, + _path: &Utf8Path, + ) -> Result> { todo!(); } } - diff --git a/dev-tools/support-bundle-reader-lib/src/index.rs b/dev-tools/support-bundle-reader-lib/src/index.rs index c0bba2ef0f8..0a7eccf6ad1 100644 --- a/dev-tools/support-bundle-reader-lib/src/index.rs +++ b/dev-tools/support-bundle-reader-lib/src/index.rs @@ -9,9 +9,8 @@ pub struct SupportBundleIndex(Vec); impl SupportBundleIndex { pub fn new(s: &str) -> Self { - let mut all_file_names: Vec<_> = s.lines().map(|line| { - Utf8PathBuf::from(line) - }).collect(); + let mut all_file_names: Vec<_> = + s.lines().map(|line| Utf8PathBuf::from(line)).collect(); all_file_names.sort(); SupportBundleIndex(all_file_names) } diff --git a/dev-tools/support-bundle-reader-lib/src/lib.rs b/dev-tools/support-bundle-reader-lib/src/lib.rs index 49d68d1cab3..9914b78be78 100644 --- a/dev-tools/support-bundle-reader-lib/src/lib.rs +++ b/dev-tools/support-bundle-reader-lib/src/lib.rs @@ -4,8 +4,9 @@ //! Utilities to help insepct support bundles -use anyhow::bail; +use anyhow::Context; use anyhow::Result; +use anyhow::bail; use camino::Utf8Path; use futures::StreamExt; use std::pin::Pin; @@ -19,13 +20,17 @@ pub use bundle_accessor::InternalApiAccess; pub use bundle_accessor::SupportBundleAccessor; pub use index::SupportBundleIndex; +enum FileState<'a> { + Open { access: Option>>, buffered: String }, + Closed, +} + /// A dashboard for inspecting a support bundle contents pub struct SupportBundleDashboard<'a, S> { access: &'a S, index: SupportBundleIndex, selected: usize, - opened: Option>>, - buffered: String, + file: FileState<'a>, } impl<'a, S: SupportBundleAccessor> SupportBundleDashboard<'a, S> { @@ -34,29 +39,28 @@ impl<'a, S: SupportBundleAccessor> SupportBundleDashboard<'a, S> { if index.files().is_empty() { bail!("No files found in support bundle"); } - Ok(Self { - access, - index, - selected: 0, - opened: None, - buffered: String::new(), - }) + Ok(Self { access, index, selected: 0, file: FileState::Closed }) } pub fn index(&self) -> &SupportBundleIndex { &self.index } - pub fn select_up(&mut self) { - if self.buffered_file_contents().is_none() { - self.selected = self.selected.saturating_sub(1); + pub async fn select_up(&mut self, count: usize) -> Result<()> { + self.selected = self.selected.saturating_sub(count); + if matches!(self.file, FileState::Open { .. }) { + self.open_and_buffer().await?; } + Ok(()) } - pub fn select_down(&mut self) { - if self.buffered_file_contents().is_none() { - self.selected = std::cmp::min(self.selected + 1, self.index.files().len() - 1); + pub async fn select_down(&mut self, count: usize) -> Result<()> { + self.selected = + std::cmp::min(self.selected + count, self.index.files().len() - 1); + if matches!(self.file, FileState::Open { .. }) { + self.open_and_buffer().await?; } + Ok(()) } pub async fn toggle_file_open(&mut self) -> Result<()> { @@ -69,7 +73,6 @@ impl<'a, S: SupportBundleAccessor> SupportBundleDashboard<'a, S> { } pub async fn open_and_buffer(&mut self) -> Result<()> { - self.close_file(); self.open_file().await?; self.read_to_buffer().await?; Ok(()) @@ -77,30 +80,45 @@ impl<'a, S: SupportBundleAccessor> SupportBundleDashboard<'a, S> { async fn open_file(&mut self) -> Result<()> { let path = &self.index.files()[self.selected]; - let file = self.access.get_file(&path).await?; - self.opened = Some(Box::pin(file)); - self.buffered = String::new(); + if path.as_str().ends_with("/") { + self.file = + FileState::Open { access: None, buffered: String::new() }; + return Ok(()); + } + + let file = self + .access + .get_file(&path) + .await + .with_context(|| format!("Failed to access {path}"))?; + self.file = FileState::Open { + access: Some(Box::pin(file)), + buffered: String::new(), + }; Ok(()) } fn close_file(&mut self) { - self.opened = None; - self.buffered = String::new(); + self.file = FileState::Closed; } async fn read_to_buffer(&mut self) -> Result<()> { - let Some(file) = self.opened.as_mut() else { + let FileState::Open { access, ref mut buffered } = &mut self.file + else { + bail!("File cannot be buffered while closed"); + }; + let Some(file) = access.as_mut() else { return Ok(()); }; - file.read_to_string(&mut self.buffered).await?; + file.read_to_string(buffered).await?; Ok(()) } pub fn buffered_file_contents(&self) -> Option<&str> { - if self.opened.is_some() { - return Some(&self.buffered); - } - None + let FileState::Open { ref buffered, .. } = &self.file else { + return None; + }; + return Some(buffered); } pub fn selected_file_index(&self) -> usize { From 0a457ddc64ac3ee032ff69b5d31e98af81a148cb Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 24 Apr 2025 16:30:30 -0700 Subject: [PATCH 12/31] Enable inspection of local files --- Cargo.lock | 1 + dev-tools/omdb/src/bin/omdb/nexus.rs | 70 ++++++++------ .../support-bundle-reader-lib/Cargo.toml | 1 + .../src/bundle_accessor.rs | 92 +++++++++++++------ .../support-bundle-reader-lib/src/lib.rs | 15 ++- 5 files changed, 121 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c006da16cc9..a0d704cdfbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12117,6 +12117,7 @@ dependencies = [ "omicron-workspace-hack", "reqwest", "tokio", + "zip 2.6.1", ] [[package]] diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 68477960d24..f6a9445d209 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -544,10 +544,19 @@ struct SupportBundleFileArgs { #[derive(Debug, Args)] struct SupportBundleInspectArgs { - /// A specific bundle to inspect. If none is supplied, the latest active - /// bundle is used. + /// A specific bundle to inspect. + /// + /// If none is supplied, the latest active bundle is used. + /// Mutually exclusive with "path". + #[arg(short, long)] id: Option, - // TODO: Option to view a local file? + + /// A local bundle file to inspect. + /// + /// If none is supplied, the latest active bundle is used. + /// Mutually exclusive with "id". + #[arg(short, long)] + path: Option, } impl NexusArgs { @@ -3613,12 +3622,11 @@ enum InspectRunStep { PipeFile, } -/// Runs `omdb nexus support-bundles inspect` -async fn cmd_nexus_support_bundles_inspect( +async fn access_bundle_from_id( client: &nexus_client::Client, - args: &SupportBundleInspectArgs, -) -> Result<(), anyhow::Error> { - let id = match args.id { + id: Option, +) -> Result, anyhow::Error> { + let id = match id { Some(id) => id, None => { // Grab the latest if one isn't supplied @@ -3647,10 +3655,29 @@ async fn cmd_nexus_support_bundles_inspect( SupportBundleUuid::from_untyped_uuid(sb.id.into_untyped_uuid()) } }; + Ok(support_bundle_reader_lib::InternalApiAccess::new(client, id)) +} - let accessor = - support_bundle_reader_lib::InternalApiAccess::new(client, id); - let mut dashboard = SupportBundleDashboard::new(&accessor).await?; +/// Runs `omdb nexus support-bundles inspect` +async fn cmd_nexus_support_bundles_inspect( + client: &nexus_client::Client, + args: &SupportBundleInspectArgs, +) -> Result<(), anyhow::Error> { + let accessor: Box = match (args.id, &args.path) { + (None, Some(path)) => { + Box::new( + support_bundle_reader_lib::LocalFileAccess::new(path)? + ) + } + (maybe_id, None) => { + Box::new(access_bundle_from_id(client, maybe_id).await?) + } + (Some(_), Some(_)) => { + bail!("Cannot specify both UUID and path"); + } + }; + + let mut dashboard = SupportBundleDashboard::new(accessor).await?; enable_raw_mode()?; @@ -3704,13 +3731,9 @@ async fn cmd_nexus_support_bundles_inspect( Ok(()) } -async fn run_support_bundle_dashboard< - 'a, - B: Backend, - S: SupportBundleAccessor, ->( +async fn run_support_bundle_dashboard( terminal: &mut Terminal, - dashboard: &mut SupportBundleDashboard<'a, S>, + dashboard: &mut SupportBundleDashboard<'_>, force_update: bool, ) -> anyhow::Result { let update = if crossterm::event::poll(Duration::from_secs(0))? { @@ -3750,9 +3773,7 @@ async fn run_support_bundle_dashboard< Ok(InspectRunStep::Continue) } -fn create_file_list<'a, S: SupportBundleAccessor>( - dashboard: &'a SupportBundleDashboard<'_, S>, -) -> List<'a> { +fn create_file_list<'a>(dashboard: &'a SupportBundleDashboard<'_>) -> List<'a> { let files = dashboard.index().files().iter().map(|f| f.as_str()); List::new(files) .highlight_symbol("> ") @@ -3760,8 +3781,8 @@ fn create_file_list<'a, S: SupportBundleAccessor>( .block(Block::new().title("Files").borders(Borders::ALL)) } -fn create_file_contents<'a, S: SupportBundleAccessor>( - dashboard: &'a SupportBundleDashboard<'_, S>, +fn create_file_contents<'a>( + dashboard: &'a SupportBundleDashboard<'_>, ) -> Option> { dashboard.buffered_file_contents().map(|c| { Paragraph::new(c).wrap(Wrap { trim: false }).block( @@ -3786,10 +3807,7 @@ const FILE_VIEWER_USAGE: [&'static str; 4] = [ "Press 'q' to quit", ]; -fn draw<'a, S: SupportBundleAccessor>( - f: &mut Frame, - dashboard: &mut SupportBundleDashboard<'a, S>, -) { +fn draw(f: &mut Frame, dashboard: &mut SupportBundleDashboard<'_>) { let file_list = create_file_list(dashboard); let file_contents = create_file_contents(dashboard); diff --git a/dev-tools/support-bundle-reader-lib/Cargo.toml b/dev-tools/support-bundle-reader-lib/Cargo.toml index 0e5c4bc5e55..1640b7a203c 100644 --- a/dev-tools/support-bundle-reader-lib/Cargo.toml +++ b/dev-tools/support-bundle-reader-lib/Cargo.toml @@ -18,3 +18,4 @@ omicron-workspace-hack.workspace = true omicron-uuid-kinds.workspace = true reqwest.workspace = true tokio.workspace = true +zip.workspace = true diff --git a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs index f73a52ea753..22facbb6fb9 100644 --- a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs +++ b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs @@ -28,22 +28,24 @@ use crate::utf8_stream_to_string; /// An I/O source which can read to a buffer /// /// This describes access to individual files within the bundle. -pub trait FileAccessor: AsyncRead {} -impl FileAccessor for T {} +pub trait FileAccessor: AsyncRead + Unpin {} +impl FileAccessor for T {} + +pub type BoxedFileAccessor<'a> = Box; /// Describes how the support bundle's data and metadata are accessed. #[async_trait] pub trait SupportBundleAccessor { - type FileAccessor<'a>: FileAccessor - where - Self: 'a; - /// Access the index of a support bundle async fn get_index(&self) -> Result; /// Access a file within the support bundle - async fn get_file(&self, path: &Utf8Path) - -> Result>; + async fn get_file<'a>( + &mut self, + path: &Utf8Path, + ) -> Result> + where + Self: 'a; } pub struct StreamedFile<'a> { @@ -150,11 +152,6 @@ impl<'a> InternalApiAccess<'a> { // Access for: The nexus internal API #[async_trait] impl<'c> SupportBundleAccessor for InternalApiAccess<'c> { - type FileAccessor<'a> - = StreamedFile<'a> - where - Self: 'a; - async fn get_index(&self) -> Result { let stream = self .client @@ -169,34 +166,75 @@ impl<'c> SupportBundleAccessor for InternalApiAccess<'c> { Ok(SupportBundleIndex::new(&s)) } - async fn get_file( - &self, + async fn get_file<'a>( + &mut self, path: &Utf8Path, - ) -> Result> { + ) -> Result> + where + 'c: 'a, + { let mut file = StreamedFile::new(self.client, self.id, path.to_path_buf()); file.start_stream() .await .with_context(|| "failed to start stream in get_file")?; - Ok(file) + Ok(Box::new(file)) } } -// TODO: Probably want to impl this on a new struct that contains a "ZipReader" +pub struct LocalFileAccess { + archive: zip::read::ZipArchive, +} + +impl LocalFileAccess { + pub fn new(path: &Utf8Path) -> Result { + let file = std::fs::File::open(path)?; + Ok(Self { archive: zip::read::ZipArchive::new(file)? }) + } +} // Access for: Local zip files #[async_trait] -impl SupportBundleAccessor for tokio::fs::File { - type FileAccessor<'a> = tokio::fs::File; - +impl SupportBundleAccessor for LocalFileAccess { async fn get_index(&self) -> Result { - todo!(); + let names: Vec<&str> = self.archive.file_names().collect(); + let all_names = names.join("\n"); + Ok(SupportBundleIndex::new(&all_names)) } - async fn get_file( - &self, - _path: &Utf8Path, - ) -> Result> { - todo!(); + async fn get_file<'a>( + &mut self, + path: &Utf8Path, + ) -> Result> { + let mut file = self.archive.by_name(path.as_str())?; + let mut buf = Vec::new(); + std::io::copy(&mut file, &mut buf)?; + + Ok(Box::new(AsyncZipFile { buf, copied: 0 })) + } +} + +// We're currently buffering the entire file into memory, mostly because dealing with the lifetime +// of ZipArchive and ZipFile objects is so difficult. +pub struct AsyncZipFile { + buf: Vec, + copied: usize, +} + +impl AsyncRead for AsyncZipFile { + fn poll_read( + mut self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + let to_copy = + std::cmp::min(self.buf.len() - self.copied, buf.remaining()); + if to_copy == 0 { + return Poll::Ready(Ok(())); + } + let src = &self.buf[self.copied..]; + buf.put_slice(&src[..to_copy]); + self.copied += to_copy; + Poll::Ready(Ok(())) } } diff --git a/dev-tools/support-bundle-reader-lib/src/lib.rs b/dev-tools/support-bundle-reader-lib/src/lib.rs index 9914b78be78..ea8b8eb952a 100644 --- a/dev-tools/support-bundle-reader-lib/src/lib.rs +++ b/dev-tools/support-bundle-reader-lib/src/lib.rs @@ -15,26 +15,31 @@ use tokio::io::AsyncReadExt; mod bundle_accessor; mod index; +pub use bundle_accessor::AsyncZipFile; +pub use bundle_accessor::BoxedFileAccessor; pub use bundle_accessor::FileAccessor; pub use bundle_accessor::InternalApiAccess; +pub use bundle_accessor::LocalFileAccess; pub use bundle_accessor::SupportBundleAccessor; pub use index::SupportBundleIndex; enum FileState<'a> { - Open { access: Option>>, buffered: String }, + Open { access: Option>>, buffered: String }, Closed, } /// A dashboard for inspecting a support bundle contents -pub struct SupportBundleDashboard<'a, S> { - access: &'a S, +pub struct SupportBundleDashboard<'a> { + access: Box, index: SupportBundleIndex, selected: usize, file: FileState<'a>, } -impl<'a, S: SupportBundleAccessor> SupportBundleDashboard<'a, S> { - pub async fn new(access: &'a S) -> Result { +impl<'a> SupportBundleDashboard<'a> { + pub async fn new( + access: Box, + ) -> Result { let index = access.get_index().await?; if index.files().is_empty() { bail!("No files found in support bundle"); From 7fe5002ccaf54cb4e82b88ce87500669502b75ef Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 24 Apr 2025 16:32:31 -0700 Subject: [PATCH 13/31] Fmt --- dev-tools/omdb/src/bin/omdb/nexus.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index f6a9445d209..f354d756526 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -3665,9 +3665,7 @@ async fn cmd_nexus_support_bundles_inspect( ) -> Result<(), anyhow::Error> { let accessor: Box = match (args.id, &args.path) { (None, Some(path)) => { - Box::new( - support_bundle_reader_lib::LocalFileAccess::new(path)? - ) + Box::new(support_bundle_reader_lib::LocalFileAccess::new(path)?) } (maybe_id, None) => { Box::new(access_bundle_from_id(client, maybe_id).await?) From 36d2d04675a7a6a43bbae732f65d7d0fc35a1309 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 25 Apr 2025 13:24:38 -0700 Subject: [PATCH 14/31] Make datasets private, add helpers to access them --- .../read_only_region_replacement_start.rs | 8 +- .../region_snapshot_replacement_start.rs | 35 ++--- nexus/src/app/sagas/disk_create.rs | 32 ++--- .../src/app/sagas/region_replacement_start.rs | 14 +- .../region_snapshot_replacement_start.rs | 29 ++-- nexus/test-utils/src/resource_helpers.rs | 28 +++- nexus/tests/integration_tests/disks.rs | 130 ++++++------------ nexus/tests/integration_tests/snapshots.rs | 7 +- .../integration_tests/support_bundles.rs | 8 +- .../integration_tests/volume_management.rs | 24 ++-- 10 files changed, 128 insertions(+), 187 deletions(-) diff --git a/nexus/src/app/background/tasks/read_only_region_replacement_start.rs b/nexus/src/app/background/tasks/read_only_region_replacement_start.rs index 5b1e40a07b7..338438c5ad1 100644 --- a/nexus/src/app/background/tasks/read_only_region_replacement_start.rs +++ b/nexus/src/app/background/tasks/read_only_region_replacement_start.rs @@ -179,7 +179,6 @@ mod test { use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external; - use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::VolumeUuid; @@ -216,11 +215,8 @@ mod test { BTreeMap::default(); for zpool in disk_test.zpools() { - for dataset in &zpool.datasets { - if matches!(dataset.kind, DatasetKind::Crucible) { - dataset_to_zpool.insert(zpool.id, dataset.id); - } - } + let dataset = zpool.crucible_dataset(); + dataset_to_zpool.insert(zpool.id, dataset.id); } let mut task = diff --git a/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs b/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs index 15b8ed92880..05edb978a55 100644 --- a/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs +++ b/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs @@ -325,7 +325,6 @@ mod test { use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external; - use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::VolumeUuid; @@ -452,24 +451,19 @@ mod test { BTreeMap::default(); for zpool in disk_test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } - - dataset_to_zpool - .insert(zpool.id.to_string(), dataset.id.to_string()); - - datastore - .region_snapshot_create(RegionSnapshot::new( - dataset.id, - region_id, - snapshot_id, - String::from("[fd00:1122:3344::101]:12345"), - )) - .await - .unwrap(); - } + let dataset = zpool.crucible_dataset(); + dataset_to_zpool + .insert(zpool.id.to_string(), dataset.id.to_string()); + + datastore + .region_snapshot_create(RegionSnapshot::new( + dataset.id, + region_id, + snapshot_id, + String::from("[fd00:1122:3344::101]:12345"), + )) + .await + .unwrap(); } // Create the fake snapshot @@ -636,7 +630,8 @@ mod test { let disk_test = DiskTest::new(cptestctx).await; - let dataset_id = disk_test.zpools().next().unwrap().datasets[0].id; + let dataset_id = + disk_test.zpools().next().unwrap().crucible_dataset().id; // Add a region snapshot replacement request for a fake region snapshot diff --git a/nexus/src/app/sagas/disk_create.rs b/nexus/src/app/sagas/disk_create.rs index 5bfdab6c025..582b74aed92 100644 --- a/nexus/src/app/sagas/disk_create.rs +++ b/nexus/src/app/sagas/disk_create.rs @@ -834,7 +834,6 @@ pub(crate) mod test { use omicron_common::api::external::ByteCount; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Name; - use omicron_common::disk::DatasetKind; use omicron_sled_agent::sim::SledAgent; use uuid::Uuid; @@ -1011,18 +1010,11 @@ pub(crate) mod test { test: &DiskTest<'_>, ) -> bool { for zpool in test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } - if datastore - .regions_total_reserved_size(dataset.id) - .await - .unwrap() - != 0 - { - return false; - } + let dataset = zpool.crucible_dataset(); + if datastore.regions_total_reserved_size(dataset.id).await.unwrap() + != 0 + { + return false; } } true @@ -1030,15 +1022,11 @@ pub(crate) mod test { fn no_regions_ensured(sled_agent: &SledAgent, test: &DiskTest<'_>) -> bool { for zpool in test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } - let crucible_dataset = - sled_agent.get_crucible_dataset(zpool.id, dataset.id); - if !crucible_dataset.is_empty() { - return false; - } + let dataset = zpool.crucible_dataset(); + let crucible_dataset = + sled_agent.get_crucible_dataset(zpool.id, dataset.id); + if !crucible_dataset.is_empty() { + return false; } } true diff --git a/nexus/src/app/sagas/region_replacement_start.rs b/nexus/src/app/sagas/region_replacement_start.rs index 4aabd9d9c3c..2a5bdefe030 100644 --- a/nexus/src/app/sagas/region_replacement_start.rs +++ b/nexus/src/app/sagas/region_replacement_start.rs @@ -1028,15 +1028,11 @@ pub(crate) mod test { let mut count = 0; for zpool in test.zpools() { - for dataset in &zpool.datasets { - if datastore - .regions_total_reserved_size(dataset.id) - .await - .unwrap() - != 0 - { - count += 1; - } + let dataset = zpool.crucible_dataset(); + if datastore.regions_total_reserved_size(dataset.id).await.unwrap() + != 0 + { + count += 1; } } diff --git a/nexus/src/app/sagas/region_snapshot_replacement_start.rs b/nexus/src/app/sagas/region_snapshot_replacement_start.rs index 5fbccdf62b0..25ad4b9f84c 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_start.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_start.rs @@ -1238,7 +1238,6 @@ pub(crate) mod test { use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::views; use nexus_types::identity::Asset; - use omicron_common::disk::DatasetKind; use omicron_uuid_kinds::GenericUuid; use sled_agent_client::VolumeConstructionRequest; @@ -1501,24 +1500,18 @@ pub(crate) mod test { let mut non_destroyed_regions_from_agent = vec![]; for zpool in test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } + let dataset = zpool.crucible_dataset(); + let crucible_dataset = + sled_agent.get_crucible_dataset(zpool.id, dataset.id); + for region in crucible_dataset.list() { + match region.state { + crucible_agent_client::types::State::Tombstoned + | crucible_agent_client::types::State::Destroyed => { + // ok + } - let crucible_dataset = - sled_agent.get_crucible_dataset(zpool.id, dataset.id); - for region in crucible_dataset.list() { - match region.state { - crucible_agent_client::types::State::Tombstoned - | crucible_agent_client::types::State::Destroyed => { - // ok - } - - _ => { - non_destroyed_regions_from_agent - .push(region.clone()); - } + _ => { + non_destroyed_regions_from_agent.push(region.clone()); } } } diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 642c7a6e68c..d580140132a 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -1139,7 +1139,33 @@ pub struct TestDataset { pub struct TestZpool { pub id: ZpoolUuid, pub size: ByteCount, - pub datasets: Vec, + datasets: Vec, +} + +impl TestZpool { + /// Returns the crucible dataset within a zpool. + /// + /// Panics if there are zero or more than one crucible datasets within the pool + pub fn crucible_dataset(&self) -> &TestDataset { + fn is_crucible(d: &&TestDataset) -> bool { + d.kind == DatasetKind::Crucible + } + + assert_eq!(self.datasets.iter().filter(is_crucible).count(), 1); + self.datasets.iter().find(is_crucible).unwrap() + } + + /// Returns the debug dataset within a zpool. + /// + /// Panics if there are zero or more than one debug datasets within the pool + pub fn debug_dataset(&self) -> &TestDataset { + fn is_debug(d: &&TestDataset) -> bool { + d.kind == DatasetKind::Debug + } + + assert_eq!(self.datasets.iter().filter(is_debug).count(), 1); + self.datasets.iter().find(is_debug).unwrap() + } } enum WhichSledAgents { diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs index 154491fe0ac..6cdfe63e28c 100644 --- a/nexus/tests/integration_tests/disks.rs +++ b/nexus/tests/integration_tests/disks.rs @@ -45,7 +45,6 @@ use omicron_common::api::external::InstanceState; use omicron_common::api::external::Name; use omicron_common::api::external::NameOrId; use omicron_common::api::external::{ByteCount, SimpleIdentityOrName as _}; -use omicron_common::disk::DatasetKind; use omicron_nexus::Nexus; use omicron_nexus::TestInterfaces as _; use omicron_nexus::app::{MAX_DISK_SIZE_BYTES, MIN_DISK_SIZE_BYTES}; @@ -1335,7 +1334,7 @@ async fn test_disk_virtual_provisioning_collection_failed_delete( // Set the third agent to fail when deleting regions let zpool = &disk_test.zpools().nth(2).expect("Expected at least three zpools"); - let dataset = &zpool.datasets[0]; + let dataset = zpool.crucible_dataset(); cptestctx .first_sled_agent() .get_crucible_dataset(zpool.id, dataset.id) @@ -1554,28 +1553,16 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // Total occupied size should start at 0 for zpool in test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } + let dataset = zpool.crucible_dataset(); + assert_eq!( + datastore.regions_total_reserved_size(dataset.id).await.unwrap(), + 0 + ); - assert_eq!( - datastore - .regions_total_reserved_size(dataset.id) - .await - .unwrap(), - 0 - ); - - assert_eq!( - datastore - .crucible_dataset_get(dataset.id) - .await - .unwrap() - .size_used, - 0, - ); - } + assert_eq!( + datastore.crucible_dataset_get(dataset.id).await.unwrap().size_used, + 0, + ); } // Ask for a 7 gibibyte disk, this should succeed @@ -1607,18 +1594,11 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // regions to make a region set for an Upstairs, one region per dataset) // plus reservation overhead for zpool in test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } - assert_eq!( - datastore - .regions_total_reserved_size(dataset.id) - .await - .unwrap(), - ByteCount::from_mebibytes_u32(8960).to_bytes(), - ); - } + let dataset = zpool.crucible_dataset(); + assert_eq!( + datastore.regions_total_reserved_size(dataset.id).await.unwrap(), + ByteCount::from_mebibytes_u32(8960).to_bytes(), + ); } // Ask for a 6 gibibyte disk, this should fail because there isn't space @@ -1647,18 +1627,11 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // Total occupied size is still 7 GiB * 3 (plus overhead) for zpool in test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } - assert_eq!( - datastore - .regions_total_reserved_size(dataset.id) - .await - .unwrap(), - ByteCount::from_mebibytes_u32(8960).to_bytes(), - ); - } + let dataset = zpool.crucible_dataset(); + assert_eq!( + datastore.regions_total_reserved_size(dataset.id).await.unwrap(), + ByteCount::from_mebibytes_u32(8960).to_bytes(), + ); } // Delete the first disk, freeing up 7 gibibytes. @@ -1674,18 +1647,11 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // Total occupied size should be 0 for zpool in test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } - assert_eq!( - datastore - .regions_total_reserved_size(dataset.id) - .await - .unwrap(), - 0, - ); - } + let dataset = zpool.crucible_dataset(); + assert_eq!( + datastore.regions_total_reserved_size(dataset.id).await.unwrap(), + 0, + ); } // Ask for a 10 gibibyte disk. @@ -1713,18 +1679,11 @@ async fn test_disk_size_accounting(cptestctx: &ControlPlaneTestContext) { // Total occupied size should be 10 GiB * 3 plus overhead for zpool in test.zpools() { - for dataset in &zpool.datasets { - if !matches!(dataset.kind, DatasetKind::Crucible) { - continue; - } - assert_eq!( - datastore - .regions_total_reserved_size(dataset.id) - .await - .unwrap(), - ByteCount::from_mebibytes_u32(12800).to_bytes(), - ); - } + let dataset = zpool.crucible_dataset(); + assert_eq!( + datastore.regions_total_reserved_size(dataset.id).await.unwrap(), + ByteCount::from_mebibytes_u32(12800).to_bytes(), + ); } } @@ -2149,19 +2108,16 @@ async fn test_single_region_allocate(cptestctx: &ControlPlaneTestContext) { let mut number_of_matching_regions = 0; for zpool in disk_test.zpools() { - for dataset in &zpool.datasets { - let total_size = datastore - .regions_total_reserved_size(dataset.id) - .await - .unwrap(); - - if total_size == allocated_region.reserved_size() { - number_of_matching_regions += 1; - } else if total_size == 0 { - // ok, unallocated - } else { - panic!("unexpected regions total size of {total_size}"); - } + let dataset = zpool.crucible_dataset(); + let total_size = + datastore.regions_total_reserved_size(dataset.id).await.unwrap(); + + if total_size == allocated_region.reserved_size() { + number_of_matching_regions += 1; + } else if total_size == 0 { + // ok, unallocated + } else { + panic!("unexpected regions total size of {total_size}"); } } @@ -2503,7 +2459,7 @@ async fn test_no_halt_disk_delete_one_region_on_expunged_agent( // Choose one of the datasets, and drop the simulated Crucible agent let zpool = disk_test.zpools().next().expect("Expected at least one zpool"); - let dataset = &zpool.datasets[0]; + let dataset = zpool.crucible_dataset(); cptestctx.first_sled_agent().drop_dataset(zpool.id, dataset.id); @@ -2623,7 +2579,7 @@ async fn test_do_not_provision_on_dataset(cptestctx: &ControlPlaneTestContext) { .await; // For one of the datasets, mark it as not provisionable - let dataset = &disk_test.zpools().next().unwrap().datasets[0]; + let dataset = disk_test.zpools().next().unwrap().crucible_dataset(); datastore .mark_crucible_dataset_not_provisionable(&opctx, dataset.id) @@ -2669,7 +2625,7 @@ async fn test_do_not_provision_on_dataset_not_enough( .await; // For one of the datasets, mark it as not provisionable - let dataset = &disk_test.zpools().next().unwrap().datasets[0]; + let dataset = disk_test.zpools().next().unwrap().crucible_dataset(); datastore .mark_crucible_dataset_not_provisionable(&opctx, dataset.id) diff --git a/nexus/tests/integration_tests/snapshots.rs b/nexus/tests/integration_tests/snapshots.rs index c6954ba0e48..6a18d13a3ca 100644 --- a/nexus/tests/integration_tests/snapshots.rs +++ b/nexus/tests/integration_tests/snapshots.rs @@ -38,7 +38,6 @@ use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; use omicron_common::api::external::Name; -use omicron_common::disk::DatasetKind; use omicron_nexus::app::MIN_DISK_SIZE_BYTES; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; @@ -979,11 +978,7 @@ async fn test_snapshot_unwind(cptestctx: &ControlPlaneTestContext) { // Set the third region's running snapshot callback so it fails let zpool = disk_test.zpools().nth(2).expect("Not enough zpools"); - let dataset = zpool - .datasets - .iter() - .find(|dataset| matches!(dataset.kind, DatasetKind::Crucible)) - .expect("No crucible dataset found on this zpool"); + let dataset = zpool.crucible_dataset(); cptestctx .first_sled_agent() .get_crucible_dataset(zpool.id, dataset.id) diff --git a/nexus/tests/integration_tests/support_bundles.rs b/nexus/tests/integration_tests/support_bundles.rs index 0e4f03c78d9..27e519d0058 100644 --- a/nexus/tests/integration_tests/support_bundles.rs +++ b/nexus/tests/integration_tests/support_bundles.rs @@ -19,7 +19,6 @@ use nexus_types::external_api::shared::SupportBundleInfo; use nexus_types::external_api::shared::SupportBundleState; use nexus_types::internal_api::background::SupportBundleCleanupReport; use nexus_types::internal_api::background::SupportBundleCollectionReport; -use omicron_common::api::internal::shared::DatasetKind; use omicron_uuid_kinds::SupportBundleUuid; use serde::Deserialize; use std::io::Cursor; @@ -348,11 +347,8 @@ async fn test_support_bundle_lifecycle(cptestctx: &ControlPlaneTestContext) { // in our disk test. let mut debug_dataset_count = 0; for zpool in disk_test.zpools() { - for dataset in &zpool.datasets { - if matches!(dataset.kind, DatasetKind::Debug) { - debug_dataset_count += 1; - } - } + let _dataset = zpool.debug_dataset(); + debug_dataset_count += 1; } assert_eq!(debug_dataset_count, 1); diff --git a/nexus/tests/integration_tests/volume_management.rs b/nexus/tests/integration_tests/volume_management.rs index 2029b1d2998..f2be7a633b2 100644 --- a/nexus/tests/integration_tests/volume_management.rs +++ b/nexus/tests/integration_tests/volume_management.rs @@ -2197,38 +2197,38 @@ async fn test_keep_your_targets_straight(cptestctx: &ControlPlaneTestContext) { let region_snapshots: Vec<(DatasetUuid, Uuid, Uuid, SocketAddr)> = vec![ // first snapshot-create ( - zpool0.datasets[0].id, + zpool0.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:101::7]:19016".parse().unwrap(), ), ( - zpool1.datasets[0].id, + zpool1.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:102::7]:19016".parse().unwrap(), ), ( - zpool2.datasets[0].id, + zpool2.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:103::7]:19016".parse().unwrap(), ), // second snapshot-create ( - zpool0.datasets[0].id, + zpool0.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:101::7]:19016".parse().unwrap(), ), ( - zpool3.datasets[0].id, + zpool3.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:104::7]:19016".parse().unwrap(), ), ( - zpool2.datasets[0].id, + zpool2.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:103::7]:19017".parse().unwrap(), @@ -2509,7 +2509,7 @@ async fn test_disk_create_saga_unwinds_correctly( // Set the third agent to fail creating the region let zpool = &disk_test.zpools().nth(2).expect("Expected three zpools"); - let dataset = &zpool.datasets[0]; + let dataset = zpool.crucible_dataset(); cptestctx .first_sled_agent() .get_crucible_dataset(zpool.id, dataset.id) @@ -2576,7 +2576,7 @@ async fn test_snapshot_create_saga_unwinds_correctly( // Set the third agent to fail creating the region for the snapshot let zpool = &disk_test.zpools().nth(2).expect("Expected at least three zpools"); - let dataset = &zpool.datasets[0]; + let dataset = zpool.crucible_dataset(); cptestctx .first_sled_agent() .get_crucible_dataset(zpool.id, dataset.id) @@ -5412,25 +5412,25 @@ async fn test_migrate_to_ref_count_with_records_region_snapshot_deleting( // (dataset_id, region_id, snapshot_id, snapshot_addr) let region_snapshots: Vec<(DatasetUuid, Uuid, Uuid, SocketAddr)> = vec![ ( - zpool0.datasets[0].id, + zpool0.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:101::7]:19016".parse().unwrap(), ), ( - zpool1.datasets[0].id, + zpool1.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:102::7]:19016".parse().unwrap(), ), ( - zpool2.datasets[0].id, + zpool2.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:103::7]:19016".parse().unwrap(), ), ( - zpool3.datasets[0].id, + zpool3.crucible_dataset().id, Uuid::new_v4(), Uuid::new_v4(), "[fd00:1122:3344:104::7]:19016".parse().unwrap(), From b21b5254174bde35f6c96826f81f337f559ad1e3 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 25 Apr 2025 13:31:10 -0700 Subject: [PATCH 15/31] feedback --- nexus/src/internal_api/http_entrypoints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/internal_api/http_entrypoints.rs b/nexus/src/internal_api/http_entrypoints.rs index 84d5320549b..241f71ea5f8 100644 --- a/nexus/src/internal_api/http_entrypoints.rs +++ b/nexus/src/internal_api/http_entrypoints.rs @@ -1164,7 +1164,7 @@ impl NexusInternalApi for NexusInternalApiImpl { crate::context::op_context_for_internal_api(&rqctx).await; let bundle = nexus - .support_bundle_create(&opctx, "Created by external API") + .support_bundle_create(&opctx, "Created by internal API") .await?; Ok(HttpResponseCreated(bundle.into())) }; From 53c0a7620bb43abff4caa30dc575b89447d4c94d Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 25 Apr 2025 13:51:23 -0700 Subject: [PATCH 16/31] feedback, less utf8 --- dev-tools/omdb/src/bin/omdb/nexus.rs | 36 +++++++++++++++------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 40788e97987..cdd8b05d5a5 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -129,6 +129,7 @@ enum NexusCommands { /// interact with sleds Sleds(SledsArgs), /// interact with support bundles + #[command(visible_alias = "sb")] SupportBundles(SupportBundleArgs), } @@ -515,6 +516,10 @@ struct SupportBundleIndexArgs { struct SupportBundleFileArgs { id: SupportBundleUuid, path: Utf8PathBuf, + /// Optional output path where the file should be written, + /// instead of stdout. + #[arg(short, long)] + output: Option, } impl NexusArgs { @@ -3516,25 +3521,15 @@ async fn cmd_nexus_support_bundles_delete( Ok(()) } -async fn print_utf8_stream_to_stdout( +async fn write_stream_to_sink( mut stream: impl futures::Stream> + std::marker::Unpin, + mut sink: impl std::io::Write, ) -> Result<(), anyhow::Error> { - let mut leftover = None; while let Some(data) = stream.next().await { match data { Err(err) => return Err(anyhow::anyhow!(err)), - Ok(data) => { - let combined = match leftover.take() { - Some(old) => [old, data].concat(), - None => data.to_vec(), - }; - - match std::str::from_utf8(&combined) { - Ok(data) => print!("{data}"), - Err(_) => leftover = Some(combined.into()), - } - } + Ok(data) => sink.write_all(&data)?, } } Ok(()) @@ -3552,9 +3547,10 @@ async fn cmd_nexus_support_bundles_get_index( format!("downloading support bundle index {}", args.id) })? .into_inner_stream(); - print_utf8_stream_to_stdout(stream).await.with_context(|| { - format!("streaming support bundle index {}", args.id) - })?; + + write_stream_to_sink(stream, std::io::stdout()).await.with_context( + || format!("streaming support bundle index {}", args.id), + )?; Ok(()) } @@ -3576,7 +3572,13 @@ async fn cmd_nexus_support_bundles_get_file( ) })? .into_inner_stream(); - print_utf8_stream_to_stdout(stream).await.with_context(|| { + + let sink: Box = match &args.output { + Some(path) => Box::new(std::fs::File::create(path)?), + None => Box::new(std::io::stdout()), + }; + + write_stream_to_sink(stream, sink).await.with_context(|| { format!("streaming support bundle file {}: {}", args.id, args.path) })?; Ok(()) From 44bd6a5cd0a16d11f76c263a7562bd3bfdcfbc8d Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 25 Apr 2025 14:14:55 -0700 Subject: [PATCH 17/31] Better support for binary files --- dev-tools/omdb/src/bin/omdb/nexus.rs | 4 +++- dev-tools/support-bundle-reader-lib/src/lib.rs | 11 +++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 7de845e2b01..89cd31e7908 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -3736,7 +3736,7 @@ async fn cmd_nexus_support_bundles_inspect( Ok(true) => { if let Some(contents) = dashboard.buffered_file_contents() { std::io::copy( - &mut std::io::Cursor::new(contents.as_bytes()), + &mut std::io::Cursor::new(contents), &mut std::io::stdout(), )?; } @@ -3801,6 +3801,8 @@ fn create_file_contents<'a>( dashboard: &'a SupportBundleDashboard<'_>, ) -> Option> { dashboard.buffered_file_contents().map(|c| { + let c = std::str::from_utf8(c).unwrap_or("Not valid UTF-8"); + Paragraph::new(c).wrap(Wrap { trim: false }).block( Block::new() .title(dashboard.selected_file_name().as_str()) diff --git a/dev-tools/support-bundle-reader-lib/src/lib.rs b/dev-tools/support-bundle-reader-lib/src/lib.rs index ea8b8eb952a..46a02084b5b 100644 --- a/dev-tools/support-bundle-reader-lib/src/lib.rs +++ b/dev-tools/support-bundle-reader-lib/src/lib.rs @@ -24,7 +24,7 @@ pub use bundle_accessor::SupportBundleAccessor; pub use index::SupportBundleIndex; enum FileState<'a> { - Open { access: Option>>, buffered: String }, + Open { access: Option>>, buffered: Vec }, Closed, } @@ -86,8 +86,7 @@ impl<'a> SupportBundleDashboard<'a> { async fn open_file(&mut self) -> Result<()> { let path = &self.index.files()[self.selected]; if path.as_str().ends_with("/") { - self.file = - FileState::Open { access: None, buffered: String::new() }; + self.file = FileState::Open { access: None, buffered: Vec::new() }; return Ok(()); } @@ -98,7 +97,7 @@ impl<'a> SupportBundleDashboard<'a> { .with_context(|| format!("Failed to access {path}"))?; self.file = FileState::Open { access: Some(Box::pin(file)), - buffered: String::new(), + buffered: Vec::new(), }; Ok(()) } @@ -115,11 +114,11 @@ impl<'a> SupportBundleDashboard<'a> { let Some(file) = access.as_mut() else { return Ok(()); }; - file.read_to_string(buffered).await?; + file.read_to_end(buffered).await?; Ok(()) } - pub fn buffered_file_contents(&self) -> Option<&str> { + pub fn buffered_file_contents(&self) -> Option<&[u8]> { let FileState::Open { ref buffered, .. } = &self.file else { return None; }; From 2790d9aa5069d28bcf2dfcdb1e0282ece53acbac Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 28 Apr 2025 10:27:35 -0700 Subject: [PATCH 18/31] expectorate --- dev-tools/omdb/tests/usage_errors.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/omdb/tests/usage_errors.out b/dev-tools/omdb/tests/usage_errors.out index 07c637d74fb..061bb516f04 100644 --- a/dev-tools/omdb/tests/usage_errors.out +++ b/dev-tools/omdb/tests/usage_errors.out @@ -706,7 +706,7 @@ Commands: oximeter-read-policy interact with oximeter read policy sagas view sagas, create and complete demo sagas sleds interact with sleds - support-bundles interact with support bundles + support-bundles interact with support bundles [aliases: sb] help Print this message or the help of the given subcommand(s) Options: From b73b6ff5d772542d60e4337bd5646e3d39aac8d0 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 28 Apr 2025 17:02:45 -0700 Subject: [PATCH 19/31] Better support for waiting for collection to finish --- dev-tools/omdb/src/bin/omdb/nexus.rs | 97 ++++++++++++++++++++++++---- 1 file changed, 83 insertions(+), 14 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 89cd31e7908..5b9c78db3a6 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -43,6 +43,8 @@ use nexus_client::types::LastResult; use nexus_client::types::PhysicalDiskPath; use nexus_client::types::SagaState; use nexus_client::types::SledSelector; +use nexus_client::types::SupportBundleInfo; +use nexus_client::types::SupportBundleState; use nexus_client::types::UninitializedSledId; use nexus_db_lookup::LookupPath; use nexus_db_queries::db::DataStore; @@ -97,6 +99,7 @@ use serde::Deserialize; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; use std::collections::BTreeSet; +use std::io::Write; use std::str::FromStr; use std::sync::Arc; use std::time::Duration; @@ -3640,12 +3643,56 @@ enum InspectRunStep { PipeFile, } +async fn wait_for_bundle_to_be_collected( + client: &nexus_client::Client, + id: SupportBundleUuid, +) -> Result { + let mut printed_wait_msg = false; + loop { + let sb = client + .support_bundle_view(id.as_untyped_uuid()) + .await + .with_context(|| { + format!("failed to query for support bundle {}", id) + })?; + + match sb.state { + SupportBundleState::Active => { + if printed_wait_msg { + eprintln!(""); + } + return Ok(sb.into_inner()); + } + SupportBundleState::Collecting => { + if !printed_wait_msg { + eprint!("Waiting for {} to finish collection...", id); + printed_wait_msg = true; + } + tokio::time::sleep(Duration::from_secs(1)).await; + eprint!("."); + std::io::stderr() + .flush() + .with_context(|| format!("cannot flush stderr"))?; + } + other => bail!("Unexepcted state: {other}"), + } + } +} + async fn access_bundle_from_id( client: &nexus_client::Client, id: Option, ) -> Result, anyhow::Error> { let id = match id { - Some(id) => id, + Some(id) => { + // Ensure the bundle has been collected + let sb = wait_for_bundle_to_be_collected( + client, + SupportBundleUuid::from_untyped_uuid(*id.as_untyped_uuid()), + ) + .await?; + SupportBundleUuid::from_untyped_uuid(sb.id.into_untyped_uuid()) + } None => { // Grab the latest if one isn't supplied let support_bundle_stream = @@ -3656,17 +3703,39 @@ async fn access_bundle_from_id( .context("listing support bundles")?; support_bundles.sort_by_key(|k| k.time_created); - let sb = support_bundles - .into_iter() - .find(|sb| { - matches!( - sb.state, - nexus_client::types::SupportBundleState::Active - ) - }) - .ok_or(anyhow::anyhow!( - "Cannot find active support bundle. Try creating one" - ))?; + let active_sb = support_bundles + .iter() + .find(|sb| matches!(sb.state, SupportBundleState::Active)); + + let sb = match active_sb { + Some(sb) => sb.clone(), + None => { + // This is a special case, but not an uncommon one: + // + // - Someone just created a bundle... + // - ... but collection is still happening. + // + // To smooth out this experience for users, we wait for the + // collection to complete. + let collecting_sb = support_bundles.iter().find(|sb| { + matches!(sb.state, SupportBundleState::Collecting) + }); + if let Some(collecting_sb) = collecting_sb { + let id = &collecting_sb.id; + wait_for_bundle_to_be_collected( + client, + SupportBundleUuid::from_untyped_uuid( + *id.as_untyped_uuid(), + ), + ) + .await? + } else { + bail!( + "Cannot find active support bundle. Try creating one" + ) + } + } + }; eprintln!("Inspecting bundle {} from {}", sb.id, sb.time_created); @@ -3757,11 +3826,11 @@ async fn run_support_bundle_dashboard( let shifted = key.modifiers.contains(event::KeyModifiers::SHIFT); match key.code { KeyCode::Char('q') => return Ok(InspectRunStep::Exit), - KeyCode::Up | KeyCode::Char('k') => { + KeyCode::Up | KeyCode::Char('k') | KeyCode::Char('K') => { let count = if shifted { 5 } else { 1 }; dashboard.select_up(count).await?; } - KeyCode::Down | KeyCode::Char('j') => { + KeyCode::Down | KeyCode::Char('j') | KeyCode::Char('J') => { let count = if shifted { 5 } else { 1 }; dashboard.select_down(count).await?; } From 82ca3209a670b103754def38c516ddf7be6250f9 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 29 Apr 2025 13:07:35 -0700 Subject: [PATCH 20/31] Refactoring TUI into support-bundle-reader-lib --- Cargo.lock | 2 + dev-tools/omdb/src/bin/omdb/nexus.rs | 313 +------------ .../support-bundle-reader-lib/Cargo.toml | 2 + .../src/bundle_accessor.rs | 33 +- .../src/dashboard.rs | 440 ++++++++++++++++++ .../support-bundle-reader-lib/src/lib.rs | 155 +----- 6 files changed, 487 insertions(+), 458 deletions(-) create mode 100644 dev-tools/support-bundle-reader-lib/src/dashboard.rs diff --git a/Cargo.lock b/Cargo.lock index 215932f0c5d..46720bf46cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12157,10 +12157,12 @@ dependencies = [ "async-trait", "bytes", "camino", + "crossterm", "futures", "nexus-client", "omicron-uuid-kinds", "omicron-workspace-hack", + "ratatui", "reqwest", "tokio", "zip 2.6.1", diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 5b9c78db3a6..f33652392e3 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -21,14 +21,6 @@ use clap::Args; use clap::ColorChoice; use clap::Subcommand; use clap::ValueEnum; -use crossterm::{ - event::{self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode}, - execute, - terminal::{ - EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, - enable_raw_mode, - }, -}; use futures::StreamExt; use futures::TryStreamExt; use futures::future::try_join; @@ -43,8 +35,6 @@ use nexus_client::types::LastResult; use nexus_client::types::PhysicalDiskPath; use nexus_client::types::SagaState; use nexus_client::types::SledSelector; -use nexus_client::types::SupportBundleInfo; -use nexus_client::types::SupportBundleState; use nexus_client::types::UninitializedSledId; use nexus_db_lookup::LookupPath; use nexus_db_queries::db::DataStore; @@ -81,30 +71,12 @@ use omicron_uuid_kinds::ParseError; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::SledUuid; use omicron_uuid_kinds::SupportBundleUuid; -use ratatui::Frame; -use ratatui::Terminal; -use ratatui::backend::Backend; -use ratatui::backend::CrosstermBackend; -use ratatui::layout::Constraint; -use ratatui::layout::Layout; -use ratatui::style::Modifier; -use ratatui::style::Style; -use ratatui::widgets::Block; -use ratatui::widgets::Borders; -use ratatui::widgets::List; -use ratatui::widgets::ListState; -use ratatui::widgets::Paragraph; -use ratatui::widgets::Wrap; use serde::Deserialize; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; use std::collections::BTreeSet; -use std::io::Write; use std::str::FromStr; use std::sync::Arc; -use std::time::Duration; -use support_bundle_reader_lib::SupportBundleAccessor; -use support_bundle_reader_lib::SupportBundleDashboard; use tabled::Tabled; use tabled::settings::Padding; use tabled::settings::object::Columns; @@ -3634,288 +3606,15 @@ async fn cmd_nexus_support_bundles_get_file( Ok(()) } -enum InspectRunStep { - // Keep running the dashboard - Continue, - // Exit the dashboard - Exit, - // Exit the dashboard GUI, but pipe a selected file to an output stream - PipeFile, -} - -async fn wait_for_bundle_to_be_collected( - client: &nexus_client::Client, - id: SupportBundleUuid, -) -> Result { - let mut printed_wait_msg = false; - loop { - let sb = client - .support_bundle_view(id.as_untyped_uuid()) - .await - .with_context(|| { - format!("failed to query for support bundle {}", id) - })?; - - match sb.state { - SupportBundleState::Active => { - if printed_wait_msg { - eprintln!(""); - } - return Ok(sb.into_inner()); - } - SupportBundleState::Collecting => { - if !printed_wait_msg { - eprint!("Waiting for {} to finish collection...", id); - printed_wait_msg = true; - } - tokio::time::sleep(Duration::from_secs(1)).await; - eprint!("."); - std::io::stderr() - .flush() - .with_context(|| format!("cannot flush stderr"))?; - } - other => bail!("Unexepcted state: {other}"), - } - } -} - -async fn access_bundle_from_id( - client: &nexus_client::Client, - id: Option, -) -> Result, anyhow::Error> { - let id = match id { - Some(id) => { - // Ensure the bundle has been collected - let sb = wait_for_bundle_to_be_collected( - client, - SupportBundleUuid::from_untyped_uuid(*id.as_untyped_uuid()), - ) - .await?; - SupportBundleUuid::from_untyped_uuid(sb.id.into_untyped_uuid()) - } - None => { - // Grab the latest if one isn't supplied - let support_bundle_stream = - client.support_bundle_list_stream(None, None); - let mut support_bundles = support_bundle_stream - .try_collect::>() - .await - .context("listing support bundles")?; - support_bundles.sort_by_key(|k| k.time_created); - - let active_sb = support_bundles - .iter() - .find(|sb| matches!(sb.state, SupportBundleState::Active)); - - let sb = match active_sb { - Some(sb) => sb.clone(), - None => { - // This is a special case, but not an uncommon one: - // - // - Someone just created a bundle... - // - ... but collection is still happening. - // - // To smooth out this experience for users, we wait for the - // collection to complete. - let collecting_sb = support_bundles.iter().find(|sb| { - matches!(sb.state, SupportBundleState::Collecting) - }); - if let Some(collecting_sb) = collecting_sb { - let id = &collecting_sb.id; - wait_for_bundle_to_be_collected( - client, - SupportBundleUuid::from_untyped_uuid( - *id.as_untyped_uuid(), - ), - ) - .await? - } else { - bail!( - "Cannot find active support bundle. Try creating one" - ) - } - } - }; - - eprintln!("Inspecting bundle {} from {}", sb.id, sb.time_created); - - SupportBundleUuid::from_untyped_uuid(sb.id.into_untyped_uuid()) - } - }; - Ok(support_bundle_reader_lib::InternalApiAccess::new(client, id)) -} - /// Runs `omdb nexus support-bundles inspect` async fn cmd_nexus_support_bundles_inspect( client: &nexus_client::Client, args: &SupportBundleInspectArgs, ) -> Result<(), anyhow::Error> { - let accessor: Box = match (args.id, &args.path) { - (None, Some(path)) => { - Box::new(support_bundle_reader_lib::LocalFileAccess::new(path)?) - } - (maybe_id, None) => { - Box::new(access_bundle_from_id(client, maybe_id).await?) - } - (Some(_), Some(_)) => { - bail!("Cannot specify both UUID and path"); - } - }; - - let mut dashboard = SupportBundleDashboard::new(accessor).await?; - - enable_raw_mode()?; - - // TODO: It should probably be a flag whether or not this is stderr or - // stdout. - let mut stderr = std::io::stderr(); - execute!(stderr, EnterAlternateScreen, EnableMouseCapture)?; - let backend = CrosstermBackend::new(stderr); - let mut terminal = Terminal::new(backend)?; - - let mut force_update = true; - let pipe_selected_file = loop { - match run_support_bundle_dashboard( - &mut terminal, - &mut dashboard, - force_update, - ) - .await - { - Err(err) => break Err(err), - Ok(InspectRunStep::Exit) => break Ok(false), - Ok(InspectRunStep::Continue) => (), - Ok(InspectRunStep::PipeFile) => break Ok(true), - }; - - force_update = false; - tokio::time::sleep(Duration::from_millis(10)).await; - }; - - // restore terminal - disable_raw_mode()?; - execute!( - terminal.backend_mut(), - LeaveAlternateScreen, - DisableMouseCapture - )?; - terminal.show_cursor()?; - - match pipe_selected_file { - Ok(true) => { - if let Some(contents) = dashboard.buffered_file_contents() { - std::io::copy( - &mut std::io::Cursor::new(contents), - &mut std::io::stdout(), - )?; - } - } - Ok(false) => (), - Err(err) => eprintln!("{err:?}"), - } - Ok(()) -} - -async fn run_support_bundle_dashboard( - terminal: &mut Terminal, - dashboard: &mut SupportBundleDashboard<'_>, - force_update: bool, -) -> anyhow::Result { - let update = if crossterm::event::poll(Duration::from_secs(0))? { - if let Event::Key(key) = event::read()? { - let shifted = key.modifiers.contains(event::KeyModifiers::SHIFT); - match key.code { - KeyCode::Char('q') => return Ok(InspectRunStep::Exit), - KeyCode::Up | KeyCode::Char('k') | KeyCode::Char('K') => { - let count = if shifted { 5 } else { 1 }; - dashboard.select_up(count).await?; - } - KeyCode::Down | KeyCode::Char('j') | KeyCode::Char('J') => { - let count = if shifted { 5 } else { 1 }; - dashboard.select_down(count).await?; - } - KeyCode::Char(' ') => { - dashboard.open_and_buffer().await?; - return Ok(InspectRunStep::PipeFile); - } - KeyCode::Enter => dashboard.toggle_file_open().await?, - _ => {} - } - } - true - } else { - force_update - }; - - if force_update { - terminal.clear()?; - } - - if update { - terminal.draw(|f| draw(f, dashboard))?; - } - - Ok(InspectRunStep::Continue) -} - -fn create_file_list<'a>(dashboard: &'a SupportBundleDashboard<'_>) -> List<'a> { - let files = dashboard.index().files().iter().map(|f| f.as_str()); - List::new(files) - .highlight_symbol("> ") - .highlight_style(Style::new().add_modifier(Modifier::BOLD)) - .block(Block::new().title("Files").borders(Borders::ALL)) -} - -fn create_file_contents<'a>( - dashboard: &'a SupportBundleDashboard<'_>, -) -> Option> { - dashboard.buffered_file_contents().map(|c| { - let c = std::str::from_utf8(c).unwrap_or("Not valid UTF-8"); - - Paragraph::new(c).wrap(Wrap { trim: false }).block( - Block::new() - .title(dashboard.selected_file_name().as_str()) - .borders(Borders::ALL), - ) - }) -} - -const FILE_PICKER_USAGE: [&'static str; 4] = [ - "Press UP or DOWN to select a file. Hold SHIFT to move faster", - "Press ENTER to view a file", - "Press SPACE to exit the terminal and dump the file to stdout", - "Press 'q' to quit", -]; - -const FILE_VIEWER_USAGE: [&'static str; 4] = [ - "Press UP or DOWN to select a file. Hold SHIFT to move faster", - "Press ENTER to stop viewing file", - "Press SPACE to exit the terminal and dump the file to stdout", - "Press 'q' to quit", -]; - -fn draw(f: &mut Frame, dashboard: &mut SupportBundleDashboard<'_>) { - let file_list = create_file_list(dashboard); - let file_contents = create_file_contents(dashboard); - - let mut file_state = ListState::default() - .with_offset(0) - .with_selected(Some(dashboard.selected_file_index())); - - let layout = Layout::vertical([Constraint::Min(0), Constraint::Length(6)]); - - let [main_display_rect, usage_rect] = layout.areas(f.area()); - - if let Some(file_contents) = file_contents { - let usage_list = List::new(FILE_VIEWER_USAGE) - .block(Block::new().title("Usage").borders(Borders::ALL)); - - f.render_widget(file_contents, main_display_rect); - f.render_widget(usage_list, usage_rect); - } else { - let usage_list = List::new(FILE_PICKER_USAGE) - .block(Block::new().title("Usage").borders(Borders::ALL)); - f.render_stateful_widget(file_list, main_display_rect, &mut file_state); - f.render_widget(usage_list, usage_rect); - } + support_bundle_reader_lib::run_dashboard( + client, + args.id, + args.path.as_ref(), + ) + .await } diff --git a/dev-tools/support-bundle-reader-lib/Cargo.toml b/dev-tools/support-bundle-reader-lib/Cargo.toml index 1640b7a203c..9817ff21f80 100644 --- a/dev-tools/support-bundle-reader-lib/Cargo.toml +++ b/dev-tools/support-bundle-reader-lib/Cargo.toml @@ -12,10 +12,12 @@ anyhow.workspace = true async-trait.workspace = true bytes.workspace = true camino.workspace = true +crossterm.workspace = true futures.workspace = true nexus-client.workspace = true omicron-workspace-hack.workspace = true omicron-uuid-kinds.workspace = true +ratatui.workspace = true reqwest.workspace = true tokio.workspace = true zip.workspace = true diff --git a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs index 22facbb6fb9..f387fe1eeb3 100644 --- a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs +++ b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -//! Utilities to help insepct support bundles +//! APIs to help access bundles use anyhow::Context as _; use anyhow::Result; @@ -13,6 +13,7 @@ use camino::Utf8Path; use camino::Utf8PathBuf; use futures::Future; use futures::Stream; +use futures::StreamExt; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::SupportBundleUuid; use std::io; @@ -23,7 +24,6 @@ use tokio::io::AsyncRead; use tokio::io::ReadBuf; use crate::SupportBundleIndex; -use crate::utf8_stream_to_string; /// An I/O source which can read to a buffer /// @@ -238,3 +238,32 @@ impl AsyncRead for AsyncZipFile { Poll::Ready(Ok(())) } } + +async fn utf8_stream_to_string( + mut stream: impl futures::Stream> + + std::marker::Unpin, +) -> Result { + let mut result = String::new(); + + // When we read from the string, we might not read a whole UTF-8 sequence. + // Keep this "leftover" type here to concatenate this partially-read data + // when we read the next sequence. + let mut leftover: Option = None; + while let Some(data) = stream.next().await { + match data { + Err(err) => return Err(anyhow::anyhow!(err)), + Ok(data) => { + let combined = match leftover.take() { + Some(old) => [old, data].concat(), + None => data.to_vec(), + }; + + match std::str::from_utf8(&combined) { + Ok(data) => result += data, + Err(_) => leftover = Some(combined.into()), + } + } + } + } + Ok(result) +} diff --git a/dev-tools/support-bundle-reader-lib/src/dashboard.rs b/dev-tools/support-bundle-reader-lib/src/dashboard.rs new file mode 100644 index 00000000000..6eb8689b18b --- /dev/null +++ b/dev-tools/support-bundle-reader-lib/src/dashboard.rs @@ -0,0 +1,440 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Dashboard for inspecting bundles + +use crate::BoxedFileAccessor; +use crate::SupportBundleAccessor; +use crate::SupportBundleIndex; +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use camino::Utf8Path; +use camino::Utf8PathBuf; +use crossterm::{ + event::{self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode}, + execute, + terminal::{ + EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, + enable_raw_mode, + }, +}; +use futures::TryStreamExt; +use nexus_client::types::SupportBundleInfo; +use nexus_client::types::SupportBundleState; +use omicron_uuid_kinds::GenericUuid; +use omicron_uuid_kinds::SupportBundleUuid; +use ratatui::Frame; +use ratatui::Terminal; +use ratatui::backend::Backend; +use ratatui::backend::CrosstermBackend; +use ratatui::layout::Constraint; +use ratatui::layout::Layout; +use ratatui::style::Modifier; +use ratatui::style::Style; +use ratatui::widgets::Block; +use ratatui::widgets::Borders; +use ratatui::widgets::List; +use ratatui::widgets::ListState; +use ratatui::widgets::Paragraph; +use ratatui::widgets::Wrap; +use std::io::Write; +use std::pin::Pin; +use std::time::Duration; +use tokio::io::AsyncReadExt; + +enum FileState<'a> { + Open { access: Option>>, buffered: Vec }, + Closed, +} + +/// A dashboard for inspecting a support bundle contents +pub struct SupportBundleDashboard<'a> { + access: Box, + index: SupportBundleIndex, + selected: usize, + file: FileState<'a>, +} + +impl<'a> SupportBundleDashboard<'a> { + pub async fn new( + access: Box, + ) -> Result { + let index = access.get_index().await?; + if index.files().is_empty() { + bail!("No files found in support bundle"); + } + Ok(Self { access, index, selected: 0, file: FileState::Closed }) + } + + pub fn index(&self) -> &SupportBundleIndex { + &self.index + } + + pub async fn select_up(&mut self, count: usize) -> Result<()> { + self.selected = self.selected.saturating_sub(count); + if matches!(self.file, FileState::Open { .. }) { + self.open_and_buffer().await?; + } + Ok(()) + } + + pub async fn select_down(&mut self, count: usize) -> Result<()> { + self.selected = + std::cmp::min(self.selected + count, self.index.files().len() - 1); + if matches!(self.file, FileState::Open { .. }) { + self.open_and_buffer().await?; + } + Ok(()) + } + + pub async fn toggle_file_open(&mut self) -> Result<()> { + if self.buffered_file_contents().is_none() { + self.open_and_buffer().await?; + } else { + self.close_file(); + } + Ok(()) + } + + pub async fn open_and_buffer(&mut self) -> Result<()> { + self.open_file().await?; + self.read_to_buffer().await?; + Ok(()) + } + + async fn open_file(&mut self) -> Result<()> { + let path = &self.index.files()[self.selected]; + if path.as_str().ends_with("/") { + self.file = FileState::Open { access: None, buffered: Vec::new() }; + return Ok(()); + } + + let file = self + .access + .get_file(&path) + .await + .with_context(|| format!("Failed to access {path}"))?; + self.file = FileState::Open { + access: Some(Box::pin(file)), + buffered: Vec::new(), + }; + Ok(()) + } + + fn close_file(&mut self) { + self.file = FileState::Closed; + } + + async fn read_to_buffer(&mut self) -> Result<()> { + let FileState::Open { access, ref mut buffered } = &mut self.file + else { + bail!("File cannot be buffered while closed"); + }; + let Some(file) = access.as_mut() else { + return Ok(()); + }; + file.read_to_end(buffered).await?; + Ok(()) + } + + pub fn buffered_file_contents(&self) -> Option<&[u8]> { + let FileState::Open { ref buffered, .. } = &self.file else { + return None; + }; + return Some(buffered); + } + + pub fn selected_file_index(&self) -> usize { + self.selected + } + + pub fn selected_file_name(&self) -> &Utf8Path { + &self.index.files()[self.selected_file_index()] + } +} + +enum InspectRunStep { + // Keep running the dashboard + Continue, + // Exit the dashboard + Exit, + // Exit the dashboard GUI, but pipe a selected file to an output stream + PipeFile, +} + +async fn wait_for_bundle_to_be_collected( + client: &nexus_client::Client, + id: SupportBundleUuid, +) -> Result { + let mut printed_wait_msg = false; + loop { + let sb = client + .support_bundle_view(id.as_untyped_uuid()) + .await + .with_context(|| { + format!("failed to query for support bundle {}", id) + })?; + + match sb.state { + SupportBundleState::Active => { + if printed_wait_msg { + eprintln!(""); + } + return Ok(sb.into_inner()); + } + SupportBundleState::Collecting => { + if !printed_wait_msg { + eprint!("Waiting for {} to finish collection...", id); + printed_wait_msg = true; + } + tokio::time::sleep(Duration::from_secs(1)).await; + eprint!("."); + std::io::stderr() + .flush() + .with_context(|| "cannot flush stderr".to_string())?; + } + other => bail!("Unexepcted state: {other}"), + } + } +} + +async fn access_bundle_from_id( + client: &nexus_client::Client, + id: Option, +) -> Result, anyhow::Error> { + let id = match id { + Some(id) => { + // Ensure the bundle has been collected + let sb = wait_for_bundle_to_be_collected( + client, + SupportBundleUuid::from_untyped_uuid(*id.as_untyped_uuid()), + ) + .await?; + SupportBundleUuid::from_untyped_uuid(sb.id.into_untyped_uuid()) + } + None => { + // Grab the latest if one isn't supplied + let support_bundle_stream = + client.support_bundle_list_stream(None, None); + let mut support_bundles = support_bundle_stream + .try_collect::>() + .await + .context("listing support bundles")?; + support_bundles.sort_by_key(|k| k.time_created); + + let active_sb = support_bundles + .iter() + .find(|sb| matches!(sb.state, SupportBundleState::Active)); + + let sb = match active_sb { + Some(sb) => sb.clone(), + None => { + // This is a special case, but not an uncommon one: + // + // - Someone just created a bundle... + // - ... but collection is still happening. + // + // To smooth out this experience for users, we wait for the + // collection to complete. + let collecting_sb = support_bundles.iter().find(|sb| { + matches!(sb.state, SupportBundleState::Collecting) + }); + if let Some(collecting_sb) = collecting_sb { + let id = &collecting_sb.id; + wait_for_bundle_to_be_collected( + client, + SupportBundleUuid::from_untyped_uuid( + *id.as_untyped_uuid(), + ), + ) + .await? + } else { + bail!( + "Cannot find active support bundle. Try creating one" + ) + } + } + }; + + eprintln!("Inspecting bundle {} from {}", sb.id, sb.time_created); + + SupportBundleUuid::from_untyped_uuid(sb.id.into_untyped_uuid()) + } + }; + Ok(crate::InternalApiAccess::new(client, id)) +} + +pub async fn run_dashboard( + client: &nexus_client::Client, + id: Option, + path: Option<&Utf8PathBuf>, +) -> Result<(), anyhow::Error> { + let accessor: Box = match (id, &path) { + (None, Some(path)) => Box::new(crate::LocalFileAccess::new(path)?), + (maybe_id, None) => { + Box::new(access_bundle_from_id(client, maybe_id).await?) + } + (Some(_), Some(_)) => { + bail!("Cannot specify both UUID and path"); + } + }; + + let mut dashboard = SupportBundleDashboard::new(accessor).await?; + + enable_raw_mode()?; + + // TODO: It should probably be a flag whether or not this is stderr or + // stdout. + let mut stderr = std::io::stderr(); + execute!(stderr, EnterAlternateScreen, EnableMouseCapture)?; + let backend = CrosstermBackend::new(stderr); + let mut terminal = Terminal::new(backend)?; + + let mut force_update = true; + let pipe_selected_file = loop { + match run_support_bundle_dashboard( + &mut terminal, + &mut dashboard, + force_update, + ) + .await + { + Err(err) => break Err(err), + Ok(InspectRunStep::Exit) => break Ok(false), + Ok(InspectRunStep::Continue) => (), + Ok(InspectRunStep::PipeFile) => break Ok(true), + }; + + force_update = false; + tokio::time::sleep(Duration::from_millis(10)).await; + }; + + // restore terminal + disable_raw_mode()?; + execute!( + terminal.backend_mut(), + LeaveAlternateScreen, + DisableMouseCapture + )?; + terminal.show_cursor()?; + + match pipe_selected_file { + Ok(true) => { + if let Some(contents) = dashboard.buffered_file_contents() { + std::io::copy( + &mut std::io::Cursor::new(contents), + &mut std::io::stdout(), + )?; + } + } + Ok(false) => (), + Err(err) => eprintln!("{err:?}"), + } + Ok(()) +} + +async fn run_support_bundle_dashboard( + terminal: &mut Terminal, + dashboard: &mut SupportBundleDashboard<'_>, + force_update: bool, +) -> anyhow::Result { + let update = if crossterm::event::poll(Duration::from_secs(0))? { + if let Event::Key(key) = event::read()? { + let shifted = key.modifiers.contains(event::KeyModifiers::SHIFT); + match key.code { + KeyCode::Char('q') => return Ok(InspectRunStep::Exit), + KeyCode::Up | KeyCode::Char('k') | KeyCode::Char('K') => { + let count = if shifted { 5 } else { 1 }; + dashboard.select_up(count).await?; + } + KeyCode::Down | KeyCode::Char('j') | KeyCode::Char('J') => { + let count = if shifted { 5 } else { 1 }; + dashboard.select_down(count).await?; + } + KeyCode::Char(' ') => { + dashboard.open_and_buffer().await?; + return Ok(InspectRunStep::PipeFile); + } + KeyCode::Enter => dashboard.toggle_file_open().await?, + _ => {} + } + } + true + } else { + force_update + }; + + if force_update { + terminal.clear()?; + } + + if update { + terminal.draw(|f| draw(f, dashboard))?; + } + + Ok(InspectRunStep::Continue) +} + +fn create_file_list<'a>(dashboard: &'a SupportBundleDashboard<'_>) -> List<'a> { + let files = dashboard.index().files().iter().map(|f| f.as_str()); + List::new(files) + .highlight_symbol("> ") + .highlight_style(Style::new().add_modifier(Modifier::BOLD)) + .block(Block::new().title("Files").borders(Borders::ALL)) +} + +fn create_file_contents<'a>( + dashboard: &'a SupportBundleDashboard<'_>, +) -> Option> { + dashboard.buffered_file_contents().map(|c| { + let c = std::str::from_utf8(c).unwrap_or("Not valid UTF-8"); + + Paragraph::new(c).wrap(Wrap { trim: false }).block( + Block::new() + .title(dashboard.selected_file_name().as_str()) + .borders(Borders::ALL), + ) + }) +} + +const FILE_PICKER_USAGE: [&'static str; 4] = [ + "Press UP or DOWN to select a file. Hold SHIFT to move faster", + "Press ENTER to view a file", + "Press SPACE to exit the terminal and dump the file to stdout", + "Press 'q' to quit", +]; + +const FILE_VIEWER_USAGE: [&'static str; 4] = [ + "Press UP or DOWN to select a file. Hold SHIFT to move faster", + "Press ENTER to stop viewing file", + "Press SPACE to exit the terminal and dump the file to stdout", + "Press 'q' to quit", +]; + +fn draw(f: &mut Frame, dashboard: &mut SupportBundleDashboard<'_>) { + let file_list = create_file_list(dashboard); + let file_contents = create_file_contents(dashboard); + + let mut file_state = ListState::default() + .with_offset(0) + .with_selected(Some(dashboard.selected_file_index())); + + let layout = Layout::vertical([Constraint::Min(0), Constraint::Length(6)]); + + let [main_display_rect, usage_rect] = layout.areas(f.area()); + + if let Some(file_contents) = file_contents { + let usage_list = List::new(FILE_VIEWER_USAGE) + .block(Block::new().title("Usage").borders(Borders::ALL)); + + f.render_widget(file_contents, main_display_rect); + f.render_widget(usage_list, usage_rect); + } else { + let usage_list = List::new(FILE_PICKER_USAGE) + .block(Block::new().title("Usage").borders(Borders::ALL)); + f.render_stateful_widget(file_list, main_display_rect, &mut file_state); + f.render_widget(usage_list, usage_rect); + } +} diff --git a/dev-tools/support-bundle-reader-lib/src/lib.rs b/dev-tools/support-bundle-reader-lib/src/lib.rs index 46a02084b5b..4ed84912ded 100644 --- a/dev-tools/support-bundle-reader-lib/src/lib.rs +++ b/dev-tools/support-bundle-reader-lib/src/lib.rs @@ -2,163 +2,20 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -//! Utilities to help insepct support bundles - -use anyhow::Context; -use anyhow::Result; -use anyhow::bail; -use camino::Utf8Path; -use futures::StreamExt; -use std::pin::Pin; -use tokio::io::AsyncReadExt; +//! Utilities to inspect support bundles mod bundle_accessor; +mod dashboard; mod index; +// TODO: Not all this needs to be external + pub use bundle_accessor::AsyncZipFile; pub use bundle_accessor::BoxedFileAccessor; pub use bundle_accessor::FileAccessor; pub use bundle_accessor::InternalApiAccess; pub use bundle_accessor::LocalFileAccess; pub use bundle_accessor::SupportBundleAccessor; +pub use dashboard::SupportBundleDashboard; +pub use dashboard::run_dashboard; pub use index::SupportBundleIndex; - -enum FileState<'a> { - Open { access: Option>>, buffered: Vec }, - Closed, -} - -/// A dashboard for inspecting a support bundle contents -pub struct SupportBundleDashboard<'a> { - access: Box, - index: SupportBundleIndex, - selected: usize, - file: FileState<'a>, -} - -impl<'a> SupportBundleDashboard<'a> { - pub async fn new( - access: Box, - ) -> Result { - let index = access.get_index().await?; - if index.files().is_empty() { - bail!("No files found in support bundle"); - } - Ok(Self { access, index, selected: 0, file: FileState::Closed }) - } - - pub fn index(&self) -> &SupportBundleIndex { - &self.index - } - - pub async fn select_up(&mut self, count: usize) -> Result<()> { - self.selected = self.selected.saturating_sub(count); - if matches!(self.file, FileState::Open { .. }) { - self.open_and_buffer().await?; - } - Ok(()) - } - - pub async fn select_down(&mut self, count: usize) -> Result<()> { - self.selected = - std::cmp::min(self.selected + count, self.index.files().len() - 1); - if matches!(self.file, FileState::Open { .. }) { - self.open_and_buffer().await?; - } - Ok(()) - } - - pub async fn toggle_file_open(&mut self) -> Result<()> { - if self.buffered_file_contents().is_none() { - self.open_and_buffer().await?; - } else { - self.close_file(); - } - Ok(()) - } - - pub async fn open_and_buffer(&mut self) -> Result<()> { - self.open_file().await?; - self.read_to_buffer().await?; - Ok(()) - } - - async fn open_file(&mut self) -> Result<()> { - let path = &self.index.files()[self.selected]; - if path.as_str().ends_with("/") { - self.file = FileState::Open { access: None, buffered: Vec::new() }; - return Ok(()); - } - - let file = self - .access - .get_file(&path) - .await - .with_context(|| format!("Failed to access {path}"))?; - self.file = FileState::Open { - access: Some(Box::pin(file)), - buffered: Vec::new(), - }; - Ok(()) - } - - fn close_file(&mut self) { - self.file = FileState::Closed; - } - - async fn read_to_buffer(&mut self) -> Result<()> { - let FileState::Open { access, ref mut buffered } = &mut self.file - else { - bail!("File cannot be buffered while closed"); - }; - let Some(file) = access.as_mut() else { - return Ok(()); - }; - file.read_to_end(buffered).await?; - Ok(()) - } - - pub fn buffered_file_contents(&self) -> Option<&[u8]> { - let FileState::Open { ref buffered, .. } = &self.file else { - return None; - }; - return Some(buffered); - } - - pub fn selected_file_index(&self) -> usize { - self.selected - } - - pub fn selected_file_name(&self) -> &Utf8Path { - &self.index.files()[self.selected_file_index()] - } -} - -pub(crate) async fn utf8_stream_to_string( - mut stream: impl futures::Stream> - + std::marker::Unpin, -) -> Result { - let mut result = String::new(); - - // When we read from the string, we might not read a whole UTF-8 sequence. - // Keep this "leftover" type here to concatenate this partially-read data - // when we read the next sequence. - let mut leftover: Option = None; - while let Some(data) = stream.next().await { - match data { - Err(err) => return Err(anyhow::anyhow!(err)), - Ok(data) => { - let combined = match leftover.take() { - Some(old) => [old, data].concat(), - None => data.to_vec(), - }; - - match std::str::from_utf8(&combined) { - Ok(data) => result += data, - Err(_) => leftover = Some(combined.into()), - } - } - } - } - Ok(result) -} From 995ca46786ba4086dfac63b658ec54e66d02faaf Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 29 Apr 2025 15:23:17 -0700 Subject: [PATCH 21/31] More private interface --- .../src/bundle_accessor.rs | 3 +-- .../support-bundle-reader-lib/src/dashboard.rs | 14 ++++++++------ dev-tools/support-bundle-reader-lib/src/lib.rs | 10 ---------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs index f387fe1eeb3..acb56cf586d 100644 --- a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs +++ b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs @@ -4,6 +4,7 @@ //! APIs to help access bundles +use crate::index::SupportBundleIndex; use anyhow::Context as _; use anyhow::Result; use async_trait::async_trait; @@ -23,8 +24,6 @@ use std::task::Poll; use tokio::io::AsyncRead; use tokio::io::ReadBuf; -use crate::SupportBundleIndex; - /// An I/O source which can read to a buffer /// /// This describes access to individual files within the bundle. diff --git a/dev-tools/support-bundle-reader-lib/src/dashboard.rs b/dev-tools/support-bundle-reader-lib/src/dashboard.rs index 6eb8689b18b..b0021c90682 100644 --- a/dev-tools/support-bundle-reader-lib/src/dashboard.rs +++ b/dev-tools/support-bundle-reader-lib/src/dashboard.rs @@ -4,9 +4,11 @@ //! Dashboard for inspecting bundles -use crate::BoxedFileAccessor; -use crate::SupportBundleAccessor; -use crate::SupportBundleIndex; +use crate::bundle_accessor::BoxedFileAccessor; +use crate::bundle_accessor::InternalApiAccess; +use crate::bundle_accessor::LocalFileAccess; +use crate::bundle_accessor::SupportBundleAccessor; +use crate::index::SupportBundleIndex; use anyhow::Context; use anyhow::Result; use anyhow::bail; @@ -203,7 +205,7 @@ async fn wait_for_bundle_to_be_collected( async fn access_bundle_from_id( client: &nexus_client::Client, id: Option, -) -> Result, anyhow::Error> { +) -> Result, anyhow::Error> { let id = match id { Some(id) => { // Ensure the bundle has been collected @@ -263,7 +265,7 @@ async fn access_bundle_from_id( SupportBundleUuid::from_untyped_uuid(sb.id.into_untyped_uuid()) } }; - Ok(crate::InternalApiAccess::new(client, id)) + Ok(InternalApiAccess::new(client, id)) } pub async fn run_dashboard( @@ -272,7 +274,7 @@ pub async fn run_dashboard( path: Option<&Utf8PathBuf>, ) -> Result<(), anyhow::Error> { let accessor: Box = match (id, &path) { - (None, Some(path)) => Box::new(crate::LocalFileAccess::new(path)?), + (None, Some(path)) => Box::new(LocalFileAccess::new(path)?), (maybe_id, None) => { Box::new(access_bundle_from_id(client, maybe_id).await?) } diff --git a/dev-tools/support-bundle-reader-lib/src/lib.rs b/dev-tools/support-bundle-reader-lib/src/lib.rs index 4ed84912ded..c5314b65660 100644 --- a/dev-tools/support-bundle-reader-lib/src/lib.rs +++ b/dev-tools/support-bundle-reader-lib/src/lib.rs @@ -8,14 +8,4 @@ mod bundle_accessor; mod dashboard; mod index; -// TODO: Not all this needs to be external - -pub use bundle_accessor::AsyncZipFile; -pub use bundle_accessor::BoxedFileAccessor; -pub use bundle_accessor::FileAccessor; -pub use bundle_accessor::InternalApiAccess; -pub use bundle_accessor::LocalFileAccess; -pub use bundle_accessor::SupportBundleAccessor; -pub use dashboard::SupportBundleDashboard; pub use dashboard::run_dashboard; -pub use index::SupportBundleIndex; From 6ad1a3193a646388b886e0d59f03d9fd7f1f11eb Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 30 Apr 2025 11:06:36 -0700 Subject: [PATCH 22/31] starting to buffer and stream more properly --- .../src/bundle_accessor.rs | 28 ++----- .../src/dashboard.rs | 84 +++++++++++-------- 2 files changed, 58 insertions(+), 54 deletions(-) diff --git a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs index acb56cf586d..f8173fda646 100644 --- a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs +++ b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs @@ -12,7 +12,6 @@ use bytes::Buf; use bytes::Bytes; use camino::Utf8Path; use camino::Utf8PathBuf; -use futures::Future; use futures::Stream; use futures::StreamExt; use omicron_uuid_kinds::GenericUuid; @@ -64,8 +63,11 @@ impl<'a> StreamedFile<'a> { Self { client, id, path, stream: None, buffer: Bytes::new() } } + // NOTE: This is a distinct method from "new", because ideally some day we could + // use range requests to stream out portions of the file. async fn start_stream(&mut self) -> Result<()> { - // TODO: Add range headers? + // TODO: Add range headers, for range requests? Though this + // will require adding support to Progenitor + Nexus too. let stream = self .client .support_bundle_download_file( @@ -93,24 +95,12 @@ impl AsyncRead for StreamedFile<'_> { buf: &mut ReadBuf<'_>, ) -> Poll> { while self.buffer.is_empty() { - if self.stream.is_none() { - // NOTE: this is broken? - let fut = self.start_stream(); - let mut fut = Box::pin(fut); - - match futures::ready!(fut.as_mut().poll(cx)) { - Ok(()) => {} - Err(e) => { - return Poll::Ready(Err(io::Error::new( - io::ErrorKind::Other, - e, - ))); - } - } - } - match futures::ready!( - self.stream.as_mut().unwrap().as_mut().poll_next(cx) + self.stream + .as_mut() + .expect("Stream must be initialized before polling") + .as_mut() + .poll_next(cx) ) { Some(Ok(bytes)) => { self.buffer = bytes; diff --git a/dev-tools/support-bundle-reader-lib/src/dashboard.rs b/dev-tools/support-bundle-reader-lib/src/dashboard.rs index b0021c90682..fa77fcdd96f 100644 --- a/dev-tools/support-bundle-reader-lib/src/dashboard.rs +++ b/dev-tools/support-bundle-reader-lib/src/dashboard.rs @@ -12,6 +12,7 @@ use crate::index::SupportBundleIndex; use anyhow::Context; use anyhow::Result; use anyhow::bail; +use bytes::BytesMut; use camino::Utf8Path; use camino::Utf8PathBuf; use crossterm::{ @@ -42,12 +43,19 @@ use ratatui::widgets::ListState; use ratatui::widgets::Paragraph; use ratatui::widgets::Wrap; use std::io::Write; -use std::pin::Pin; use std::time::Duration; +use tokio::io::AsyncRead; use tokio::io::AsyncReadExt; +use tokio::io::BufReader; + +const BUF_READER_CAPACITY: usize = 1 << 16; enum FileState<'a> { - Open { access: Option>>, buffered: Vec }, + Open { + // TODO; actually use bufreader, rather than manually buffering? + access: Option>>, + preview: BytesMut, + }, Closed, } @@ -60,9 +68,7 @@ pub struct SupportBundleDashboard<'a> { } impl<'a> SupportBundleDashboard<'a> { - pub async fn new( - access: Box, - ) -> Result { + async fn new(access: Box) -> Result { let index = access.get_index().await?; if index.files().is_empty() { bail!("No files found in support bundle"); @@ -70,11 +76,11 @@ impl<'a> SupportBundleDashboard<'a> { Ok(Self { access, index, selected: 0, file: FileState::Closed }) } - pub fn index(&self) -> &SupportBundleIndex { + fn index(&self) -> &SupportBundleIndex { &self.index } - pub async fn select_up(&mut self, count: usize) -> Result<()> { + async fn select_up(&mut self, count: usize) -> Result<()> { self.selected = self.selected.saturating_sub(count); if matches!(self.file, FileState::Open { .. }) { self.open_and_buffer().await?; @@ -82,7 +88,7 @@ impl<'a> SupportBundleDashboard<'a> { Ok(()) } - pub async fn select_down(&mut self, count: usize) -> Result<()> { + async fn select_down(&mut self, count: usize) -> Result<()> { self.selected = std::cmp::min(self.selected + count, self.index.files().len() - 1); if matches!(self.file, FileState::Open { .. }) { @@ -91,16 +97,15 @@ impl<'a> SupportBundleDashboard<'a> { Ok(()) } - pub async fn toggle_file_open(&mut self) -> Result<()> { - if self.buffered_file_contents().is_none() { - self.open_and_buffer().await?; - } else { - self.close_file(); + async fn toggle_file_open(&mut self) -> Result<()> { + match self.file { + FileState::Open { .. } => self.close_file(), + FileState::Closed => self.open_and_buffer().await?, } Ok(()) } - pub async fn open_and_buffer(&mut self) -> Result<()> { + async fn open_and_buffer(&mut self) -> Result<()> { self.open_file().await?; self.read_to_buffer().await?; Ok(()) @@ -109,7 +114,8 @@ impl<'a> SupportBundleDashboard<'a> { async fn open_file(&mut self) -> Result<()> { let path = &self.index.files()[self.selected]; if path.as_str().ends_with("/") { - self.file = FileState::Open { access: None, buffered: Vec::new() }; + self.file = + FileState::Open { access: None, preview: BytesMut::new() }; return Ok(()); } @@ -119,8 +125,8 @@ impl<'a> SupportBundleDashboard<'a> { .await .with_context(|| format!("Failed to access {path}"))?; self.file = FileState::Open { - access: Some(Box::pin(file)), - buffered: Vec::new(), + access: Some(BufReader::with_capacity(BUF_READER_CAPACITY, file)), + preview: BytesMut::new(), }; Ok(()) } @@ -130,29 +136,40 @@ impl<'a> SupportBundleDashboard<'a> { } async fn read_to_buffer(&mut self) -> Result<()> { - let FileState::Open { access, ref mut buffered } = &mut self.file - else { + let FileState::Open { access, ref mut preview } = &mut self.file else { bail!("File cannot be buffered while closed"); }; let Some(file) = access.as_mut() else { return Ok(()); }; - file.read_to_end(buffered).await?; + preview.reserve(BUF_READER_CAPACITY); + file.read_buf(preview).await?; Ok(()) } - pub fn buffered_file_contents(&self) -> Option<&[u8]> { - let FileState::Open { ref buffered, .. } = &self.file else { + fn buffered_file_preview(&self) -> Option<&[u8]> { + let FileState::Open { ref preview, .. } = &self.file else { return None; }; - return Some(buffered); + return Some(preview); + } + + fn streaming_file_contents( + &mut self, + ) -> Option> { + match &mut self.file { + FileState::Open { access, preview } => { + Some(preview.chain(access.as_mut().unwrap())) + } + FileState::Closed => None, + } } - pub fn selected_file_index(&self) -> usize { + fn selected_file_index(&self) -> usize { self.selected } - pub fn selected_file_name(&self) -> &Utf8Path { + fn selected_file_name(&self) -> &Utf8Path { &self.index.files()[self.selected_file_index()] } } @@ -324,11 +341,8 @@ pub async fn run_dashboard( match pipe_selected_file { Ok(true) => { - if let Some(contents) = dashboard.buffered_file_contents() { - std::io::copy( - &mut std::io::Cursor::new(contents), - &mut std::io::stdout(), - )?; + if let Some(mut stream) = dashboard.streaming_file_contents() { + tokio::io::copy(&mut stream, &mut tokio::io::stdout()).await?; } } Ok(false) => (), @@ -387,10 +401,10 @@ fn create_file_list<'a>(dashboard: &'a SupportBundleDashboard<'_>) -> List<'a> { .block(Block::new().title("Files").borders(Borders::ALL)) } -fn create_file_contents<'a>( +fn create_file_preview<'a>( dashboard: &'a SupportBundleDashboard<'_>, ) -> Option> { - dashboard.buffered_file_contents().map(|c| { + dashboard.buffered_file_preview().map(|c| { let c = std::str::from_utf8(c).unwrap_or("Not valid UTF-8"); Paragraph::new(c).wrap(Wrap { trim: false }).block( @@ -417,7 +431,7 @@ const FILE_VIEWER_USAGE: [&'static str; 4] = [ fn draw(f: &mut Frame, dashboard: &mut SupportBundleDashboard<'_>) { let file_list = create_file_list(dashboard); - let file_contents = create_file_contents(dashboard); + let file_preview = create_file_preview(dashboard); let mut file_state = ListState::default() .with_offset(0) @@ -427,11 +441,11 @@ fn draw(f: &mut Frame, dashboard: &mut SupportBundleDashboard<'_>) { let [main_display_rect, usage_rect] = layout.areas(f.area()); - if let Some(file_contents) = file_contents { + if let Some(file_preview) = file_preview { let usage_list = List::new(FILE_VIEWER_USAGE) .block(Block::new().title("Usage").borders(Borders::ALL)); - f.render_widget(file_contents, main_display_rect); + f.render_widget(file_preview, main_display_rect); f.render_widget(usage_list, usage_rect); } else { let usage_list = List::new(FILE_PICKER_USAGE) From 865467ac6479f50944381a9bd709fb5d74ac9095 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 30 Apr 2025 12:45:23 -0700 Subject: [PATCH 23/31] Less unwrapping, more cleanup --- .../src/bundle_accessor.rs | 2 ++ .../support-bundle-reader-lib/src/dashboard.rs | 18 ++++++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs index f8173fda646..757e9cb78c1 100644 --- a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs +++ b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs @@ -65,6 +65,8 @@ impl<'a> StreamedFile<'a> { // NOTE: This is a distinct method from "new", because ideally some day we could // use range requests to stream out portions of the file. + // + // This means that we would potentially want to restart the stream with a different position. async fn start_stream(&mut self) -> Result<()> { // TODO: Add range headers, for range requests? Though this // will require adding support to Progenitor + Nexus too. diff --git a/dev-tools/support-bundle-reader-lib/src/dashboard.rs b/dev-tools/support-bundle-reader-lib/src/dashboard.rs index fa77fcdd96f..775fbe5b75c 100644 --- a/dev-tools/support-bundle-reader-lib/src/dashboard.rs +++ b/dev-tools/support-bundle-reader-lib/src/dashboard.rs @@ -51,11 +51,7 @@ use tokio::io::BufReader; const BUF_READER_CAPACITY: usize = 1 << 16; enum FileState<'a> { - Open { - // TODO; actually use bufreader, rather than manually buffering? - access: Option>>, - preview: BytesMut, - }, + Open { access: Option>>, preview: BytesMut }, Closed, } @@ -114,8 +110,10 @@ impl<'a> SupportBundleDashboard<'a> { async fn open_file(&mut self) -> Result<()> { let path = &self.index.files()[self.selected]; if path.as_str().ends_with("/") { - self.file = - FileState::Open { access: None, preview: BytesMut::new() }; + self.file = FileState::Open { + access: None, + preview: BytesMut::from(&b""[..]), + }; return Ok(()); } @@ -158,10 +156,10 @@ impl<'a> SupportBundleDashboard<'a> { &mut self, ) -> Option> { match &mut self.file { - FileState::Open { access, preview } => { - Some(preview.chain(access.as_mut().unwrap())) + FileState::Open { access: Some(access), preview } => { + Some(preview.chain(access)) } - FileState::Closed => None, + _ => None, } } From f2aae61f6c5b6b742c49ae3fddc94dad34babf42 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 30 Apr 2025 13:20:29 -0700 Subject: [PATCH 24/31] [omdb] Add command to download entire support bundle --- dev-tools/omdb/src/bin/omdb/nexus.rs | 41 ++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index ce7bbf8a034..ad91b37560a 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -495,13 +495,15 @@ enum SupportBundleCommands { Create, /// Delete a support bundle Delete(SupportBundleDeleteArgs), + /// Download an entire support bundle + Download(SupportBundleDownloadArgs), /// Download the index of a support bundle /// /// This is a "list of files", from which individual files can be accessed GetIndex(SupportBundleIndexArgs), - /// View a file within a support bundle + /// Download a single file within a support bundle GetFile(SupportBundleFileArgs), - /// Creates dashboard for viewing the contents of a support bundle + /// Run a dashboard for viewing the contents of a support bundle Inspect(SupportBundleInspectArgs), } @@ -510,6 +512,16 @@ struct SupportBundleDeleteArgs { id: SupportBundleUuid, } +#[derive(Debug, Args)] +struct SupportBundleDownloadArgs { + id: SupportBundleUuid, + + /// Optional output path where the file should be written, + /// instead of stdout. + #[arg(short, long)] + output: Option, +} + #[derive(Debug, Args)] struct SupportBundleIndexArgs { id: SupportBundleUuid, @@ -750,6 +762,9 @@ impl NexusArgs { let token = omdb.check_allow_destructive()?; cmd_nexus_support_bundles_delete(&client, args, token).await } + NexusCommands::SupportBundles(SupportBundleArgs { + command: SupportBundleCommands::Download(args), + }) => cmd_nexus_support_bundles_download(&client, args).await, NexusCommands::SupportBundles(SupportBundleArgs { command: SupportBundleCommands::GetIndex(args), }) => cmd_nexus_support_bundles_get_index(&client, args).await, @@ -3854,6 +3869,28 @@ async fn write_stream_to_sink( Ok(()) } +/// Runs `omdb nexus support-bundles download` +async fn cmd_nexus_support_bundles_download( + client: &nexus_client::Client, + args: &SupportBundleDownloadArgs, +) -> Result<(), anyhow::Error> { + let stream = client + .support_bundle_download(args.id.as_untyped_uuid()) + .await + .with_context(|| format!("downloading support bundle {}", args.id))? + .into_inner_stream(); + + let sink: Box = match &args.output { + Some(path) => Box::new(std::fs::File::create(path)?), + None => Box::new(std::io::stdout()), + }; + + write_stream_to_sink(stream, sink) + .await + .with_context(|| format!("streaming support bundle {}", args.id))?; + Ok(()) +} + /// Runs `omdb nexus support-bundles get-index` async fn cmd_nexus_support_bundles_get_index( client: &nexus_client::Client, From 70a95181dcaf21cbb08e937830b2639da3e3779a Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 6 May 2025 10:54:45 -0700 Subject: [PATCH 25/31] [support bundle] Add health checks to support bundles --- .../tasks/support_bundle_collector.rs | 6 ++ openapi/sled-agent.json | 27 ++++++ sled-agent/api/src/lib.rs | 8 ++ sled-agent/src/http_entrypoints.rs | 14 +++ sled-agent/src/sim/http_entrypoints.rs | 7 ++ sled-agent/src/sled_agent.rs | 9 +- sled-diagnostics/Cargo.toml | 2 +- sled-diagnostics/src/lib.rs | 22 +++++ sled-diagnostics/src/queries.rs | 96 +++++++++++++++++++ 9 files changed, 189 insertions(+), 2 deletions(-) diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index 9003f19038d..3629ffce9ba 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -662,6 +662,12 @@ impl BundleCollection<'_> { sled_client.support_zpool_info(), ) .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "health-check", + sled_client.support_health_check(), + ) + .boxed(), ]) // Currently we execute up to 10 commands concurrently which // might be doing their own concurrent work, for example diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index c3f1cbcee51..fbc523b7744 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -659,6 +659,33 @@ } } }, + "/support/health-check": { + "get": { + "operationId": "support_health_check", + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "title": "Array_of_SledDiagnosticsQueryOutput", + "type": "array", + "items": { + "$ref": "#/components/schemas/SledDiagnosticsQueryOutput" + } + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/support/ipadm-info": { "get": { "operationId": "support_ipadm_info", diff --git a/sled-agent/api/src/lib.rs b/sled-agent/api/src/lib.rs index 6033650fa4e..7bdba01e1bc 100644 --- a/sled-agent/api/src/lib.rs +++ b/sled-agent/api/src/lib.rs @@ -673,6 +673,14 @@ pub trait SledAgentApi { request_context: RequestContext, ) -> Result, HttpError>; + #[endpoint { + method = GET, + path = "/support/health-check", + }] + async fn support_health_check( + request_context: RequestContext, + ) -> Result>, HttpError>; + /// This endpoint returns a list of known zones on a sled that have service /// logs that can be collected into a support bundle. #[endpoint { diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs index 9abd2dfb5c2..0056056eb33 100644 --- a/sled-agent/src/http_entrypoints.rs +++ b/sled-agent/src/http_entrypoints.rs @@ -1036,6 +1036,20 @@ impl SledAgentApi for SledAgentImpl { Ok(HttpResponseOk(res.get_output())) } + async fn support_health_check( + request_context: RequestContext, + ) -> Result>, HttpError> + { + let sa = request_context.context(); + Ok(HttpResponseOk( + sa.support_health_check() + .await + .into_iter() + .map(|cmd| cmd.get_output()) + .collect::>(), + )) + } + async fn support_logs( request_context: RequestContext, ) -> Result>, HttpError> { diff --git a/sled-agent/src/sim/http_entrypoints.rs b/sled-agent/src/sim/http_entrypoints.rs index 57ea9491484..59eb99a4d6e 100644 --- a/sled-agent/src/sim/http_entrypoints.rs +++ b/sled-agent/src/sim/http_entrypoints.rs @@ -745,6 +745,13 @@ impl SledAgentApi for SledAgentSimImpl { method_unimplemented() } + async fn support_health_check( + _request_context: RequestContext, + ) -> Result>, HttpError> + { + method_unimplemented() + } + async fn support_logs( _request_context: RequestContext, ) -> Result>, HttpError> { diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index e1976c2b29a..8013b2a37c0 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -67,7 +67,8 @@ use sled_agent_types::zone_bundle::{ BundleUtilization, CleanupContext, CleanupCount, CleanupPeriod, PriorityOrder, StorageLimit, ZoneBundleMetadata, }; -use sled_diagnostics::{SledDiagnosticsCmdError, SledDiagnosticsCmdOutput}; +use sled_diagnostics::SledDiagnosticsCmdError; +use sled_diagnostics::SledDiagnosticsCmdOutput; use sled_hardware::{HardwareManager, MemoryReservations, underlay}; use sled_hardware_types::Baseboard; use sled_hardware_types::underlay::BootstrapInterface; @@ -1474,6 +1475,12 @@ impl SledAgent { ) -> Result { sled_diagnostics::zpool_info().await } + + pub(crate) async fn support_health_check( + &self, + ) -> Vec> { + sled_diagnostics::health_check().await + } } #[derive(From, thiserror::Error, Debug)] diff --git a/sled-diagnostics/Cargo.toml b/sled-diagnostics/Cargo.toml index 40a56c5d705..70ca27f157d 100644 --- a/sled-diagnostics/Cargo.toml +++ b/sled-diagnostics/Cargo.toml @@ -29,4 +29,4 @@ zip = { workspace = true, features = ["zstd"] } omicron-common.workspace = true omicron-test-utils.workspace = true omicron-uuid-kinds.workspace = true -sled-storage = { workspace = true, features = ["testing"] } +sled-storage = { workspace = true, features = ["testing"] } diff --git a/sled-diagnostics/src/lib.rs b/sled-diagnostics/src/lib.rs index 8b7639a45b1..e3bc6b5e6c8 100644 --- a/sled-diagnostics/src/lib.rs +++ b/sled-diagnostics/src/lib.rs @@ -146,3 +146,25 @@ pub async fn zpool_info() -> Result { execute_command_with_timeout(zpool_status(), DEFAULT_TIMEOUT).await } + +pub async fn health_check() +-> Vec> { + [ + uptime(), + kstat_low_page(), + svcs_show_disabled(), + count_disks(), + zfs_list_unmounted(), + count_crucibles(), + identify_datasets_close_to_quota(), + identify_datasets_with_less_than_300_gib_avail(), + dimm_check(), + ] + .into_iter() + .map(|c| async move { + execute_command_with_timeout(c, DEFAULT_TIMEOUT).await + }) + .collect::>() + .collect::>>() + .await +} diff --git a/sled-diagnostics/src/queries.rs b/sled-diagnostics/src/queries.rs index c7da33ecefb..039c552acc3 100644 --- a/sled-diagnostics/src/queries.rs +++ b/sled-diagnostics/src/queries.rs @@ -22,11 +22,14 @@ use crate::contract_stub::ContractError; const DLADM: &str = "/usr/sbin/dladm"; const IPADM: &str = "/usr/sbin/ipadm"; +const KSTAT: &str = "/usr/bin/kstat"; const NVMEADM: &str = "/usr/sbin/nvmeadm"; const PFEXEC: &str = "/usr/bin/pfexec"; const PFILES: &str = "/usr/bin/pfiles"; const PSTACK: &str = "/usr/bin/pstack"; const PARGS: &str = "/usr/bin/pargs"; +const SVCS: &str = "/usr/bin/svcs"; +const UPTIME: &str = "/usr/bin/uptime"; const ZFS: &str = "/usr/sbin/zfs"; const ZONEADM: &str = "/usr/sbin/zoneadm"; const ZPOOL: &str = "/usr/sbin/zpool"; @@ -263,6 +266,99 @@ pub fn pfiles_process(pid: i32) -> Command { cmd } +pub fn uptime() -> Command { + let mut cmd = std::process::Command::new(UPTIME); + cmd.env_clear(); + cmd +} + +pub fn kstat_low_page() -> Command { + let mut cmd = std::process::Command::new(PFEXEC); + cmd.env_clear().arg(KSTAT).arg("-p").arg("unix::system_pages:low_mem_scan"); + cmd +} + +pub fn svcs_show_disabled() -> Command { + let mut cmd = std::process::Command::new(PFEXEC); + cmd.env_clear().arg(SVCS).arg("-xZ"); + cmd +} + +pub fn count_disks() -> Command { + let mut cmd = std::process::Command::new("bash"); + cmd.env_clear().args([ + "-c", + "(pfexec diskinfo -pH | tee | wc -l | xargs | grep -x '12' > /dev/null) + && echo 'OK: All expected disks found' + || echo 'WARN: Unexpected number of physical disks (expected 12)'", + ]); + cmd +} + +pub fn zfs_list_unmounted() -> Command { + let mut cmd = std::process::Command::new("bash"); + cmd.env_clear().args([ + "-c", + "pfexec zfs list -r -o name,mounted | grep oxp | grep -v yes$ + && echo 'WARN: Found unmounted dataset(s)' + || echo 'OK: No unmounted datasets'", + ]); + cmd +} + +pub fn count_crucibles() -> Command { + let mut cmd = std::process::Command::new("bash"); + cmd.env_clear() + .args([ + "-c", + "(zoneadm list | grep crucible | grep -v pantry | tee | wc -l | xargs | grep -x '10' > /dev/null) + && echo 'OK: 10 Crucibles found' + || echo 'WARN: Unexpected number of crucible zones (expected 10)'" + ]); + cmd +} + +pub fn identify_datasets_close_to_quota() -> Command { + let mut cmd = std::process::Command::new("bash"); + cmd.env_clear() + .args([ + "-c", + "zfs list -Hp -o used,quota,name,avail,mountpoint | + egrep 'oxp|oxi' | + egrep -v 'none|crucible' | + awk '$2 > 0 && $1 / $2 >= 0.8 { any=1; print } END { exit !any }' + && echo 'WARN: Found near-quota datasets' + || echo 'OK: No near-quota datasets found'" + ]); + cmd +} + +pub fn identify_datasets_with_less_than_300_gib_avail() -> Command { + let mut cmd = std::process::Command::new("bash"); + cmd.env_clear().args([ + "-c", + "zfs list -Hp -o used,quota,name,avail,mountpoint | + egrep 'oxp|oxi' | + egrep -v 'none|crucible' | + awk '$4 < (300 * (1024^3)) { any=1; print } END { exit !any }' + && echo 'WARN: Found low-space datasets' + || echo 'OK: No low-space datasets found'", + ]); + cmd +} + +pub fn dimm_check() -> Command { + let mut cmd = std::process::Command::new("bash"); + cmd.env_clear().args([ + "-c", + "prtconf -m | + grep -v -e 1036271 -e 2084847 + && echo 'WARN: Unexpected quantity of system memory' + || echo 'OK: Found expected quantity of system memory'", + ]); + cmd +} + pub fn zfs_list() -> Command { let mut cmd = std::process::Command::new(PFEXEC); cmd.env_clear() From 1c6386ec4704638c1b50b3c991d5bbfe26f9708b Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 6 May 2025 14:26:17 -0700 Subject: [PATCH 26/31] [support bundle] Collect bundles concurrently --- .../tasks/support_bundle_collector.rs | 300 ++++++++++-------- 1 file changed, 160 insertions(+), 140 deletions(-) diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index 3629ffce9ba..f17ad219a4d 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -568,153 +568,173 @@ impl BundleCollection<'_> { { report.listed_in_service_sleds = true; - // NOTE: This could be, and probably should be, done concurrently. - for sled in &all_sleds { - info!(&log, "Collecting bundle info from sled"; "sled" => %sled.id()); - let sled_path = dir - .path() - .join("rack") - .join(sled.rack_id.to_string()) - .join("sled") - .join(sled.id().to_string()); - tokio::fs::create_dir_all(&sled_path).await?; - tokio::fs::write( - sled_path.join("sled.txt"), - format!("{sled:?}"), - ) - .await?; + const MAX_CONCURRENT_SLED_REQUESTS: usize = 16; + let mut sled_stream = futures::stream::iter(all_sleds) + .map(|sled| async move { + self.collect_data_from_sled(&log, &sled, dir).await + }) + .buffer_unordered(MAX_CONCURRENT_SLED_REQUESTS); - if self.request.skip_sled_info { - continue; + while let Some(result) = sled_stream.next().await { + if let Err(err) = result { + warn!( + &self.log, + "Failed to fully collect support bundle info from sled"; + "err" => ?err + ); } + } + } - let Ok(sled_client) = nexus_networking::sled_client( - &self.collector.datastore, - &self.opctx, - sled.id(), - log, - ) - .await - else { - tokio::fs::write( - sled_path.join("error.txt"), - "Could not contact sled", - ) - .await?; - continue; - }; - - // NB: As new sled-diagnostic commands are added they should - // be added to this array so that their output can be saved - // within the support bundle. - let mut diag_cmds = futures::stream::iter([ - save_diag_cmd_output_or_error( - &sled_path, - "zoneadm", - sled_client.support_zoneadm_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "dladm", - sled_client.support_dladm_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "ipadm", - sled_client.support_ipadm_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "nvmeadm", - sled_client.support_nvmeadm_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "pargs", - sled_client.support_pargs_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "pfiles", - sled_client.support_pfiles_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "pstack", - sled_client.support_pstack_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "zfs", - sled_client.support_zfs_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "zpool", - sled_client.support_zpool_info(), - ) - .boxed(), - save_diag_cmd_output_or_error( - &sled_path, - "health-check", - sled_client.support_health_check(), - ) - .boxed(), - ]) - // Currently we execute up to 10 commands concurrently which - // might be doing their own concurrent work, for example - // collectiong `pstack` output of every Oxide process that is - // found on a sled. - .buffer_unordered(10); - - while let Some(result) = diag_cmds.next().await { - // Log that we failed to write the diag command output to a - // file but don't return early as we wish to get as much - // information as we can. - if let Err(e) = result { - error!( - &self.log, - "failed to write diagnostic command output to \ - file: {e}" - ); - } - } + Ok(report) + } - // For each zone we concurrently fire off a request to its - // sled-agent to collect its logs in a zip file and write the - // result to the support bundle. - let zones = sled_client.support_logs().await?.into_inner(); - let mut log_futs: FuturesUnordered<_> = zones - .iter() - .map(|zone| { - save_zone_log_zip_or_error( - log, - &sled_client, - zone, - &sled_path, - ) - }) - .collect(); - - while let Some(log_collection_result) = log_futs.next().await { - // We log any errors saving the zip file to disk and - // continue on. - if let Err(e) = log_collection_result { - error!(&self.log, "failed to write logs output: {e}"); - } - } + // Collect data from a sled, storing it into a directory that will + // be turned into a support bundle. + // + // - "sled" is the sled from which we should collect data. + // - "dir" is a directory where data can be stored, to be turned + // into a bundle after collection completes. + async fn collect_data_from_sled( + &self, + log: &slog::Logger, + sled: &nexus_db_model::Sled, + dir: &Utf8TempDir, + ) -> anyhow::Result<()> { + info!(&log, "Collecting bundle info from sled"; "sled" => %sled.id()); + let sled_path = dir + .path() + .join("rack") + .join(sled.rack_id.to_string()) + .join("sled") + .join(sled.id().to_string()); + tokio::fs::create_dir_all(&sled_path).await?; + tokio::fs::write(sled_path.join("sled.txt"), format!("{sled:?}")) + .await?; + + if self.request.skip_sled_info { + return Ok(()); + } + + let Ok(sled_client) = nexus_networking::sled_client( + &self.collector.datastore, + &self.opctx, + sled.id(), + log, + ) + .await + else { + tokio::fs::write( + sled_path.join("error.txt"), + "Could not contact sled", + ) + .await?; + return Ok(()); + }; + + // NB: As new sled-diagnostic commands are added they should + // be added to this array so that their output can be saved + // within the support bundle. + let mut diag_cmds = futures::stream::iter([ + save_diag_cmd_output_or_error( + &sled_path, + "zoneadm", + sled_client.support_zoneadm_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "dladm", + sled_client.support_dladm_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "ipadm", + sled_client.support_ipadm_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "nvmeadm", + sled_client.support_nvmeadm_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "pargs", + sled_client.support_pargs_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "pfiles", + sled_client.support_pfiles_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "pstack", + sled_client.support_pstack_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "zfs", + sled_client.support_zfs_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "zpool", + sled_client.support_zpool_info(), + ) + .boxed(), + save_diag_cmd_output_or_error( + &sled_path, + "health-check", + sled_client.support_health_check(), + ) + .boxed(), + ]) + // Currently we execute up to 10 commands concurrently which + // might be doing their own concurrent work, for example + // collectiong `pstack` output of every Oxide process that is + // found on a sled. + .buffer_unordered(10); + + while let Some(result) = diag_cmds.next().await { + // Log that we failed to write the diag command output to a + // file but don't return early as we wish to get as much + // information as we can. + if let Err(e) = result { + error!( + &self.log, + "failed to write diagnostic command output to \ + file: {e}" + ); } } - Ok(report) + // For each zone we concurrently fire off a request to its + // sled-agent to collect its logs in a zip file and write the + // result to the support bundle. + let zones = sled_client.support_logs().await?.into_inner(); + let mut log_futs: FuturesUnordered<_> = zones + .iter() + .map(|zone| { + save_zone_log_zip_or_error(log, &sled_client, zone, &sled_path) + }) + .collect(); + + while let Some(log_collection_result) = log_futs.next().await { + // We log any errors saving the zip file to disk and + // continue on. + if let Err(e) = log_collection_result { + error!(&self.log, "failed to write logs output: {e}"); + } + } + return Ok(()); } } From 12d00d89e9f19d7c609ca0ab71815f310f56f707 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 7 May 2025 12:04:33 -0700 Subject: [PATCH 27/31] feedback --- dev-tools/omdb/src/bin/omdb/nexus.rs | 2 +- .../src/bundle_accessor.rs | 27 +++------------ .../src/dashboard.rs | 34 +++++++++++++------ 3 files changed, 29 insertions(+), 34 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index ce7bbf8a034..70c8b360afb 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -501,7 +501,7 @@ enum SupportBundleCommands { GetIndex(SupportBundleIndexArgs), /// View a file within a support bundle GetFile(SupportBundleFileArgs), - /// Creates dashboard for viewing the contents of a support bundle + /// Creates a dashboard for viewing the contents of a support bundle Inspect(SupportBundleInspectArgs), } diff --git a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs index 757e9cb78c1..08b9e2cfe01 100644 --- a/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs +++ b/dev-tools/support-bundle-reader-lib/src/bundle_accessor.rs @@ -234,27 +234,10 @@ async fn utf8_stream_to_string( mut stream: impl futures::Stream> + std::marker::Unpin, ) -> Result { - let mut result = String::new(); - - // When we read from the string, we might not read a whole UTF-8 sequence. - // Keep this "leftover" type here to concatenate this partially-read data - // when we read the next sequence. - let mut leftover: Option = None; - while let Some(data) = stream.next().await { - match data { - Err(err) => return Err(anyhow::anyhow!(err)), - Ok(data) => { - let combined = match leftover.take() { - Some(old) => [old, data].concat(), - None => data.to_vec(), - }; - - match std::str::from_utf8(&combined) { - Ok(data) => result += data, - Err(_) => leftover = Some(combined.into()), - } - } - } + let mut bytes = Vec::new(); + while let Some(chunk) = stream.next().await { + let chunk = chunk?; + bytes.extend_from_slice(&chunk); } - Ok(result) + Ok(String::from_utf8(bytes)?) } diff --git a/dev-tools/support-bundle-reader-lib/src/dashboard.rs b/dev-tools/support-bundle-reader-lib/src/dashboard.rs index 775fbe5b75c..eb60d4cd7eb 100644 --- a/dev-tools/support-bundle-reader-lib/src/dashboard.rs +++ b/dev-tools/support-bundle-reader-lib/src/dashboard.rs @@ -15,14 +15,16 @@ use anyhow::bail; use bytes::BytesMut; use camino::Utf8Path; use camino::Utf8PathBuf; -use crossterm::{ - event::{self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode}, - execute, - terminal::{ - EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, - enable_raw_mode, - }, -}; +use crossterm::event; +use crossterm::event::DisableMouseCapture; +use crossterm::event::EnableMouseCapture; +use crossterm::event::Event; +use crossterm::event::KeyCode; +use crossterm::execute; +use crossterm::terminal::EnterAlternateScreen; +use crossterm::terminal::LeaveAlternateScreen; +use crossterm::terminal::disable_raw_mode; +use crossterm::terminal::enable_raw_mode; use futures::TryStreamExt; use nexus_client::types::SupportBundleInfo; use nexus_client::types::SupportBundleState; @@ -55,7 +57,7 @@ enum FileState<'a> { Closed, } -/// A dashboard for inspecting a support bundle contents +/// A dashboard for inspecting a support bundle's contents pub struct SupportBundleDashboard<'a> { access: Box, index: SupportBundleIndex, @@ -77,17 +79,27 @@ impl<'a> SupportBundleDashboard<'a> { } async fn select_up(&mut self, count: usize) -> Result<()> { + let old_selection = self.selected; self.selected = self.selected.saturating_sub(count); - if matches!(self.file, FileState::Open { .. }) { + + // Buffer the new file if we're currently viewing open files + if old_selection != self.selected + && matches!(self.file, FileState::Open { .. }) + { self.open_and_buffer().await?; } Ok(()) } async fn select_down(&mut self, count: usize) -> Result<()> { + let old_selection = self.selected; self.selected = std::cmp::min(self.selected + count, self.index.files().len() - 1); - if matches!(self.file, FileState::Open { .. }) { + + // Buffer the new file if we're currently viewing open files + if old_selection != self.selected + && matches!(self.file, FileState::Open { .. }) + { self.open_and_buffer().await?; } Ok(()) From b2522d1259a03f363c11fb0638c6ca83171dccf5 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 7 May 2025 12:40:20 -0700 Subject: [PATCH 28/31] feedback --- sled-diagnostics/src/lib.rs | 2 +- sled-diagnostics/src/queries.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sled-diagnostics/src/lib.rs b/sled-diagnostics/src/lib.rs index e3bc6b5e6c8..b61267af01e 100644 --- a/sled-diagnostics/src/lib.rs +++ b/sled-diagnostics/src/lib.rs @@ -152,7 +152,7 @@ pub async fn health_check() [ uptime(), kstat_low_page(), - svcs_show_disabled(), + svcs_enabled_but_not_running(), count_disks(), zfs_list_unmounted(), count_crucibles(), diff --git a/sled-diagnostics/src/queries.rs b/sled-diagnostics/src/queries.rs index 039c552acc3..0f249149376 100644 --- a/sled-diagnostics/src/queries.rs +++ b/sled-diagnostics/src/queries.rs @@ -278,7 +278,7 @@ pub fn kstat_low_page() -> Command { cmd } -pub fn svcs_show_disabled() -> Command { +pub fn svcs_enabled_but_not_running() -> Command { let mut cmd = std::process::Command::new(PFEXEC); cmd.env_clear().arg(SVCS).arg("-xZ"); cmd From 59e8750d8d79d9deecc21046b5869fff9118dc1c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 7 May 2025 16:08:15 -0700 Subject: [PATCH 29/31] Parallel collection --- .../tasks/support_bundle_collector.rs | 90 +++++++++++-------- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index f17ad219a4d..638335c314d 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -42,6 +42,7 @@ use std::sync::Arc; use tokio::io::AsyncReadExt; use tokio::io::AsyncSeekExt; use tokio::io::SeekFrom; +use tokio::task::JoinSet; use tufaceous_artifact::ArtifactHash; use zip::ZipArchive; use zip::ZipWriter; @@ -60,7 +61,7 @@ fn authz_support_bundle_from_id(id: SupportBundleUuid) -> authz::SupportBundle { } // Specifies the data to be collected within the Support Bundle. -#[derive(Default)] +#[derive(Clone, Default)] struct BundleRequest { // If "false": Skip collecting host-specific info from each sled. skip_sled_info: bool, @@ -373,13 +374,13 @@ impl SupportBundleCollector { } }; - let collection = BundleCollection { - collector: &self, + let collection = Arc::new(BundleCollection { + datastore: self.datastore.clone(), log: opctx.log.new(slog::o!("bundle" => bundle.id.to_string())), - opctx, - request, - bundle: &bundle, - }; + opctx: opctx.child(std::collections::BTreeMap::new()), + request: request.clone(), + bundle: bundle.clone(), + }); let authz_bundle = authz_support_bundle_from_id(bundle.id.into()); let mut report = collection.collect_bundle_and_store_on_sled().await?; @@ -416,20 +417,18 @@ impl SupportBundleCollector { } // Wraps up all arguments to perform a single support bundle collection -struct BundleCollection<'a> { - // The task responsible for this collection - collector: &'a SupportBundleCollector, - +struct BundleCollection { + datastore: Arc, log: slog::Logger, - opctx: &'a OpContext, - request: &'a BundleRequest, - bundle: &'a SupportBundle, + opctx: OpContext, + request: BundleRequest, + bundle: SupportBundle, } -impl BundleCollection<'_> { +impl BundleCollection { // Collect the bundle within Nexus, and store it on a target sled. async fn collect_bundle_and_store_on_sled( - &self, + self: &Arc, ) -> anyhow::Result { // Create a temporary directory where we'll store the support bundle // as it's being collected. @@ -456,7 +455,7 @@ impl BundleCollection<'_> { "bundle" => %self.bundle.id ); - let bundle = self.collector.datastore.support_bundle_get( + let bundle = self.datastore.support_bundle_get( &self.opctx, self.bundle.id.into() ).await?; @@ -491,7 +490,6 @@ impl BundleCollection<'_> { // Find the sled where we're storing this bundle. let sled_id = self - .collector .datastore .zpool_get_sled_if_in_service( &self.opctx, @@ -499,7 +497,7 @@ impl BundleCollection<'_> { ) .await?; let sled_client = nexus_networking::sled_client( - &self.collector.datastore, + &self.datastore, &self.opctx, sled_id.into_untyped_uuid(), &self.log, @@ -545,7 +543,7 @@ impl BundleCollection<'_> { // As a result, it is important that this function be implemented as // cancel-safe. async fn collect_bundle_as_file( - &self, + self: &Arc, dir: &Utf8TempDir, ) -> anyhow::Result { let log = &self.log; @@ -561,7 +559,6 @@ impl BundleCollection<'_> { .await?; if let Ok(all_sleds) = self - .collector .datastore .sled_list_all_batched(&self.opctx, SledFilter::InService) .await @@ -569,19 +566,39 @@ impl BundleCollection<'_> { report.listed_in_service_sleds = true; const MAX_CONCURRENT_SLED_REQUESTS: usize = 16; - let mut sled_stream = futures::stream::iter(all_sleds) - .map(|sled| async move { - self.collect_data_from_sled(&log, &sled, dir).await - }) - .buffer_unordered(MAX_CONCURRENT_SLED_REQUESTS); + let mut sleds_iter = all_sleds.into_iter().peekable(); + let mut tasks = JoinSet::new(); + + // While we have incoming work to send to tasks (sleds_iter) + // or a task operating on that data (tasks)... + while sleds_iter.peek().is_some() || !tasks.is_empty() { + // Spawn tasks up to the concurrency limit + while tasks.len() < MAX_CONCURRENT_SLED_REQUESTS { + if let Some(sled) = sleds_iter.next() { + let collection: Arc = self.clone(); + let dir = dir.path().to_path_buf(); + tasks.spawn({ + async move { + collection + .collect_data_from_sled(&sled, &dir) + .await + } + }); + } + } - while let Some(result) = sled_stream.next().await { - if let Err(err) = result { - warn!( - &self.log, - "Failed to fully collect support bundle info from sled"; - "err" => ?err - ); + // Await the completion of ongoing tasks. + // + // Keep collecting from other sleds, even if one or more of the + // sled collection tasks fail. + if let Some(result) = tasks.join_next().await { + if let Err(err) = result { + warn!( + &self.log, + "Failed to fully collect support bundle info from sled"; + "err" => ?err + ); + } } } } @@ -597,13 +614,12 @@ impl BundleCollection<'_> { // into a bundle after collection completes. async fn collect_data_from_sled( &self, - log: &slog::Logger, sled: &nexus_db_model::Sled, - dir: &Utf8TempDir, + dir: &Utf8Path, ) -> anyhow::Result<()> { + let log = &self.log; info!(&log, "Collecting bundle info from sled"; "sled" => %sled.id()); let sled_path = dir - .path() .join("rack") .join(sled.rack_id.to_string()) .join("sled") @@ -617,7 +633,7 @@ impl BundleCollection<'_> { } let Ok(sled_client) = nexus_networking::sled_client( - &self.collector.datastore, + &self.datastore, &self.opctx, sled.id(), log, From bbe9fcbe3a1c5460f0a38e1f75c752d0b54e6c95 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 8 May 2025 15:52:24 -0700 Subject: [PATCH 30/31] fix boolean logic --- nexus/src/app/background/tasks/support_bundle_collector.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nexus/src/app/background/tasks/support_bundle_collector.rs b/nexus/src/app/background/tasks/support_bundle_collector.rs index 638335c314d..ab61422332c 100644 --- a/nexus/src/app/background/tasks/support_bundle_collector.rs +++ b/nexus/src/app/background/tasks/support_bundle_collector.rs @@ -573,7 +573,9 @@ impl BundleCollection { // or a task operating on that data (tasks)... while sleds_iter.peek().is_some() || !tasks.is_empty() { // Spawn tasks up to the concurrency limit - while tasks.len() < MAX_CONCURRENT_SLED_REQUESTS { + while tasks.len() < MAX_CONCURRENT_SLED_REQUESTS + && sleds_iter.peek().is_some() + { if let Some(sled) = sleds_iter.next() { let collection: Arc = self.clone(); let dir = dir.path().to_path_buf(); From 5e3cf0492df56b662c037d29a8c0e089f9f8ee61 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 9 May 2025 10:49:50 -0700 Subject: [PATCH 31/31] avoid linebreaks --- sled-diagnostics/src/queries.rs | 38 ++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/sled-diagnostics/src/queries.rs b/sled-diagnostics/src/queries.rs index 0f249149376..c1452abbc4d 100644 --- a/sled-diagnostics/src/queries.rs +++ b/sled-diagnostics/src/queries.rs @@ -288,8 +288,8 @@ pub fn count_disks() -> Command { let mut cmd = std::process::Command::new("bash"); cmd.env_clear().args([ "-c", - "(pfexec diskinfo -pH | tee | wc -l | xargs | grep -x '12' > /dev/null) - && echo 'OK: All expected disks found' + "(pfexec diskinfo -pH | tee | wc -l | xargs | grep -x '12' > /dev/null) \ + && echo 'OK: All expected disks found' \ || echo 'WARN: Unexpected number of physical disks (expected 12)'", ]); cmd @@ -299,8 +299,8 @@ pub fn zfs_list_unmounted() -> Command { let mut cmd = std::process::Command::new("bash"); cmd.env_clear().args([ "-c", - "pfexec zfs list -r -o name,mounted | grep oxp | grep -v yes$ - && echo 'WARN: Found unmounted dataset(s)' + "pfexec zfs list -r -o name,mounted | grep oxp | grep -v yes$ \ + && echo 'WARN: Found unmounted dataset(s)' \ || echo 'OK: No unmounted datasets'", ]); cmd @@ -311,8 +311,8 @@ pub fn count_crucibles() -> Command { cmd.env_clear() .args([ "-c", - "(zoneadm list | grep crucible | grep -v pantry | tee | wc -l | xargs | grep -x '10' > /dev/null) - && echo 'OK: 10 Crucibles found' + "(zoneadm list | grep crucible | grep -v pantry | tee | wc -l | xargs | grep -x '10' > /dev/null) \ + && echo 'OK: 10 Crucibles found' \ || echo 'WARN: Unexpected number of crucible zones (expected 10)'" ]); cmd @@ -323,11 +323,11 @@ pub fn identify_datasets_close_to_quota() -> Command { cmd.env_clear() .args([ "-c", - "zfs list -Hp -o used,quota,name,avail,mountpoint | - egrep 'oxp|oxi' | - egrep -v 'none|crucible' | - awk '$2 > 0 && $1 / $2 >= 0.8 { any=1; print } END { exit !any }' - && echo 'WARN: Found near-quota datasets' + "zfs list -Hp -o used,quota,name,avail,mountpoint | \ + egrep 'oxp|oxi' | \ + egrep -v 'none|crucible' | \ + awk '$2 > 0 && $1 / $2 >= 0.8 { any=1; print } END { exit !any }' \ + && echo 'WARN: Found near-quota datasets' \ || echo 'OK: No near-quota datasets found'" ]); cmd @@ -337,11 +337,11 @@ pub fn identify_datasets_with_less_than_300_gib_avail() -> Command { let mut cmd = std::process::Command::new("bash"); cmd.env_clear().args([ "-c", - "zfs list -Hp -o used,quota,name,avail,mountpoint | - egrep 'oxp|oxi' | - egrep -v 'none|crucible' | - awk '$4 < (300 * (1024^3)) { any=1; print } END { exit !any }' - && echo 'WARN: Found low-space datasets' + "zfs list -Hp -o used,quota,name,avail,mountpoint | \ + egrep 'oxp|oxi' | \ + egrep -v 'none|crucible' | \ + awk '$4 < (300 * (1024^3)) { any=1; print } END { exit !any }' \ + && echo 'WARN: Found low-space datasets' \ || echo 'OK: No low-space datasets found'", ]); cmd @@ -351,9 +351,9 @@ pub fn dimm_check() -> Command { let mut cmd = std::process::Command::new("bash"); cmd.env_clear().args([ "-c", - "prtconf -m | - grep -v -e 1036271 -e 2084847 - && echo 'WARN: Unexpected quantity of system memory' + "prtconf -m | \ + grep -v -e 1036271 -e 2084847 \ + && echo 'WARN: Unexpected quantity of system memory' \ || echo 'OK: Found expected quantity of system memory'", ]); cmd