Skip to content

Commit

Permalink
(Existing) Test fixup.
Browse files Browse the repository at this point in the history
  • Loading branch information
FelixMcFelix committed Dec 28, 2023
1 parent dcdec48 commit 4b65a67
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 10 deletions.
8 changes: 5 additions & 3 deletions nexus/db-queries/src/db/datastore/external_ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,11 @@ impl DataStore {
AttachError::NoUpdate { attached_count, resource, collection } => {
match resource.state {
// Idempotent errors: is in progress forsame resource pair -- this is fine.
IpAttachState::Attaching if resource.parent_id == Some(instance_id) => return Ok(Some((collection, resource))),
// Double attach can be hit by, e.g., repeated call during instance create.
IpAttachState::Attaching
| IpAttachState::Attached
if resource.parent_id == Some(instance_id) =>
return Ok(Some((collection, resource))),
IpAttachState::Attached => return Err(Error::invalid_request(
"floating IP cannot be attached to one \
instance while still attached to another"
Expand Down Expand Up @@ -225,8 +229,6 @@ impl DataStore {
}
// Idempotent cases:
Ok(Some((_, eip))) if eip.id != temp_ip.id => {
// Is this even possible?
eprintln!("mismatch?");
self.deallocate_external_ip(opctx, temp_ip.id).await?;
Ok((eip, true))
}
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-queries/src/db/datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1965,9 +1965,10 @@ mod test {
.execute_async(&*conn)
.await;
let ip_type = if is_service { "Service" } else { "Instance" };
let null_snat_parent = parent_id.is_none() && kind == IpKind::SNat;
if name.is_none()
&& description.is_none()
&& parent_id.is_some()
&& !null_snat_parent
&& project_id.is_none()
{
// Name/description must be NULL, instance ID cannot
Expand Down
49 changes: 43 additions & 6 deletions nexus/db-queries/src/db/queries/external_ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,11 @@ mod tests {
use async_bb8_diesel::AsyncRunQueryDsl;
use diesel::{ExpressionMethods, QueryDsl, SelectableHelper};
use dropshot::test_util::LogContext;
use nexus_db_model::ByteCount;
use nexus_db_model::Instance;
use nexus_db_model::InstanceCpuCount;
use nexus_test_utils::db::test_setup_database;
use nexus_types::external_api::params::InstanceCreate;
use nexus_types::external_api::shared::IpRange;
use omicron_common::address::NUM_SOURCE_NAT_PORTS;
use omicron_common::api::external::Error;
Expand Down Expand Up @@ -973,6 +977,37 @@ mod tests {
.expect("Failed to create IP Pool range");
}

async fn create_instance(&self, name: &str) -> Uuid {
let instance_id = Uuid::new_v4();
let project_id = Uuid::new_v4();
let instance = Instance::new(instance_id, project_id, &InstanceCreate {
identity: IdentityMetadataCreateParams { name: String::from(name).parse().unwrap(), description: format!("instance {}", name) },
ncpus: InstanceCpuCount(omicron_common::api::external::InstanceCpuCount(1)).into(),
memory: ByteCount(omicron_common::api::external::ByteCount::from_gibibytes_u32(1)).into(),
hostname: "test".into(),
user_data: vec![],
network_interfaces: Default::default(),
external_ips: vec![],
disks: vec![],
start: false,
});

let conn = self
.db_datastore
.pool_connection_authorized(&self.opctx)
.await
.unwrap();

use crate::db::schema::instance::dsl as instance_dsl;
diesel::insert_into(instance_dsl::instance)
.values(instance.clone())
.execute_async(&*conn)
.await
.expect("Failed to create Instance");

instance_id
}

async fn default_pool_id(&self) -> Uuid {
let pool = self
.db_datastore
Expand Down Expand Up @@ -1057,7 +1092,7 @@ mod tests {

// Allocate an Ephemeral IP, which should take the entire port range of
// the only address in the pool.
let instance_id = Uuid::new_v4();
let instance_id = context.create_instance("for-eph").await;
let ephemeral_ip = context
.db_datastore
.allocate_instance_ephemeral_ip(
Expand All @@ -1076,7 +1111,7 @@ mod tests {

// At this point, we should be able to allocate neither a new Ephemeral
// nor any SNAT IPs.
let instance_id = Uuid::new_v4();
let instance_id = context.create_instance("for-snat").await;
let res = context
.db_datastore
.allocate_instance_snat_ip(
Expand Down Expand Up @@ -1242,7 +1277,7 @@ mod tests {
.unwrap();
context.initialize_ip_pool("default", range).await;

let instance_id = Uuid::new_v4();
let instance_id = context.create_instance("all-the-ports").await;
let id = Uuid::new_v4();
let pool_name = None;

Expand Down Expand Up @@ -1774,7 +1809,7 @@ mod tests {

// Allocating an address on an instance in the second pool should be
// respected, even though there are IPs available in the first.
let instance_id = Uuid::new_v4();
let instance_id = context.create_instance("test").await;
let id = Uuid::new_v4();
let pool_name = Some(Name("p1".parse().unwrap()));

Expand Down Expand Up @@ -1822,7 +1857,8 @@ mod tests {
let first_octet = first_address.octets()[3];
let last_octet = last_address.octets()[3];
for octet in first_octet..=last_octet {
let instance_id = Uuid::new_v4();
let instance_id =
context.create_instance(&format!("o{octet}")).await;
let ip = context
.db_datastore
.allocate_instance_ephemeral_ip(
Expand All @@ -1844,12 +1880,13 @@ mod tests {
}

// Allocating another address should _fail_, and not use the first pool.
let instance_id = context.create_instance("final").await;
context
.db_datastore
.allocate_instance_ephemeral_ip(
&context.opctx,
Uuid::new_v4(),
Uuid::new_v4(),
instance_id,
pool_name,
true,
)
Expand Down

0 comments on commit 4b65a67

Please sign in to comment.