Skip to content

Commit bccd77c

Browse files
mergeconflictmergeconflict
committed
EreportFilters: make fields private, add builder API
Add builder methods to BundleDataSelection (with_reconfigurator, with_sled_cubby_info, with_sp_dumps, with_all_sleds, with_specific_sleds, with_ereports) and EreportFilters (with_start_time, with_end_time returning Result with validation, with_serials, with_classes taking IntoIterator). Time range validation is now enforced eagerly by the builder, removing the need for check_time_range(). Addresses review feedback from PR #10090 and PR #10089.
1 parent 72b5433 commit bccd77c

5 files changed

Lines changed: 224 additions & 145 deletions

File tree

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

Lines changed: 27 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ impl DataStore {
9595
pagparams: &DataPageParams<'_, (Uuid, DbEna)>,
9696
) -> ListResultVec<Ereport> {
9797
opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?;
98-
filters.check_time_range()?;
9998

10099
let query = Self::ereport_fetch_matching_query(filters, pagparams);
101100
query
@@ -116,23 +115,23 @@ impl DataStore {
116115
.filter(dsl::time_deleted.is_null())
117116
.select(Ereport::as_select());
118117

119-
if let Some(start) = filters.start_time {
118+
if let Some(start) = filters.start_time() {
120119
query = query.filter(dsl::time_collected.ge(start));
121120
}
122121

123-
if let Some(end) = filters.end_time {
122+
if let Some(end) = filters.end_time() {
124123
query = query.filter(dsl::time_collected.le(end));
125124
}
126125

127-
if !filters.only_serials.is_empty() {
126+
if !filters.only_serials().is_empty() {
128127
query = query.filter(
129-
dsl::serial_number.eq_any(filters.only_serials.clone()),
128+
dsl::serial_number.eq_any(filters.only_serials().to_vec()),
130129
);
131130
}
132131

133-
if !filters.only_classes.is_empty() {
134-
query =
135-
query.filter(dsl::class.eq_any(filters.only_classes.clone()));
132+
if !filters.only_classes().is_empty() {
133+
query = query
134+
.filter(dsl::class.eq_any(filters.only_classes().to_vec()));
136135
}
137136

138137
query
@@ -419,13 +418,7 @@ mod tests {
419418
async fn explain_ereport_fetch_matching_only_serials() {
420419
explain_fetch_matching_query(
421420
"explain_ereport_fetch_matching_only_serials",
422-
EreportFilters {
423-
only_serials: vec![
424-
"BRM6900420".to_string(),
425-
"BRM5555555".to_string(),
426-
],
427-
..Default::default()
428-
},
421+
EreportFilters::new().with_serials(["BRM6900420", "BRM5555555"]),
429422
)
430423
.await
431424
}
@@ -434,17 +427,12 @@ mod tests {
434427
async fn explain_ereport_fetch_matching_serials_and_classes() {
435428
explain_fetch_matching_query(
436429
"explain_ereport_fetch_matching_serials_and_classes",
437-
EreportFilters {
438-
only_serials: vec![
439-
"BRM6900420".to_string(),
440-
"BRM5555555".to_string(),
441-
],
442-
only_classes: vec![
443-
"my.cool.ereport".to_string(),
444-
"hw.frobulator.fault.frobulation_failed".to_string(),
445-
],
446-
..Default::default()
447-
},
430+
EreportFilters::new()
431+
.with_serials(["BRM6900420", "BRM5555555"])
432+
.with_classes([
433+
"my.cool.ereport",
434+
"hw.frobulator.fault.frobulation_failed",
435+
]),
448436
)
449437
.await
450438
}
@@ -453,10 +441,9 @@ mod tests {
453441
async fn explain_ereport_fetch_matching_only_time() {
454442
explain_fetch_matching_query(
455443
"explain_ereport_fetch_matching_only_time",
456-
EreportFilters {
457-
end_time: Some(chrono::Utc::now()),
458-
..Default::default()
459-
},
444+
EreportFilters::new()
445+
.with_end_time(chrono::Utc::now())
446+
.expect("no start time set"),
460447
)
461448
.await
462449
}
@@ -465,14 +452,10 @@ mod tests {
465452
async fn explain_ereport_fetch_matching_time_and_serials() {
466453
explain_fetch_matching_query(
467454
"explain_ereport_fetch_matching_only_time",
468-
EreportFilters {
469-
only_serials: vec![
470-
"BRM6900420".to_string(),
471-
"BRM5555555".to_string(),
472-
],
473-
end_time: Some(chrono::Utc::now()),
474-
..Default::default()
475-
},
455+
EreportFilters::new()
456+
.with_serials(["BRM6900420", "BRM5555555"])
457+
.with_end_time(chrono::Utc::now())
458+
.expect("no start time set"),
476459
)
477460
.await
478461
}
@@ -585,12 +568,11 @@ mod tests {
585568
let found_by_time_range = datastore
586569
.ereport_fetch_matching(
587570
opctx,
588-
&EreportFilters {
589-
start_time: Some(
571+
&EreportFilters::new()
572+
.with_start_time(
590573
ereport.time_collected - Duration::from_secs(600),
591-
),
592-
..Default::default()
593-
},
574+
)
575+
.expect("no end time set"),
594576
&pagparams,
595577
)
596578
.await
@@ -600,10 +582,7 @@ mod tests {
600582
let found_by_serial = datastore
601583
.ereport_fetch_matching(
602584
opctx,
603-
&EreportFilters {
604-
only_serials: vec!["my cool serial".to_string()],
605-
..Default::default()
606-
},
585+
&EreportFilters::new().with_serials(["my cool serial"]),
607586
&pagparams,
608587
)
609588
.await
@@ -613,10 +592,7 @@ mod tests {
613592
let found_by_class = datastore
614593
.ereport_fetch_matching(
615594
opctx,
616-
&EreportFilters {
617-
only_classes: vec!["my cool ereport".to_string()],
618-
..Default::default()
619-
},
595+
&EreportFilters::new().with_classes(["my cool ereport"]),
620596
&pagparams,
621597
)
622598
.await

‎nexus/src/app/background/tasks/support_bundle_collector.rs‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -831,9 +831,7 @@ mod test {
831831
// NOTE: The support bundle querying interface isn't supported on
832832
// the simulated sled agent (yet?) so we're using an empty sled selection.
833833
let mut request = BundleRequest::default();
834-
request.data_selection.insert(BundleData::HostInfo(
835-
SledSelection::Specific(HashSet::new()),
836-
));
834+
request.data_selection = request.data_selection.with_specific_sleds([]);
837835
let report = collector
838836
.collect_bundle(&opctx, &request)
839837
.await
@@ -910,9 +908,7 @@ mod test {
910908

911909
// Collect the bundle
912910
let mut request = BundleRequest::default();
913-
request.data_selection.insert(BundleData::HostInfo(
914-
SledSelection::Specific(HashSet::new()),
915-
));
911+
request.data_selection = request.data_selection.with_specific_sleds([]);
916912
let report = collector
917913
.collect_bundle(&opctx, &request)
918914
.await
@@ -1131,9 +1127,7 @@ mod test {
11311127

11321128
// Each time we call "collect_bundle", we collect a SINGLE bundle.
11331129
let mut request = BundleRequest::default();
1134-
request.data_selection.insert(BundleData::HostInfo(
1135-
SledSelection::Specific(HashSet::new()),
1136-
));
1130+
request.data_selection = request.data_selection.with_specific_sleds([]);
11371131
let report = collector
11381132
.collect_bundle(&opctx, &request)
11391133
.await
@@ -1298,9 +1292,7 @@ mod test {
12981292
nexus.id(),
12991293
);
13001294
let mut request = BundleRequest::default();
1301-
request.data_selection.insert(BundleData::HostInfo(
1302-
SledSelection::Specific(HashSet::new()),
1303-
));
1295+
request.data_selection = request.data_selection.with_specific_sleds([]);
13041296
let report = collector
13051297
.collect_bundle(&opctx, &request)
13061298
.await
@@ -1455,9 +1447,7 @@ mod test {
14551447
nexus.id(),
14561448
);
14571449
let mut request = BundleRequest::default();
1458-
request.data_selection.insert(BundleData::HostInfo(
1459-
SledSelection::Specific(HashSet::new()),
1460-
));
1450+
request.data_selection = request.data_selection.with_specific_sleds([]);
14611451
let report = collector
14621452
.collect_bundle(&opctx, &request)
14631453
.await
@@ -1542,9 +1532,7 @@ mod test {
15421532
nexus.id(),
15431533
);
15441534
let mut request = BundleRequest::default();
1545-
request.data_selection.insert(BundleData::HostInfo(
1546-
SledSelection::Specific(HashSet::new()),
1547-
));
1535+
request.data_selection = request.data_selection.with_specific_sleds([]);
15481536
let report = collector
15491537
.collect_bundle(&opctx, &request)
15501538
.await
@@ -1630,9 +1618,7 @@ mod test {
16301618

16311619
// Collect the bundle
16321620
let mut request = BundleRequest::default();
1633-
request.data_selection.insert(BundleData::HostInfo(
1634-
SledSelection::Specific(HashSet::new()),
1635-
));
1621+
request.data_selection = request.data_selection.with_specific_sleds([]);
16361622
let report = collector
16371623
.collect_bundle(&opctx, &request)
16381624
.await

‎nexus/types/src/fm/case.rs‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,7 @@ mod tests {
299299
use crate::fm::DiagnosisEngineKind;
300300
use crate::fm::ereport::EreportFilters;
301301
use crate::inventory::SpType;
302-
use crate::support_bundle::{
303-
BundleData, BundleDataSelection, SledSelection,
304-
};
302+
use crate::support_bundle::BundleDataSelection;
305303
use ereport_types::{Ena, EreportId};
306304
use omicron_uuid_kinds::{
307305
AlertUuid, CaseUuid, EreporterRestartUuid, OmicronZoneUuid, SitrepUuid,
@@ -413,14 +411,11 @@ mod tests {
413411
})
414412
.unwrap();
415413

416-
let mut bundle1_data = BundleDataSelection::new();
417-
bundle1_data.insert(BundleData::Reconfigurator);
418-
bundle1_data.insert(BundleData::SpDumps);
419-
bundle1_data.insert(BundleData::HostInfo(SledSelection::All));
420-
bundle1_data.insert(BundleData::Ereports(EreportFilters {
421-
only_classes: vec!["hw.pwr.*".to_string()],
422-
..Default::default()
423-
}));
414+
let bundle1_data = BundleDataSelection::new()
415+
.with_reconfigurator()
416+
.with_sp_dumps()
417+
.with_all_sleds()
418+
.with_ereports(EreportFilters::new().with_classes(["hw.pwr.*"]));
424419

425420
let mut support_bundles_requested = IdOrdMap::new();
426421
support_bundles_requested

0 commit comments

Comments
 (0)