Skip to content

Support Bundles: Add user comment support #8582

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion dev-tools/omdb/src/bin/omdb/nexus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4009,6 +4009,7 @@ async fn cmd_nexus_support_bundles_list(
reason_for_creation: String,
reason_for_failure: String,
state: String,
user_comment: String,
}
let rows = support_bundles.into_iter().map(|sb| SupportBundleInfo {
id: *sb.id,
Expand All @@ -4018,6 +4019,7 @@ async fn cmd_nexus_support_bundles_list(
.reason_for_failure
.unwrap_or_else(|| "-".to_string()),
state: format!("{:?}", sb.state),
user_comment: sb.user_comment.unwrap_or_else(|| "-".to_string()),
});
let table = tabled::Table::new(rows)
.with(tabled::settings::Style::empty())
Expand All @@ -4033,7 +4035,9 @@ async fn cmd_nexus_support_bundles_create(
_destruction_token: DestructiveOperationToken,
) -> Result<(), anyhow::Error> {
let support_bundle_id = client
.support_bundle_create()
.support_bundle_create(&nexus_client::types::SupportBundleCreate {
user_comment: None,
})
.await
.context("creating support bundle")?
.into_inner()
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-model/src/schema_versions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
///
/// This must be updated when you change the database schema. Refer to
/// schema/crdb/README.adoc in the root of this repository for details.
pub const SCHEMA_VERSION: Version = Version::new(165, 0, 0);
pub const SCHEMA_VERSION: Version = Version::new(166, 0, 0);

/// List of all past database schema versions, in *reverse* order
///
Expand All @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
// | leaving the first copy as an example for the next person.
// v
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
KnownVersion::new(166, "bundle-user-comment"),
KnownVersion::new(165, "route-config-rib-priority"),
KnownVersion::new(164, "fix-leaked-bp-oximeter-read-policy-rows"),
KnownVersion::new(163, "bp-desired-host-phase-2"),
Expand Down
4 changes: 4 additions & 0 deletions nexus/db-model/src/support_bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ pub struct SupportBundle {
pub zpool_id: DbTypedUuid<ZpoolKind>,
pub dataset_id: DbTypedUuid<DatasetKind>,
pub assigned_nexus: Option<DbTypedUuid<OmicronZoneKind>>,
pub user_comment: Option<String>,
}

impl SupportBundle {
Expand All @@ -103,6 +104,7 @@ impl SupportBundle {
zpool_id: ZpoolUuid,
dataset_id: DatasetUuid,
nexus_id: OmicronZoneUuid,
user_comment: Option<String>,
) -> Self {
Self {
id: SupportBundleUuid::new_v4().into(),
Expand All @@ -113,6 +115,7 @@ impl SupportBundle {
zpool_id: zpool_id.into(),
dataset_id: dataset_id.into(),
assigned_nexus: Some(nexus_id.into()),
user_comment,
}
}

Expand All @@ -128,6 +131,7 @@ impl From<SupportBundle> for SupportBundleView {
time_created: bundle.time_created,
reason_for_creation: bundle.reason_for_creation,
reason_for_failure: bundle.reason_for_failure,
user_comment: bundle.user_comment,
state: bundle.state.into(),
}
}
Expand Down
61 changes: 51 additions & 10 deletions nexus/db-queries/src/db/datastore/support_bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ impl DataStore {
opctx: &OpContext,
reason_for_creation: &'static str,
this_nexus_id: OmicronZoneUuid,
user_comment: Option<String>,
) -> CreateResult<SupportBundle> {
opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;
let conn = self.pool_connection_authorized(opctx).await?;
Expand All @@ -89,6 +90,7 @@ impl DataStore {
self.transaction_retry_wrapper("support_bundle_create")
.transaction(&conn, |conn| {
let err = err.clone();
let user_comment = user_comment.clone();

async move {
use nexus_db_schema::schema::rendezvous_debug_dataset::dsl as dataset_dsl;
Expand Down Expand Up @@ -129,6 +131,7 @@ impl DataStore {
dataset.pool_id(),
dataset.id(),
this_nexus_id,
user_comment,
);

diesel::insert_into(support_bundle_dsl::support_bundle)
Expand Down Expand Up @@ -438,6 +441,42 @@ impl DataStore {
}
}

/// Updates the user comment of a support bundle.
///
/// Returns:
/// - "Ok" if the bundle was updated successfully.
/// - "Err::InvalidRequest" if the comment exceeds the maximum length.
pub async fn support_bundle_update_user_comment(
&self,
opctx: &OpContext,
authz_bundle: &authz::SupportBundle,
user_comment: Option<String>,
) -> Result<(), Error> {
opctx.authorize(authz::Action::Modify, authz_bundle).await?;

if let Some(ref comment) = user_comment {
if comment.len() > 4096 {
return Err(Error::invalid_request(
"User comment cannot exceed 4096 bytes",
));
}
}

use nexus_db_schema::schema::support_bundle::dsl;

let id = authz_bundle.id().into_untyped_uuid();
let conn = self.pool_connection_authorized(opctx).await?;
diesel::update(dsl::support_bundle)
.filter(dsl::id.eq(id))
.set(dsl::user_comment.eq(user_comment))
.execute_async(&*conn)
.await
.map(|_rows_modified| ())
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?;

Ok(())
}

/// Deletes a support bundle.
///
/// This should only be invoked after all storage for the support bundle has
Expand Down Expand Up @@ -628,7 +667,7 @@ mod test {
this_nexus_id: OmicronZoneUuid,
) {
let err = datastore
.support_bundle_create(&opctx, "for tests", this_nexus_id)
.support_bundle_create(&opctx, "for tests", this_nexus_id, None)
.await
.expect_err("Shouldn't provision bundle without datasets");
let Error::InsufficientCapacity { message } = err else {
Expand Down Expand Up @@ -674,15 +713,15 @@ mod test {
// Create two bundles on "nexus A", one bundle on "nexus B"

let bundle_a1 = datastore
.support_bundle_create(&opctx, "for the test", nexus_a)
.support_bundle_create(&opctx, "for the test", nexus_a, None)
.await
.expect("Should be able to create bundle");
let bundle_a2 = datastore
.support_bundle_create(&opctx, "for the test", nexus_a)
.support_bundle_create(&opctx, "for the test", nexus_a, None)
.await
.expect("Should be able to create bundle");
let bundle_b1 = datastore
.support_bundle_create(&opctx, "for the test", nexus_b)
.support_bundle_create(&opctx, "for the test", nexus_b, None)
.await
.expect("Should be able to create bundle");

Expand Down Expand Up @@ -800,6 +839,7 @@ mod test {
&opctx,
"for the test",
this_nexus_id,
None,
)
.await
.expect("Should be able to create bundle"),
Expand Down Expand Up @@ -846,7 +886,7 @@ mod test {
.await
.expect("Should be able to destroy this bundle");
datastore
.support_bundle_create(&opctx, "for the test", this_nexus_id)
.support_bundle_create(&opctx, "for the test", this_nexus_id, None)
.await
.expect("Should be able to create bundle");

Expand All @@ -867,7 +907,7 @@ mod test {
// Create the bundle, then observe it through the "getter" APIs

let mut bundle = datastore
.support_bundle_create(&opctx, reason, this_nexus_id)
.support_bundle_create(&opctx, reason, this_nexus_id, None)
.await
.expect("Should be able to create bundle");
assert_eq!(bundle.reason_for_creation, reason);
Expand Down Expand Up @@ -1031,7 +1071,7 @@ mod test {
// When we create a bundle, it should exist on a dataset provisioned by
// the blueprint.
let bundle = datastore
.support_bundle_create(&opctx, "for the test", this_nexus_id)
.support_bundle_create(&opctx, "for the test", this_nexus_id, None)
.await
.expect("Should be able to create bundle");
assert_eq!(bundle.assigned_nexus, Some(this_nexus_id.into()));
Expand Down Expand Up @@ -1136,7 +1176,7 @@ mod test {
// When we create a bundle, it should exist on a dataset provisioned by
// the blueprint.
let bundle = datastore
.support_bundle_create(&opctx, "for the test", this_nexus_id)
.support_bundle_create(&opctx, "for the test", this_nexus_id, None)
.await
.expect("Should be able to create bundle");
assert_eq!(bundle.assigned_nexus, Some(this_nexus_id.into()));
Expand Down Expand Up @@ -1237,7 +1277,7 @@ mod test {
// When we create a bundle, it should exist on a dataset provisioned by
// the blueprint.
let bundle = datastore
.support_bundle_create(&opctx, "for the test", nexus_ids[0])
.support_bundle_create(&opctx, "for the test", nexus_ids[0], None)
.await
.expect("Should be able to create bundle");

Expand Down Expand Up @@ -1358,7 +1398,7 @@ mod test {
// When we create a bundle, it should exist on a dataset provisioned by
// the blueprint.
let bundle = datastore
.support_bundle_create(&opctx, "for the test", nexus_ids[0])
.support_bundle_create(&opctx, "for the test", nexus_ids[0], None)
.await
.expect("Should be able to create bundle");

Expand Down Expand Up @@ -1442,6 +1482,7 @@ mod test {
&opctx,
"Bundle for time ordering test",
this_nexus_id,
None,
)
.await
.expect("Should be able to create bundle");
Expand Down
1 change: 1 addition & 0 deletions nexus/db-schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1470,6 +1470,7 @@ table! {
dataset_id -> Uuid,

assigned_nexus -> Nullable<Uuid>,
user_comment -> Nullable<Text>,
}
}

Expand Down
1 change: 1 addition & 0 deletions nexus/external-api/output/nexus_tags.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ support_bundle_head HEAD /experimental/v1/system/suppor
support_bundle_head_file HEAD /experimental/v1/system/support-bundles/{bundle_id}/download/{file}
support_bundle_index GET /experimental/v1/system/support-bundles/{bundle_id}/index
support_bundle_list GET /experimental/v1/system/support-bundles
support_bundle_update PUT /experimental/v1/system/support-bundles/{bundle_id}
support_bundle_view GET /experimental/v1/system/support-bundles/{bundle_id}
system_update_get_repository GET /v1/system/update/repository/{system_version}
system_update_put_repository PUT /v1/system/update/repository
Expand Down
13 changes: 13 additions & 0 deletions nexus/external-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3368,6 +3368,7 @@ pub trait NexusExternalApi {
}]
async fn support_bundle_create(
rqctx: RequestContext<Self::Context>,
body: TypedBody<params::SupportBundleCreate>,
) -> Result<HttpResponseCreated<shared::SupportBundleInfo>, HttpError>;

/// Delete an existing support bundle
Expand All @@ -3384,6 +3385,18 @@ pub trait NexusExternalApi {
path_params: Path<params::SupportBundlePath>,
) -> Result<HttpResponseDeleted, HttpError>;

/// Update a support bundle
#[endpoint {
method = PUT,
path = "/experimental/v1/system/support-bundles/{bundle_id}",
tags = ["experimental"], // system/support-bundles: only one tag is allowed
}]
async fn support_bundle_update(
rqctx: RequestContext<Self::Context>,
path_params: Path<params::SupportBundlePath>,
body: TypedBody<params::SupportBundleUpdate>,
) -> Result<HttpResponseOk<shared::SupportBundleInfo>, HttpError>;

// Probes (experimental)

/// List instrumentation probes
Expand Down
12 changes: 12 additions & 0 deletions nexus/internal-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,7 @@ pub trait NexusInternalApi {
}]
async fn support_bundle_create(
rqctx: RequestContext<Self::Context>,
body: TypedBody<params::SupportBundleCreate>,
) -> Result<HttpResponseCreated<shared::SupportBundleInfo>, HttpError>;

/// Delete an existing support bundle
Expand All @@ -680,6 +681,17 @@ pub trait NexusInternalApi {
path_params: Path<params::SupportBundlePath>,
) -> Result<HttpResponseDeleted, HttpError>;

/// Update a support bundle
#[endpoint {
method = PUT,
path = "/experimental/v1/system/support-bundles/{bundle_id}",
}]
async fn support_bundle_update(
rqctx: RequestContext<Self::Context>,
path_params: Path<params::SupportBundlePath>,
body: TypedBody<params::SupportBundleUpdate>,
) -> Result<HttpResponseOk<shared::SupportBundleInfo>, HttpError>;

/// Get all the probes associated with a given sled.
#[endpoint {
method = GET,
Expand Down
Loading
Loading