diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index aa1a0d221cd..d3602366f35 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -1999,7 +1999,7 @@ impl DataStore { .select(NetworkInterface::as_select()); // This returns the state of the associated instance. - let stopped_instance_query = db::schema::instance::dsl::instance + let instance_query = db::schema::instance::dsl::instance .filter(db::schema::instance::dsl::id.eq(instance_id)) .filter(db::schema::instance::dsl::time_deleted.is_null()) .select(Instance::as_select()); @@ -2025,10 +2025,8 @@ impl DataStore { let pool = self.pool_authorized(opctx).await?; if make_primary { pool.transaction(move |conn| { - let instance_state = stopped_instance_query - .get_result(conn)? - .runtime_state - .state; + let instance_state = + instance_query.get_result(conn)?.runtime_state.state; if instance_state != stopped { return Err(TxnError::CustomError( NetworkInterfaceUpdateError::InstanceNotStopped, @@ -2065,10 +2063,8 @@ impl DataStore { // we're only hitting a single row. Note that we still need to // verify the instance is stopped. pool.transaction(move |conn| { - let instance_state = stopped_instance_query - .get_result(conn)? - .runtime_state - .state; + let instance_state = + instance_query.get_result(conn)?.runtime_state.state; if instance_state != stopped { return Err(TxnError::CustomError( NetworkInterfaceUpdateError::InstanceNotStopped, diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 77dec0b33eb..1bf0015cd2b 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -566,7 +566,7 @@ async fn test_instance_create_saga_removes_instance_database_record( description: String::from("instance to test saga unwind"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("inst"), user_data: vec![], network_interfaces: interface_params.clone(), @@ -588,7 +588,7 @@ async fn test_instance_create_saga_removes_instance_database_record( description: String::from("instance to test saga unwind 2"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("inst2"), user_data: vec![], network_interfaces: interface_params, @@ -674,7 +674,7 @@ async fn test_instance_with_single_explicit_ip_address( description: String::from("instance to test multiple nics"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nic-test"), user_data: vec![], network_interfaces: interface_params, @@ -791,7 +791,7 @@ async fn test_instance_with_new_custom_network_interfaces( description: String::from("instance to test multiple nics"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nic-test"), user_data: vec![], network_interfaces: interface_params, @@ -905,7 +905,7 @@ async fn test_instance_create_delete_network_interface( description: String::from("instance to test attaching new nic"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nic-test"), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::None, @@ -1155,7 +1155,7 @@ async fn test_instance_update_network_interfaces( description: String::from("instance to test updatin nics"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nic-test"), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::None, @@ -1224,7 +1224,7 @@ async fn test_instance_update_network_interfaces( instance_post(client, url_instance.as_str(), InstanceOp::Start).await; instance_simulate(nexus, &instance.identity.id).await; - // We'll the interface's name and description + // We'll change the interface's name and description let new_name = Name::try_from(String::from("new-if0")).unwrap(); let new_description = String::from("new description"); let updates = params::NetworkInterfaceUpdate { @@ -1279,22 +1279,28 @@ async fn test_instance_update_network_interfaces( // description, and modification time. assert_eq!(updated_primary_iface.identity.name, new_name); assert_eq!(updated_primary_iface.identity.description, new_description); - assert!( - primary_iface.identity.time_modified - < updated_primary_iface.identity.time_modified - ); - - // Nothing else about the primary interface should have changed - assert_eq!( - primary_iface.identity.time_created, - updated_primary_iface.identity.time_created - ); assert!(updated_primary_iface.primary); - assert_eq!(primary_iface.ip, updated_primary_iface.ip); - assert_eq!(primary_iface.mac, updated_primary_iface.mac); - assert_eq!(primary_iface.subnet_id, updated_primary_iface.subnet_id); - assert_eq!(primary_iface.vpc_id, updated_primary_iface.vpc_id); - assert_eq!(primary_iface.instance_id, updated_primary_iface.instance_id); + + // Helper to check that most attributes are unchanged when updating an + // interface, and that the modification time for the new is later than the + // old. + let verify_unchanged_attributes = + |original_iface: &NetworkInterface, new_iface: &NetworkInterface| { + assert_eq!( + original_iface.identity.time_created, + new_iface.identity.time_created + ); + assert!( + original_iface.identity.time_modified + < new_iface.identity.time_modified + ); + assert_eq!(original_iface.ip, new_iface.ip); + assert_eq!(original_iface.mac, new_iface.mac); + assert_eq!(original_iface.subnet_id, new_iface.subnet_id); + assert_eq!(original_iface.vpc_id, new_iface.vpc_id); + assert_eq!(original_iface.instance_id, new_iface.instance_id); + }; + verify_unchanged_attributes(&primary_iface, &updated_primary_iface); // Try with the same request again, but this time only changing // `make_primary`. This should have no effect. @@ -1323,10 +1329,6 @@ async fn test_instance_update_network_interfaces( .unwrap(); // Everything should still be the same, except the modification time. - assert!( - updated_primary_iface.identity.time_modified - < updated_primary_iface1.identity.time_modified - ); assert_eq!( updated_primary_iface.identity.name, updated_primary_iface1.identity.name @@ -1335,21 +1337,10 @@ async fn test_instance_update_network_interfaces( updated_primary_iface.identity.description, updated_primary_iface1.identity.description ); - assert_eq!( - updated_primary_iface.identity.time_created, - updated_primary_iface1.identity.time_created - ); - assert!(updated_primary_iface1.primary); - assert_eq!(updated_primary_iface.ip, updated_primary_iface1.ip); - assert_eq!(updated_primary_iface.mac, updated_primary_iface1.mac); - assert_eq!( - updated_primary_iface.subnet_id, - updated_primary_iface1.subnet_id - ); - assert_eq!(updated_primary_iface.vpc_id, updated_primary_iface1.vpc_id); - assert_eq!( - updated_primary_iface.instance_id, - updated_primary_iface1.instance_id + assert_eq!(updated_primary_iface.primary, updated_primary_iface1.primary); + verify_unchanged_attributes( + &updated_primary_iface, + &updated_primary_iface1, ); // Add a secondary interface to the instance. We'll use this to check @@ -1436,10 +1427,6 @@ async fn test_instance_update_network_interfaces( // It should now be the primary and have an updated modification time assert!(new_primary_iface.primary, "Failed to set the new primary"); - assert!( - new_primary_iface.identity.time_modified - > secondary_iface.identity.time_modified - ); // Nothing else about the new primary should have changed assert_eq!(new_primary_iface.identity.name, secondary_iface.identity.name); @@ -1447,16 +1434,7 @@ async fn test_instance_update_network_interfaces( new_primary_iface.identity.description, secondary_iface.identity.description ); - assert_eq!(new_primary_iface.identity.id, secondary_iface.identity.id); - assert_eq!( - new_primary_iface.identity.time_created, - secondary_iface.identity.time_created - ); - assert_eq!(new_primary_iface.subnet_id, secondary_iface.subnet_id); - assert_eq!(new_primary_iface.vpc_id, secondary_iface.vpc_id); - assert_eq!(new_primary_iface.instance_id, secondary_iface.instance_id); - assert_eq!(new_primary_iface.mac, secondary_iface.mac); - assert_eq!(new_primary_iface.ip, secondary_iface.ip); + verify_unchanged_attributes(&secondary_iface, &new_primary_iface); // Get the newly-made secondary interface to test let new_secondary_iface = NexusRequest::object_get( @@ -1481,12 +1459,6 @@ async fn test_instance_update_network_interfaces( "The old primary interface should now be a seconary" ); - // The modification time should have changed - assert!( - new_secondary_iface.identity.time_modified - > updated_primary_iface.identity.time_modified - ); - // Nothing else about the old primary should have changed assert_eq!( new_secondary_iface.identity.name, @@ -1496,22 +1468,7 @@ async fn test_instance_update_network_interfaces( new_secondary_iface.identity.description, updated_primary_iface.identity.description ); - assert_eq!( - new_secondary_iface.identity.id, - updated_primary_iface.identity.id - ); - assert_eq!( - new_secondary_iface.identity.time_created, - updated_primary_iface.identity.time_created - ); - assert_eq!(new_secondary_iface.subnet_id, updated_primary_iface.subnet_id); - assert_eq!(new_secondary_iface.vpc_id, updated_primary_iface.vpc_id); - assert_eq!( - new_secondary_iface.instance_id, - updated_primary_iface.instance_id - ); - assert_eq!(new_secondary_iface.mac, updated_primary_iface.mac); - assert_eq!(new_secondary_iface.ip, updated_primary_iface.ip); + verify_unchanged_attributes(&updated_primary_iface, &new_secondary_iface); } /// This test specifically creates two NICs, the second of which will fail the @@ -1564,7 +1521,7 @@ async fn test_instance_with_multiple_nics_unwinds_completely( description: String::from("instance to test multiple bad nics"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nic-test"), user_data: vec![], network_interfaces: interface_params, @@ -1638,7 +1595,7 @@ async fn test_attach_one_disk_to_instance(cptestctx: &ControlPlaneTestContext) { description: String::from("probably serving data"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nfs"), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -1734,7 +1691,7 @@ async fn test_attach_eight_disks_to_instance( description: String::from("probably serving data"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nfs"), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -1840,7 +1797,7 @@ async fn test_cannot_attach_nine_disks_to_instance( description: String::from("probably serving data"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nfs"), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -1967,7 +1924,7 @@ async fn test_cannot_attach_faulted_disks(cptestctx: &ControlPlaneTestContext) { description: String::from("probably serving data"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nfs"), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -2079,7 +2036,7 @@ async fn test_disks_detached_when_instance_destroyed( description: String::from("probably serving data"), }, ncpus: InstanceCpuCount::try_from(2).unwrap(), - memory: ByteCount::from_mebibytes_u32(4), + memory: ByteCount::from_gibibytes_u32(4), hostname: String::from("nfs"), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default,