[sled-agent] Replace tokio::spawn'd mg-ddm calls with a reconciler task (#7521)

Prior to this PR, sled-agent would spawn three infinite
retry-until-success tasks on startup:

1. Sending mg-ddm enough information to turn on its Oximeter stats
2. Telling mg-ddm to advertise the sled's bootstrap prefix
3. Telling mg-ddm to advertise the sled's underlay prefix

Additionally, it would spawn an infinite retry-until-success task for
any internal DNS zone it started (telling mg-ddm to advertise the
internal DNS prefix).

This PR replaces all of these with a `DdmReconciler`, which is
responsible for taking all of the above information and periodically
syncing with the local mg-ddm instance to ensure it has up-to-date
information.

Fixes #7377. Builds on #7507.

Author: jgallagher

clients/ddm-admin-client/src/lib.rs

@@ -75,14 +75,30 @@ impl Client {
 
     pub async fn advertise_prefixes(
         &self,
-        prefixes: Vec<Ipv6Net>,
+        prefixes: &Vec<Ipv6Net>,
     ) -> Result<(), Error<types::Error>> {
         self.inner
-            .advertise_prefixes(&prefixes)
+            .advertise_prefixes(prefixes)
             .await
             .map(|resp| resp.into_inner())
     }
 
+    pub async fn withdraw_prefixes(
+        &self,
+        prefixes: &Vec<Ipv6Net>,
+    ) -> Result<(), Error<types::Error>> {
+        self.inner
+            .withdraw_prefixes(prefixes)
+            .await
+            .map(|resp| resp.into_inner())
+    }
+
+    pub async fn get_originated(
+        &self,
+    ) -> Result<Vec<Ipv6Net>, Error<types::Error>> {
+        self.inner.get_originated().await.map(|resp| resp.into_inner())
+    }
+
     pub async fn enable_stats(
         &self,
         request: &EnableStatsRequest,

installinator/src/bootstrap.rs

@@ -92,7 +92,7 @@ pub(crate) async fn bootstrap_sled(
                         "Sending prefix to ddmd for advertisement";
                         "prefix" => ?prefix,
                     );
-                    ddmd_client.advertise_prefixes(vec![prefix]).await?;
+                    ddmd_client.advertise_prefixes(&vec![prefix]).await?;
                     Ok(())
                 },
                 |err, duration| {

sled-agent/src/bootstrap/pre_server.rs

@@ -15,6 +15,7 @@ use super::pumpkind;
 use super::server::StartError;
 use crate::config::Config;
 use crate::config::SidecarRevision;
+use crate::ddm_reconciler::DdmReconciler;
 use crate::long_running_tasks::{
     LongRunningTaskHandles, spawn_all_longrunning_tasks,
 };
@@ -34,25 +35,19 @@ use illumos_utils::zone;
 use illumos_utils::zone::Zones;
 use omicron_common::FileKv;
 use omicron_common::address::Ipv6Subnet;
-use omicron_common::address::SLED_PREFIX;
-use omicron_common::backoff::retry_notify;
-use omicron_common::backoff::retry_policy_internal_service_aggressive;
-use omicron_ddm_admin_client::Client as DdmAdminClient;
 use sled_hardware::DendriteAsic;
 use sled_hardware::SledMode;
 use sled_hardware::underlay;
 use sled_hardware_types::underlay::BootstrapInterface;
 use slog::Drain;
 use slog::Logger;
-use slog_error_chain::InlineErrorChain;
 use std::net::IpAddr;
 use std::net::Ipv6Addr;
 use tokio::sync::oneshot;
 
 pub(super) struct BootstrapAgentStartup {
     pub(super) config: Config,
     pub(super) global_zone_bootstrap_ip: Ipv6Addr,
-    pub(super) ddm_admin_localhost_client: DdmAdminClient,
     pub(super) base_log: Logger,
     pub(super) startup_log: Logger,
     pub(super) service_manager: ServiceManager,
@@ -77,72 +72,32 @@ impl BootstrapAgentStartup {
 
         // Perform several blocking startup tasks first; we move `config` and
         // `log` into this task, and on success, it gives them back to us.
-        let (config, log, ddm_admin_localhost_client, startup_networking) =
+        let (config, log, startup_networking) =
             tokio::task::spawn_blocking(move || {
                 enable_mg_ddm(&config, &log)?;
                 pumpkind::enable_pumpkind_service(&log)?;
                 ensure_zfs_key_directory_exists(&log)?;
 
                 let startup_networking = BootstrapNetworking::setup(&config)?;
 
-                // Spawn a background task to notify our local ddmd of our
-                // bootstrap address so it can advertise it to other sleds.
-                //
-                // TODO-cleanup Spawning a task here is a little suspect;
-                // consider moving to a reconciler loop when we do omicron#7377.
-                let ddmd_client = DdmAdminClient::localhost(&log)
-                    .map_err(StartError::CreateDdmAdminLocalhostClient)?;
-                tokio::spawn({
-                    let prefix = Ipv6Subnet::<SLED_PREFIX>::new(
-                        startup_networking.global_zone_bootstrap_ip,
-                    )
-                    .net();
-                    let ddmd_client = ddmd_client.clone();
-                    async move {
-                        retry_notify(
-                            retry_policy_internal_service_aggressive(),
-                            || async {
-                                info!(
-                                    ddmd_client.log(),
-                                    "Sending underlay prefix to \
-                                     ddmd for advertisement";
-                                    "prefix" => ?prefix,
-                                );
-                                ddmd_client
-                                    .advertise_prefixes(vec![prefix])
-                                    .await?;
-                                Ok(())
-                            },
-                            |err, duration| {
-                                info!(
-                                    ddmd_client.log(),
-                                    "Failed to notify ddmd \
-                                     of our address (will retry)";
-                                    "retry_after" => ?duration,
-                                    InlineErrorChain::new(&err),
-                                );
-                            },
-                        )
-                        .await
-                        .expect("retry policy retries until success");
-                    }
-                });
-
                 // Before we create the switch zone, we need to ensure that the
                 // necessary ZFS and Zone resources are ready. All other zones
                 // are created on U.2 drives.
                 ensure_zfs_ramdisk_dataset()?;
 
-                Ok::<_, StartError>((
-                    config,
-                    log,
-                    ddmd_client,
-                    startup_networking,
-                ))
+                Ok::<_, StartError>((config, log, startup_networking))
             })
             .await
             .unwrap()?;
 
+        // Start the DDM reconciler, giving it our bootstrap subnet to
+        // advertise to other sleds.
+        let ddm_reconciler = DdmReconciler::new(
+            Ipv6Subnet::new(startup_networking.global_zone_bootstrap_ip),
+            &base_log,
+        )
+        .map_err(StartError::CreateDdmAdminLocalhostClient)?;
+
         // Before we start monitoring for hardware, ensure we're running from a
         // predictable state.
         //
@@ -184,7 +139,7 @@ impl BootstrapAgentStartup {
 
         let service_manager = ServiceManager::new(
             &base_log,
-            ddm_admin_localhost_client.clone(),
+            ddm_reconciler,
             startup_networking,
             sled_mode,
             time_sync,
@@ -204,7 +159,6 @@ impl BootstrapAgentStartup {
         Ok(Self {
             config,
             global_zone_bootstrap_ip,
-            ddm_admin_localhost_client,
             base_log,
             startup_log: log,
             service_manager,

sled-agent/src/bootstrap/server.rs

@@ -34,11 +34,8 @@ use illumos_utils::zone;
 use illumos_utils::zone::Zones;
 use internal_dns_resolver::Resolver;
 use omicron_common::address::{AZ_PREFIX, Ipv6Subnet};
-use omicron_common::backoff::retry_notify;
-use omicron_common::backoff::retry_policy_internal_service_aggressive;
 use omicron_common::ledger;
 use omicron_common::ledger::Ledger;
-use omicron_ddm_admin_client::Client as DdmAdminClient;
 use omicron_ddm_admin_client::DdmError;
 use omicron_ddm_admin_client::types::EnableStatsRequest;
 use omicron_uuid_kinds::GenericUuid;
@@ -49,7 +46,6 @@ use sled_hardware::underlay;
 use sled_storage::dataset::CONFIG_DATASET;
 use sled_storage::manager::StorageHandle;
 use slog::Logger;
-use slog_error_chain::InlineErrorChain;
 use std::io;
 use std::net::SocketAddr;
 use std::net::SocketAddrV6;
@@ -180,7 +176,6 @@ impl Server {
         let BootstrapAgentStartup {
             config,
             global_zone_bootstrap_ip,
-            ddm_admin_localhost_client,
             base_log,
             startup_log,
             service_manager,
@@ -251,7 +246,6 @@ impl Server {
                 start_sled_agent_request,
                 long_running_task_handles.clone(),
                 service_manager.clone(),
-                &ddm_admin_localhost_client,
                 &base_log,
                 &startup_log,
             )
@@ -282,7 +276,6 @@ impl Server {
             state,
             sled_init_rx,
             sled_reset_rx,
-            ddm_admin_localhost_client,
             long_running_task_handles,
             service_manager,
             _sprockets_server_handle: sprockets_server_handle,
@@ -359,7 +352,6 @@ async fn start_sled_agent(
     request: StartSledAgentRequest,
     long_running_task_handles: LongRunningTaskHandles,
     service_manager: ServiceManager,
-    ddmd_client: &DdmAdminClient,
     base_log: &Logger,
     log: &Logger,
 ) -> Result<SledAgentServer, SledAgentServerStartError> {
@@ -398,88 +390,24 @@ async fn start_sled_agent(
     // Inform the storage service that the key manager is available
     long_running_task_handles.storage_manager.key_manager_ready().await;
 
-    // Spawn a background task to notify our local ddmd of our underlay address
-    // so it can advertise it to other sleds.
-    //
-    // TODO-cleanup Spawning a task here is a little suspect; consider moving to
-    // a reconciler loop when we do omicron#7377.
-    //
-    // TODO-security This ddmd_client is used to advertise both this
-    // (underlay) address and our bootstrap address. Bootstrap addresses are
-    // unauthenticated (connections made on them are auth'd via sprockets),
-    // but underlay addresses should be exchanged via authenticated channels
-    // between ddmd instances. It's TBD how that will work, but presumably
-    // we'll need to do something different here for underlay vs bootstrap
-    // addrs (either talk to a differently-configured ddmd, or include info
-    // indicating which kind of address we're advertising).
-    tokio::spawn({
-        let prefix = request.body.subnet.net();
-        let ddmd_client = ddmd_client.clone();
-        async move {
-            retry_notify(
-                retry_policy_internal_service_aggressive(),
-                || async {
-                    info!(
-                        ddmd_client.log(),
-                        "Sending underlay prefix to ddmd for advertisement";
-                        "prefix" => ?prefix,
-                    );
-                    ddmd_client.advertise_prefixes(vec![prefix]).await?;
-                    Ok(())
-                },
-                |err, duration| {
-                    info!(
-                        ddmd_client.log(),
-                        "Failed to notify ddmd of our address (will retry)";
-                        "retry_after" => ?duration,
-                        InlineErrorChain::new(&err),
-                    );
-                },
-            )
-            .await
-            .expect("retry policy retries until success");
-        }
-    });
+    // Inform our DDM reconciler of our underlay subnet and the information it
+    // needs for maghemite to enable Oximeter stats.
+    {
+        let ddm_reconciler = service_manager.ddm_reconciler();
 
-    // Also spawn a task to give ddmd the config it needs to start its oximeter
-    // stats producer.
-    tokio::spawn({
         let az_prefix =
             Ipv6Subnet::<AZ_PREFIX>::new(request.body.subnet.net().addr());
         let addr = request.body.subnet.net().iter().nth(1).unwrap();
         let dns_servers = Resolver::servers_from_subnet(az_prefix);
-        let request = EnableStatsRequest {
+
+        ddm_reconciler.set_underlay_subnet(request.body.subnet);
+        ddm_reconciler.enable_stats(EnableStatsRequest {
             addr: addr.into(),
             dns_servers: dns_servers.iter().map(|x| x.to_string()).collect(),
             rack_id: request.body.rack_id,
             sled_id: request.body.id.into_untyped_uuid(),
-        };
-        let ddmd_client = ddmd_client.clone();
-
-        async move {
-            retry_notify(
-                retry_policy_internal_service_aggressive(),
-                || async {
-                    info!(
-                        ddmd_client.log(), "Enabling ddm stats";
-                        "request" => ?request,
-                    );
-                    ddmd_client.enable_stats(&request).await?;
-                    Ok(())
-                },
-                |err, duration| {
-                    info!(
-                        ddmd_client.log(),
-                        "Failed enable ddm stats (will retry)";
-                        "retry_after" => ?duration,
-                        InlineErrorChain::new(&err),
-                    );
-                },
-            )
-            .await
-            .unwrap();
-        }
-    });
+        });
+    }
 
     // Server does not exist, initialize it.
     let server = SledAgentServer::start(
@@ -574,7 +502,6 @@ struct Inner {
         oneshot::Sender<Result<SledAgentResponse, String>>,
     )>,
     sled_reset_rx: mpsc::Receiver<oneshot::Sender<Result<(), BootstrapError>>>,
-    ddm_admin_localhost_client: DdmAdminClient,
     long_running_task_handles: LongRunningTaskHandles,
     service_manager: ServiceManager,
     _sprockets_server_handle: JoinHandle<()>,
@@ -637,7 +564,6 @@ impl Inner {
                     request,
                     self.long_running_task_handles.clone(),
                     self.service_manager.clone(),
-                    &self.ddm_admin_localhost_client,
                     &self.base_log,
                     &log,
                 )