[master] Merge branch 'trac1424'

src/bin/resolver/main.cc

@@ -220,7 +220,10 @@ main(int argc, char* argv[]) {
         // Install all initial configurations.  If loading configuration
         // fails, it will be logged, but we start the server anyway, giving
         // the user a second chance to correct the configuration.
-        resolver->updateConfig(config_session->getFullConfig());
+        // By setting the 'startup' parameter to true, we ensure most of
+        // the default configuration will be installed even if listen_on
+        // fails.
+        resolver->updateConfig(config_session->getFullConfig(), true);
         LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_CONFIG_LOADED);
 
         LOG_INFO(resolver_logger, RESOLVER_STARTED);

src/bin/resolver/resolver.cc

@@ -356,8 +356,7 @@ Resolver::Resolver() :
     dns_lookup_(NULL),
     dns_answer_(new MessageAnswer),
     nsas_(NULL),
-    cache_(NULL),
-    configured_(false)
+    cache_(NULL)
 {
     // Operations referring to "this" must be done in the constructor body
     // (some compilers will issue warnings if "this" is referred to in the
@@ -585,7 +584,7 @@ ResolverImpl::processNormalQuery(const IOMessage& io_message,
 }
 
 ConstElementPtr
-Resolver::updateConfig(ConstElementPtr config) {
+Resolver::updateConfig(ConstElementPtr config, bool startup) {
     LOG_DEBUG(resolver_logger, RESOLVER_DBG_CONFIG, RESOLVER_CONFIG_UPDATED)
               .arg(*config);
 
@@ -658,7 +657,7 @@ Resolver::updateConfig(ConstElementPtr config) {
         // listenAddresses can fail to bind, so try them first
         bool need_query_restart = false;
         
-        if (listenAddressesE) {
+        if (!startup && listenAddressesE) {
             setListenAddresses(listenAddresses);
             need_query_restart = true;
         }
@@ -677,12 +676,15 @@ Resolver::updateConfig(ConstElementPtr config) {
         if (query_acl) {
             setQueryACL(query_acl);
         }
+        if (startup && listenAddressesE) {
+            setListenAddresses(listenAddresses);
+            need_query_restart = true;
+        }
 
         if (need_query_restart) {
             impl_->queryShutdown();
             impl_->querySetup(*dnss_, *nsas_, *cache_);
         }
-        setConfigured();
         return (isc::config::createAnswer());
 
     } catch (const isc::Exception& error) {

src/bin/resolver/resolver.h

@@ -95,8 +95,13 @@ public:
     isc::config::ModuleCCSession* getConfigSession() const;
     void setConfigSession(isc::config::ModuleCCSession* config_session);
 
-    /// \brief Handle commands from the config session
-    isc::data::ConstElementPtr updateConfig(isc::data::ConstElementPtr config);
+    /// \brief Handle commands from the config session.
+    ///
+    /// By default, or if \c startup is set to false explicitly, this method
+    /// will ignore any other configuration parameters if listen_on fails;
+    /// if \c startup is true, it will install as many parameters as possible.
+    isc::data::ConstElementPtr updateConfig(isc::data::ConstElementPtr config,
+                                            bool startup = false);
 
     /// \brief Assign an ASIO IO Service queue to this Resolver object
     void setDNSService(isc::asiodns::DNSService& dnss);
@@ -130,13 +135,6 @@ public:
     isc::asiolink::SimpleCallback* getCheckinProvider() { return (checkin_); }
 
     /**
-     * \brief Tell the Resolver that is has already been configured
-     *        so that it will only set some defaults the first time
-     *        (used by updateConfig() and tests)
-     */
-    void setConfigured() { configured_ = true; };
-
-    /**
      * \brief Specify the list of upstream servers.
      *
      * Specify the list off addresses of upstream servers to forward queries
@@ -266,10 +264,6 @@ private:
     isc::asiodns::DNSAnswer* dns_answer_;
     isc::nsas::NameserverAddressStore* nsas_;
     isc::cache::ResolverCache* cache_;
-    // This value is initally false, and will be set to true
-    // when the initial configuration is done (updateConfig
-    // should act a tiny bit different on the very first call)
-    bool configured_;
 };
 
 #endif // __RESOLVER_H

src/bin/resolver/tests/resolver_config_unittest.cc

@@ -14,12 +14,22 @@
 
 #include <config.h>
 
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <netdb.h>
+
+#include <cerrno>
+#include <cstring>
 #include <string>
 
 #include <boost/scoped_ptr.hpp>
+#include <boost/noncopyable.hpp>
 
 #include <gtest/gtest.h>
 
+#include <exceptions/exceptions.h>
+
 #include <cc/data.h>
 
 #include <config/ccsession.h>
@@ -52,6 +62,16 @@ using namespace isc::server_common;
 using isc::UnitTestUtil;
 
 namespace {
+const char* const TEST_ADDRESS = "127.0.0.1";
+const char* const TEST_PORT = "53535";
+
+// An internal exception class
+class TestConfigError : public isc::Exception {
+public:
+    TestConfigError(const char *file, size_t line, const char *what):
+        isc::Exception(file, line, what) {}
+};
+
 class ResolverConfig : public ::testing::Test {
 protected:
     IOService ios;
@@ -63,7 +83,6 @@ protected:
     scoped_ptr<const RequestContext> request;
     ResolverConfig() : dnss(ios, NULL, NULL, NULL) {
         server.setDNSService(dnss);
-        server.setConfigured();
     }
     const RequestContext& createRequest(const string& source_addr) {
         endpoint.reset(IOEndpoint::create(IPPROTO_UDP, IOAddress(source_addr),
@@ -155,6 +174,109 @@ TEST_F(ResolverConfig, rootAddressConfig) {
     EXPECT_EQ(0, server.getRootAddresses().size());
 }
 
+// The following two are helper classes to manage some temporary system
+// resources in an RAII manner.
+class ScopedAddrInfo : public boost::noncopyable {
+public:
+    ScopedAddrInfo(struct addrinfo* ai) : ai_(ai) {}
+    ~ScopedAddrInfo() { freeaddrinfo(ai_);}
+private:
+    struct addrinfo* ai_;
+};
+
+struct ScopedSocket : public boost::noncopyable {
+public:
+    ScopedSocket(int fd) : fd_(fd) {}
+    ~ScopedSocket() { close(fd_); }
+private:
+    const int fd_;
+};
+
+int
+createSocket(const char* address, const char* port) {
+    struct addrinfo hints, *res;
+    memset(&hints, 0, sizeof(hints));
+    hints.ai_family = AF_UNSPEC;
+    hints.ai_socktype = SOCK_DGRAM;
+    hints.ai_protocol = IPPROTO_UDP;
+    hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
+    const int error = getaddrinfo(address, port, &hints, &res);
+    if (error != 0) {
+        isc_throw(TestConfigError, "getaddrinfo failed: " <<
+                  gai_strerror(error));
+    }
+    ScopedAddrInfo scoped_res(res);
+    const int s = socket(res->ai_family, res->ai_socktype, res->ai_protocol);
+    if (s == -1) {
+        isc_throw(TestConfigError, "socket system call failed: " <<
+                  strerror(errno));
+    }
+    if (bind(s, res->ai_addr, res->ai_addrlen) == -1) {
+        close(s);
+        isc_throw(TestConfigError, "bind system call failed: " <<
+                  strerror(errno));
+    }
+    return (s);
+}
+
+void
+configAnswerCheck(ConstElementPtr config_answer, bool expect_success) {
+    EXPECT_EQ(Element::map, config_answer->getType());
+    EXPECT_TRUE(config_answer->contains("result"));
+
+    ConstElementPtr result = config_answer->get("result");
+    EXPECT_EQ(Element::list, result->getType());
+    EXPECT_EQ(expect_success ? 0 : 1, result->get(0)->intValue());
+}
+
+TEST_F(ResolverConfig, listenOnConfig) {
+    ConstElementPtr config(Element::fromJSON("{"
+                                             "\"listen_on\": ["
+                                             " {"
+                                             "    \"address\": \"" +
+                                             string(TEST_ADDRESS) + "\","
+                                             "    \"port\": " +
+                                             string(TEST_PORT) + "}]}"));
+    configAnswerCheck(server.updateConfig(config), true);
+}
+
+TEST_F(ResolverConfig, listenOnConfigFail) {
+    // Create and bind a socket that would make the subsequent listen_on fail
+    ScopedSocket sock(createSocket(TEST_ADDRESS, TEST_PORT));
+
+    ConstElementPtr config(Element::fromJSON("{"
+                                             "\"listen_on\": ["
+                                             " {"
+                                             "    \"address\": \"" +
+                                             string(TEST_ADDRESS) + "\","
+                                             "    \"port\": " +
+                                             string(TEST_PORT) + "}]}"));
+    configAnswerCheck(server.updateConfig(config), false);
+}
+
+TEST_F(ResolverConfig, listenOnAndOtherConfig) {
+    // Create and bind a socket that would make the subsequent listen_on fail
+    ScopedSocket sock(createSocket(TEST_ADDRESS, TEST_PORT));
+
+    // We are going to install a pair of "root_addresses" and "listen_on"
+    // in a single update.
+    const string config_str("{\"root_addresses\": ["
+                            " {\"address\": \"192.0.2.1\","
+                            "   \"port\": 53}], "
+                            "\"listen_on\": ["
+                            " {\"address\": \"" + string(TEST_ADDRESS) + "\","
+                            "  \"port\": " + string(TEST_PORT) + "}]}");
+    // Normally, if listen_on fails the rest of the config parameters will
+    // be ignored.
+    ConstElementPtr config(Element::fromJSON(config_str));
+    configAnswerCheck(server.updateConfig(config), false);
+    EXPECT_EQ(0, server.getRootAddresses().size());
+
+    // On startup the other parameters will be installed anyway.
+    configAnswerCheck(server.updateConfig(config, true), false);
+    EXPECT_EQ(1, server.getRootAddresses().size());
+}
+
 void
 ResolverConfig::invalidTest(const string &JSON, const string& name) {
     isc::testutils::portconfig::configRejected(server, JSON, name);

src/bin/resolver/tests/resolver_unittest.cc

@@ -39,7 +39,6 @@ protected:
     ResolverTest() : server() {
         // By default queries from the "default remote address" will be
         // rejected, so we'll need to add an explicit ACL entry to allow that.
-        server.setConfigured();
         server.updateConfig(Element::fromJSON(
                                 "{ \"query_acl\": "
                                 "  [ {\"action\": \"ACCEPT\","