WIP: Blueprint planner test

Author: plotnick

nexus/src/app/background/tasks/blueprint_planner.rs

@@ -69,7 +69,7 @@ impl BlueprintPlanner {
             return status;
         };
         let (target, parent) = &*loaded;
-        status.blueprint_id = parent.id;
+        status.parent_id = parent.id;
 
         // Get the inventory most recently seen by the collection
         // background task. The value is `Copy`, so with the deref
@@ -105,7 +105,7 @@ impl BlueprintPlanner {
             }
         };
 
-        // Assemble the planning context.
+        // Assemble and adjust the planning context.
         let input =
             match PlanningInputFromDb::assemble(opctx, &self.datastore).await {
                 Ok(input) => input,
@@ -155,72 +155,71 @@ impl BlueprintPlanner {
 
         // Compare the new blueprint to its parent.
         let summary = blueprint.diff_since_blueprint(&parent);
-        if summary.has_changes() {
+        if !summary.has_changes() {
+            // Blueprint is unchanged, do nothing.
             info!(
                 &opctx.log,
-                "planning produced new blueprint";
+                "blueprint unchanged from current target";
                 "parent_blueprint_id" => %parent.id,
-                "blueprint_id" => %blueprint.id,
             );
-            status.blueprint_id = blueprint.id;
+            return status;
+        }
 
-            // Save it.
-            match self.datastore.blueprint_insert(opctx, &blueprint).await {
-                Ok(()) => (),
-                Err(error) => {
-                    error!(
-                        &opctx.log,
-                        "can't save blueprint";
-                        "error" => %error,
-                        "blueprint_id" => %blueprint.id,
-                    );
-                    status.error = Some(format!(
-                        "can't save blueprint {}: {}",
-                        blueprint.id, error
-                    ));
-                    return status;
-                }
-            }
+        // We have a fresh blueprint.
+        info!(
+            &opctx.log,
+            "planning produced new blueprint";
+            "parent_blueprint_id" => %parent.id,
+            "blueprint_id" => %blueprint.id,
+        );
+        status.blueprint_id = Some(blueprint.id);
 
-            // Make it the current target.
-            let target = BlueprintTarget {
-                target_id: blueprint.id,
-                enabled: target.enabled,
-                time_made_target: Utc::now(),
-            };
-            match self
-                .datastore
-                .blueprint_target_set_current(opctx, target)
-                .await
-            {
-                Ok(()) => (),
-                Err(error) => {
-                    warn!(
-                        &opctx.log,
-                        "can't make blueprint the current target";
-                        "error" => %error,
-                        "blueprint_id" => %blueprint.id
-                    );
-                    status.error = Some(format!(
-                        "can't make blueprint {} the current target: {}",
-                        blueprint.id, error,
-                    ));
-                    return status;
-                }
+        // Save it.
+        match self.datastore.blueprint_insert(opctx, &blueprint).await {
+            Ok(()) => (),
+            Err(error) => {
+                error!(
+                    &opctx.log,
+                    "can't save blueprint";
+                    "error" => %error,
+                    "blueprint_id" => %blueprint.id,
+                );
+                status.error = Some(format!(
+                    "can't save blueprint {}: {}",
+                    blueprint.id, error
+                ));
+                return status;
             }
+        }
 
-            // Notify watchers that we have a new target.
-            self.tx_blueprint.send_replace(Some(Arc::new((target, blueprint))));
-        } else {
-            // Blueprint is unchanged, do nothing.
-            info!(
-                &opctx.log,
-                "blueprint unchanged from current target";
-                "parent_blueprint_id" => %parent.id,
-            );
-            status.unchanged = true;
+        // Try to make it the current target.
+        let target = BlueprintTarget {
+            target_id: blueprint.id,
+            enabled: target.enabled,
+            time_made_target: Utc::now(),
+        };
+        match self.datastore.blueprint_target_set_current(opctx, target).await {
+            Ok(()) => {
+                status.new_target = true;
+            }
+            Err(error) => {
+                warn!(
+                    &opctx.log,
+                    "can't make blueprint the current target";
+                    "error" => %error,
+                    "blueprint_id" => %blueprint.id
+                );
+                status.error = Some(format!(
+                    "can't make blueprint {} the current target: {}",
+                    blueprint.id, error,
+                ));
+                return status;
+            }
         }
 
+        // Notify watchers that we have a new target.
+        self.tx_blueprint.send_replace(Some(Arc::new((target, blueprint))));
+
         status
     }
 }
@@ -233,3 +232,100 @@ impl BackgroundTask for BlueprintPlanner {
         Box::pin(async move { json!(self.plan(opctx).await) })
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::app::background::tasks::blueprint_load::TargetBlueprintLoader;
+    use crate::app::background::tasks::inventory_collection::InventoryCollector;
+    use nexus_test_utils_macros::nexus_test;
+
+    type ControlPlaneTestContext =
+        nexus_test_utils::ControlPlaneTestContext<crate::Server>;
+
+    #[nexus_test(server = crate::Server)]
+    async fn test_blueprint_planner(cptestctx: &ControlPlaneTestContext) {
+        // Set up the test context.
+        let nexus = &cptestctx.server.server_context().nexus;
+        let datastore = nexus.datastore();
+        let opctx = OpContext::for_tests(
+            cptestctx.logctx.log.clone(),
+            datastore.clone(),
+        );
+
+        // Spin up the blueprint loader background task.
+        let mut loader = TargetBlueprintLoader::new(datastore.clone());
+        let mut rx_loader = loader.watcher();
+        loader.activate(&opctx).await;
+        let (_initial_target, initial_blueprint) = &*rx_loader
+            .borrow_and_update()
+            .clone()
+            .expect("no initial blueprint");
+
+        // Spin up the inventory collector background task.
+        let resolver = internal_dns_resolver::Resolver::new_from_addrs(
+            cptestctx.logctx.log.clone(),
+            &[cptestctx.internal_dns.dns_server.local_address()],
+        )
+        .unwrap();
+        let mut collector = InventoryCollector::new(
+            datastore.clone(),
+            resolver.clone(),
+            "test_planner",
+            1,
+            false,
+        );
+        let rx_collector = collector.watcher();
+        collector.activate(&opctx).await;
+
+        // Finally, spin up the planner background task.
+        let mut planner = BlueprintPlanner::new(
+            datastore.clone(),
+            false,
+            rx_collector,
+            rx_loader.clone(),
+        );
+        let _rx_planner = planner.watcher();
+
+        // On activation, the planner should run successfully and generate
+        // a new target blueprint.
+        let status = serde_json::from_value::<BlueprintPlannerStatus>(
+            planner.activate(&opctx).await,
+        )
+        .unwrap();
+        assert!(!status.disabled);
+        assert!(status.error.is_none());
+        assert_eq!(status.parent_id, initial_blueprint.id);
+        let blueprint_id = status.blueprint_id.unwrap();
+        assert_ne!(blueprint_id, initial_blueprint.id);
+        assert!(status.new_target);
+
+        // Load and check the new target blueprint.
+        loader.activate(&opctx).await;
+        let (target, blueprint) = &*rx_loader
+            .borrow_and_update()
+            .clone()
+            .expect("failed to load blueprint");
+        assert_eq!(target.target_id, blueprint.id);
+        assert_eq!(target.target_id, blueprint_id);
+        assert!(
+            blueprint.diff_since_blueprint(initial_blueprint).has_changes()
+        );
+
+        // Planning again should not change the plan.
+        let status = serde_json::from_value::<BlueprintPlannerStatus>(
+            planner.activate(&opctx).await,
+        )
+        .unwrap();
+        assert_eq!(
+            status,
+            BlueprintPlannerStatus {
+                disabled: false,
+                error: None,
+                parent_id: blueprint_id,
+                blueprint_id: None,
+                new_target: false,
+            }
+        );
+    }
+}

nexus/src/lib.rs

@@ -269,22 +269,18 @@ impl nexus_test_interface::NexusServer for Server {
         let opctx =
             internal_server.apictx.context.nexus.opctx_for_internal_api();
 
-        // Allocation of the initial Nexus's external IP is a little funny.  In
+        // Allocation of initial external IP addresses is a little funny.  In
         // a real system, it'd be allocated by RSS and provided with the rack
         // initialization request (which we're about to simulate).  RSS also
         // provides information about the external IP pool ranges available for
-        // system services.  The Nexus external IP that it picks comes from this
-        // range.  During rack initialization, Nexus "allocates" the IP (which
-        // was really already allocated) -- recording that allocation like any
-        // other one.
-        //
-        // In this context, the IP was "allocated" by the user.  Most likely,
-        // it's 127.0.0.1, having come straight from the stock testing config
-        // file.  Whatever it is, we fake up an IP pool range for use by system
-        // services that includes solely this IP.
+        // system services.  But here, we fake up IP pool ranges based on the
+        // external addresses of services that we start or mock.
         let internal_services_ip_pool_ranges = blueprint
             .all_omicron_zones(BlueprintZoneDisposition::is_in_service)
             .filter_map(|(_, zc)| match &zc.zone_type {
+                BlueprintZoneType::BoundaryNtp(
+                    blueprint_zone_type::BoundaryNtp { external_ip, .. },
+                ) => Some(IpRange::from(external_ip.snat_cfg.ip)),
                 BlueprintZoneType::ExternalDns(
                     blueprint_zone_type::ExternalDns { dns_address, .. },
                 ) => Some(IpRange::from(dns_address.addr.ip())),

nexus/test-utils/src/lib.rs

@@ -50,12 +50,14 @@ use nexus_types::deployment::BlueprintZoneType;
 use nexus_types::deployment::CockroachDbPreserveDowngrade;
 use nexus_types::deployment::OmicronZoneExternalFloatingAddr;
 use nexus_types::deployment::OmicronZoneExternalFloatingIp;
+use nexus_types::deployment::OmicronZoneExternalSnatIp;
 use nexus_types::deployment::OximeterReadMode;
 use nexus_types::deployment::blueprint_zone_type;
 use nexus_types::external_api::views::SledState;
 use nexus_types::internal_api::params::DnsConfigParams;
 use omicron_common::address::DNS_OPTE_IPV4_SUBNET;
 use omicron_common::address::NEXUS_OPTE_IPV4_SUBNET;
+use omicron_common::address::NTP_OPTE_IPV4_SUBNET;
 use omicron_common::api::external::Generation;
 use omicron_common::api::external::MacAddr;
 use omicron_common::api::external::UserId;
@@ -67,6 +69,7 @@ use omicron_common::api::internal::nexus::ProducerKind;
 use omicron_common::api::internal::shared::DatasetKind;
 use omicron_common::api::internal::shared::NetworkInterface;
 use omicron_common::api::internal::shared::NetworkInterfaceKind;
+use omicron_common::api::internal::shared::SourceNatConfig;
 use omicron_common::api::internal::shared::SwitchLocation;
 use omicron_common::disk::CompressionAlgorithm;
 use omicron_common::zpool_name::ZpoolName;
@@ -91,7 +94,7 @@ use slog::{Logger, debug, error, o};
 use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::fmt::Debug;
-use std::net::{IpAddr, Ipv6Addr, SocketAddr, SocketAddrV6};
+use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV6};
 use std::sync::Arc;
 use std::time::Duration;
 use uuid::Uuid;
@@ -1199,6 +1202,57 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
         })
     }
 
+    /// Configure a mock boundary-NTP server on the first sled agent
+    pub async fn configure_boundary_ntp(&mut self) {
+        let mac = self
+            .rack_init_builder
+            .mac_addrs
+            .next()
+            .expect("ran out of MAC addresses");
+        let internal_ip = NTP_OPTE_IPV4_SUBNET
+            .nth(NUM_INITIAL_RESERVED_IP_ADDRESSES + 1)
+            .unwrap();
+        let external_ip = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4));
+        let zone_id = OmicronZoneUuid::new_v4();
+        let zpool_id = ZpoolUuid::new_v4();
+
+        self.blueprint_zones.push(BlueprintZoneConfig {
+            disposition: BlueprintZoneDisposition::InService,
+            id: zone_id,
+            filesystem_pool: ZpoolName::new_external(zpool_id),
+            zone_type: BlueprintZoneType::BoundaryNtp(
+                blueprint_zone_type::BoundaryNtp {
+                    address: "[::1]:80".parse().unwrap(),
+                    ntp_servers: vec![],
+                    dns_servers: vec![],
+                    domain: None,
+                    nic: NetworkInterface {
+                        id: Uuid::new_v4(),
+                        kind: NetworkInterfaceKind::Service {
+                            id: zone_id.into_untyped_uuid(),
+                        },
+                        ip: internal_ip.into(),
+                        mac,
+                        name: format!("boundary-ntp-{zone_id}")
+                            .parse()
+                            .unwrap(),
+                        primary: true,
+                        slot: 0,
+                        subnet: (*NTP_OPTE_IPV4_SUBNET).into(),
+                        vni: Vni::SERVICES_VNI,
+                        transit_ips: vec![],
+                    },
+                    external_ip: OmicronZoneExternalSnatIp {
+                        id: ExternalIpUuid::new_v4(),
+                        snat_cfg: SourceNatConfig::new(external_ip, 0, 16383)
+                            .unwrap(),
+                    },
+                },
+            ),
+            image_source: BlueprintZoneImageSource::InstallDataset,
+        });
+    }
+
     /// Set up the Crucible Pantry on the first sled agent
     pub async fn start_crucible_pantry(&mut self) {
         let pantry = self.sled_agents[0].start_pantry().await;
@@ -1685,6 +1739,12 @@ async fn setup_with_config_impl<N: NexusServer>(
     builder
         .init_with_steps(
             vec![
+                (
+                    "configure_boundary_ntp",
+                    Box::new(|builder| {
+                        builder.configure_boundary_ntp().boxed()
+                    }),
+                ),
                 (
                     "start_crucible_pantry",
                     Box::new(|builder| builder.start_crucible_pantry().boxed()),