review nits

Author: iliana

Cargo.toml

@@ -120,7 +120,7 @@ branch = "oxide/omicron"
 #
 # libsodium-sys is a dependency of thrussh, used in `end-to-end-tests`.  We
 # maintain a fork of the sodiumoxide repository to address an illumos build
-# issue.  See the README.oxide.md in the "oxide" branch of our fork for
+# issue.  See the README.oxide.md in the "omicron/oxide" branch of our fork for
 # details.
 #
 [patch.crates-io.libsodium-sys]

end-to-end-tests/src/bin/bootstrap.rs

@@ -1,12 +1,11 @@
-use anyhow::{bail, Result};
-use end_to_end_tests::helpers::ctx::{build_client, nexus_addr, Context};
-use end_to_end_tests::helpers::generate_name;
+use anyhow::Result;
+use end_to_end_tests::helpers::ctx::{build_client, Context};
+use end_to_end_tests::helpers::{generate_name, get_system_ip_pool};
 use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError};
 use oxide_client::types::{
     ByteCount, DiskCreate, DiskSource, IpPoolCreate, IpRange, Ipv4Range,
 };
 use oxide_client::{ClientDisksExt, ClientOrganizationsExt, ClientSystemExt};
-use std::net::IpAddr;
 use std::time::Duration;
 
 #[tokio::main]
@@ -30,17 +29,7 @@ async fn main() -> Result<()> {
 
     // ===== CREATE IP POOL ===== //
     eprintln!("creating IP pool...");
-    let nexus_addr = match nexus_addr().ip() {
-        IpAddr::V4(addr) => addr.octets(),
-        IpAddr::V6(_) => bail!("not sure what to do about IPv6 here"),
-    };
-    // TODO: not really sure about a good heuristic for selecting an IP address
-    // range here. in both my (iliana's) environment and the lab, the last octet
-    // is 20; in my environment the DHCP range is 100-249, and in the buildomat
-    // lab environment the network is currently private.
-    let first = [nexus_addr[0], nexus_addr[1], nexus_addr[2], 50].into();
-    let last = [nexus_addr[0], nexus_addr[1], nexus_addr[2], 90].into();
-
+    let (first, last) = get_system_ip_pool()?;
     let pool_name = client
         .ip_pool_create()
         .body(IpPoolCreate {

end-to-end-tests/src/helpers/mod.rs

@@ -1,11 +1,30 @@
 pub mod ctx;
 
-use anyhow::Result;
+use self::ctx::nexus_addr;
+use anyhow::{bail, Result};
 use oxide_client::types::Name;
 use rand::{thread_rng, Rng};
+use std::net::{IpAddr, Ipv4Addr};
 
 pub fn generate_name(prefix: &str) -> Result<Name> {
     format!("{}-{:x}", prefix, thread_rng().gen_range(0..0xfff_ffff_ffffu64))
         .try_into()
         .map_err(anyhow::Error::msg)
 }
+
+/// HACK: we're picking a range that doesn't conflict with either iliana's or
+/// the lab environment's IP ranges. This is not terribly robust. (in both
+/// iliana's environment and the lab, the last octet is 20; in my environment
+/// the DHCP range is 100-249, and in the buildomat lab environment the network
+/// is currently private.)
+pub fn get_system_ip_pool() -> Result<(Ipv4Addr, Ipv4Addr)> {
+    let nexus_addr = match nexus_addr().ip() {
+        IpAddr::V4(addr) => addr.octets(),
+        IpAddr::V6(_) => bail!("not sure what to do about IPv6 here"),
+    };
+
+    let first = [nexus_addr[0], nexus_addr[1], nexus_addr[2], 50].into();
+    let last = [nexus_addr[0], nexus_addr[1], nexus_addr[2], 90].into();
+
+    Ok((first, last))
+}

end-to-end-tests/src/instance_launch.rs

@@ -219,14 +219,32 @@ async fn instance_launch() -> Result<()> {
     );
 
     // tear-down
-    eprintln!("tear-down");
+    eprintln!("stopping instance");
     ctx.client
         .instance_stop()
         .organization_name(ctx.org_name.clone())
         .project_name(ctx.project_name.clone())
         .instance_name(instance.name.clone())
         .send()
         .await?;
+
+    eprintln!("deleting instance");
+    wait_for_condition(
+        || async {
+            ctx.client
+                .instance_delete()
+                .organization_name(ctx.org_name.clone())
+                .project_name(ctx.project_name.clone())
+                .instance_name(instance.name.clone())
+                .send()
+                .await
+                .map_err(|_| CondCheckError::<oxide_client::Error>::NotYet)
+        },
+        &Duration::from_secs(1),
+        &Duration::from_secs(60),
+    )
+    .await?;
+
     ctx.cleanup().await
 }