Continue name resolution fixing, add unit test

name_lookup() now returns all results given by getnameinfo(). When searching
for a server, finding one matching address in the lookup-results is enough for
a match.

Also, added a test for name_lookup(). The test is minimal on its own, as hardcoded
test cases are not generally valid.

maxutils/maxbase/include/maxbase/host.hh

@@ -16,6 +16,7 @@
 
 #include <string>
 #include <iosfwd>
+#include <unordered_set>
 
 /** Host is a streamable class that represents an address and port, or a unix domain socket.
  */
@@ -110,14 +111,15 @@ inline bool operator!=(const Host& l, const Host& r)
 }
 
 /**
- * Perform DNS resolution on a hostname or text-form IP address.
+ * Perform domain name resolution on a hostname or text-form IP address.
  *
  * @param host Hostname to convert.
- * @param addr_out Output buffer. The output is in IPv6-form as returned by "inet_ntop(AF_INET6, ...)".
+ * @param addresses_out Output buffer. The output is in IPv6-form as returned by "inet_ntop(AF_INET6, ...)".
  * @param error_out Error output
  * @return True if successful
  */
-bool name_lookup(const std::string& host, std::string* addr_out, std::string* error_out = nullptr);
+bool name_lookup(const std::string& host, std::unordered_set<std::string>* addresses_out,
+                 std::string* error_out = nullptr);
 
 /**
  * Perform reverse DNS on an IP address. This may involve network communication so can be slow.

maxutils/maxbase/src/host.cc

@@ -227,43 +227,51 @@ std::istream& operator>>(std::istream& is, Host& host)
     return is;
 }
 
-bool name_lookup(const std::string& host, std::string* addr_out, std::string* error_out)
+bool name_lookup(const std::string& host, std::unordered_set<std::string>* addresses_out, std::string* error_out)
 {
     addrinfo hints;
     memset(&hints, 0, sizeof(hints));
-    hints.ai_family = AF_INET6;     /* Only return IPv6-addresses, possibly mapped from IPv4 */
+    hints.ai_family = AF_UNSPEC;     /* Return any address type */
     hints.ai_socktype = SOCK_DGRAM;  /* Datagram socket */
-    hints.ai_flags = AI_V4MAPPED;   /* Mapped IPv4 */
+    hints.ai_flags = 0;              /* Mapped IPv4 */
     hints.ai_protocol = 0;           /* Any protocol */
-    hints.ai_canonname = NULL;
+    hints.ai_canonname = nullptr;
-    hints.ai_addr = NULL;
+    hints.ai_addr = nullptr;
-    hints.ai_next = NULL;
+    hints.ai_next = nullptr;
 
     addrinfo* results = nullptr;
     bool success = false;
     std::string error_msg;
+
     int rv_addrinfo = getaddrinfo(host.c_str(), nullptr, &hints, &results);
     if (rv_addrinfo == 0)
     {
-        mxb_assert(results);
+        // getaddrinfo may return multiple result addresses. Save all of them.
-        if (results)
+        for (auto iter = results; iter; iter = iter->ai_next)
         {
-            // getaddrinfo may return multiple result addresses. Only consider the first.
+            int address_family = iter->ai_family;
-            char buf[INET6_ADDRSTRLEN];
+            void* addr = nullptr;
-            in6_addr* addr = &((sockaddr_in6*)results->ai_addr)->sin6_addr;
+            char buf[INET6_ADDRSTRLEN]; // Enough for both address types
-
+            if (iter->ai_family == AF_INET)
-            if (inet_ntop(AF_INET6, addr, buf, sizeof(buf)))
             {
-                *addr_out = buf;
+                auto sa_in = (sockaddr_in*)(iter->ai_addr);
-                success = true;
+                addr = &sa_in->sin_addr;
             }
-            else
+            else if (iter->ai_family == AF_INET6)
             {
-                error_msg = mxb::string_printf("inet_ntop() failed: '%s'.", strerror(errno));
+                auto sa_in = (sockaddr_in6*)(iter->ai_addr);
+                addr = &sa_in->sin6_addr;
+            }
+
+            inet_ntop(address_family, addr, buf, sizeof(buf));
+            if (buf[0])
+            {
+                addresses_out->insert(buf);
+                success = true; // One successful lookup is enough.
+            }
         }
         freeaddrinfo(results);
     }
-    }
     else
     {
         error_msg = mxb::string_printf("getaddrinfo() failed: '%s'.", gai_strerror(rv_addrinfo));

maxutils/maxbase/src/test/CMakeLists.txt

@@ -24,3 +24,7 @@ add_test(test_worker test_worker)
 add_executable(test_host_class test_host_class.cc)
 target_link_libraries(test_host_class maxbase pthread rt)
 add_test(test_host_class test_host_class)
+
+add_executable(test_name_lookup test_name_lookup.cc)
+target_link_libraries(test_name_lookup maxbase)
+add_test(test_name_lookup test_name_lookup)

maxutils/maxbase/src/test/test_name_lookup.cc

@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2019 MariaDB Corporation Ab
+ *
+ * Use of this software is governed by the Business Source License included
+ * in the LICENSE.TXT file and at www.mariadb.com/bsl11.
+ *
+ * Change Date: 2023-01-01
+ *
+ * On the date above, in accordance with the Business Source License, use
+ * of this software will be governed by version 2 or later of the General
+ * Public License.
+ */
+#include <maxbase/host.hh>
+
+#include <algorithm>
+
+int main()
+{
+    using std::string;
+    using std::unordered_set;
+
+    struct Test
+    {
+        string host;
+        string ip;
+        string result;
+    };
+
+    Test tests[] = {
+            {"localhost", "127.0.0.1", "127.0.0.1"},
+    };
+    /*
+     * Here are some additional test case examples. They may not work
+     * as is.
+     *
+     * Test tests[] = {
+     *      {"yle.fi", "13.32.43.102", "13.32.43.102"},
+     *      {"mariadb.com", "35.235.124.140", "35.235.124.140"},
+     *      {"reddit.com", "151.101.1.140", "151.101.1.140"},
+     *      {"max-tst-02.mariadb.com", "94.23.248.118", "94.23.248.118"},
+     *      {"wikipedia.org", "2620:0:862:ed1a::1", "2620:0:862:ed1a::1"},
+     *      {"one.one.one.one", "2606:4700:4700::1111", "2606:4700:4700::1111"},
+     * };
+     */
+
+    bool ok = true;
+    for (auto& item : tests)
+    {
+        string error;
+        auto& host = item.host;
+        auto& ip = item.ip;
+        auto& expected_result = item.result;
+        for (auto& subitem : {host, ip})
+        {
+            unordered_set<string> lookup_result;
+            bool lookup_ok = mxb::name_lookup(subitem, &lookup_result, &error);
+            if (!lookup_ok)
+            {
+                printf("Lookup of '%s' failed: %s\n", subitem.c_str(), error.c_str());
+                ok = false;
+            }
+            else if (lookup_result.count(expected_result) == 0)
+            {
+                string results_conc;
+                string sep;
+                for (auto result_elem : lookup_result)
+                {
+                    results_conc += sep + result_elem;
+                    sep = ", ";
+                }
+                printf("Lookup of '%s' gave incorrect results. Expected '%s', got '%s'.\n",
+                       subitem.c_str(), expected_result.c_str(), results_conc.c_str());
+                ok = false;
+            }
+        }
+    }
+
+    return ok ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+

server/modules/monitor/mariadbmon/cluster_discovery.cc

@@ -1005,34 +1005,32 @@ bool MariaDBMonitor::is_candidate_valid(MariaDBServer* cand, RequireRunning req_
     return is_valid;
 };
 
-string MariaDBMonitor::DNSResolver::resolve_server(const string& host)
+MariaDBMonitor::DNSResolver::StringSet MariaDBMonitor::DNSResolver::resolve_server(const string& host)
 {
     auto now = mxb::Clock::now();
     const auto MAX_AGE = mxb::Duration((double)5*60); // Refresh interval for cache entries.
     auto recent_time = now - MAX_AGE;
+    DNSResolver::StringSet rval;
 
-    string rval;
     auto iter = m_mapping.find(host);
-
     if (iter == m_mapping.end() || iter->second.timestamp < recent_time)
     {
         // Map did not have a record, or it was too old. In either case, generate a new one.
-        string addr;
+        DNSResolver::StringSet addresses;
         string error_msg;
-        bool dns_success = mxb::name_lookup(host, &addr, &error_msg);
+        bool dns_success = mxb::name_lookup(host, &addresses, &error_msg);
         if (!dns_success)
         {
             MXB_ERROR("Could not resolve host '%s'. %s", host.c_str(), error_msg.c_str());
         }
-        // If dns failed, addr will be empty. Add the element anyway to prevent repeated lookups.
+        // If dns failed, the array will be empty. Add/replace the element anyway to prevent repeated lookups.
-        MapElement newelem = {addr, now};
+        m_mapping[host] = {addresses, now};
-        m_mapping[host] = newelem;
+        rval = std::move(addresses);
-        rval = addr;
     }
     else
     {
         // Return recent value.
-        rval = iter->second.address;
+        rval = iter->second.addresses;
     }
     return rval;
 }

server/modules/monitor/mariadbmon/mariadbmon.cc

@@ -124,22 +124,28 @@ MariaDBServer* MariaDBMonitor::get_server(const EndPoint& search_ep)
     {
         // Phase 2: Was not found with simple string compare. Try DNS resolving for endpoints with
         // matching ports.
-        string target_addr = m_resolver.resolve_server(search_ep.host());
+        DNSResolver::StringSet target_addresses = m_resolver.resolve_server(search_ep.host());
-        if (!target_addr.empty())
+        if (!target_addresses.empty())
         {
             for (auto server : m_servers)
             {
-                if (server->m_server_base->server->port == search_ep.port())
+                SERVER* srv = server->m_server_base->server;
+                if (srv->port == search_ep.port())
                 {
-                    string server_addr = m_resolver.resolve_server(server->m_server_base->server->address);
+                    auto server_addresses = m_resolver.resolve_server(srv->address);
-                    if (server_addr == target_addr)
+                    // The number of elements in the arrays is rarely over 1.
+                    for (auto& address : server_addresses)
+                    {
+                        if (target_addresses.count(address) > 0)
                         {
                             found = server;
-                        break;
+                            goto breakout;
                         }
                     }
                 }
             }
+            breakout:;
+        }
     }
     return found;
 }

server/modules/monitor/mariadbmon/mariadbmon.hh

@@ -172,12 +172,13 @@ private:
     class DNSResolver
     {
     public:
-        std::string resolve_server(const std::string& host);
+        using StringSet = std::unordered_set<std::string>;
+        StringSet resolve_server(const std::string& host);
 
     private:
         struct MapElement
         {
-            std::string    address;
+            StringSet   addresses; // A hostname can map to multiple addresses
             mxb::TimePoint timestamp;
         };