Skip to content
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

[nexus] Explicitly terminate pools for qorb #6881

Merged
merged 22 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ propolis_api_types = { git = "https://github.com/oxidecomputer/propolis", rev =
propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "11371b0f3743f8df5b047dc0edc2699f4bdf3927" }
propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "11371b0f3743f8df5b047dc0edc2699f4bdf3927" }
proptest = "1.5.0"
qorb = "0.0.2"
qorb = "0.1.0"
quote = "1.0"
rand = "0.8.5"
rand_core = "0.6.4"
Expand Down
12 changes: 5 additions & 7 deletions dev-tools/omdb/src/bin/omdb/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use crate::check_allow_destructive::DestructiveOperationToken;
use crate::helpers::CONNECTION_OPTIONS_HEADING;
use crate::helpers::DATABASE_OPTIONS_HEADING;
use crate::Omdb;
use anyhow::anyhow;
use anyhow::bail;
use anyhow::Context;
use async_bb8_diesel::AsyncConnection;
Expand Down Expand Up @@ -255,10 +254,7 @@ impl DbUrlOptions {
// doesn't match what we expect. So we use `DataStore::new_unchecked()`
// here. We will then check the schema version explicitly and warn the
// user if it doesn't match.
let datastore = Arc::new(
DataStore::new_unchecked(log.clone(), pool)
.map_err(|e| anyhow!(e).context("creating datastore"))?,
);
let datastore = Arc::new(DataStore::new_unchecked(log.clone(), pool));
check_schema_version(&datastore).await;
Ok(datastore)
}
Expand Down Expand Up @@ -785,7 +781,7 @@ impl DbArgs {
) -> Result<(), anyhow::Error> {
let datastore = self.db_url_opts.connect(omdb, log).await?;
let opctx = OpContext::for_tests(log.clone(), datastore.clone());
match &self.command {
let res = match &self.command {
DbCommands::Rack(RackArgs { command: RackCommands::List }) => {
cmd_db_rack_list(&opctx, &datastore, &self.fetch_opts).await
}
Expand Down Expand Up @@ -1013,7 +1009,9 @@ impl DbArgs {
DbCommands::Volumes(VolumeArgs {
command: VolumeCommands::List,
}) => cmd_db_volume_list(&datastore, &self.fetch_opts).await,
}
};
datastore.terminate().await;
res
}
}

Expand Down
2 changes: 1 addition & 1 deletion live-tests/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub fn live_test(_attrs: TokenStream, input: TokenStream) -> TokenStream {
#func_ident_string
).await.expect("setting up LiveTestContext");
#func_ident(&ctx).await;
ctx.cleanup_successful();
ctx.cleanup_successful().await;
}
};
let mut sig = input_func.sig.clone();
Expand Down
3 changes: 2 additions & 1 deletion live-tests/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ impl LiveTestContext {
/// This mainly removes log files created by the test. We do this in this
smklein marked this conversation as resolved.
Show resolved Hide resolved
/// explicit cleanup function rather than on `Drop` because we want the log
/// files preserved on test failure.
pub fn cleanup_successful(self) {
pub async fn cleanup_successful(self) {
self.datastore.terminate().await;
self.logctx.cleanup_successful();
}

Expand Down
17 changes: 15 additions & 2 deletions nexus-config/src/nexus_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ pub struct MgdConfig {
#[derive(Clone, Debug, Deserialize, PartialEq)]
struct UnvalidatedTunables {
max_vpc_ipv4_subnet_prefix: u8,
load_timeout: Option<std::time::Duration>,
}

/// Tunable configuration parameters, intended for use in test environments or
Expand All @@ -282,6 +283,11 @@ pub struct Tunables {
/// Note that this is the maximum _prefix_ size, which sets the minimum size
/// of the subnet.
pub max_vpc_ipv4_subnet_prefix: u8,

/// How long should we attempt to loop until the schema matches?
smklein marked this conversation as resolved.
Show resolved Hide resolved
///
/// If "None", nexus loops forever during initialization.
pub load_timeout: Option<std::time::Duration>,
}

// Convert from the unvalidated tunables, verifying each parameter as needed.
Expand All @@ -292,6 +298,7 @@ impl TryFrom<UnvalidatedTunables> for Tunables {
Tunables::validate_ipv4_prefix(unvalidated.max_vpc_ipv4_subnet_prefix)?;
Ok(Tunables {
max_vpc_ipv4_subnet_prefix: unvalidated.max_vpc_ipv4_subnet_prefix,
load_timeout: unvalidated.load_timeout,
})
}
}
Expand Down Expand Up @@ -341,7 +348,10 @@ pub const MAX_VPC_IPV4_SUBNET_PREFIX: u8 = 26;

impl Default for Tunables {
fn default() -> Self {
Tunables { max_vpc_ipv4_subnet_prefix: MAX_VPC_IPV4_SUBNET_PREFIX }
Tunables {
max_vpc_ipv4_subnet_prefix: MAX_VPC_IPV4_SUBNET_PREFIX,
load_timeout: None,
}
}
}

Expand Down Expand Up @@ -1003,7 +1013,10 @@ mod test {
trusted_root: Utf8PathBuf::from("/path/to/root.json"),
}),
schema: None,
tunables: Tunables { max_vpc_ipv4_subnet_prefix: 27 },
tunables: Tunables {
max_vpc_ipv4_subnet_prefix: 27,
load_timeout: None
},
dendrite: HashMap::from([(
SwitchLocation::Switch0,
DpdConfig {
Expand Down
10 changes: 10 additions & 0 deletions nexus/db-queries/src/db/collection_attach.rs
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,7 @@ mod test {

assert!(matches!(attach, Err(AttachError::CollectionNotFound)));

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -928,6 +929,7 @@ mod test {
// The collection should remain unchanged.
assert_eq!(collection, get_collection(collection_id, &conn).await);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -976,6 +978,7 @@ mod test {
);
assert_eq!(returned_resource, get_resource(resource_id, &conn).await);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1025,6 +1028,7 @@ mod test {
);
assert_eq!(returned_resource, get_resource(resource_id, &conn).await);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1081,6 +1085,7 @@ mod test {
);
}

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1145,6 +1150,7 @@ mod test {
_ => panic!("Unexpected error: {:?}", err),
};

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1252,6 +1258,7 @@ mod test {
_ => panic!("Unexpected error: {:?}", err),
};

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1307,6 +1314,7 @@ mod test {
assert_eq!(returned_resource, get_resource(resource_id, &conn).await);
assert_eq!(returned_resource.description(), "new description");

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1352,6 +1360,7 @@ mod test {
.await;
assert!(matches!(attach, Err(AttachError::ResourceNotFound)));

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1408,6 +1417,7 @@ mod test {
.collection_id
.is_none());

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down
6 changes: 6 additions & 0 deletions nexus/db-queries/src/db/collection_detach.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,7 @@ mod test {

assert!(matches!(detach, Err(DetachError::CollectionNotFound)));

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -839,6 +840,7 @@ mod test {
// The collection should remain unchanged.
assert_eq!(collection, get_collection(collection_id, &conn).await);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -879,6 +881,7 @@ mod test {
// The returned value should be the latest value in the DB.
assert_eq!(returned_resource, get_resource(resource_id, &conn).await);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -943,6 +946,7 @@ mod test {
_ => panic!("Unexpected error: {:?}", err),
};

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -987,6 +991,7 @@ mod test {
.await;
assert!(matches!(detach, Err(DetachError::ResourceNotFound)));

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1044,6 +1049,7 @@ mod test {
.collection_id
.is_some());

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down
8 changes: 8 additions & 0 deletions nexus/db-queries/src/db/collection_detach_many.rs
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,7 @@ mod test {

assert!(matches!(detach, Err(DetachManyError::CollectionNotFound)));

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -838,6 +839,7 @@ mod test {
get_collection(collection_id, &conn).await
);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -881,6 +883,7 @@ mod test {
get_collection(collection_id, &conn).await
);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -926,6 +929,7 @@ mod test {
get_collection(collection_id, &conn).await
);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -982,6 +986,7 @@ mod test {
"... and again!"
);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1033,6 +1038,7 @@ mod test {
_ => panic!("Unexpected error: {:?}", err),
};

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1091,6 +1097,7 @@ mod test {
&collection_id,
);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -1166,6 +1173,7 @@ mod test {
&collection_id2
);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down
2 changes: 2 additions & 0 deletions nexus/db-queries/src/db/collection_insert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ mod test {
.await;
assert!(matches!(insert, Err(AsyncInsertError::CollectionNotFound)));

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down Expand Up @@ -642,6 +643,7 @@ mod test {
// Make sure rcgen got incremented
assert_eq!(collection_rcgen, 2);

pool.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down
1 change: 1 addition & 0 deletions nexus/db-queries/src/db/datastore/allow_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ mod tests {
"Updated allowed IPs are incorrect"
);

datastore.terminate().await;
db.cleanup().await.expect("failed to cleanup database");
logctx.cleanup_successful();
}
Expand Down
1 change: 1 addition & 0 deletions nexus/db-queries/src/db/datastore/bgp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,7 @@ mod tests {
.await
.expect("delete announce set by name");

datastore.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down
1 change: 1 addition & 0 deletions nexus/db-queries/src/db/datastore/cockroachdb_node_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ mod tests {
.expect("looked up node ID");
assert_eq!(node_id.as_deref(), Some(fake_node_id));

datastore.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down
1 change: 1 addition & 0 deletions nexus/db-queries/src/db/datastore/cockroachdb_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ mod test {
}
}

datastore.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down
1 change: 1 addition & 0 deletions nexus/db-queries/src/db/datastore/dataset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ mod test {
expected_datasets,
);

datastore.terminate().await;
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}
Expand Down
Loading
Loading