Skip to content

Commit 53aea85

Browse files
smkleinsmklein
authored
Support Bundles: Add user comment support (#8582)
Adds a text field for support bundles, letting users supply string labels as comments Fixes #8550
1 parent 565e59d commit 53aea85

File tree

21 files changed

+588
-31
lines changed

21 files changed

+588
-31
lines changed

‎dev-tools/omdb/src/bin/omdb/nexus.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4009,6 +4009,7 @@ async fn cmd_nexus_support_bundles_list(
40094009
reason_for_creation: String,
40104010
reason_for_failure: String,
40114011
state: String,
4012+
user_comment: String,
40124013
}
40134014
let rows = support_bundles.into_iter().map(|sb| SupportBundleInfo {
40144015
id: *sb.id,
@@ -4018,6 +4019,7 @@ async fn cmd_nexus_support_bundles_list(
40184019
.reason_for_failure
40194020
.unwrap_or_else(|| "-".to_string()),
40204021
state: format!("{:?}", sb.state),
4022+
user_comment: sb.user_comment.unwrap_or_else(|| "-".to_string()),
40214023
});
40224024
let table = tabled::Table::new(rows)
40234025
.with(tabled::settings::Style::empty())
@@ -4033,7 +4035,9 @@ async fn cmd_nexus_support_bundles_create(
40334035
_destruction_token: DestructiveOperationToken,
40344036
) -> Result<(), anyhow::Error> {
40354037
let support_bundle_id = client
4036-
.support_bundle_create()
4038+
.support_bundle_create(&nexus_client::types::SupportBundleCreate {
4039+
user_comment: None,
4040+
})
40374041
.await
40384042
.context("creating support bundle")?
40394043
.into_inner()

‎nexus/db-model/src/schema_versions.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
1616
///
1717
/// This must be updated when you change the database schema. Refer to
1818
/// schema/crdb/README.adoc in the root of this repository for details.
19-
pub const SCHEMA_VERSION: Version = Version::new(165, 0, 0);
19+
pub const SCHEMA_VERSION: Version = Version::new(166, 0, 0);
2020

2121
/// List of all past database schema versions, in *reverse* order
2222
///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
2828
// | leaving the first copy as an example for the next person.
2929
// v
3030
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
31+
KnownVersion::new(166, "bundle-user-comment"),
3132
KnownVersion::new(165, "route-config-rib-priority"),
3233
KnownVersion::new(164, "fix-leaked-bp-oximeter-read-policy-rows"),
3334
KnownVersion::new(163, "bp-desired-host-phase-2"),

‎nexus/db-model/src/support_bundle.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ pub struct SupportBundle {
9595
pub zpool_id: DbTypedUuid<ZpoolKind>,
9696
pub dataset_id: DbTypedUuid<DatasetKind>,
9797
pub assigned_nexus: Option<DbTypedUuid<OmicronZoneKind>>,
98+
pub user_comment: Option<String>,
9899
}
99100

100101
impl SupportBundle {
@@ -103,6 +104,7 @@ impl SupportBundle {
103104
zpool_id: ZpoolUuid,
104105
dataset_id: DatasetUuid,
105106
nexus_id: OmicronZoneUuid,
107+
user_comment: Option<String>,
106108
) -> Self {
107109
Self {
108110
id: SupportBundleUuid::new_v4().into(),
@@ -113,6 +115,7 @@ impl SupportBundle {
113115
zpool_id: zpool_id.into(),
114116
dataset_id: dataset_id.into(),
115117
assigned_nexus: Some(nexus_id.into()),
118+
user_comment,
116119
}
117120
}
118121

@@ -128,6 +131,7 @@ impl From<SupportBundle> for SupportBundleView {
128131
time_created: bundle.time_created,
129132
reason_for_creation: bundle.reason_for_creation,
130133
reason_for_failure: bundle.reason_for_failure,
134+
user_comment: bundle.user_comment,
131135
state: bundle.state.into(),
132136
}
133137
}

‎nexus/db-queries/src/db/datastore/support_bundle.rs‎

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ impl DataStore {
7676
opctx: &OpContext,
7777
reason_for_creation: &'static str,
7878
this_nexus_id: OmicronZoneUuid,
79+
user_comment: Option<String>,
7980
) -> CreateResult<SupportBundle> {
8081
opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;
8182
let conn = self.pool_connection_authorized(opctx).await?;
@@ -89,6 +90,7 @@ impl DataStore {
8990
self.transaction_retry_wrapper("support_bundle_create")
9091
.transaction(&conn, |conn| {
9192
let err = err.clone();
93+
let user_comment = user_comment.clone();
9294

9395
async move {
9496
use nexus_db_schema::schema::rendezvous_debug_dataset::dsl as dataset_dsl;
@@ -129,6 +131,7 @@ impl DataStore {
129131
dataset.pool_id(),
130132
dataset.id(),
131133
this_nexus_id,
134+
user_comment,
132135
);
133136

134137
diesel::insert_into(support_bundle_dsl::support_bundle)
@@ -438,6 +441,42 @@ impl DataStore {
438441
}
439442
}
440443

444+
/// Updates the user comment of a support bundle.
445+
///
446+
/// Returns:
447+
/// - "Ok" if the bundle was updated successfully.
448+
/// - "Err::InvalidRequest" if the comment exceeds the maximum length.
449+
pub async fn support_bundle_update_user_comment(
450+
&self,
451+
opctx: &OpContext,
452+
authz_bundle: &authz::SupportBundle,
453+
user_comment: Option<String>,
454+
) -> Result<(), Error> {
455+
opctx.authorize(authz::Action::Modify, authz_bundle).await?;
456+
457+
if let Some(ref comment) = user_comment {
458+
if comment.len() > 4096 {
459+
return Err(Error::invalid_request(
460+
"User comment cannot exceed 4096 bytes",
461+
));
462+
}
463+
}
464+
465+
use nexus_db_schema::schema::support_bundle::dsl;
466+
467+
let id = authz_bundle.id().into_untyped_uuid();
468+
let conn = self.pool_connection_authorized(opctx).await?;
469+
diesel::update(dsl::support_bundle)
470+
.filter(dsl::id.eq(id))
471+
.set(dsl::user_comment.eq(user_comment))
472+
.execute_async(&*conn)
473+
.await
474+
.map(|_rows_modified| ())
475+
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?;
476+
477+
Ok(())
478+
}
479+
441480
/// Deletes a support bundle.
442481
///
443482
/// This should only be invoked after all storage for the support bundle has
@@ -628,7 +667,7 @@ mod test {
628667
this_nexus_id: OmicronZoneUuid,
629668
) {
630669
let err = datastore
631-
.support_bundle_create(&opctx, "for tests", this_nexus_id)
670+
.support_bundle_create(&opctx, "for tests", this_nexus_id, None)
632671
.await
633672
.expect_err("Shouldn't provision bundle without datasets");
634673
let Error::InsufficientCapacity { message } = err else {
@@ -674,15 +713,15 @@ mod test {
674713
// Create two bundles on "nexus A", one bundle on "nexus B"
675714

676715
let bundle_a1 = datastore
677-
.support_bundle_create(&opctx, "for the test", nexus_a)
716+
.support_bundle_create(&opctx, "for the test", nexus_a, None)
678717
.await
679718
.expect("Should be able to create bundle");
680719
let bundle_a2 = datastore
681-
.support_bundle_create(&opctx, "for the test", nexus_a)
720+
.support_bundle_create(&opctx, "for the test", nexus_a, None)
682721
.await
683722
.expect("Should be able to create bundle");
684723
let bundle_b1 = datastore
685-
.support_bundle_create(&opctx, "for the test", nexus_b)
724+
.support_bundle_create(&opctx, "for the test", nexus_b, None)
686725
.await
687726
.expect("Should be able to create bundle");
688727

@@ -800,6 +839,7 @@ mod test {
800839
&opctx,
801840
"for the test",
802841
this_nexus_id,
842+
None,
803843
)
804844
.await
805845
.expect("Should be able to create bundle"),
@@ -846,7 +886,7 @@ mod test {
846886
.await
847887
.expect("Should be able to destroy this bundle");
848888
datastore
849-
.support_bundle_create(&opctx, "for the test", this_nexus_id)
889+
.support_bundle_create(&opctx, "for the test", this_nexus_id, None)
850890
.await
851891
.expect("Should be able to create bundle");
852892

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

869909
let mut bundle = datastore
870-
.support_bundle_create(&opctx, reason, this_nexus_id)
910+
.support_bundle_create(&opctx, reason, this_nexus_id, None)
871911
.await
872912
.expect("Should be able to create bundle");
873913
assert_eq!(bundle.reason_for_creation, reason);
@@ -1031,7 +1071,7 @@ mod test {
10311071
// When we create a bundle, it should exist on a dataset provisioned by
10321072
// the blueprint.
10331073
let bundle = datastore
1034-
.support_bundle_create(&opctx, "for the test", this_nexus_id)
1074+
.support_bundle_create(&opctx, "for the test", this_nexus_id, None)
10351075
.await
10361076
.expect("Should be able to create bundle");
10371077
assert_eq!(bundle.assigned_nexus, Some(this_nexus_id.into()));
@@ -1136,7 +1176,7 @@ mod test {
11361176
// When we create a bundle, it should exist on a dataset provisioned by
11371177
// the blueprint.
11381178
let bundle = datastore
1139-
.support_bundle_create(&opctx, "for the test", this_nexus_id)
1179+
.support_bundle_create(&opctx, "for the test", this_nexus_id, None)
11401180
.await
11411181
.expect("Should be able to create bundle");
11421182
assert_eq!(bundle.assigned_nexus, Some(this_nexus_id.into()));
@@ -1237,7 +1277,7 @@ mod test {
12371277
// When we create a bundle, it should exist on a dataset provisioned by
12381278
// the blueprint.
12391279
let bundle = datastore
1240-
.support_bundle_create(&opctx, "for the test", nexus_ids[0])
1280+
.support_bundle_create(&opctx, "for the test", nexus_ids[0], None)
12411281
.await
12421282
.expect("Should be able to create bundle");
12431283

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

@@ -1442,6 +1482,7 @@ mod test {
14421482
&opctx,
14431483
"Bundle for time ordering test",
14441484
this_nexus_id,
1485+
None,
14451486
)
14461487
.await
14471488
.expect("Should be able to create bundle");

‎nexus/db-schema/src/schema.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,6 +1470,7 @@ table! {
14701470
dataset_id -> Uuid,
14711471

14721472
assigned_nexus -> Nullable<Uuid>,
1473+
user_comment -> Nullable<Text>,
14731474
}
14741475
}
14751476

‎nexus/external-api/output/nexus_tags.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ support_bundle_head HEAD /experimental/v1/system/suppor
6262
support_bundle_head_file HEAD /experimental/v1/system/support-bundles/{bundle_id}/download/{file}
6363
support_bundle_index GET /experimental/v1/system/support-bundles/{bundle_id}/index
6464
support_bundle_list GET /experimental/v1/system/support-bundles
65+
support_bundle_update PUT /experimental/v1/system/support-bundles/{bundle_id}
6566
support_bundle_view GET /experimental/v1/system/support-bundles/{bundle_id}
6667
system_update_get_repository GET /v1/system/update/repository/{system_version}
6768
system_update_put_repository PUT /v1/system/update/repository

‎nexus/external-api/src/lib.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3368,6 +3368,7 @@ pub trait NexusExternalApi {
33683368
}]
33693369
async fn support_bundle_create(
33703370
rqctx: RequestContext<Self::Context>,
3371+
body: TypedBody<params::SupportBundleCreate>,
33713372
) -> Result<HttpResponseCreated<shared::SupportBundleInfo>, HttpError>;
33723373

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

3388+
/// Update a support bundle
3389+
#[endpoint {
3390+
method = PUT,
3391+
path = "/experimental/v1/system/support-bundles/{bundle_id}",
3392+
tags = ["experimental"], // system/support-bundles: only one tag is allowed
3393+
}]
3394+
async fn support_bundle_update(
3395+
rqctx: RequestContext<Self::Context>,
3396+
path_params: Path<params::SupportBundlePath>,
3397+
body: TypedBody<params::SupportBundleUpdate>,
3398+
) -> Result<HttpResponseOk<shared::SupportBundleInfo>, HttpError>;
3399+
33873400
// Probes (experimental)
33883401

33893402
/// List instrumentation probes

‎nexus/internal-api/src/lib.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,7 @@ pub trait NexusInternalApi {
665665
}]
666666
async fn support_bundle_create(
667667
rqctx: RequestContext<Self::Context>,
668+
body: TypedBody<params::SupportBundleCreate>,
668669
) -> Result<HttpResponseCreated<shared::SupportBundleInfo>, HttpError>;
669670

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

684+
/// Update a support bundle
685+
#[endpoint {
686+
method = PUT,
687+
path = "/experimental/v1/system/support-bundles/{bundle_id}",
688+
}]
689+
async fn support_bundle_update(
690+
rqctx: RequestContext<Self::Context>,
691+
path_params: Path<params::SupportBundlePath>,
692+
body: TypedBody<params::SupportBundleUpdate>,
693+
) -> Result<HttpResponseOk<shared::SupportBundleInfo>, HttpError>;
694+
683695
/// Get all the probes associated with a given sled.
684696
#[endpoint {
685697
method = GET,

0 commit comments

Comments
 (0)