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

Deprecate manually adding simulated sled agents #7375

Merged
merged 3 commits into from
Jan 21, 2025
+129 −245
Merged

Deprecate manually adding simulated sled agents #7375

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
b668b00
Deprecate manually adding simulated sled agents
jmpesp Jan 20, 2025
a6803f8
Merge branch 'main' into deprecate_add_sleds
jmpesp Jan 20, 2025
b7de7c6
some good suggested improvements
jmpesp Jan 21, 2025
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
8 changes: 4 additions & 4 deletions nexus/src/app/sagas/instance_migrate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,11 +624,11 @@ mod tests {
.await
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_saga_basic_usage_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();
let client = &cptestctx.external_client;
let nexus = &cptestctx.server.server_context().nexus;
let _project_id = setup_test_project(&client).await;
Expand Down Expand Up @@ -671,12 +671,12 @@ mod tests {
);
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_action_failure_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
let log = &cptestctx.logctx.log;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();
let client = &cptestctx.external_client;
let nexus = &cptestctx.server.server_context().nexus;
let _project_id = setup_test_project(&client).await;
Expand Down
88 changes: 44 additions & 44 deletions nexus/src/app/sagas/instance_update/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1490,7 +1490,6 @@ mod test {
};
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::PropolisUuid;
use omicron_uuid_kinds::SledUuid;
use std::sync::Arc;
use std::sync::Mutex;
use uuid::Uuid;
Expand Down Expand Up @@ -1944,12 +1943,12 @@ mod test {

// === migration source completed tests ===

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_source_completed_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, take it or leave it: we write this a lot...maybe ControlPlaneTestContext should just have a method that returns all of the extra sled agents?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, perhaps we could have the ControlPlaneTextContext return that as a slice, using &self.sled_agents[1..], so that callers don't need to always iterate over the sled agents and collect them?

Edit: Oh, disregard that last bit, I see that all_ssled_agents is returning the sa.server(), so it does have to be an iterator. Carry on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in b7de7c6, thanks :)

MigrationOutcome::default()
.source(MigrationState::Completed, VmmState::Stopping)
.setup_test(cptestctx, &other_sleds)
Expand All @@ -1958,12 +1957,12 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_source_completed_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.source(MigrationState::Completed, VmmState::Stopping)
Expand All @@ -1973,7 +1972,7 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_source_completed_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
Expand All @@ -1985,12 +1984,12 @@ mod test {

// === migration target completed tests ===

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_completed_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Completed, VmmState::Running)
Expand All @@ -2000,12 +1999,12 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_completed_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Completed, VmmState::Running)
Expand All @@ -2015,7 +2014,7 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_completed_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
Expand All @@ -2027,12 +2026,12 @@ mod test {

// === migration completed and source destroyed tests ===

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_completed_source_destroyed_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Completed, VmmState::Running)
Expand All @@ -2043,12 +2042,12 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_completed_source_destroyed_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Completed, VmmState::Running)
Expand All @@ -2059,7 +2058,7 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_completed_source_destroyed_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
Expand All @@ -2072,12 +2071,12 @@ mod test {

// === migration failed, target not destroyed ===

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_failed_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Failed, VmmState::Failed)
Expand All @@ -2088,12 +2087,12 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_failed_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Failed, VmmState::Failed)
Expand All @@ -2104,7 +2103,7 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_failed_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
Expand All @@ -2117,12 +2116,12 @@ mod test {

// === migration failed, migration target destroyed tests ===

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_failed_destroyed_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Failed, VmmState::Destroyed)
Expand All @@ -2133,12 +2132,12 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_failed_destroyed_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Failed, VmmState::Destroyed)
Expand All @@ -2149,7 +2148,7 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_target_failed_destroyed_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
Expand All @@ -2162,12 +2161,12 @@ mod test {

// === migration failed, migration source destroyed tests ===

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_source_failed_destroyed_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::InProgress, VmmState::Running)
Expand All @@ -2178,12 +2177,12 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_source_failed_destroyed_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::InProgress, VmmState::Running)
Expand All @@ -2194,7 +2193,7 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_source_failed_destroyed_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
Expand All @@ -2207,12 +2206,12 @@ mod test {

// === migration failed, source and target both destroyed ===

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_failed_everyone_died_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Failed, VmmState::Destroyed)
Expand All @@ -2223,12 +2222,12 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_failed_everyone_died_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Failed, VmmState::Destroyed)
Expand All @@ -2239,7 +2238,7 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_failed_everyone_died_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
Expand All @@ -2252,12 +2251,12 @@ mod test {

// === migration completed, but then the target was destroyed ===

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_completed_but_target_destroyed_succeeds(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Completed, VmmState::Destroyed)
Expand All @@ -2268,12 +2267,12 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_completed_but_target_destroyed_actions_succeed_idempotently(
cptestctx: &ControlPlaneTestContext,
) {
let _project_id = setup_test_project(&cptestctx.external_client).await;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> = cptestctx.all_sled_agents().skip(1).collect();

MigrationOutcome::default()
.target(MigrationState::Completed, VmmState::Destroyed)
Expand All @@ -2284,7 +2283,7 @@ mod test {
.await;
}

#[nexus_test(server = crate::Server)]
#[nexus_test(server = crate::Server, extra_sled_agents = 1)]
async fn test_migration_completed_but_target_destroyed_can_unwind(
cptestctx: &ControlPlaneTestContext,
) {
Expand Down Expand Up @@ -2320,7 +2319,7 @@ mod test {
async fn setup_test(
self,
cptestctx: &ControlPlaneTestContext,
other_sleds: &[(SledUuid, omicron_sled_agent::sim::Server)],
other_sleds: &[&omicron_sled_agent::sim::Server],
) -> MigrationTest {
MigrationTest::setup(self, cptestctx, other_sleds).await
}
Expand All @@ -2330,7 +2329,8 @@ mod test {
cptestctx: &ControlPlaneTestContext,
) {
let nexus = &cptestctx.server.server_context().nexus;
let other_sleds = test_helpers::add_sleds(cptestctx, 1).await;
let other_sleds: Vec<_> =
cptestctx.all_sled_agents().skip(1).collect();
let _project_id =
setup_test_project(&cptestctx.external_client).await;
let opctx = test_helpers::test_opctx(&cptestctx);
Expand Down Expand Up @@ -2418,7 +2418,7 @@ mod test {
async fn setup(
outcome: MigrationOutcome,
cptestctx: &ControlPlaneTestContext,
other_sleds: &[(SledUuid, omicron_sled_agent::sim::Server)],
other_sleds: &[&omicron_sled_agent::sim::Server],
) -> Self {
use crate::app::sagas::instance_migrate;

Expand Down
Loading
Loading