dnsdist: Be consistent with regard to health-check modes transition

Calling `setAuto()` on a backend used to set the health-check mode
to `active`, even if it had been set to `lazy` before, which was
quite confusing.
This commit introduces a new method, `setAutoActive()` which can
be used to change the health-check mode to `active`, and alters the
behaviour of `setAuto()` to restore the previous health-check mode
instead. This is a breaking change but since the default health-check
mode is `active` I don't expect to break any existing configurations.
It also introduces a new method, `getHealthCheckMode()`, to inspect
the current mode.

diff --git a/pdns/dnsdistdist/dnsdist-backend.cc b/pdns/dnsdistdist/dnsdist-backend.cc
index 435024ed615eabbcb948b33e3a577b84301f0b71..607c709c358aab69a83d640a4638c02726e89021 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-backend.cc
+++ b/pdns/dnsdistdist/dnsdist-backend.cc
@@ -310,7 +310,7 @@ DownstreamState::DownstreamState(DownstreamState::Config&& config, std::shared_p
     setWeight(d_config.d_weight);
   }
 
-  if (d_config.availability == Availability::Lazy && d_config.d_lazyHealthCheckSampleSize > 0) {
+  if (d_config.d_availability == Availability::Auto && d_config.d_healthCheckMode == HealthCheckMode::Lazy && d_config.d_lazyHealthCheckSampleSize > 0) {
     d_lazyHealthCheckStats.lock()->d_lastResults.set_capacity(d_config.d_lazyHealthCheckSampleSize);
     setUpStatus(true);
   }
@@ -475,7 +475,7 @@ void DownstreamState::handleUDPTimeout(IDState& ids)
 
 void DownstreamState::reportResponse(uint8_t rcode)
 {
-  if (d_config.availability == Availability::Lazy && d_config.d_lazyHealthCheckSampleSize > 0) {
+  if (d_config.d_availability == Availability::Auto && d_config.d_healthCheckMode == HealthCheckMode::Lazy && d_config.d_lazyHealthCheckSampleSize > 0) {
     bool failure = d_config.d_lazyHealthCheckMode == LazyHealthCheckMode::TimeoutOrServFail ? rcode == RCode::ServFail : false;
     d_lazyHealthCheckStats.lock()->d_lastResults.push_back(failure);
   }
@@ -483,7 +483,7 @@ void DownstreamState::reportResponse(uint8_t rcode)
 
 void DownstreamState::reportTimeoutOrError()
 {
-  if (d_config.availability == Availability::Lazy && d_config.d_lazyHealthCheckSampleSize > 0) {
+  if (d_config.d_availability == Availability::Auto && d_config.d_healthCheckMode == HealthCheckMode::Lazy && d_config.d_lazyHealthCheckSampleSize > 0) {
     d_lazyHealthCheckStats.lock()->d_lastResults.push_back(true);
   }
 }
@@ -674,7 +674,11 @@ std::optional<InternalQueryState> DownstreamState::getState(uint16_t id)
 
 bool DownstreamState::healthCheckRequired(std::optional<time_t> currentTime)
 {
-  if (d_config.availability == DownstreamState::Availability::Lazy) {
+  if (d_config.d_availability != DownstreamState::Availability::Auto) {
+    return false;
+  }
+
+  if (d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Lazy) {
     auto stats = d_lazyHealthCheckStats.lock();
     if (stats->d_status == LazyHealthCheckStats::LazyStatus::PotentialFailure) {
       vinfolog("Sending health-check query for %s which is still in the Potential Failure state", getNameWithAddr());
@@ -723,7 +727,7 @@ bool DownstreamState::healthCheckRequired(std::optional<time_t> currentTime)
 
     return false;
   }
-  else if (d_config.availability == DownstreamState::Availability::Auto) {
+  else if (d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Active) {
 
     if (d_nextCheck > 1) {
       --d_nextCheck;
@@ -807,7 +811,7 @@ void DownstreamState::submitHealthCheckResult(bool initial, bool newResult)
     setUpStatus(newResult);
     if (newResult == false) {
       currentCheckFailures++;
-      if (d_config.availability == DownstreamState::Availability::Lazy) {
+      if (d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Lazy) {
         auto stats = d_lazyHealthCheckStats.lock();
         stats->d_status = LazyHealthCheckStats::LazyStatus::Failed;
         updateNextLazyHealthCheck(*stats, false);
@@ -832,7 +836,7 @@ void DownstreamState::submitHealthCheckResult(bool initial, bool newResult)
            and we didn't reach the threshold yet, let's stay down */
         newState = false;
 
-        if (d_config.availability == DownstreamState::Availability::Lazy) {
+        if (d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Lazy) {
           auto stats = d_lazyHealthCheckStats.lock();
           updateNextLazyHealthCheck(*stats, false);
         }
@@ -840,7 +844,7 @@ void DownstreamState::submitHealthCheckResult(bool initial, bool newResult)
     }
 
     if (newState) {
-      if (d_config.availability == DownstreamState::Availability::Lazy) {
+      if (d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Lazy) {
         auto stats = d_lazyHealthCheckStats.lock();
         vinfolog("Backend %s had %d successful checks, moving to Healthy", getNameWithAddr(), std::to_string(consecutiveSuccessfulChecks));
         stats->d_status = LazyHealthCheckStats::LazyStatus::Healthy;
@@ -863,7 +867,7 @@ void DownstreamState::submitHealthCheckResult(bool initial, bool newResult)
            and we did not reach the threshold yet, let's stay up */
         newState = true;
       }
-      else if (d_config.availability == DownstreamState::Availability::Lazy) {
+      else if (d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Lazy) {
         auto stats = d_lazyHealthCheckStats.lock();
         vinfolog("Backend %s failed its health-check, moving from Potential failure to Failed", getNameWithAddr());
         stats->d_status = LazyHealthCheckStats::LazyStatus::Failed;
@@ -981,21 +985,27 @@ bool DownstreamState::parseSourceParameter(const std::string& source, Downstream
   return false;
 }
 
-std::optional<DownstreamState::Availability> DownstreamState::getAvailabilityFromStr(const std::string& mode)
+bool DownstreamState::parseAvailabilityConfigFromStr(DownstreamState::Config& config, const std::string str)
 {
-  if (pdns_iequals(mode, "auto")) {
-    return DownstreamState::Availability::Auto;
+  if (pdns_iequals(str, "auto")) {
+    config.d_availability = DownstreamState::Availability::Auto;
+    config.d_healthCheckMode = DownstreamState::HealthCheckMode::Active;
+    return true;
   }
-  if (pdns_iequals(mode, "lazy")) {
-    return DownstreamState::Availability::Lazy;
+  if (pdns_iequals(str, "lazy")) {
+    config.d_availability = DownstreamState::Availability::Auto;
+    config.d_healthCheckMode = DownstreamState::HealthCheckMode::Lazy;
+    return true;
   }
-  if (pdns_iequals(mode, "up")) {
-    return DownstreamState::Availability::Up;
+  if (pdns_iequals(str, "up")) {
+    config.d_availability = DownstreamState::Availability::Up;
+    return true;
   }
-  if (pdns_iequals(mode, "down")) {
-    return DownstreamState::Availability::Down;
+  if (pdns_iequals(str, "down")) {
+    config.d_availability = DownstreamState::Availability::Down;
+    return true;
   }
-  return std::nullopt;
+  return false;
 }
 
 size_t ServerPool::countServers(bool upOnly)

diff --git a/pdns/dnsdistdist/dnsdist-carbon.cc b/pdns/dnsdistdist/dnsdist-carbon.cc
index 81075c4ab16ee77d90c791987d16b6220516034f..b14f5359c9148aae844843c6e419d23ed7878d80 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-carbon.cc
+++ b/pdns/dnsdistdist/dnsdist-carbon.cc
@@ -90,8 +90,8 @@ static bool doOneCarbonExport(const Carbon::Endpoint& endpoint)
       str << base << "queries" << ' ' << state->queries.load() << " " << now << "\r\n";
       str << base << "responses" << ' ' << state->responses.load() << " " << now << "\r\n";
       str << base << "drops" << ' ' << state->reuseds.load() << " " << now << "\r\n";
-      str << base << "latency" << ' ' << (state->d_config.availability != DownstreamState::Availability::Down ? state->latencyUsec / 1000.0 : 0) << " " << now << "\r\n";
-      str << base << "latencytcp" << ' ' << (state->d_config.availability != DownstreamState::Availability::Down ? state->latencyUsecTCP / 1000.0 : 0) << " " << now << "\r\n";
+      str << base << "latency" << ' ' << (state->d_config.d_availability != DownstreamState::Availability::Down ? state->latencyUsec / 1000.0 : 0) << " " << now << "\r\n";
+      str << base << "latencytcp" << ' ' << (state->d_config.d_availability != DownstreamState::Availability::Down ? state->latencyUsecTCP / 1000.0 : 0) << " " << now << "\r\n";
       str << base << "senderrors" << ' ' << state->sendErrors.load() << " " << now << "\r\n";
       str << base << "outstanding" << ' ' << state->outstanding.load() << " " << now << "\r\n";
       str << base << "tcpdiedsendingquery" << ' ' << state->tcpDiedSendingQuery.load() << " " << now << "\r\n";

diff --git a/pdns/dnsdistdist/dnsdist-configuration-yaml.cc b/pdns/dnsdistdist/dnsdist-configuration-yaml.cc
index be82deb681198187f2b9d4318f6b21c5a704589e..68f06990f54c06dbc87115b8ce0d5ea4fe51a5cb 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-configuration-yaml.cc
+++ b/pdns/dnsdistdist/dnsdist-configuration-yaml.cc
@@ -417,10 +417,7 @@ static std::shared_ptr<DownstreamState> createBackendFromConfiguration(const dns
 
   getLuaFunctionFromConfiguration<DownstreamState::checkfunc_t>(backendConfig.checkFunction, hcConf.function, hcConf.lua, hcConf.lua_file, "backend health-check");
 
-  auto availability = DownstreamState::getAvailabilityFromStr(std::string(hcConf.mode));
-  if (availability) {
-    backendConfig.availability = *availability;
-  }
+  DownstreamState::parseAvailabilityConfigFromStr(backendConfig, std::string(hcConf.mode));
 
   backendConfig.d_lazyHealthCheckSampleSize = hcConf.lazy.sample_size;
   backendConfig.d_lazyHealthCheckMinSampleCount = hcConf.lazy.min_sample_count;

diff --git a/pdns/dnsdistdist/dnsdist-discovery.cc b/pdns/dnsdistdist/dnsdist-discovery.cc
index 07a2a85c6e70ffe4deeb7c293872917b83c6b78a..e65d54ecb390d6e413fbb64b987eca37ecfb73e0 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-discovery.cc
+++ b/pdns/dnsdistdist/dnsdist-discovery.cc
@@ -401,16 +401,18 @@ bool ServiceDiscovery::tryToUpgradeBackend(const UpgradeableBackend& backend)
   config.remote = discoveredConfig.d_addr;
   config.remote.setPort(discoveredConfig.d_port);
 
-  if (backend.keepAfterUpgrade && config.availability == DownstreamState::Availability::Up) {
+  if (backend.keepAfterUpgrade && config.d_availability == DownstreamState::Availability::Up) {
     /* it's OK to keep the forced state if we replace the initial
        backend, but if we are adding a new backend, it should not
        inherit that setting, especially since DoX backends are much
        more likely to fail (certificate errors, ...) */
     if (config.d_upgradeToLazyHealthChecks) {
-      config.availability = DownstreamState::Availability::Lazy;
+      config.d_availability = DownstreamState::Availability::Auto;
+      config.d_healthCheckMode = DownstreamState::HealthCheckMode::Lazy;
     }
     else {
-      config.availability = DownstreamState::Availability::Auto;
+      config.d_availability = DownstreamState::Availability::Auto;
+      config.d_healthCheckMode = DownstreamState::HealthCheckMode::Active;
     }
   }
 

diff --git a/pdns/dnsdistdist/dnsdist-lua-bindings.cc b/pdns/dnsdistdist/dnsdist-lua-bindings.cc
index 0a4c26772c8403c9bb077448f76f62a8e8827da1..a0e4ed0d6693b24fec729e9500d2d5bd50a75fa8 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua-bindings.cc
+++ b/pdns/dnsdistdist/dnsdist-lua-bindings.cc
@@ -125,12 +125,24 @@ void setupLuaBindings(LuaContext& luaCtx, bool client, bool configCheck)
   luaCtx.registerFunction("isUp", &DownstreamState::isUp);
   luaCtx.registerFunction("setDown", &DownstreamState::setDown);
   luaCtx.registerFunction("setUp", &DownstreamState::setUp);
+  luaCtx.registerFunction<std::string (DownstreamState::*)() const>("getHealthCheckMode", [](const DownstreamState& state) -> std::string {
+    if (state.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Active) {
+      return "active";
+    }
+    return "lazy";
+  });
   luaCtx.registerFunction<void (DownstreamState::*)(boost::optional<bool> newStatus)>("setAuto", [](DownstreamState& state, boost::optional<bool> newStatus) {
     if (newStatus) {
       state.setUpStatus(*newStatus);
     }
     state.setAuto();
   });
+  luaCtx.registerFunction<void (DownstreamState::*)(boost::optional<bool> newStatus)>("setActiveAuto", [](DownstreamState& state, boost::optional<bool> newStatus) {
+    if (newStatus) {
+      state.setUpStatus(*newStatus);
+    }
+    state.setActiveAuto();
+  });
   luaCtx.registerFunction<void (DownstreamState::*)(boost::optional<bool> newStatus)>("setLazyAuto", [](DownstreamState& state, boost::optional<bool> newStatus) {
     if (newStatus) {
       state.setUpStatus(*newStatus);

diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc
index f52ae0cfe28ba400eca88962cd4538d40cecd1e0..71397f2552d8eba42d604323a6a38a7e8bc2c301 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua.cc
+++ b/pdns/dnsdistdist/dnsdist-lua.cc
@@ -324,11 +324,7 @@ static void handleNewServerHealthCheckParameters(boost::optional<newserver_t>& v
 
   if (getOptionalValue<std::string>(vars, "healthCheckMode", valueStr) > 0) {
     const auto& mode = valueStr;
-    auto availability = DownstreamState::getAvailabilityFromStr(mode);
-    if (availability) {
-      config.availability = *availability;
-    }
-    else {
+    if (!DownstreamState::parseAvailabilityConfigFromStr(config, valueStr)) {
       warnlog("Ignoring unknown value '%s' for 'healthCheckMode' on 'newServer'", mode);
     }
   }

diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc
index ed8a28d621cfc93de1de363d19690c058a4b3d4a..2cba82f5e0340bf96ce5007b2f4102e9a6bcb1f2 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-web.cc
+++ b/pdns/dnsdistdist/dnsdist-web.cc
@@ -1069,10 +1069,10 @@ static void handleJSONStats(const YaHTTP::Request& req, YaHTTP::Response& resp)
 static void addServerToJSON(Json::array& servers, int identifier, const std::shared_ptr<DownstreamState>& backend)
 {
   string status;
-  if (backend->d_config.availability == DownstreamState::Availability::Up) {
+  if (backend->d_config.d_availability == DownstreamState::Availability::Up) {
     status = "UP";
   }
-  else if (backend->d_config.availability == DownstreamState::Availability::Down) {
+  else if (backend->d_config.d_availability == DownstreamState::Availability::Down) {
     status = "DOWN";
   }
   else {
@@ -1127,7 +1127,7 @@ static void addServerToJSON(Json::array& servers, int identifier, const std::sha
     {"dropRate", (double)backend->dropRate}};
 
   /* sending a latency for a DOWN server doesn't make sense */
-  if (backend->d_config.availability == DownstreamState::Availability::Down) {
+  if (backend->d_config.d_availability == DownstreamState::Availability::Down) {
     server["latency"] = nullptr;
     server["tcpLatency"] = nullptr;
   }

diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc
index 23cda549eae9ccc0b218c80c59f79511ed20293b..b35b40628b47b1483834a056bc5f2828eb564d23 100644 (file)
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -3631,8 +3631,8 @@ int main(int argc, char** argv)
       auto mplexer = std::unique_ptr<FDMultiplexer>(FDMultiplexer::getMultiplexerSilent(states.size()));
       for (auto& dss : states) {
 
-        if (dss->d_config.availability == DownstreamState::Availability::Auto || dss->d_config.availability == DownstreamState::Availability::Lazy) {
-          if (dss->d_config.availability == DownstreamState::Availability::Auto) {
+        if (dss->d_config.d_availability == DownstreamState::Availability::Auto) {
+          if (dss->d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Active) {
             dss->d_nextCheck = dss->d_config.checkInterval;
           }
 

diff --git a/pdns/dnsdistdist/dnsdist.hh b/pdns/dnsdistdist/dnsdist.hh
index ddfaf0d8abb8cd4638909b32cf5433a88f6f74e3..3c3c135659a2b567a03c7326ed50a3644ffa4d55 100644 (file)
--- a/pdns/dnsdistdist/dnsdist.hh
+++ b/pdns/dnsdistdist/dnsdist.hh
@@ -534,7 +534,11 @@ struct DownstreamState : public std::enable_shared_from_this<DownstreamState>
   {
     Up,
     Down,
-    Auto,
+    Auto
+  };
+  enum class HealthCheckMode : uint8_t
+  {
+    Active,
     Lazy
   };
   enum class LazyHealthCheckMode : uint8_t
@@ -595,7 +599,8 @@ struct DownstreamState : public std::enable_shared_from_this<DownstreamState>
     uint8_t minRiseSuccesses{1};
     uint8_t udpTimeout{0};
     uint8_t dscp{0};
-    Availability availability{Availability::Auto};
+    Availability d_availability{Availability::Auto};
+    HealthCheckMode d_healthCheckMode{HealthCheckMode::Active};
     bool d_tlsSubjectIsAddr{false};
     bool mustResolve{false};
     bool useECS{false};
@@ -726,7 +731,7 @@ private:
 
 public:
   static bool parseSourceParameter(const std::string& source, Config& config);
-  static std::optional<DownstreamState::Availability> getAvailabilityFromStr(const std::string& mode);
+  static bool parseAvailabilityConfigFromStr(DownstreamState::Config& config, const std::string str);
 
   void updateStatisticsInfo()
   {
@@ -740,10 +745,10 @@ public:
 
   bool isUp() const
   {
-    if (d_config.availability == Availability::Down) {
+    if (d_config.d_availability == Availability::Down) {
       return false;
     }
-    else if (d_config.availability == Availability::Up) {
+    else if (d_config.d_availability == Availability::Up) {
       return true;
     }
     return upStatus.load(std::memory_order_relaxed);
@@ -751,7 +756,7 @@ public:
 
   void setUp()
   {
-    d_config.availability = Availability::Up;
+    d_config.d_availability = Availability::Up;
   }
 
   void setUpStatus(bool newStatus)
@@ -764,17 +769,23 @@ public:
   }
   void setDown()
   {
-    d_config.availability = Availability::Down;
+    d_config.d_availability = Availability::Down;
     latencyUsec = 0.0;
     latencyUsecTCP = 0.0;
   }
   void setAuto()
   {
-    d_config.availability = Availability::Auto;
+    d_config.d_availability = Availability::Auto;
+  }
+  void setActiveAuto()
+  {
+    d_config.d_availability = Availability::Auto;
+    d_config.d_healthCheckMode = HealthCheckMode::Active;
   }
   void setLazyAuto()
   {
-    d_config.availability = Availability::Lazy;
+    d_config.d_availability = Availability::Auto;
+    d_config.d_healthCheckMode = HealthCheckMode::Lazy;
     d_lazyHealthCheckStats.lock()->d_lastResults.set_capacity(d_config.d_lazyHealthCheckSampleSize);
   }
   bool healthCheckRequired(std::optional<time_t> currentTime = std::nullopt);
@@ -796,10 +807,10 @@ public:
   string getStatus() const
   {
     string status;
-    if (d_config.availability == DownstreamState::Availability::Up) {
+    if (d_config.d_availability == DownstreamState::Availability::Up) {
       status = "UP";
     }
-    else if (d_config.availability == DownstreamState::Availability::Down) {
+    else if (d_config.d_availability == DownstreamState::Availability::Down) {
       status = "DOWN";
     }
     else {

diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst
index b8f8eab9d95a0c8b1693e7486e5a07d0676a3469..4e706c21d60dccae5345f785bb610ce933d55c78 100644 (file)
--- a/pdns/dnsdistdist/docs/reference/config.rst
+++ b/pdns/dnsdistdist/docs/reference/config.rst
@@ -829,6 +829,15 @@ A server object returned by :func:`getServer` can be manipulated with these func
 
     :returns: The number of dropped queries
 
+  .. method:: Server:getHealthCheckMode() -> str
+
+    .. versionadded:: 2.0.0
+
+    Get the current health-check mode, ``active`` or ``lazy``. Note that health-checks might be disabled because :meth:`Server:setUp` or :meth:`Server:setDown`
+    were called, in which case this method will return the health-check more that will be restored ud :meth:`Server:setAuto` is called.
+
+    :returns: The current health-check mode
+
   .. method:: Server:getOutstanding() -> int
 
     Get the number of outstanding queries for this server.
@@ -839,7 +848,7 @@ A server object returned by :func:`getServer` can be manipulated with these func
 
     Returns the up status of the server.
     Result is based on the administrative status of the server (as set by either :meth:`Server:setDown` or :meth:`Server:setUp`).
-    If no administrative status is set (see :meth:`Server:setAuto` and :meth:`Server:setLazyAuto`), result is based on :attr:`Server.upStatus`
+    If no administrative status is set (see :meth:`Server:setAuto`, :meth:`Server:setActiveAuto` and :meth:`Server:setLazyAuto`), result is based on :attr:`Server.upStatus`
 
     :returns: true when the server is up, false otherwise
 
@@ -849,11 +858,23 @@ A server object returned by :func:`getServer` can be manipulated with these func
 
     :param str pool: The pool to remove the server from
 
+  .. method:: Server:setActiveAuto([status])
+
+    .. versionadded:: 2.0.0
+
+    Set the server in the 'active' health-check mode, which will send health-check queries to the backend every ``checkInterval`` seconds.
+    See also :meth:`Server:setLazyAuto` for a passive mode where health-check queries are only sent after a configurable threshold of regular queries failing,
+    and :ref:`Healthcheck` for a more detailed explanation.
+
+    :param bool status: Set the initial status of the server to ``up`` (true) or ``down`` (false) instead of using the last known status
+
   .. method:: Server:setAuto([status])
 
-    Set the server in the default ``auto`` state, regularly sending health-check queries to the backend. See also :meth:`Server:setLazyAuto` for a
-    passive mode where health-check queries are only sent after a configurable threshold of regular queries failing.
-    This will enable health check queries that will set the server ``up`` and ``down`` appropriately.
+    .. versionchanged:: 2.0.0
+      Before 2.0.0 this option forced the health-check mode to ``active`` (see :meth:`Server:setActiveAuto`). After 2.0.0 it restores the previous health-check mode instead.
+
+    Set the server in the default ``auto`` state, enabling health check queries that will set the server ``up`` and ``down`` appropriately.
+    See :meth:`Server:setActiveAuto`, :meth:`Server:setLazyAuto` and :ref:`Healthcheck` to understand the different health-check modes.
 
     :param bool status: Set the initial status of the server to ``up`` (true) or ``down`` (false) instead of using the last known status
 

diff --git a/pdns/dnsdistdist/docs/upgrade_guide.rst b/pdns/dnsdistdist/docs/upgrade_guide.rst
index b221f36091cd2e0edaff425761e4da8602a53c32..165eccffaf931b205bc80bc34642f4bd1aa3d0cf 100644 (file)
--- a/pdns/dnsdistdist/docs/upgrade_guide.rst
+++ b/pdns/dnsdistdist/docs/upgrade_guide.rst
@@ -13,6 +13,8 @@ Our eBPF filtering code no longer treats the ``255``/``ANY`` qtype as a special
 
 XPF support has been removed.
 
+:meth:`Server:setAuto` used to reset the health-check mode to ``active`` even if it had previously been set to ``lazy`` via :meth:`Server:setLazyAuto`. This is no longer the case, and :meth:`Server:setActiveAuto` should be used instead to set the health-check mode to ``Active``.
+
 1.8.x to 1.9.0
 --------------
 

diff --git a/pdns/dnsdistdist/test-dnsdistbackend_cc.cc b/pdns/dnsdistdist/test-dnsdistbackend_cc.cc
index 28bf5109a79b65ec494e337a967db00d86a1b46a..3b93b9cf7a88670b981ef67550fb579e26661231 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdistbackend_cc.cc
+++ b/pdns/dnsdistdist/test-dnsdistbackend_cc.cc
@@ -15,31 +15,32 @@ BOOST_AUTO_TEST_CASE(test_Basic)
 {
   DownstreamState::Config config;
   DownstreamState ds(std::move(config), nullptr, false);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Active);
   BOOST_CHECK_EQUAL(ds.isUp(), false);
   BOOST_CHECK_EQUAL(ds.getStatus(), "down");
   BOOST_CHECK_EQUAL(ds.healthCheckRequired(), true);
 
   ds.setUp();
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Up);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Up);
   BOOST_CHECK_EQUAL(ds.isUp(), true);
   BOOST_CHECK_EQUAL(ds.getStatus(), "UP");
   BOOST_CHECK_EQUAL(ds.healthCheckRequired(), false);
 
   ds.setDown();
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Down);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Down);
   BOOST_CHECK_EQUAL(ds.isUp(), false);
   BOOST_CHECK_EQUAL(ds.getStatus(), "DOWN");
   BOOST_CHECK_EQUAL(ds.healthCheckRequired(), false);
 
   ds.setAuto();
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), false);
   BOOST_CHECK_EQUAL(ds.getStatus(), "down");
   BOOST_CHECK_EQUAL(ds.healthCheckRequired(), true);
 
   ds.submitHealthCheckResult(true, true);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), true);
   BOOST_CHECK_EQUAL(ds.getStatus(), "up");
   BOOST_CHECK_EQUAL(ds.healthCheckRequired(), true);
@@ -54,7 +55,8 @@ BOOST_AUTO_TEST_CASE(test_MaxCheckFailures)
   config.remote = ComboAddress("0.0.0.0");
 
   DownstreamState ds(std::move(config), nullptr, false);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Active);
   ds.setUpStatus(true);
   BOOST_CHECK_EQUAL(ds.isUp(), true);
   BOOST_CHECK_EQUAL(ds.getStatus(), "up");
@@ -64,19 +66,19 @@ BOOST_AUTO_TEST_CASE(test_MaxCheckFailures)
   }
 
   /* four failed checks is not enough */
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), true);
   BOOST_CHECK_EQUAL(ds.getStatus(), "up");
 
   /* but five is */
   ds.submitHealthCheckResult(false, false);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), false);
   BOOST_CHECK_EQUAL(ds.getStatus(), "down");
 
   /* only one successful check is needed to go back up */
   ds.submitHealthCheckResult(false, true);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), true);
   BOOST_CHECK_EQUAL(ds.getStatus(), "up");
 }
@@ -90,7 +92,8 @@ BOOST_AUTO_TEST_CASE(test_Rise)
   config.remote = ComboAddress("0.0.0.0");
 
   DownstreamState ds(std::move(config), nullptr, false);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Active);
   BOOST_CHECK_EQUAL(ds.isUp(), false);
   BOOST_CHECK_EQUAL(ds.getStatus(), "down");
 
@@ -99,19 +102,19 @@ BOOST_AUTO_TEST_CASE(test_Rise)
   }
 
   /* four successful checks is not enough */
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), false);
   BOOST_CHECK_EQUAL(ds.getStatus(), "down");
 
   /* but five is */
   ds.submitHealthCheckResult(false, true);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), true);
   BOOST_CHECK_EQUAL(ds.getStatus(), "up");
 
   /* only one failed check is needed to go back down */
   ds.submitHealthCheckResult(false, false);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), false);
   BOOST_CHECK_EQUAL(ds.getStatus(), "down");
 }
@@ -124,12 +127,14 @@ BOOST_AUTO_TEST_CASE(test_Lazy)
   config.d_lazyHealthCheckMinSampleCount = 11;
   config.d_lazyHealthCheckThreshold = 20;
   config.d_lazyHealthCheckUseExponentialBackOff = false;
-  config.availability = DownstreamState::Availability::Lazy;
+  config.d_availability = DownstreamState::Availability::Auto;
+  config.d_healthCheckMode = DownstreamState::HealthCheckMode::Lazy;
   /* prevents a re-connection */
   config.remote = ComboAddress("0.0.0.0");
 
   DownstreamState ds(std::move(config), nullptr, false);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Lazy);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
   BOOST_CHECK_EQUAL(ds.isUp(), true);
   BOOST_CHECK_EQUAL(ds.getStatus(), "up");
   BOOST_CHECK_EQUAL(ds.healthCheckRequired(), false);
@@ -227,12 +232,14 @@ BOOST_AUTO_TEST_CASE(test_LazyExponentialBackOff)
   config.d_lazyHealthCheckUseExponentialBackOff = true;
   config.d_lazyHealthCheckMaxBackOff = 600;
   config.d_lazyHealthCheckFailedInterval = 15;
-  config.availability = DownstreamState::Availability::Lazy;
+  DownstreamState::parseAvailabilityConfigFromStr(config, "lazy");
+
   /* prevents a re-connection */
   config.remote = ComboAddress("0.0.0.0");
 
   DownstreamState ds(std::move(config), nullptr, false);
-  BOOST_CHECK(ds.d_config.availability == DownstreamState::Availability::Lazy);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Lazy);
   BOOST_CHECK_EQUAL(ds.isUp(), true);
   BOOST_CHECK_EQUAL(ds.getStatus(), "up");
   BOOST_CHECK_EQUAL(ds.healthCheckRequired(), false);
@@ -291,4 +298,29 @@ BOOST_AUTO_TEST_CASE(test_LazyExponentialBackOff)
   BOOST_CHECK_EQUAL(ds.healthCheckRequired(), false);
 }
 
+BOOST_AUTO_TEST_CASE(test_CheckAutoRestorePreviousHealthCheckMode)
+{
+  DownstreamState::Config config;
+  DownstreamState::parseAvailabilityConfigFromStr(config, "lazy");
+  /* prevents a re-connection */
+  config.remote = ComboAddress("0.0.0.0");
+
+  DownstreamState ds(std::move(config), nullptr, false);
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Lazy);
+  ds.setUp();
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Up);
+  ds.setAuto();
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Lazy);
+  ds.setActiveAuto();
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Active);
+  ds.setUp();
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Up);
+  ds.setAuto();
+  BOOST_CHECK(ds.d_config.d_availability == DownstreamState::Availability::Auto);
+  BOOST_CHECK(ds.d_config.d_healthCheckMode == DownstreamState::HealthCheckMode::Active);
+}
+
 BOOST_AUTO_TEST_SUITE_END()

diff --git a/regression-tests.dnsdist/test_HealthChecks.py b/regression-tests.dnsdist/test_HealthChecks.py
index 7a5c6b7e0c06f79af6ccd676e7d103970a1f9044..f90fae62ffc417aabb1c2018929e7a37d7cefdcf 100644 (file)
--- a/regression-tests.dnsdist/test_HealthChecks.py
+++ b/regression-tests.dnsdist/test_HealthChecks.py
@@ -56,6 +56,7 @@ class TestDefaultHealthCheck(HealthCheckTest):
 
         self.sendConsoleCommand("getServer(0):setUp()")
         self.assertEqual(self.getBackendStatus(), 'up')
+        self.assertEqual(self.sendConsoleCommand("getServer(0):getHealthCheckMode()").rstrip(), "active")
 
         before = TestDefaultHealthCheck._healthCheckCounter
         time.sleep(1.5)
@@ -71,6 +72,7 @@ class TestDefaultHealthCheck(HealthCheckTest):
         self.sendConsoleCommand("getServer(0):setAuto()")
         # we get back the previous state, which was up
         self.assertEqual(self.getBackendStatus(), 'up')
+        self.assertEqual(self.sendConsoleCommand("getServer(0):getHealthCheckMode()").rstrip(), "active")
 
         before = TestDefaultHealthCheck._healthCheckCounter
         time.sleep(1.5)
@@ -79,6 +81,7 @@ class TestDefaultHealthCheck(HealthCheckTest):
 
         self.sendConsoleCommand("getServer(0):setDown()")
         self.assertEqual(self.getBackendStatus(), 'down')
+        self.assertEqual(self.sendConsoleCommand("getServer(0):getHealthCheckMode()").rstrip(), "active")
         self.sendConsoleCommand("getServer(0):setAuto(false)")
 
         before = TestDefaultHealthCheck._healthCheckCounter
@@ -86,6 +89,15 @@ class TestDefaultHealthCheck(HealthCheckTest):
         self.assertGreater(TestDefaultHealthCheck._healthCheckCounter, before)
         self.assertEqual(self.getBackendStatus(), 'up')
         self.assertEqual(self.getBackendMetric(0, 'healthCheckFailures'), 0)
+        self.assertEqual(self.sendConsoleCommand("getServer(0):getHealthCheckMode()").rstrip(), "active")
+
+        self.sendConsoleCommand("getServer(0):setLazyAuto()")
+        self.assertEqual(self.sendConsoleCommand("getServer(0):getHealthCheckMode()").rstrip(), "lazy")
+        self.sendConsoleCommand("getServer(0):setDown()")
+        self.sendConsoleCommand("getServer(0):setAuto()")
+        self.assertEqual(self.sendConsoleCommand("getServer(0):getHealthCheckMode()").rstrip(), "lazy")
+        self.sendConsoleCommand("getServer(0):setActiveAuto()")
+        self.assertEqual(self.sendConsoleCommand("getServer(0):getHealthCheckMode()").rstrip(), "active")
 
 class TestHealthCheckForcedUP(HealthCheckTest):
     # this test suite uses a different responder port