fix(dns): add a lower-bound negative TTL (#4449)

see linkerd/linkerd2#14954.

some user reports describe situations in which, when the linkerd control
plane's destination controller is OOM-killed, DNS resolution can
momentarily cause the proxy to compute a negative-TTL duration of zero.

this causes a panic in production environments, because
`tokio::time::interval` asserts that it has not been provided a duration
of zero. this manifests in errors that look like this:

```
thread 'main' panicked at linkerd/app/core/src/control.rs:118:49: period must be non-zero.
```

this commit introduces a minimum negative TTL that will be used when
backing off from resolution errors.

this commit moves the logic responsible for enforcing a minimum TTL
(added #3807) from the outer layer in
`linkerd-dns-resolve` and into the `linkerd-dns` library.

this helps us reuse/consolidate the same logic for negative TTL's when
recovering from resolution errors, and will facilitate making this
configurable in the future, should we so desire.

X-Ref: #3807
Signed-off-by: katelyn martin <kate@buoyant.io>

Author: cratelyn

linkerd/dns/src/lib.rs

@@ -11,6 +11,8 @@ use thiserror::Error;
 use tokio::time::{self, Instant};
 use tracing::{debug, trace};
 
+pub mod minimum_ttl;
+
 #[derive(Clone)]
 pub struct Resolver {
     dns: TokioResolver,
@@ -201,32 +203,40 @@ impl fmt::Debug for Resolver {
 // === impl ResolveError ===
 
 impl ResolveError {
-    /// Returns the amount of time that the resolver should wait before
-    /// retrying.
+    /// Returns the amount of time that the resolver should wait before retrying.
     pub fn negative_ttl(&self) -> Option<time::Duration> {
-        if let Some(hickory_resolver::proto::ProtoErrorKind::NoRecordsFound {
-            negative_ttl: Some(ttl_secs),
-            ..
-        }) = self
-            .a_error
-            .0
-            .proto()
-            .map(hickory_resolver::proto::ProtoError::kind)
-        {
-            return Some(time::Duration::from_secs(*ttl_secs as u64));
+        let Self {
+            a_error: ARecordError(a_error),
+            srv_error,
+        } = self;
+
+        if let ttl @ Some(_) = Self::negative_ttl_of(a_error) {
+            return ttl;
         }
 
-        if let SrvRecordError::Resolve(error) = &self.srv_error {
-            if let Some(hickory_resolver::proto::ProtoErrorKind::NoRecordsFound {
-                negative_ttl: Some(ttl_secs),
-                ..
-            }) = error.proto().map(hickory_resolver::proto::ProtoError::kind)
-            {
-                return Some(time::Duration::from_secs(*ttl_secs as u64));
-            }
+        match srv_error {
+            SrvRecordError::Resolve(srv_error) => Self::negative_ttl_of(srv_error),
+            SrvRecordError::Invalid(_) => None,
         }
+    }
+
+    /// Returns the negative TTL [`time::Duration`] of a [`hickory_resolver::ResolveError`].
+    ///
+    /// This function will defensively enforce a minimum negative TTL.
+    fn negative_ttl_of(error: &hickory_resolver::ResolveError) -> Option<time::Duration> {
+        use hickory_resolver::proto::{ProtoError, ProtoErrorKind};
 
-        None
+        let Some(ProtoErrorKind::NoRecordsFound {
+            negative_ttl: Some(ttl_secs),
+            ..
+        }) = error.proto().map(ProtoError::kind)
+        else {
+            return None;
+        };
+
+        let ttl = time::Duration::from_secs(*ttl_secs as u64);
+        let ttl = minimum_ttl::with_minimum_duration(ttl);
+        Some(ttl)
     }
 }
 

linkerd/dns/src/minimum_ttl.rs

@@ -0,0 +1,43 @@
+//! Minimum TTL enforcement.
+//!
+//! This module contains functions to enforce a lower-bound for TTL's (including negative TTL's
+//! used during error recovery) to prevent DNS resolution from spinning in a hot-loop.
+
+use tokio::time::{Duration, Instant};
+use tracing::debug;
+
+/// The minimum TTL duration that will be respected.
+const MINIMUM_TTL: Duration = Duration::from_secs(5);
+
+/// Apply a lower-bound to the given [`Instant`].
+///
+/// NB: This enforces a lower-bound for TTL's to prevent DNS resolution from spinning in a
+/// hot-loop.
+#[tracing::instrument(level = "debug")]
+pub fn with_minimum_expiry(valid_until: Instant) -> Instant {
+    let minimum = Instant::now() + MINIMUM_TTL;
+
+    // Choose a deadline; if the expiry is too short, fall back to the minimum TTL.
+    let deadline = if valid_until >= minimum {
+        valid_until
+    } else {
+        debug!(ttl.min = ?MINIMUM_TTL, "Given TTL too short, using a minimum TTL");
+        minimum
+    };
+
+    deadline
+}
+
+/// Apply a lower-bound to the given [`Duration`].
+///
+/// NB: This enforces a lower-bound for negative TTL's to prevent DNS resolution error recovery
+/// from spinning in a hot-loop.
+pub fn with_minimum_duration(ttl: Duration) -> Duration {
+    if ttl < MINIMUM_TTL {
+        // Choose a deadline; if the expiry is too short, fall back to the minimum TTL.
+        debug!(ttl.min = ?MINIMUM_TTL, ?ttl, "Given Negative TTL too short, using a minimum TTL");
+        return MINIMUM_TTL;
+    }
+
+    ttl
+}

linkerd/proxy/dns-resolve/src/lib.rs

@@ -64,6 +64,8 @@ impl<T: Param<Addr>> tower::Service<T> for DnsResolve {
 }
 
 async fn resolution(dns: dns::Resolver, na: NameAddr) -> Result<UpdateStream, Error> {
+    use linkerd_dns::minimum_ttl::with_minimum_expiry;
+    use tokio::time::sleep_until;
     use tokio_stream::wrappers::ReceiverStream;
 
     // Don't return a stream before the initial resolution completes. Then,
@@ -80,7 +82,7 @@ async fn resolution(dns: dns::Resolver, na: NameAddr) -> Result<UpdateStream, Er
                 trace!("Closed");
                 return;
             }
-            sleep_until_expired(expiry).await;
+            sleep_until(with_minimum_expiry(expiry)).await;
 
             loop {
                 match dns.resolve_addrs(na.name().as_ref(), na.port()).await {
@@ -91,7 +93,7 @@ async fn resolution(dns: dns::Resolver, na: NameAddr) -> Result<UpdateStream, Er
                             trace!("Closed");
                             return;
                         }
-                        sleep_until_expired(expiry).await;
+                        sleep_until(with_minimum_expiry(expiry)).await;
                     }
                     Err(error) => {
                         debug!(%error);
@@ -107,26 +109,3 @@ async fn resolution(dns: dns::Resolver, na: NameAddr) -> Result<UpdateStream, Er
 
     Ok(Box::pin(ReceiverStream::new(rx)))
 }
-
-/// Sleep for the provided [`Duration`][tokio::time::Duration].
-///
-/// NB: This enforces a lower-bound for TTL's to prevent [`resolution()`], above, from spinning
-/// in a hot-loop.
-#[tracing::instrument(level = "debug")]
-async fn sleep_until_expired(valid_until: tokio::time::Instant) {
-    use tokio::time::{sleep_until, Duration, Instant};
-
-    /// The minimum TTL duration that will be respected.
-    const MINIMUM_TTL: Duration = Duration::from_secs(5);
-    let minimum = Instant::now() + MINIMUM_TTL;
-
-    // Choose a deadline; if the expiry is too short, fall back to the minimum TTL.
-    let deadline = if valid_until >= minimum {
-        valid_until
-    } else {
-        debug!(ttl.min = ?MINIMUM_TTL, "Given TTL too short, using a minimum TTL");
-        minimum
-    };
-
-    sleep_until(deadline).await;
-}