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.

Sign up for GitHub

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

[sled-agent][sim] Use a shared support bundle implementation #7264

Open
wants to merge 3 commits into
base: support-bundles-crdb
Choose a base branch
from
Open

[sled-agent][sim] Use a shared support bundle implementation #7264

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d16fda0
Use a shared support bundle implementation for the simulated sled agent
smklein Dec 17, 2024
60c27d3
cleanup println
smklein Dec 17, 2024
053adca
Merge branch 'support-bundles-crdb' into support-bundle-simulated-imp…
smklein Dec 19, 2024
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
11 changes: 4 additions & 7 deletions nexus/src/app/sagas/disk_create.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,15 +1034,12 @@ pub(crate) mod test {
true
}

async fn no_regions_ensured(
sled_agent: &SledAgent,
test: &DiskTest<'_>,
) -> bool {
fn no_regions_ensured(sled_agent: &SledAgent, test: &DiskTest<'_>) -> bool {
for zpool in test.zpools() {
for dataset in &zpool.datasets {
let crucible_dataset =
sled_agent.get_crucible_dataset(zpool.id, dataset.id).await;
if !crucible_dataset.is_empty().await {
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
if !crucible_dataset.is_empty() {
return false;
}
}
Expand Down Expand Up @@ -1073,7 +1070,7 @@ pub(crate) mod test {
.await
);
assert!(no_region_allocations_exist(datastore, &test).await);
assert!(no_regions_ensured(&sled_agent, &test).await);
assert!(no_regions_ensured(&sled_agent, &test));

assert!(test.crucible_resources_deleted().await);
}
Expand Down
2 changes: 1 addition & 1 deletion nexus/src/app/sagas/instance_create.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,7 @@ pub mod test {
assert!(disk_is_detached(datastore).await);
assert!(no_instances_or_disks_on_sled(&sled_agent).await);

let v2p_mappings = &*sled_agent.v2p_mappings.lock().await;
let v2p_mappings = &*sled_agent.v2p_mappings.lock().unwrap();
assert!(v2p_mappings.is_empty());
}

Expand Down
20 changes: 11 additions & 9 deletions nexus/src/app/sagas/instance_ip_attach.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,14 +435,16 @@ pub(crate) mod test {
&instance_id,
)
.await;
let mut eips = sled_agent.external_ips.lock().await;
let my_eips = eips.entry(vmm_id).or_default();
assert!(my_eips
.iter()
.any(|v| matches!(v, InstanceExternalIpBody::Floating(_))));
assert!(my_eips
.iter()
.any(|v| matches!(v, InstanceExternalIpBody::Ephemeral(_))));
{
let mut eips = sled_agent.external_ips.lock().unwrap();
let my_eips = eips.entry(vmm_id).or_default();
assert!(my_eips
.iter()
.any(|v| matches!(v, InstanceExternalIpBody::Floating(_))));
assert!(my_eips
.iter()
.any(|v| matches!(v, InstanceExternalIpBody::Ephemeral(_))));
}

// DB has records for SNAT plus the new IPs.
let db_eips = datastore
Expand Down Expand Up @@ -497,7 +499,7 @@ pub(crate) mod test {
&instance_id,
)
.await;
let mut eips = sled_agent.external_ips.lock().await;
let mut eips = sled_agent.external_ips.lock().unwrap();
let my_eips = eips.entry(vmm_id).or_default();
assert!(my_eips.is_empty());
}
Expand Down
10 changes: 6 additions & 4 deletions nexus/src/app/sagas/instance_ip_detach.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,11 @@ pub(crate) mod test {
&instance_id,
)
.await;
let mut eips = sled_agent.external_ips.lock().await;
let my_eips = eips.entry(vmm_id).or_default();
assert!(my_eips.is_empty());
{
let mut eips = sled_agent.external_ips.lock().unwrap();
let my_eips = eips.entry(vmm_id).or_default();
assert!(my_eips.is_empty());
}

// DB only has record for SNAT.
let db_eips = datastore
Expand Down Expand Up @@ -467,7 +469,7 @@ pub(crate) mod test {
assert!(db_eips.iter().any(|v| v.kind == IpKind::SNat));

// No IP bindings remain on sled-agent.
let eips = &*sled_agent.external_ips.lock().await;
let eips = &*sled_agent.external_ips.lock().unwrap();
for (_nic_id, eip_set) in eips {
assert_eq!(eip_set.len(), 2);
}
Expand Down
65 changes: 28 additions & 37 deletions nexus/test-utils/src/resource_helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1282,26 +1282,24 @@ impl<'a, N: NexusServer> DiskTest<'a, N> {
// Tell the simulated sled agent to create the disk and zpool containing
// these datasets.

sled_agent
.create_external_physical_disk(
physical_disk_id,
disk_identity.clone(),
)
.await;
sled_agent
.create_zpool(zpool.id, physical_disk_id, zpool.size.to_bytes())
.await;
sled_agent.create_external_physical_disk(
physical_disk_id,
disk_identity.clone(),
);
sled_agent.create_zpool(
zpool.id,
physical_disk_id,
zpool.size.to_bytes(),
);

for dataset in &zpool.datasets {
// Sled Agent side: Create the Dataset, make sure regions can be
// created immediately if Nexus requests anything.
let address =
sled_agent.create_crucible_dataset(zpool.id, dataset.id).await;
sled_agent.create_crucible_dataset(zpool.id, dataset.id);
let crucible =
sled_agent.get_crucible_dataset(zpool.id, dataset.id).await;
crucible
.set_create_callback(Box::new(|_| RegionState::Created))
.await;
sled_agent.get_crucible_dataset(zpool.id, dataset.id);
crucible.set_create_callback(Box::new(|_| RegionState::Created));

// Nexus side: Notify Nexus of the physical disk/zpool/dataset
// combination that exists.
Expand Down Expand Up @@ -1381,23 +1379,19 @@ impl<'a, N: NexusServer> DiskTest<'a, N> {
for dataset in &zpool.datasets {
let crucible = self
.get_sled(*sled_id)
.get_crucible_dataset(zpool.id, dataset.id)
.await;
.get_crucible_dataset(zpool.id, dataset.id);
let called = std::sync::atomic::AtomicBool::new(false);
crucible
.set_create_callback(Box::new(move |_| {
if !called.load(std::sync::atomic::Ordering::SeqCst)
{
called.store(
true,
std::sync::atomic::Ordering::SeqCst,
);
RegionState::Requested
} else {
RegionState::Created
}
}))
.await;
crucible.set_create_callback(Box::new(move |_| {
if !called.load(std::sync::atomic::Ordering::SeqCst) {
called.store(
true,
std::sync::atomic::Ordering::SeqCst,
);
RegionState::Requested
} else {
RegionState::Created
}
}));
}
}
}
Expand All @@ -1409,11 +1403,9 @@ impl<'a, N: NexusServer> DiskTest<'a, N> {
for dataset in &zpool.datasets {
let crucible = self
.get_sled(*sled_id)
.get_crucible_dataset(zpool.id, dataset.id)
.await;
.get_crucible_dataset(zpool.id, dataset.id);
crucible
.set_create_callback(Box::new(|_| RegionState::Failed))
.await;
.set_create_callback(Box::new(|_| RegionState::Failed));
}
}
}
Expand All @@ -1430,9 +1422,8 @@ impl<'a, N: NexusServer> DiskTest<'a, N> {
for dataset in &zpool.datasets {
let crucible = self
.get_sled(*sled_id)
.get_crucible_dataset(zpool.id, dataset.id)
.await;
if !crucible.is_empty().await {
.get_crucible_dataset(zpool.id, dataset.id);
if !crucible.is_empty() {
return false;
}
}
Expand Down
1 change: 0 additions & 1 deletion nexus/tests/integration_tests/crucible_replacements.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,6 @@ impl<'a> RegionReplacementDeletedVolumeTest<'a> {
.activate_background_attachment(
region_replacement.volume_id.to_string(),
)
.await
.unwrap();
}

Expand Down
10 changes: 3 additions & 7 deletions nexus/tests/integration_tests/disks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1341,9 +1341,7 @@ async fn test_disk_virtual_provisioning_collection_failed_delete(
.sled_agent
.sled_agent
.get_crucible_dataset(zpool.id, dataset.id)
.await
.set_region_deletion_error(true)
.await;
.set_region_deletion_error(true);

// Delete the disk - expect this to fail
NexusRequest::new(
Expand Down Expand Up @@ -1379,9 +1377,7 @@ async fn test_disk_virtual_provisioning_collection_failed_delete(
.sled_agent
.sled_agent
.get_crucible_dataset(zpool.id, dataset.id)
.await
.set_region_deletion_error(false)
.await;
.set_region_deletion_error(false);

// Request disk delete again
NexusRequest::new(
Expand Down Expand Up @@ -2479,7 +2475,7 @@ async fn test_no_halt_disk_delete_one_region_on_expunged_agent(
let zpool = disk_test.zpools().next().expect("Expected at least one zpool");
let dataset = &zpool.datasets[0];

cptestctx.sled_agent.sled_agent.drop_dataset(zpool.id, dataset.id).await;
cptestctx.sled_agent.sled_agent.drop_dataset(zpool.id, dataset.id);

// Spawn a task that tries to delete the disk
let disk_url = get_disk_url(DISK_NAME);
Expand Down
8 changes: 4 additions & 4 deletions nexus/tests/integration_tests/instances.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ async fn test_instance_start_creates_networking_state(

sled_agents.push(&cptestctx.sled_agent.sled_agent);
for agent in &sled_agents {
agent.v2p_mappings.lock().await.clear();
agent.v2p_mappings.lock().unwrap().clear();
}

// Start the instance and make sure that it gets to Running.
Expand Down Expand Up @@ -6244,7 +6244,7 @@ async fn test_instance_v2p_mappings(cptestctx: &ControlPlaneTestContext) {
// Validate that every sled no longer has the V2P mapping for this instance
for sled_agent in &sled_agents {
let condition = || async {
let v2p_mappings = sled_agent.v2p_mappings.lock().await;
let v2p_mappings = sled_agent.v2p_mappings.lock().unwrap();
if v2p_mappings.is_empty() {
Ok(())
} else {
Expand Down Expand Up @@ -6501,7 +6501,7 @@ async fn assert_sled_v2p_mappings(
vni: Vni,
) {
let condition = || async {
let v2p_mappings = sled_agent.v2p_mappings.lock().await;
let v2p_mappings = sled_agent.v2p_mappings.lock().unwrap();
let mapping = v2p_mappings.iter().find(|mapping| {
mapping.virtual_ip == nic.ip
&& mapping.virtual_mac == nic.mac
Expand Down Expand Up @@ -6573,7 +6573,7 @@ pub async fn assert_sled_vpc_routes(
kind: RouterKind::Custom(db_subnet.ipv4_block.0.into()),
};

let vpc_routes = sled_agent.vpc_routes.lock().await;
let vpc_routes = sled_agent.vpc_routes.lock().unwrap();
let sys_routes_found = vpc_routes
.iter()
.any(|(id, set)| *id == sys_key && set.routes == system_routes);
Expand Down
4 changes: 1 addition & 3 deletions nexus/tests/integration_tests/snapshots.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,9 +980,7 @@ async fn test_snapshot_unwind(cptestctx: &ControlPlaneTestContext) {
.sled_agent
.sled_agent
.get_crucible_dataset(zpool.id, dataset.id)
.await
.set_creating_a_running_snapshot_should_fail()
.await;
.set_creating_a_running_snapshot_should_fail();

// Issue snapshot request, expecting it to fail
let snapshots_url = format!("/v1/snapshots?project={}", PROJECT_NAME);
Expand Down
12 changes: 2 additions & 10 deletions nexus/tests/integration_tests/volume_management.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2528,9 +2528,7 @@ async fn test_disk_create_saga_unwinds_correctly(
.sled_agent
.sled_agent
.get_crucible_dataset(zpool.id, dataset.id)
.await
.set_region_creation_error(true)
.await;
.set_region_creation_error(true);

let disk_size = ByteCount::from_gibibytes_u32(2);
let base_disk = params::DiskCreate {
Expand Down Expand Up @@ -2598,9 +2596,7 @@ async fn test_snapshot_create_saga_unwinds_correctly(
.sled_agent
.sled_agent
.get_crucible_dataset(zpool.id, dataset.id)
.await
.set_region_creation_error(true)
.await;
.set_region_creation_error(true);

// Create a snapshot
let snapshot_create = params::SnapshotCreate {
Expand Down Expand Up @@ -4225,11 +4221,9 @@ async fn test_read_only_region_reference_counting(
TypedUuid::from_untyped_uuid(db_read_only_dataset.pool_id),
db_read_only_dataset.id(),
)
.await
.get(crucible_agent_client::types::RegionId(
read_only_region.id().to_string()
))
.await
.unwrap()
.state,
crucible_agent_client::types::State::Created
Expand Down Expand Up @@ -4297,11 +4291,9 @@ async fn test_read_only_region_reference_counting(
TypedUuid::from_untyped_uuid(db_read_only_dataset.pool_id),
db_read_only_dataset.id(),
)
.await
.get(crucible_agent_client::types::RegionId(
read_only_region.id().to_string()
))
.await
.unwrap()
.state,
crucible_agent_client::types::State::Destroyed
Expand Down
9 changes: 2 additions & 7 deletions sled-agent/src/sim/artifact_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,19 @@
//! Implementation of `crate::artifact_store::StorageBackend` for our simulated
//! storage.

use std::sync::Arc;

use camino_tempfile::Utf8TempDir;
use futures::lock::Mutex;
use sled_storage::error::Error as StorageError;

use super::storage::Storage;
use crate::artifact_store::DatasetsManager;

pub(super) struct SimArtifactStorage {
root: Utf8TempDir,
backend: Arc<Mutex<Storage>>,
backend: Storage,
}

impl SimArtifactStorage {
pub(super) fn new(backend: Arc<Mutex<Storage>>) -> SimArtifactStorage {
pub(super) fn new(backend: Storage) -> SimArtifactStorage {
SimArtifactStorage {
root: camino_tempfile::tempdir().unwrap(),
backend,
Expand All @@ -36,9 +33,7 @@ impl DatasetsManager for SimArtifactStorage {
let config = self
.backend
.lock()
.await
.datasets_config_list()
.await
.map_err(|_| StorageError::LedgerNotFound)?;
Ok(crate::artifact_store::filter_dataset_mountpoints(
config,
Expand Down
Loading
Loading