From ed2e63b85b6843899ba271da3a6883f4079132e4 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Fri, 2 Feb 2024 10:32:51 -0800 Subject: [PATCH] move blueprint execution to its own package (#4963) --- Cargo.lock | 26 ++ Cargo.toml | 3 + nexus/Cargo.toml | 1 + nexus/blueprint-execution/Cargo.toml | 34 ++ nexus/blueprint-execution/build.rs | 10 + nexus/blueprint-execution/src/lib.rs | 34 ++ .../blueprint-execution/src/omicron_zones.rs | 346 ++++++++++++++++++ .../tests/config.test.toml | 1 + .../src/app/background/blueprint_execution.rs | 274 ++++---------- 9 files changed, 530 insertions(+), 199 deletions(-) create mode 100644 nexus/blueprint-execution/Cargo.toml create mode 100644 nexus/blueprint-execution/build.rs create mode 100644 nexus/blueprint-execution/src/lib.rs create mode 100644 nexus/blueprint-execution/src/omicron_zones.rs create mode 120000 nexus/blueprint-execution/tests/config.test.toml diff --git a/Cargo.lock b/Cargo.lock index a54d03e52a..0ab396bf4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4213,6 +4213,31 @@ dependencies = [ "rustc_version 0.1.7", ] +[[package]] +name = "nexus-blueprint-execution" +version = "0.1.0" +dependencies = [ + "anyhow", + "chrono", + "futures", + "httptest", + "nexus-db-model", + "nexus-db-queries", + "nexus-test-utils", + "nexus-test-utils-macros", + "nexus-types", + "omicron-common", + "omicron-nexus", + "omicron-rpaths", + "omicron-workspace-hack", + "pq-sys", + "reqwest", + "sled-agent-client", + "slog", + "tokio", + "uuid", +] + [[package]] name = "nexus-client" version = "0.1.0" @@ -4948,6 +4973,7 @@ dependencies = [ "macaddr", "mg-admin-client", "mime_guess", + "nexus-blueprint-execution", "nexus-db-model", "nexus-db-queries", "nexus-defaults", diff --git a/Cargo.toml b/Cargo.toml index 600fbf185c..ea679498ce 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ members = [ "key-manager", "nexus", "nexus/authz-macros", + "nexus/blueprint-execution", "nexus/db-macros", "nexus/db-model", "nexus/db-queries", @@ -113,6 +114,7 @@ default-members = [ "key-manager", "nexus", "nexus/authz-macros", + "nexus/blueprint-execution", "nexus/db-macros", "nexus/db-model", "nexus/db-queries", @@ -246,6 +248,7 @@ mime_guess = "2.0.4" mockall = "0.12" newtype_derive = "0.1.6" mg-admin-client = { path = "clients/mg-admin-client" } +nexus-blueprint-execution = { path = "nexus/blueprint-execution" } nexus-client = { path = "clients/nexus-client" } nexus-db-model = { path = "nexus/db-model" } nexus-db-queries = { path = "nexus/db-queries" } diff --git a/nexus/Cargo.toml b/nexus/Cargo.toml index 87703cce77..6e9f2f135d 100644 --- a/nexus/Cargo.toml +++ b/nexus/Cargo.toml @@ -77,6 +77,7 @@ tough.workspace = true trust-dns-resolver.workspace = true uuid.workspace = true +nexus-blueprint-execution.workspace = true nexus-defaults.workspace = true nexus-db-model.workspace = true nexus-db-queries.workspace = true diff --git a/nexus/blueprint-execution/Cargo.toml b/nexus/blueprint-execution/Cargo.toml new file mode 100644 index 0000000000..11d8003599 --- /dev/null +++ b/nexus/blueprint-execution/Cargo.toml @@ -0,0 +1,34 @@ +[package] +name = "nexus-blueprint-execution" +version = "0.1.0" +edition = "2021" + +[build-dependencies] +omicron-rpaths.workspace = true + +[dependencies] +anyhow.workspace = true +futures.workspace = true +nexus-db-queries.workspace = true +nexus-types.workspace = true +reqwest.workspace = true +sled-agent-client.workspace = true +slog.workspace = true +uuid.workspace = true + +# See omicron-rpaths for more about the "pq-sys" dependency. This is needed +# because we use the database in the test suite, though it doesn't appear to +# work to put the pq-sys dependency only in dev-dependencies. +pq-sys = "*" + +omicron-workspace-hack.workspace = true + +[dev-dependencies] +chrono.workspace = true +httptest.workspace = true +nexus-db-model.workspace = true +nexus-test-utils.workspace = true +nexus-test-utils-macros.workspace = true +omicron-common.workspace = true +omicron-nexus.workspace = true +tokio.workspace = true diff --git a/nexus/blueprint-execution/build.rs b/nexus/blueprint-execution/build.rs new file mode 100644 index 0000000000..1ba9acd41c --- /dev/null +++ b/nexus/blueprint-execution/build.rs @@ -0,0 +1,10 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +// See omicron-rpaths for documentation. +// NOTE: This file MUST be kept in sync with the other build.rs files in this +// repository. +fn main() { + omicron_rpaths::configure_default_omicron_rpaths(); +} diff --git a/nexus/blueprint-execution/src/lib.rs b/nexus/blueprint-execution/src/lib.rs new file mode 100644 index 0000000000..f7bfd7d30c --- /dev/null +++ b/nexus/blueprint-execution/src/lib.rs @@ -0,0 +1,34 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Execution of Nexus blueprints +//! +//! See `nexus_deployment` crate-level docs for background. + +use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::DataStore; +use nexus_types::deployment::Blueprint; +use slog::o; + +mod omicron_zones; + +/// Make one attempt to realize the given blueprint, meaning to take actions to +/// alter the real system to match the blueprint +/// +/// The assumption is that callers are running this periodically or in a loop to +/// deal with transient errors or changes in the underlying system state. +pub async fn realize_blueprint( + opctx: &OpContext, + datastore: &DataStore, + blueprint: &Blueprint, +) -> Result<(), Vec> { + let log = opctx.log.new(o!("comment" => blueprint.comment.clone())); + omicron_zones::deploy_zones( + &log, + opctx, + datastore, + &blueprint.omicron_zones, + ) + .await +} diff --git a/nexus/blueprint-execution/src/omicron_zones.rs b/nexus/blueprint-execution/src/omicron_zones.rs new file mode 100644 index 0000000000..f3e81d283d --- /dev/null +++ b/nexus/blueprint-execution/src/omicron_zones.rs @@ -0,0 +1,346 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Manges deployment of Omicron zones to Sled Agents + +use anyhow::Context; +use futures::stream; +use futures::StreamExt; +use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::lookup::LookupPath; +use nexus_db_queries::db::DataStore; +use nexus_types::deployment::OmicronZonesConfig; +use sled_agent_client::Client as SledAgentClient; +use slog::info; +use slog::warn; +use slog::Logger; +use std::collections::BTreeMap; +use uuid::Uuid; + +/// Idempotently ensure that the specified Omicron zones are deployed to the +/// corresponding sleds +pub(crate) async fn deploy_zones( + log: &Logger, + opctx: &OpContext, + datastore: &DataStore, + zones: &BTreeMap, +) -> Result<(), Vec> { + let errors: Vec<_> = stream::iter(zones) + .filter_map(|(sled_id, config)| async move { + let client = match sled_client(opctx, datastore, *sled_id).await { + Ok(client) => client, + Err(err) => { + warn!(log, "{err:#}"); + return Some(err); + } + }; + let result = + client.omicron_zones_put(&config).await.with_context(|| { + format!("Failed to put {config:#?} to sled {sled_id}") + }); + + match result { + Err(error) => { + warn!(log, "{error:#}"); + Some(error) + } + Ok(_) => { + info!( + log, + "Successfully deployed zones for sled agent"; + "sled_id" => %sled_id, + "generation" => %config.generation, + ); + None + } + } + }) + .collect() + .await; + + if errors.is_empty() { + Ok(()) + } else { + Err(errors) + } +} + +// This is a modified copy of the functionality from `nexus/src/app/sled.rs`. +// There's no good way to access this functionality right now since it is a +// method on the `Nexus` type. We want to have a more constrained type we can +// pass into background tasks for this type of functionality, but for now we +// just copy the functionality. +async fn sled_client( + opctx: &OpContext, + datastore: &DataStore, + sled_id: Uuid, +) -> Result { + let (.., sled) = LookupPath::new(opctx, datastore) + .sled_id(sled_id) + .fetch() + .await + .with_context(|| { + format!( + "Failed to create sled_agent::Client for sled_id: {}", + sled_id + ) + })?; + let dur = std::time::Duration::from_secs(60); + let client = reqwest::ClientBuilder::new() + .connect_timeout(dur) + .timeout(dur) + .build() + .unwrap(); + Ok(SledAgentClient::new_with_client( + &format!("http://{}", sled.address()), + client, + opctx.log.clone(), + )) +} + +#[cfg(test)] +mod test { + use super::deploy_zones; + use httptest::matchers::{all_of, json_decoded, request}; + use httptest::responders::status_code; + use httptest::Expectation; + use nexus_db_model::{ + ByteCount, SledBaseboard, SledSystemHardware, SledUpdate, + }; + use nexus_db_queries::context::OpContext; + use nexus_test_utils_macros::nexus_test; + use nexus_types::deployment::OmicronZonesConfig; + use nexus_types::deployment::{Blueprint, BlueprintTarget}; + use nexus_types::inventory::{ + OmicronZoneConfig, OmicronZoneDataset, OmicronZoneType, + }; + use omicron_common::api::external::Generation; + use std::collections::BTreeMap; + use std::collections::BTreeSet; + use std::net::SocketAddr; + use std::sync::Arc; + use uuid::Uuid; + + type ControlPlaneTestContext = + nexus_test_utils::ControlPlaneTestContext; + + fn create_blueprint( + omicron_zones: BTreeMap, + ) -> (BlueprintTarget, Blueprint) { + let id = Uuid::new_v4(); + ( + BlueprintTarget { + target_id: id, + enabled: true, + time_made_target: chrono::Utc::now(), + }, + Blueprint { + id, + omicron_zones, + zones_in_service: BTreeSet::new(), + parent_blueprint_id: None, + time_created: chrono::Utc::now(), + creator: "test".to_string(), + comment: "test blueprint".to_string(), + }, + ) + } + + #[nexus_test] + async fn test_deploy_omicron_zones(cptestctx: &ControlPlaneTestContext) { + let nexus = &cptestctx.server.apictx().nexus; + let datastore = nexus.datastore(); + let opctx = OpContext::for_tests( + cptestctx.logctx.log.clone(), + datastore.clone(), + ); + let log = opctx.log.clone(); + + // Get a success result back when the blueprint has an empty set of + // zones. + let blueprint = Arc::new(create_blueprint(BTreeMap::new())); + deploy_zones(&log, &opctx, &datastore, &blueprint.1.omicron_zones) + .await + .expect("failed to deploy no zones"); + + // Create some fake sled-agent servers to respond to zone puts and add + // sleds to CRDB. + let mut s1 = httptest::Server::run(); + let mut s2 = httptest::Server::run(); + let sled_id1 = Uuid::new_v4(); + let sled_id2 = Uuid::new_v4(); + let rack_id = Uuid::new_v4(); + for (i, (sled_id, server)) in + [(sled_id1, &s1), (sled_id2, &s2)].iter().enumerate() + { + let SocketAddr::V6(addr) = server.addr() else { + panic!("Expected Ipv6 address. Got {}", server.addr()); + }; + let update = SledUpdate::new( + *sled_id, + addr, + SledBaseboard { + serial_number: i.to_string(), + part_number: "test".into(), + revision: 1, + }, + SledSystemHardware { + is_scrimlet: false, + usable_hardware_threads: 4, + usable_physical_ram: ByteCount(1000.into()), + reservoir_size: ByteCount(999.into()), + }, + rack_id, + ); + datastore + .sled_upsert(update) + .await + .expect("Failed to insert sled to db"); + } + + // The particular dataset doesn't matter for this test. + // We re-use the same one to not obfuscate things + let dataset = OmicronZoneDataset { + pool_name: format!("oxp_{}", Uuid::new_v4()).parse().unwrap(), + }; + + let generation = Generation::new(); + + // Zones are updated in a particular order, but each request contains + // the full set of zones that must be running. + // See `rack_setup::service::ServiceInner::run` for more details. + let mut zones = OmicronZonesConfig { + generation, + zones: vec![OmicronZoneConfig { + id: Uuid::new_v4(), + underlay_address: "::1".parse().unwrap(), + zone_type: OmicronZoneType::InternalDns { + dataset, + dns_address: "oh-hello-internal-dns".into(), + gz_address: "::1".parse().unwrap(), + gz_address_index: 0, + http_address: "some-ipv6-address".into(), + }, + }], + }; + + // Create a blueprint with only the `InternalDns` zone for both servers + // We reuse the same `OmicronZonesConfig` because the details don't + // matter for this test. + let blueprint = Arc::new(create_blueprint(BTreeMap::from([ + (sled_id1, zones.clone()), + (sled_id2, zones.clone()), + ]))); + + // Set expectations for the initial requests sent to the fake + // sled-agents. + for s in [&mut s1, &mut s2] { + s.expect( + Expectation::matching(all_of![ + request::method_path("PUT", "/omicron-zones",), + // Our generation number should be 1 and there should + // be only a single zone. + request::body(json_decoded(|c: &OmicronZonesConfig| { + c.generation == 1u32.into() && c.zones.len() == 1 + })) + ]) + .respond_with(status_code(204)), + ); + } + + // Execute it. + deploy_zones(&log, &opctx, &datastore, &blueprint.1.omicron_zones) + .await + .expect("failed to deploy initial zones"); + + s1.verify_and_clear(); + s2.verify_and_clear(); + + // Do it again. This should trigger the same request. + for s in [&mut s1, &mut s2] { + s.expect( + Expectation::matching(request::method_path( + "PUT", + "/omicron-zones", + )) + .respond_with(status_code(204)), + ); + } + deploy_zones(&log, &opctx, &datastore, &blueprint.1.omicron_zones) + .await + .expect("failed to deploy same zones"); + s1.verify_and_clear(); + s2.verify_and_clear(); + + // Take another lap, but this time, have one server fail the request and + // try again. + s1.expect( + Expectation::matching(request::method_path( + "PUT", + "/omicron-zones", + )) + .respond_with(status_code(204)), + ); + s2.expect( + Expectation::matching(request::method_path( + "PUT", + "/omicron-zones", + )) + .respond_with(status_code(500)), + ); + + let errors = + deploy_zones(&log, &opctx, &datastore, &blueprint.1.omicron_zones) + .await + .expect_err("unexpectedly succeeded in deploying zones"); + + println!("{:?}", errors); + assert_eq!(errors.len(), 1); + assert!(errors[0] + .to_string() + .starts_with("Failed to put OmicronZonesConfig")); + s1.verify_and_clear(); + s2.verify_and_clear(); + + // Add an `InternalNtp` zone for our next update + zones.generation = generation.next(); + zones.zones.push(OmicronZoneConfig { + id: Uuid::new_v4(), + underlay_address: "::1".parse().unwrap(), + zone_type: OmicronZoneType::InternalNtp { + address: "::1".into(), + dns_servers: vec!["::1".parse().unwrap()], + domain: None, + ntp_servers: vec!["some-ntp-server-addr".into()], + }, + }); + + let blueprint = Arc::new(create_blueprint(BTreeMap::from([ + (sled_id1, zones.clone()), + (sled_id2, zones.clone()), + ]))); + + // Set our new expectations + for s in [&mut s1, &mut s2] { + s.expect( + Expectation::matching(all_of![ + request::method_path("PUT", "/omicron-zones",), + // Our generation number should be bumped and there should + // be two zones. + request::body(json_decoded(|c: &OmicronZonesConfig| { + c.generation == 2u32.into() && c.zones.len() == 2 + })) + ]) + .respond_with(status_code(204)), + ); + } + + // Activate the task + deploy_zones(&log, &opctx, &datastore, &blueprint.1.omicron_zones) + .await + .expect("failed to deploy last round of zones"); + s1.verify_and_clear(); + s2.verify_and_clear(); + } +} diff --git a/nexus/blueprint-execution/tests/config.test.toml b/nexus/blueprint-execution/tests/config.test.toml new file mode 120000 index 0000000000..52f00171fd --- /dev/null +++ b/nexus/blueprint-execution/tests/config.test.toml @@ -0,0 +1 @@ +../../tests/config.test.toml \ No newline at end of file diff --git a/nexus/src/app/background/blueprint_execution.rs b/nexus/src/app/background/blueprint_execution.rs index 8d6ea8d8ce..84d4cef212 100644 --- a/nexus/src/app/background/blueprint_execution.rs +++ b/nexus/src/app/background/blueprint_execution.rs @@ -5,22 +5,14 @@ //! Background task for realizing a plan blueprint use super::common::BackgroundTask; -use anyhow::Context; use futures::future::BoxFuture; -use futures::stream; use futures::FutureExt; -use futures::StreamExt; use nexus_db_queries::context::OpContext; -use nexus_db_queries::db::lookup::LookupPath; use nexus_db_queries::db::DataStore; -use nexus_types::deployment::{Blueprint, BlueprintTarget, OmicronZonesConfig}; +use nexus_types::deployment::{Blueprint, BlueprintTarget}; use serde_json::json; -use sled_agent_client::Client as SledAgentClient; -use slog::Logger; -use std::collections::BTreeMap; use std::sync::Arc; use tokio::sync::watch; -use uuid::Uuid; /// Background task that takes a [`Blueprint`] and realizes the change to /// the state of the system based on the `Blueprint`. @@ -38,96 +30,6 @@ impl BlueprintExecutor { ) -> BlueprintExecutor { BlueprintExecutor { datastore, rx_blueprint } } - - // This is a modified copy of the functionality from `nexus/src/app/sled.rs`. - // There's no good way to access this functionality right now since it is a - // method on the `Nexus` type. We want to have a more constrained type we can - // pass into background tasks for this type of functionality, but for now we - // just copy the functionality. - async fn sled_client( - &self, - opctx: &OpContext, - sled_id: &Uuid, - ) -> Result { - let (.., sled) = LookupPath::new(opctx, &self.datastore) - .sled_id(*sled_id) - .fetch() - .await - .with_context(|| { - format!( - "Failed to create sled_agent::Client for sled_id: {}", - sled_id - ) - })?; - let dur = std::time::Duration::from_secs(60); - let client = reqwest::ClientBuilder::new() - .connect_timeout(dur) - .timeout(dur) - .build() - .unwrap(); - Ok(SledAgentClient::new_with_client( - &format!("http://{}", sled.address()), - client, - opctx.log.clone(), - )) - } - - async fn realize_blueprint( - &self, - opctx: &OpContext, - blueprint: &Blueprint, - ) -> Result<(), Vec> { - let log = opctx.log.new(o!("comment" => blueprint.comment.clone())); - self.deploy_zones(&log, opctx, &blueprint.omicron_zones).await - } - - async fn deploy_zones( - &self, - log: &Logger, - opctx: &OpContext, - zones: &BTreeMap, - ) -> Result<(), Vec> { - let errors: Vec<_> = stream::iter(zones.clone()) - .filter_map(|(sled_id, config)| async move { - let client = match self.sled_client(&opctx, &sled_id).await { - Ok(client) => client, - Err(err) => { - warn!(log, "{err:#}"); - return Some(err); - } - }; - let result = client - .omicron_zones_put(&config) - .await - .with_context(|| { - format!("Failed to put {config:#?} to sled {sled_id}") - }); - - match result { - Err(error) => { - warn!(log, "{error:#}"); - Some(error) - } - Ok(_) => { - info!( - log, - "Successfully deployed zones for sled agent"; - "sled_id" => %sled_id, - "generation" => config.generation.to_string() - ); - None - } - } - }) - .collect() - .await; - - if errors.is_empty() { - Ok(()) - } else { - Err(errors) - } - } } impl BackgroundTask for BlueprintExecutor { @@ -159,7 +61,12 @@ impl BackgroundTask for BlueprintExecutor { }); } - let result = self.realize_blueprint(opctx, blueprint).await; + let result = nexus_blueprint_execution::realize_blueprint( + opctx, + &self.datastore, + blueprint, + ) + .await; // Return the result as a `serde_json::Value` match result { @@ -179,24 +86,33 @@ impl BackgroundTask for BlueprintExecutor { .boxed() } } + #[cfg(test)] mod test { - use super::*; + use super::BlueprintExecutor; use crate::app::background::common::BackgroundTask; - use httptest::matchers::{all_of, json_decoded, request}; + use httptest::matchers::{all_of, request}; use httptest::responders::status_code; use httptest::Expectation; use nexus_db_model::{ ByteCount, SledBaseboard, SledSystemHardware, SledUpdate, }; + use nexus_db_queries::context::OpContext; use nexus_test_utils_macros::nexus_test; + use nexus_types::deployment::OmicronZonesConfig; + use nexus_types::deployment::{Blueprint, BlueprintTarget}; use nexus_types::inventory::{ OmicronZoneConfig, OmicronZoneDataset, OmicronZoneType, }; use omicron_common::api::external::Generation; use serde::Deserialize; + use serde_json::json; + use std::collections::BTreeMap; use std::collections::BTreeSet; use std::net::SocketAddr; + use std::sync::Arc; + use tokio::sync::watch; + use uuid::Uuid; type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; @@ -225,6 +141,7 @@ mod test { #[nexus_test(server = crate::Server)] async fn test_deploy_omicron_zones(cptestctx: &ControlPlaneTestContext) { + // Set up the test. let nexus = &cptestctx.server.apictx().nexus; let datastore = nexus.datastore(); let opctx = OpContext::for_tests( @@ -232,19 +149,6 @@ mod test { datastore.clone(), ); - let (blueprint_tx, blueprint_rx) = watch::channel(None); - let mut task = BlueprintExecutor::new(datastore.clone(), blueprint_rx); - - // With no blueprint we should fail with an appropriate message. - let value = task.activate(&opctx).await; - assert_eq!(value, json!({"error": "no blueprint"})); - - // Get a success (empty) result back when the blueprint has an empty set of zones - let blueprint = Arc::new(create_blueprint(BTreeMap::new())); - blueprint_tx.send(Some(blueprint)).unwrap(); - let value = task.activate(&opctx).await; - assert_eq!(value, json!({})); - // Create some fake sled-agent servers to respond to zone puts and add // sleds to CRDB. let mut s1 = httptest::Server::run(); @@ -280,24 +184,40 @@ mod test { .expect("Failed to insert sled to db"); } - // The particular dataset doesn't matter for this test. - // We re-use the same one to not obfuscate things - let dataset = OmicronZoneDataset { - pool_name: format!("oxp_{}", Uuid::new_v4()).parse().unwrap(), - }; + let (blueprint_tx, blueprint_rx) = watch::channel(None); + let mut task = BlueprintExecutor::new(datastore.clone(), blueprint_rx); - let generation = Generation::new(); + // Now we're ready. + // + // With no target blueprint, the task should fail with an appropriate + // message. + let value = task.activate(&opctx).await; + assert_eq!(value, json!({"error": "no blueprint"})); - // Zones are updated in a particular order, but each request contains - // the full set of zones that must be running. - // See `rack_setup::service::ServiceInner::run` for more details. - let mut zones = OmicronZonesConfig { - generation, + // With a target blueprint having no zones, the task should trivially + // complete and report a successful (empty) summary. + let blueprint = Arc::new(create_blueprint(BTreeMap::new())); + blueprint_tx.send(Some(blueprint)).unwrap(); + let value = task.activate(&opctx).await; + println!("activating with no zones: {:?}", value); + assert_eq!(value, json!({})); + + // Create a non-empty blueprint describing two servers and verify that + // the task correctly winds up making requests to both of them and + // reporting success. We reuse the same `OmicronZonesConfig` in + // constructing the blueprint because the details don't matter for this + // test. + let zones = OmicronZonesConfig { + generation: Generation::new(), zones: vec![OmicronZoneConfig { id: Uuid::new_v4(), underlay_address: "::1".parse().unwrap(), zone_type: OmicronZoneType::InternalDns { - dataset, + dataset: OmicronZoneDataset { + pool_name: format!("oxp_{}", Uuid::new_v4()) + .parse() + .unwrap(), + }, dns_address: "oh-hello-internal-dns".into(), gz_address: "::1".parse().unwrap(), gz_address_index: 0, @@ -306,55 +226,53 @@ mod test { }], }; - // Create a blueprint with only the `InternalDns` zone for both servers - // We reuse the same `OmicronZonesConfig` because the details don't - // matter for this test. - let blueprint = Arc::new(create_blueprint(BTreeMap::from([ + let mut blueprint = create_blueprint(BTreeMap::from([ (sled_id1, zones.clone()), (sled_id2, zones.clone()), - ]))); + ])); - // Send the blueprint with the first set of zones to the task - blueprint_tx.send(Some(blueprint)).unwrap(); + blueprint_tx.send(Some(Arc::new(blueprint.clone()))).unwrap(); - // Check that the initial requests were sent to the fake sled-agents + // Make sure that requests get made to the sled agent. This is not a + // careful check of exactly what gets sent. For that, see the tests in + // nexus-blueprint-execution. for s in [&mut s1, &mut s2] { s.expect( - Expectation::matching(all_of![ - request::method_path("PUT", "/omicron-zones",), - // Our generation number should be 1 and there should - // be only a single zone. - request::body(json_decoded(|c: &OmicronZonesConfig| { - c.generation == 1u32.into() && c.zones.len() == 1 - })) - ]) + Expectation::matching(all_of![request::method_path( + "PUT", + "/omicron-zones" + ),]) .respond_with(status_code(204)), ); } // Activate the task to trigger zone configuration on the sled-agents let value = task.activate(&opctx).await; + println!("activating two sled agents: {:?}", value); assert_eq!(value, json!({})); s1.verify_and_clear(); s2.verify_and_clear(); - // Do it again. This should trigger the same request. - for s in [&mut s1, &mut s2] { - s.expect( - Expectation::matching(request::method_path( - "PUT", - "/omicron-zones", - )) - .respond_with(status_code(204)), - ); - } + // Now, disable the target and make sure that we _don't_ invoke the sled + // agent. It's enough to just not set expectations. + blueprint.0.enabled = false; + blueprint_tx.send(Some(Arc::new(blueprint.clone()))).unwrap(); let value = task.activate(&opctx).await; - assert_eq!(value, json!({})); + println!("when disabled: {:?}", value); + assert_eq!( + value, + json!({ + "error": "blueprint disabled", + "target_id": blueprint.1.id.to_string() + }) + ); s1.verify_and_clear(); s2.verify_and_clear(); - // Take another lap, but this time, have one server fail the request and - // try again. + // Do it all again, but configure one of the servers to fail so we can + // verify the task's returned summary of what happened. + blueprint.0.enabled = true; + blueprint_tx.send(Some(Arc::new(blueprint))).unwrap(); s1.expect( Expectation::matching(request::method_path( "PUT", @@ -370,14 +288,13 @@ mod test { .respond_with(status_code(500)), ); - // Define a type we can use to pick stuff out of error objects. #[derive(Deserialize)] struct ErrorResult { errors: Vec, } let value = task.activate(&opctx).await; - println!("{:?}", value); + println!("after failure: {:?}", value); let result: ErrorResult = serde_json::from_value(value).unwrap(); assert_eq!(result.errors.len(), 1); assert!( @@ -385,46 +302,5 @@ mod test { ); s1.verify_and_clear(); s2.verify_and_clear(); - - // Add an `InternalNtp` zone for our next update - zones.generation = generation.next(); - zones.zones.push(OmicronZoneConfig { - id: Uuid::new_v4(), - underlay_address: "::1".parse().unwrap(), - zone_type: OmicronZoneType::InternalNtp { - address: "::1".into(), - dns_servers: vec!["::1".parse().unwrap()], - domain: None, - ntp_servers: vec!["some-ntp-server-addr".into()], - }, - }); - - // Update our watch channel - let blueprint = Arc::new(create_blueprint(BTreeMap::from([ - (sled_id1, zones.clone()), - (sled_id2, zones.clone()), - ]))); - blueprint_tx.send(Some(blueprint)).unwrap(); - - // Set our new expectations - for s in [&mut s1, &mut s2] { - s.expect( - Expectation::matching(all_of![ - request::method_path("PUT", "/omicron-zones",), - // Our generation number should be bumped and there should - // be two zones. - request::body(json_decoded(|c: &OmicronZonesConfig| { - c.generation == 2u32.into() && c.zones.len() == 2 - })) - ]) - .respond_with(status_code(204)), - ); - } - - // Activate the task - let value = task.activate(&opctx).await; - assert_eq!(value, json!({})); - s1.verify_and_clear(); - s2.verify_and_clear(); } }