From 5934ba88182de40736b694bfddfff2317a1b270f Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Mon, 2 Oct 2023 17:43:18 -0700 Subject: [PATCH] review feedback --- dev-tools/omdb/src/bin/omdb/db.rs | 32 ++++++++++++++++--- dev-tools/omdb/tests/successes.out | 18 ----------- dev-tools/omdb/tests/test_all_output.rs | 2 +- dev-tools/omdb/tests/usage_errors.out | 18 +++++++++++ .../src/db/datastore/external_ip.rs | 14 -------- 5 files changed, 47 insertions(+), 37 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index 9650acdd7d7..b1443b32b91 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -32,6 +32,7 @@ use nexus_db_model::DnsGroup; use nexus_db_model::DnsName; use nexus_db_model::DnsVersion; use nexus_db_model::DnsZone; +use nexus_db_model::ExternalIp; use nexus_db_model::Instance; use nexus_db_model::Project; use nexus_db_model::Sled; @@ -968,10 +969,18 @@ async fn cmd_db_eips( limit: NonZeroU32, verbose: bool, ) -> Result<(), anyhow::Error> { + /* let ips = datastore .lookup_external_ips(&opctx) .await .context("listing external ips")?; + */ + use db::schema::external_ip::dsl; + let ips: Vec = dsl::external_ip + .filter(dsl::time_deleted.is_null()) + .select(ExternalIp::as_select()) + .get_results_async(datastore.pool_for_tests().await?) + .await?; check_limit(&ips, limit, || String::from("listing external ips")); @@ -1034,7 +1043,7 @@ async fn cmd_db_eips( Owner::Service { kind: format!("{:?}", service.1.kind) } } else { use db::schema::instance::dsl as instance_dsl; - let instance = instance_dsl::instance + let instance = match instance_dsl::instance .filter(instance_dsl::id.eq(owner_id)) .limit(1) .select(Instance::as_select()) @@ -1042,10 +1051,16 @@ async fn cmd_db_eips( .await .context("loading requested instance")? .pop() - .context("requested instance not found")?; + { + Some(instance) => instance, + None => { + eprintln!("instance with id {owner_id} not found"); + continue; + } + }; use db::schema::project::dsl as project_dsl; - let project = project_dsl::project + let project = match project_dsl::project .filter(project_dsl::id.eq(instance.project_id)) .limit(1) .select(Project::as_select()) @@ -1053,7 +1068,16 @@ async fn cmd_db_eips( .await .context("loading requested project")? .pop() - .context("requested project not found")?; + { + Some(instance) => instance, + None => { + eprintln!( + "project with id {} not found", + instance.project_id + ); + continue; + } + }; Owner::Instance { project: project.name().to_string(), diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index c935d4ba982..7532e9b61e0 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -84,24 +84,6 @@ stderr: note: using database URL postgresql://root@[::1]:REDACTED_PORT/omicron?sslmode=disable note: database schema version matches expected (4.0.0) ============================================= -EXECUTING COMMAND: omdb ["db", "network"] -termination: Exited(2) ---------------------------------------------- -stdout: ---------------------------------------------- -stderr: -Print information about the network - -Usage: omdb db network [OPTIONS] - -Commands: - list-eips List external IPs - help Print this message or the help of the given subcommand(s) - -Options: - --verbose Print out raw data structures from the data store - -h, --help Print help -============================================= EXECUTING COMMAND: omdb ["nexus", "background-tasks", "doc"] termination: Exited(0) --------------------------------------------- diff --git a/dev-tools/omdb/tests/test_all_output.rs b/dev-tools/omdb/tests/test_all_output.rs index 7028ec57106..d757369eadc 100644 --- a/dev-tools/omdb/tests/test_all_output.rs +++ b/dev-tools/omdb/tests/test_all_output.rs @@ -41,6 +41,7 @@ async fn test_omdb_usage_errors() { &["db", "dns", "diff"], &["db", "dns", "names"], &["db", "services"], + &["db", "network"], &["nexus"], &["nexus", "background-tasks"], &["sled-agent"], @@ -69,7 +70,6 @@ async fn test_omdb_success_cases(cptestctx: &ControlPlaneTestContext) { &["db", "services", "list-instances"], &["db", "services", "list-by-sled"], &["db", "sleds"], - &["db", "network"], &["nexus", "background-tasks", "doc"], &["nexus", "background-tasks", "show"], // We can't easily test the sled agent output because that's only diff --git a/dev-tools/omdb/tests/usage_errors.out b/dev-tools/omdb/tests/usage_errors.out index cebbb261ea8..b5421b76afb 100644 --- a/dev-tools/omdb/tests/usage_errors.out +++ b/dev-tools/omdb/tests/usage_errors.out @@ -188,6 +188,24 @@ Commands: Options: -h, --help Print help ============================================= +EXECUTING COMMAND: omdb ["db", "network"] +termination: Exited(2) +--------------------------------------------- +stdout: +--------------------------------------------- +stderr: +Print information about the network + +Usage: omdb db network [OPTIONS] + +Commands: + list-eips List external IPs + help Print this message or the help of the given subcommand(s) + +Options: + --verbose Print out raw data structures from the data store + -h, --help Print help +============================================= EXECUTING COMMAND: omdb ["nexus"] termination: Exited(2) --------------------------------------------- diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 251f12ecaa7..8f5e9ba4c1e 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -289,18 +289,4 @@ impl DataStore { .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } - - /// Fetch all external IP addresses of any kind for the provided instance - pub async fn lookup_external_ips( - &self, - opctx: &OpContext, - ) -> LookupResult> { - use db::schema::external_ip::dsl; - dsl::external_ip - .filter(dsl::time_deleted.is_null()) - .select(ExternalIp::as_select()) - .get_results_async(self.pool_authorized(opctx).await?) - .await - .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) - } }