From: Remi Gacogne Date: Tue, 23 Sep 2025 13:06:58 +0000 (+0200) Subject: dnsdist: Add unit tests for server consistency X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5a8c68ca564c08c62fe02ebb67d3133a73351fab;p=thirdparty%2Fpdns.git dnsdist: Add unit tests for server consistency Signed-off-by: Remi Gacogne --- diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am index a493f453a2..918dfa98d7 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/pdns/dnsdistdist/Makefile.am @@ -411,6 +411,7 @@ testrunner_SOURCES = \ test-dnsdistpacketcache_cc.cc \ test-dnsdistrings_cc.cc \ test-dnsdistrules_cc.cc \ + test-dnsdistserverpool.cc \ test-dnsdistsvc_cc.cc \ test-dnsdisttcp_cc.cc \ test-dnsparser_cc.cc \ diff --git a/pdns/dnsdistdist/dnsdist-backend.cc b/pdns/dnsdistdist/dnsdist-backend.cc index a04b36e3eb..9bd23b966e 100644 --- a/pdns/dnsdistdist/dnsdist-backend.cc +++ b/pdns/dnsdistdist/dnsdist-backend.cc @@ -1099,10 +1099,11 @@ void ServerPool::removeServer(shared_ptr& server) void ServerPool::updateConsistency() { + bool consistent{true}; bool first{true}; bool useECS{false}; bool tcpOnly{false}; - bool zeroScope{false}; + bool zeroScope{true}; for (const auto& serverPair : d_servers) { const auto& server = serverPair.second; @@ -1113,23 +1114,30 @@ void ServerPool::updateConsistency() zeroScope = !server->d_config.disableZeroScope; } else { - if (server->d_config.useECS != useECS || - server->isTCPOnly() != tcpOnly || - server->d_config.disableZeroScope == zeroScope) { - d_tcpOnly = false; - d_isConsistent = false; - return; + if (consistent) { + if (server->d_config.useECS != useECS) { + consistent = false; + } + if (server->d_config.disableZeroScope == zeroScope) { + consistent = false; + } + } + if (server->isTCPOnly() != tcpOnly) { + consistent = false; + tcpOnly = false; } } } d_tcpOnly = tcpOnly; - /* at this point we know that all servers agree - on these settings, so let's just use the same - values for the pool itself */ - d_useECS = useECS; - d_zeroScope = zeroScope; - d_isConsistent = true; + if (consistent) { + /* at this point we know that all servers agree + on these settings, so let's just use the same + values for the pool itself */ + d_useECS = useECS; + d_zeroScope = zeroScope; + } + d_isConsistent = consistent; } void ServerPool::setZeroScope(bool enabled) diff --git a/pdns/dnsdistdist/dnsdist-server-pool.hh b/pdns/dnsdistdist/dnsdist-server-pool.hh index 2336e8e9bc..78ba591aa9 100644 --- a/pdns/dnsdistdist/dnsdist-server-pool.hh +++ b/pdns/dnsdistdist/dnsdist-server-pool.hh @@ -38,6 +38,10 @@ struct ServerPool return d_useECS; } + /* Note that the pool will do a consistency check, + and might decide to override the supplied value + if all backends in the pool have the same ECS + value and the value differs from the supplied one */ void setECS(bool useECS); bool getZeroScope() const @@ -45,6 +49,10 @@ struct ServerPool return d_zeroScope; } + /* Note that the pool will do a consistency check, + and might decide to override the supplied value + if all backends in the pool have the same disable zero scope setting + value and the value differs from the supplied one */ void setZeroScope(bool enabled); bool isConsistent() const @@ -52,6 +60,7 @@ struct ServerPool return d_isConsistent; } + /* sum of outstanding queries for all servers in this pool */ size_t poolLoad() const; size_t countServers(bool upOnly) const; bool hasAtLeastOneServerAvailable() const; @@ -60,6 +69,7 @@ struct ServerPool void removeServer(std::shared_ptr& server); bool isTCPOnly() const { + // coverity[missing_lock] return d_tcpOnly; } diff --git a/pdns/dnsdistdist/meson.build b/pdns/dnsdistdist/meson.build index 75ff2e0599..9f13bec05d 100644 --- a/pdns/dnsdistdist/meson.build +++ b/pdns/dnsdistdist/meson.build @@ -537,6 +537,7 @@ test_sources += files( src_dir / 'test-dnsdistrings_cc.cc', src_dir / 'test-dnsdistrules_cc.cc', src_dir / 'test-dnsdistsvc_cc.cc', + src_dir / 'test-dnsdistserverpool.cc', src_dir / 'test-dnsdisttcp_cc.cc', src_dir / 'test-dnsparser_cc.cc', src_dir / 'test-iputils_hh.cc', diff --git a/pdns/dnsdistdist/test-dnsdistserverpool.cc b/pdns/dnsdistdist/test-dnsdistserverpool.cc new file mode 100644 index 0000000000..07c4ad472b --- /dev/null +++ b/pdns/dnsdistdist/test-dnsdistserverpool.cc @@ -0,0 +1,191 @@ +/* + * This file is part of PowerDNS or dnsdist. + * Copyright -- PowerDNS.COM B.V. and its contributors + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * In addition, for the avoidance of any doubt, permission is granted to + * link this program with OpenSSL and to (re)distribute the binaries + * produced as the result of such linking. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef BOOST_TEST_DYN_LINK +#define BOOST_TEST_DYN_LINK +#endif +#define BOOST_TEST_NO_MAIN + +#include + +#include "dnsdist-server-pool.hh" +#include "dnsdist.hh" + +BOOST_AUTO_TEST_SUITE(dnsdistserverpool) + +BOOST_AUTO_TEST_CASE(test_ServerPoolBasics) +{ + ServerPool pool{}; + BOOST_CHECK(!pool.getCache()); + + BOOST_CHECK(!pool.getECS()); + /* will be ignored because there is no server with ECS + in the pool at this point */ + pool.setECS(true); + BOOST_CHECK(!pool.getECS()); + pool.setECS(false); + BOOST_CHECK(!pool.getECS()); + + BOOST_CHECK(pool.getZeroScope()); + /* will be ignored because there is no server with the + zero scope feature disabled in the pool at this point */ + pool.setZeroScope(false); + BOOST_CHECK(pool.getZeroScope()); + pool.setZeroScope(true); + BOOST_CHECK(pool.getZeroScope()); + + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(pool.isConsistent()); + BOOST_CHECK(!pool.hasAtLeastOneServerAvailable()); + BOOST_CHECK_EQUAL(pool.countServers(true), 0U); + BOOST_CHECK_EQUAL(pool.countServers(false), 0U); + BOOST_CHECK_EQUAL(pool.poolLoad(), 0U); + + { + const auto& servers = pool.getServers(); + BOOST_CHECK(servers.empty()); + } + + { + /* add one server */ + DownstreamState::Config config; + auto ds = std::make_shared(std::move(config), nullptr, false); + pool.addServer(ds); + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(pool.isConsistent()); + /* the server is unavailable by default */ + BOOST_CHECK(!pool.hasAtLeastOneServerAvailable()); + BOOST_CHECK_EQUAL(pool.countServers(true), 0U); + BOOST_CHECK_EQUAL(pool.countServers(false), 1U); + + BOOST_CHECK_EQUAL(pool.poolLoad(), 0U); + { + const auto& servers = pool.getServers(); + BOOST_CHECK(!servers.empty()); + } + + /* let's make it available */ + ds->setUp(); + BOOST_CHECK(pool.hasAtLeastOneServerAvailable()); + BOOST_CHECK_EQUAL(pool.countServers(true), 1U); + BOOST_CHECK_EQUAL(pool.countServers(false), 1U); + + /* now remove it */ + pool.removeServer(ds); + BOOST_CHECK_EQUAL(pool.countServers(true), 0U); + BOOST_CHECK_EQUAL(pool.countServers(false), 0U); + { + const auto& servers = pool.getServers(); + BOOST_CHECK(servers.empty()); + } + } +} + +BOOST_AUTO_TEST_CASE(test_ServerPoolECSConsistency) +{ + ServerPool pool{}; + + /* one server with ECS */ + DownstreamState::Config config1; + config1.useECS = true; + auto ds1 = std::make_shared(std::move(config1), nullptr, false); + pool.addServer(ds1); + BOOST_CHECK_EQUAL(pool.countServers(false), 1U); + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(pool.isConsistent()); + BOOST_CHECK(pool.getECS()); + /* should be ignored */ + pool.setECS(false); + BOOST_CHECK(pool.getECS()); + + /* and now one without ECS */ + DownstreamState::Config config2; + config2.useECS = false; + auto ds2 = std::make_shared(std::move(config2), nullptr, false); + pool.addServer(ds2); + BOOST_CHECK_EQUAL(pool.countServers(false), 2U); + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(!pool.isConsistent()); + /* the first backend has ECS so the pool automatically picked up + ECS-enabled, and after that the inconsistency meant the state + was not longer automatically updated */ + BOOST_CHECK(pool.getECS()); + /* should NOT be ignored */ + pool.setECS(false); + BOOST_CHECK(!pool.getECS()); + /* should NOT be ignored */ + pool.setECS(true); + BOOST_CHECK(pool.getECS()); + + /* remove the server with ECS */ + pool.removeServer(ds1); + BOOST_CHECK_EQUAL(pool.countServers(false), 1U); + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(pool.isConsistent()); + BOOST_CHECK(!pool.getECS()); + + /* re-add the server with ECS */ + pool.addServer(ds1); + BOOST_CHECK_EQUAL(pool.countServers(false), 2U); + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(!pool.isConsistent()); + /* the pool was in a consistent state without ECS, + and now is in a inconsistent state so it not automatically + updated */ + BOOST_CHECK(!pool.getECS()); +} + +BOOST_AUTO_TEST_CASE(test_ServerPoolTCPOnlyConsistency) +{ + ServerPool pool{}; + + DownstreamState::Config config1; + config1.d_tcpOnly = true; + auto ds1 = std::make_shared(std::move(config1), nullptr, false); + pool.addServer(ds1); + BOOST_CHECK_EQUAL(pool.countServers(false), 1U); + BOOST_CHECK(pool.isTCPOnly()); + BOOST_CHECK(pool.isConsistent()); + BOOST_CHECK(!pool.getECS()); + + DownstreamState::Config config2; + config2.d_tcpOnly = false; + auto ds2 = std::make_shared(std::move(config2), nullptr, false); + pool.addServer(ds2); + BOOST_CHECK_EQUAL(pool.countServers(false), 2U); + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(!pool.isConsistent()); + + /* remove the TCP-only server */ + pool.removeServer(ds1); + BOOST_CHECK_EQUAL(pool.countServers(false), 1U); + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(pool.isConsistent()); + + /* re-add the server with TCP-only */ + pool.addServer(ds1); + BOOST_CHECK_EQUAL(pool.countServers(false), 2U); + BOOST_CHECK(!pool.isTCPOnly()); + BOOST_CHECK(!pool.isConsistent()); +} + +BOOST_AUTO_TEST_SUITE_END()