From bcf22c29b4358fbfbe56036064f546a988f86010 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 3 May 2023 14:44:16 -0700 Subject: [PATCH 1/2] fix disk test --- nexus/tests/integration_tests/disks.rs | 24 +++++++++++++----------- nexus/tests/integration_tests/metrics.rs | 15 +++++---------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs index 91b796289cc..57f517b6b29 100644 --- a/nexus/tests/integration_tests/disks.rs +++ b/nexus/tests/integration_tests/disks.rs @@ -4,9 +4,7 @@ //! Tests basic disk support in the API -use super::metrics::{ - query_for_latest_metric, query_for_metrics_until_they_exist, -}; +use super::metrics::{query_for_latest_metric, query_for_metrics}; use chrono::Utc; use crucible_agent_client::types::State as RegionState; @@ -1349,6 +1347,11 @@ const ALL_METRICS: [&'static str; 6] = #[nexus_test] async fn test_disk_metrics(cptestctx: &ControlPlaneTestContext) { + // Record the current start time. When fetching data points, we never need + // to look at times before this timestamp, since all the events we're + // interested in happen after this test starts. + let test_start_time = Utc::now(); + // Normally, Nexus is not registered as a producer for tests. // Turn this bit on so we can also test some metrics from Nexus itself. cptestctx.server.register_as_producer().await; @@ -1360,14 +1363,15 @@ async fn test_disk_metrics(cptestctx: &ControlPlaneTestContext) { let disk = create_disk(&client, PROJECT_NAME, DISK_NAME).await; oximeter.force_collect().await; - // Whenever we grab this URL, get the surrounding few seconds of metrics. + // When grabbing a metric, we look for data points going back to the + // start of this test all the way up to the current time. let metric_url = |metric: &str| { format!( "/v1/disks/{}/metrics/{}?start_time={:?}&end_time={:?}&project={}", DISK_NAME, metric, - Utc::now() - chrono::Duration::seconds(10), - Utc::now() + chrono::Duration::seconds(10), + test_start_time, + Utc::now(), PROJECT_NAME, ) }; @@ -1375,8 +1379,8 @@ async fn test_disk_metrics(cptestctx: &ControlPlaneTestContext) { let utilization_url = |id: Uuid| { format!( "/v1/system/metrics/virtual_disk_space_provisioned?start_time={:?}&end_time={:?}&id={:?}", - Utc::now() - chrono::Duration::seconds(10), - Utc::now() + chrono::Duration::seconds(10), + test_start_time, + Utc::now(), id, ) }; @@ -1400,9 +1404,7 @@ async fn test_disk_metrics(cptestctx: &ControlPlaneTestContext) { oximeter.force_collect().await; for metric in &ALL_METRICS { - let measurements = - query_for_metrics_until_they_exist(client, &metric_url(metric)) - .await; + let measurements = query_for_metrics(client, &metric_url(metric)).await; assert!(!measurements.items.is_empty()); for item in &measurements.items { diff --git a/nexus/tests/integration_tests/metrics.rs b/nexus/tests/integration_tests/metrics.rs index 160ad9951a1..71cda497ac1 100644 --- a/nexus/tests/integration_tests/metrics.rs +++ b/nexus/tests/integration_tests/metrics.rs @@ -8,19 +8,14 @@ use nexus_test_utils::resource_helpers::objects_list_page_authz; use oximeter::types::Datum; use oximeter::types::Measurement; -pub async fn query_for_metrics_until_they_exist( +pub async fn query_for_metrics( client: &ClientTestContext, path: &str, ) -> ResultsPage { - loop { - let measurements: ResultsPage = - objects_list_page_authz(client, path).await; - - if !measurements.items.is_empty() { - return measurements; - } - tokio::time::sleep(tokio::time::Duration::from_millis(10)).await; - } + let measurements: ResultsPage = + objects_list_page_authz(client, path).await; + assert!(!measurements.items.is_empty()); + measurements } pub async fn query_for_latest_metric( From cdefe2a330bfae5c3c462d942e084f61e781283b Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 3 May 2023 16:59:33 -0700 Subject: [PATCH 2/2] fix the instance test too --- nexus/test-utils/src/lib.rs | 3 +++ nexus/tests/integration_tests/disks.rs | 13 ++++--------- nexus/tests/integration_tests/instances.rs | 4 ++-- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 0033d1b080b..0bf6b834da2 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -57,6 +57,7 @@ pub const TEST_PHYSICAL_RAM: u64 = 32 * (1 << 30); pub const TEST_SUITE_PASSWORD: &str = "oxide"; pub struct ControlPlaneTestContext { + pub start_time: chrono::DateTime, pub external_client: ClientTestContext, pub internal_client: ClientTestContext, pub server: N, @@ -157,6 +158,7 @@ pub async fn test_setup_with_config( config: &mut omicron_common::nexus_config::Config, sim_mode: sim::SimMode, ) -> ControlPlaneTestContext { + let start_time = chrono::Utc::now(); let logctx = LogContext::new(test_name, &config.pkg.log); let log = &logctx.log; @@ -360,6 +362,7 @@ pub async fn test_setup_with_config( register_test_producer(&producer).unwrap(); ControlPlaneTestContext { + start_time, server, external_client: testctx_external, internal_client: testctx_internal, diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs index 57f517b6b29..021b77761c7 100644 --- a/nexus/tests/integration_tests/disks.rs +++ b/nexus/tests/integration_tests/disks.rs @@ -1347,11 +1347,6 @@ const ALL_METRICS: [&'static str; 6] = #[nexus_test] async fn test_disk_metrics(cptestctx: &ControlPlaneTestContext) { - // Record the current start time. When fetching data points, we never need - // to look at times before this timestamp, since all the events we're - // interested in happen after this test starts. - let test_start_time = Utc::now(); - // Normally, Nexus is not registered as a producer for tests. // Turn this bit on so we can also test some metrics from Nexus itself. cptestctx.server.register_as_producer().await; @@ -1370,7 +1365,7 @@ async fn test_disk_metrics(cptestctx: &ControlPlaneTestContext) { "/v1/disks/{}/metrics/{}?start_time={:?}&end_time={:?}&project={}", DISK_NAME, metric, - test_start_time, + cptestctx.start_time, Utc::now(), PROJECT_NAME, ) @@ -1379,7 +1374,7 @@ async fn test_disk_metrics(cptestctx: &ControlPlaneTestContext) { let utilization_url = |id: Uuid| { format!( "/v1/system/metrics/virtual_disk_space_provisioned?start_time={:?}&end_time={:?}&id={:?}", - test_start_time, + cptestctx.start_time, Utc::now(), id, ) @@ -1444,8 +1439,8 @@ async fn test_disk_metrics_paginated(cptestctx: &ControlPlaneTestContext) { ); let initial_params = format!( "start_time={:?}&end_time={:?}", - Utc::now() - chrono::Duration::seconds(2), - Utc::now() + chrono::Duration::seconds(2), + cptestctx.start_time, + Utc::now(), ); objects_list_page_authz::( diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index d5b3e17d345..71e1a391dc9 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -519,8 +519,8 @@ async fn test_instance_metrics(cptestctx: &ControlPlaneTestContext) { let metric_url = |metric_type: &str, id: Uuid| { format!( "/v1/system/metrics/{metric_type}?start_time={:?}&end_time={:?}&id={id}", - Utc::now() - chrono::Duration::seconds(30), - Utc::now() + chrono::Duration::seconds(30), + cptestctx.start_time, + Utc::now(), ) }; oximeter.force_collect().await;