add background tasks for propagating DNS (#2797)

Author: davepacheco

Cargo.lock

@@ -277,7 +277,7 @@ signal-hook-tokio = { version = "0.3", features = [ "futures-v0_3" ] }
 sled = "0.34"
 sled-agent-client = { path = "sled-agent-client" }
 sled-hardware = { path = "sled-hardware" }
-slog = { version = "2.7", features = [ "max_level_trace", "release_max_level_debug" ] }
+slog = { version = "2.7", features = [ "dynamic-keys", "max_level_trace", "release_max_level_debug" ] }
 slog-async = "2.7"
 slog-dtrace = "0.2"
 slog-envlogger = "2.2"

Cargo.toml

@@ -14,10 +14,12 @@ use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
 use serde_with::DeserializeFromStr;
 use serde_with::DisplayFromStr;
+use serde_with::DurationSeconds;
 use serde_with::SerializeDisplay;
 use std::fmt;
 use std::net::SocketAddr;
 use std::path::{Path, PathBuf};
+use std::time::Duration;
 use uuid::Uuid;
 
 #[derive(Debug)]
@@ -263,6 +265,37 @@ fn default_https_port() -> u16 {
     443
 }
 
+/// Background task configuration
+#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
+pub struct BackgroundTaskConfig {
+    /// configuration for internal DNS background tasks
+    pub dns_internal: DnsTasksConfig,
+    /// configuration for external DNS background tasks
+    pub dns_external: DnsTasksConfig,
+}
+
+#[serde_as]
+#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
+pub struct DnsTasksConfig {
+    /// period (in seconds) for periodic activations of the background task that
+    /// reads the latest DNS configuration from the database
+    #[serde_as(as = "DurationSeconds<u64>")]
+    pub period_secs_config: Duration,
+
+    /// period (in seconds) for periodic activations of the background task that
+    /// reads the latest list of DNS servers from the database
+    #[serde_as(as = "DurationSeconds<u64>")]
+    pub period_secs_servers: Duration,
+
+    /// period (in seconds) for periodic activations of the background task that
+    /// propagates the latest DNS configuration to the latest set of DNS servers
+    #[serde_as(as = "DurationSeconds<u64>")]
+    pub period_secs_propagation: Duration,
+
+    /// maximum number of concurrent DNS server updates
+    pub max_concurrent_server_updates: usize,
+}
+
 /// Configuration for a nexus server
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub struct PackageConfig {
@@ -278,8 +311,8 @@ pub struct PackageConfig {
     /// Timeseries database configuration.
     #[serde(default)]
     pub timeseries_db: TimeseriesDbConfig,
-    /// Updates-related configuration. Updates APIs return 400 Bad Request when this is
-    /// unconfigured.
+    /// Updates-related configuration. Updates APIs return 400 Bad Request when
+    /// this is unconfigured.
     #[serde(default)]
     pub updates: Option<UpdatesConfig>,
     /// Tunable configuration for testing and experimentation
@@ -288,6 +321,8 @@ pub struct PackageConfig {
     /// `Dendrite` dataplane daemon configuration
     #[serde(default)]
     pub dendrite: DpdConfig,
+    /// Background task configuration
+    pub background_tasks: BackgroundTaskConfig,
 }
 
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
@@ -361,7 +396,8 @@ mod test {
     };
     use crate::address::{Ipv6Subnet, RACK_PREFIX};
     use crate::nexus_config::{
-        Database, DeploymentConfig, DpdConfig, LoadErrorKind,
+        BackgroundTaskConfig, Database, DeploymentConfig, DnsTasksConfig,
+        DpdConfig, LoadErrorKind,
     };
     use dropshot::ConfigDropshot;
     use dropshot::ConfigLogging;
@@ -373,6 +409,7 @@ mod test {
     use std::path::Path;
     use std::path::PathBuf;
     use std::str::FromStr;
+    use std::time::Duration;
 
     /// Generates a temporary filesystem path unique for the given label.
     fn temp_path(label: &str) -> PathBuf {
@@ -494,6 +531,15 @@ mod test {
             type = "from_dns"
             [dendrite]
             address = "[::1]:12224"
+            [background_tasks]
+            dns_internal.period_secs_config = 1
+            dns_internal.period_secs_servers = 2
+            dns_internal.period_secs_propagation = 3
+            dns_internal.max_concurrent_server_updates = 4
+            dns_external.period_secs_config = 5
+            dns_external.period_secs_servers = 6
+            dns_external.period_secs_propagation = 7
+            dns_external.max_concurrent_server_updates = 8
             "##,
         )
         .unwrap();
@@ -548,6 +594,20 @@ mod test {
                             SocketAddr::from_str("[::1]:12224").unwrap()
                         )
                     },
+                    background_tasks: BackgroundTaskConfig {
+                        dns_internal: DnsTasksConfig {
+                            period_secs_config: Duration::from_secs(1),
+                            period_secs_servers: Duration::from_secs(2),
+                            period_secs_propagation: Duration::from_secs(3),
+                            max_concurrent_server_updates: 4,
+                        },
+                        dns_external: DnsTasksConfig {
+                            period_secs_config: Duration::from_secs(5),
+                            period_secs_servers: Duration::from_secs(6),
+                            period_secs_propagation: Duration::from_secs(7),
+                            max_concurrent_server_updates: 8,
+                        },
+                    },
                 },
             }
         );
@@ -584,6 +644,15 @@ mod test {
             type = "from_dns"
             [dendrite]
             address = "[::1]:12224"
+            [background_tasks]
+            dns_internal.period_secs_config = 1
+            dns_internal.period_secs_servers = 2
+            dns_internal.period_secs_propagation = 3
+            dns_internal.max_concurrent_server_updates = 4
+            dns_external.period_secs_config = 5
+            dns_external.period_secs_servers = 6
+            dns_external.period_secs_propagation = 7
+            dns_external.max_concurrent_server_updates = 8
             "##,
         )
         .unwrap();

common/src/nexus_config.rs

@@ -90,6 +90,7 @@ oximeter-producer.workspace = true
 [dev-dependencies]
 assert_matches.workspace = true
 criterion.workspace = true
+dns-server.workspace = true
 expectorate.workspace = true
 hyper-rustls.workspace = true
 itertools.workspace = true

nexus/Cargo.toml

@@ -12,7 +12,7 @@ impl_enum_type!(
     #[diesel(postgres_type(name = "service_kind"))]
     pub struct ServiceKindEnum;
 
-    #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, Serialize, Deserialize, PartialEq)]
+    #[derive(Clone, Copy, Debug, Eq, AsExpression, FromSqlRow, Serialize, Deserialize, PartialEq)]
     #[diesel(sql_type = ServiceKindEnum)]
     pub enum ServiceKind;
 

nexus/db-model/src/service_kind.rs

@@ -62,6 +62,7 @@ impl Authz {
 /// This is the primary external interface for the authorization subsystem,
 /// through which Nexus at-large makes authorization checks.  This is almost
 /// always done through [`OpContext::authorize()`].
+#[derive(Clone)]
 pub struct Context {
     authn: Arc<authn::Context>,
     authz: Arc<Authz>,

nexus/db-queries/src/authz/context.rs

@@ -46,6 +46,7 @@ pub struct OpContext {
     kind: OpKind,
 }
 
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum OpKind {
     /// Handling an external API request
     ExternalApiRequest,
@@ -200,6 +201,34 @@ impl OpContext {
         }
     }
 
+    /// Creates a new `OpContext` with extra metadata (including log metadata)
+    ///
+    /// This is intended for cases where you want an OpContext that's
+    /// functionally the same as one that you already have, but where you want
+    /// to provide extra debugging information (in the form of key-value pairs)
+    /// in both the OpContext itself and its logger.
+    pub fn child(&self, new_metadata: BTreeMap<String, String>) -> OpContext {
+        let created_instant = Instant::now();
+        let created_walltime = SystemTime::now();
+        let mut metadata = self.metadata.clone();
+        let mut log = self.log.clone();
+
+        for (k, v) in new_metadata {
+            metadata.insert(k.clone(), v.clone());
+            log = log.new(o!(k => v));
+        }
+
+        OpContext {
+            log,
+            authn: self.authn.clone(),
+            authz: self.authz.clone(),
+            created_instant,
+            created_walltime,
+            metadata,
+            kind: self.kind,
+        }
+    }
+
     /// Check whether the actor performing this request is authorized for
     /// `action` on `resource`.
     pub async fn authorize<Resource>(
@@ -254,6 +283,7 @@ mod test {
     use nexus_test_utils::db::test_setup_database;
     use omicron_common::api::external::Error;
     use omicron_test_utils::dev;
+    use std::collections::BTreeMap;
     use std::sync::Arc;
 
     #[tokio::test]
@@ -308,4 +338,51 @@ mod test {
         db.cleanup().await.unwrap();
         logctx.cleanup_successful();
     }
+
+    #[tokio::test]
+    async fn test_child_context() {
+        let logctx = dev::test_setup_log("test_child_context");
+        let mut db = test_setup_database(&logctx.log).await;
+        let (_, datastore) =
+            crate::db::datastore::datastore_test(&logctx, &db).await;
+        let opctx = OpContext::for_background(
+            logctx.log.new(o!()),
+            Arc::new(authz::Authz::new(&logctx.log)),
+            authn::Context::internal_unauthenticated(),
+            datastore,
+        );
+
+        let child_opctx = opctx.child(BTreeMap::from([
+            (String::from("one"), String::from("two")),
+            (String::from("three"), String::from("four")),
+        ]));
+        let grandchild_opctx = opctx.child(BTreeMap::from([
+            (String::from("one"), String::from("seven")),
+            (String::from("five"), String::from("six")),
+        ]));
+
+        // Verify they're the same "kind".
+        assert_eq!(opctx.kind, child_opctx.kind);
+        assert_eq!(opctx.kind, grandchild_opctx.kind);
+
+        // Verify that both descendants have metadata from the root.
+        for (k, v) in opctx.metadata.iter() {
+            assert_eq!(v, &child_opctx.metadata[k]);
+            assert_eq!(v, &grandchild_opctx.metadata[k]);
+        }
+
+        // The child opctx ought to have its own metadata and not any of its
+        // child's metadata.
+        assert_eq!(child_opctx.metadata["one"], "two");
+        assert_eq!(child_opctx.metadata["three"], "four");
+        assert!(!child_opctx.metadata.contains_key("five"));
+
+        // The granchild opctx ought to have its own metadata, one key of which
+        // overrides its parent's.
+        assert_eq!(grandchild_opctx.metadata["one"], "seven");
+        assert_eq!(grandchild_opctx.metadata["five"], "six");
+
+        db.cleanup().await.unwrap();
+        logctx.cleanup_successful();
+    }
 }

nexus/db-queries/src/context.rs

@@ -36,7 +36,7 @@ const NMAX_DNS_ZONES: u32 = 10;
 
 impl DataStore {
     /// List all DNS zones in a DNS group (paginated)
-    async fn dns_zones_list(
+    pub async fn dns_zones_list(
         &self,
         opctx: &OpContext,
         dns_group: DnsGroup,
@@ -53,7 +53,7 @@ impl DataStore {
     }
 
     /// Get the latest version for a given DNS group
-    async fn dns_group_latest_version(
+    pub async fn dns_group_latest_version(
         &self,
         opctx: &OpContext,
         dns_group: DnsGroup,