Skip to content

Commit

Permalink
fix instance_commit_update idempotency
Browse files Browse the repository at this point in the history
  • Loading branch information
hawkw committed Aug 7, 2024
1 parent 8a4486b commit 9e18f2b
Showing 1 changed file with 91 additions and 1 deletion.
92 changes: 91 additions & 1 deletion nexus/db-queries/src/db/datastore/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@ impl DataStore {
// generation has advanced past ours. That's fine --- assume we
// already updated the instance.
UpdateAndQueryResult { ref found, .. }
if found.runtime().r#gen > new_runtime.r#gen =>
if found.runtime().r#gen >= new_runtime.r#gen =>
{
debug!(
&opctx.log,
Expand Down Expand Up @@ -1842,6 +1842,96 @@ mod tests {
logctx.cleanup_successful();
}

#[tokio::test]
async fn test_instance_commit_update_is_idempotent() {
// Setup
let logctx =
dev::test_setup_log("test_instance_commit_update_is_idempotent");
let mut db = test_setup_database(&logctx.log).await;
let (opctx, datastore) = datastore_test(&logctx, &db).await;
let authz_instance = create_test_instance(&datastore, &opctx).await;
let saga1 = Uuid::new_v4();

// lock the instance once.
let lock = dbg!(
datastore
.instance_updater_lock(&opctx, &authz_instance, saga1)
.await
)
.expect("instance should be locked");
let new_runtime = &InstanceRuntimeState {
time_updated: Utc::now(),
r#gen: Generation(external::Generation::from_u32(2)),
propolis_id: Some(Uuid::new_v4()),
dst_propolis_id: None,
migration_id: None,
nexus_state: InstanceState::Vmm,
};

let updated = dbg!(
datastore
.instance_commit_update(
&opctx,
&authz_instance,
&lock,
&new_runtime
)
.await
)
.expect("instance_commit_update should succeed");
assert!(updated, "it should be updated");

// okay, let's do it again at the same generation.
let updated = dbg!(
datastore
.instance_commit_update(
&opctx,
&authz_instance,
&lock,
&new_runtime
)
.await
)
.expect("instance_commit_update should succeed");
assert!(!updated, "it was already updated");
let instance =
dbg!(datastore.instance_refetch(&opctx, &authz_instance).await)
.expect("instance should exist");
assert_eq!(instance.runtime().propolis_id, new_runtime.propolis_id);
assert_eq!(instance.runtime().r#gen, new_runtime.r#gen);

// Doing it again at the same generation with a *different* state
// shouldn't change the instance at all.
let updated = dbg!(
datastore
.instance_commit_update(
&opctx,
&authz_instance,
&lock,
&InstanceRuntimeState {
propolis_id: Some(Uuid::new_v4()),
migration_id: Some(Uuid::new_v4()),
dst_propolis_id: Some(Uuid::new_v4()),
..new_runtime.clone()
}
)
.await
)
.expect("instance_commit_update should succeed");
assert!(!updated, "it was already updated");
let instance =
dbg!(datastore.instance_refetch(&opctx, &authz_instance).await)
.expect("instance should exist");
assert_eq!(instance.runtime().propolis_id, new_runtime.propolis_id);
assert_eq!(instance.runtime().dst_propolis_id, None);
assert_eq!(instance.runtime().migration_id, None);
assert_eq!(instance.runtime().r#gen, new_runtime.r#gen);

// Clean up.
db.cleanup().await.unwrap();
logctx.cleanup_successful();
}

#[tokio::test]
async fn test_instance_fetch_all() {
// Setup
Expand Down

0 comments on commit 9e18f2b

Please sign in to comment.