Skip to content

Commit

Permalink
misc review feedback cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
hawkw committed Jul 11, 2024
1 parent aa752bd commit 0ea7623
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 12 deletions.
8 changes: 2 additions & 6 deletions nexus/db-queries/src/db/queries/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,10 @@ pub struct InstanceAndVmmUpdateResult {
/// indicates whether the row was updated. `None` if the VMM was not found.
pub vmm_status: Option<UpdateStatus>,

/// `Some(status)` if the inbound migration was found; the wrapped `UpdateStatus`
/// indicates whether the row was updated. `None` if the inbound migration
/// was not found, or no migration update was performed.
/// Indicates whether a migration-in update was performed.
pub migration_in_status: RecordUpdateStatus,

/// `Some(status)` if the outbound migration was found; the wrapped `UpdateStatus`
/// indicates whether the row was updated. `None` if the inbound migration
/// was not found, or no migration update was performed.
/// Indicates whether a migration-out update was performed.
pub migration_out_status: RecordUpdateStatus,
}

Expand Down
2 changes: 1 addition & 1 deletion nexus/src/app/sagas/snapshot_create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2210,7 +2210,7 @@ mod test {
&Duration::from_secs(5),
&Duration::from_secs(300),
)
.await.expect("instance did not advance to NoVmm after 400 seconds");
.await.expect("instance did not advance to NoVmm after 300 seconds");
test_helpers::instance_delete_by_name(
cptestctx,
INSTANCE_NAME,
Expand Down
9 changes: 4 additions & 5 deletions nexus/tests/integration_tests/instances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,11 +838,6 @@ async fn test_instance_migrate(cptestctx: &ControlPlaneTestContext) {
assert_eq!(migration.target_state, MigrationState::Pending.into());
assert_eq!(migration.source_state, MigrationState::Pending.into());

// Explicitly simulate the migration action on the target. Simulated
// migrations always succeed. The state transition on the target is
// sufficient to move the instance back into a Running state (strictly
// speaking no further updates from the source are required if the target
// successfully takes over).
instance_simulate_migration_source(
cptestctx,
nexus,
Expand All @@ -851,6 +846,10 @@ async fn test_instance_migrate(cptestctx: &ControlPlaneTestContext) {
migration_id,
)
.await;
// TODO(eliza): it would be nice to single-step both simulated sled agents
// through each migration phase and assert that we see all the intermediate
// states, instead of just letting them run straight to completion...

// Ensure that both sled agents report that the migration has completed.
instance_simulate_on_sled(cptestctx, nexus, original_sled, instance_id)
.await;
Expand Down

0 comments on commit 0ea7623

Please sign in to comment.