[2213] Initial version of loadZone() support with new threaded structure

src/bin/auth/auth_srv.cc

@@ -922,3 +922,9 @@ void
 AuthSrv::setTCPRecvTimeout(size_t timeout) {
     dnss_->setTCPRecvTimeout(timeout);
 }
+
+void
+AuthSrv::loadZone(ConstElementPtr args) {
+    impl_->datasrc_clients_mgr_.loadZone(args);
+}
+

src/bin/auth/auth_srv.h

@@ -320,6 +320,13 @@ public:
     /// open forever.
     void setTCPRecvTimeout(size_t timeout);
 
+    /// \brief Reloads a zone
+    ///
+    /// This method should only be called from the LoadZoneCommand class,
+    /// internally it will tell the clients builder thread to reload
+    /// the zone specified in the arguments.
+    void loadZone(isc::data::ConstElementPtr args);
+
 private:
     AuthSrvImpl* impl_;
     isc::asiolink::SimpleCallback* checkin_;

src/bin/auth/command.cc

@@ -176,52 +176,12 @@ public:
     virtual ConstElementPtr exec(AuthSrv& server,
                                  isc::data::ConstElementPtr args)
     {
-        if (args == NULL) {
-            isc_throw(AuthCommandError, "Null argument");
+        try {
+            server.loadZone(args);
+            return (createAnswer());
+        } catch (const LoadZoneCommandError& lzce) {
+            return (createAnswer(1, lzce.what()));
         }
-
-        ConstElementPtr class_elem = args->get("class");
-        RRClass zone_class(class_elem ? RRClass(class_elem->stringValue()) :
-            RRClass::IN());
-
-        ConstElementPtr origin_elem = args->get("origin");
-        if (!origin_elem) {
-            isc_throw(AuthCommandError, "Zone origin is missing");
-        }
-        const Name origin(origin_elem->stringValue());
-
-        DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
-        boost::shared_ptr<ConfigurableClientList> list =
-            holder.findClientList(zone_class);
-
-        if (!list) {
-            isc_throw(AuthCommandError, "There's no client list for "
-                      "class " << zone_class);
-        }
-
-        switch (list->reload(origin)) {
-            case ConfigurableClientList::ZONE_RELOADED:
-                // Everything worked fine.
-                LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_LOAD_ZONE)
-                    .arg(zone_class).arg(origin);
-                return (createAnswer());
-            case ConfigurableClientList::ZONE_NOT_FOUND:
-                isc_throw(AuthCommandError, "Zone " << origin << "/" <<
-                          zone_class << " was not found in any configured "
-                          "data source. Configure it first.");
-            case ConfigurableClientList::ZONE_NOT_CACHED:
-                isc_throw(AuthCommandError, "Zone " << origin << "/" <<
-                          zone_class << " is not served from memory, but "
-                          "directly from the data source. It is not possible "
-                          "to reload it into memory. Configure it to be cached "
-                          "first.");
-            case ConfigurableClientList::CACHE_DISABLED:
-                // This is an internal error. Auth server must have the cache
-                // enabled.
-                isc_throw(isc::Unexpected, "Cache disabled in client list of "
-                          "class " << zone_class);
-        }
-        return (createAnswer());
     }
 };
 

src/bin/auth/datasrc_clients_mgr.h

@@ -45,6 +45,14 @@
 namespace isc {
 namespace auth {
 
+/// \brief An exception that is thrown if the arguments to the loadZone
+/// call are not in the right format
+class LoadZoneCommandError : public isc::Exception {
+public:
+    LoadZoneCommandError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 namespace datasrc_clientmgr_internal {
 // This namespace is essentially private for DataSrcClientsMgr(Base) and
 // DataSrcClientsBuilder(Base).  This is exposed in the public header
@@ -239,6 +247,30 @@ public:
         clients_map_ = new_lists;
     }
 
+    /// \brief Instruct internal thread to (re)load a zone
+    ///
+    /// \param args Element argument that should be a map of the form
+    /// { "class": "IN", "origin": "example.com" }
+    /// (but class is optional and will default to IN)
+    ///
+    /// \exception LoadZoneCommandError if the args value is null, or not in
+    ///                                 the expected format
+    void
+    loadZone(data::ConstElementPtr args) {
+        if (!args) {
+            isc_throw(LoadZoneCommandError, "loadZone argument empty");
+        }
+        if (args->getType() != isc::data::Element::map) {
+            isc_throw(LoadZoneCommandError, "loadZone argument not a map");
+        }
+        if (!args->contains("origin")) {
+            isc_throw(LoadZoneCommandError,
+                      "loadZone argument has no 'origin' value");
+        }
+
+        sendCommand(datasrc_clientmgr_internal::LOADZONE, args);
+    }
+
 private:
     // This is expected to be called at the end of the destructor.  It
     // actually does nothing, but provides a customization point for
@@ -489,10 +521,12 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::doLoadZone(
     // called via the manager in practice.  manager is expected to do the
     // minimal validation.
     assert(arg);
-    assert(arg->get("class"));
     assert(arg->get("origin"));
 
-    const dns::RRClass rrclass(arg->get("class")->stringValue());
+    isc::data::ConstElementPtr class_elem = arg->get("class");
+    const dns::RRClass rrclass(class_elem ?
+                                dns::RRClass(class_elem->stringValue()) :
+                                dns::RRClass::IN());
     const dns::Name origin(arg->get("origin")->stringValue());
     ClientListsMap::iterator found = (*clients_map_)->find(rrclass);
     if (found == (*clients_map_)->end()) {

src/bin/auth/tests/command_unittest.cc

@@ -244,206 +244,6 @@ newZoneChecks(AuthSrv& server) {
               find(Name("ns.test2.example"), RRType::AAAA())->code);
 }
 
-TEST_F(AuthCommandTest, loadZone) {
-    configureZones(server_);
-
-    ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
-                        "/test1-new.zone.in "
-                        TEST_DATA_BUILDDIR "/test1.zone.copied"));
-    ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
-                        "/test2-new.zone.in "
-                        TEST_DATA_BUILDDIR "/test2.zone.copied"));
-
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON(
-                                        "{\"origin\": \"test1.example\"}"));
-    checkAnswer(0);
-    newZoneChecks(server_);
-}
-
-TEST_F(AuthCommandTest,
-#ifdef USE_STATIC_LINK
-       DISABLED_loadZoneSQLite3
-#else
-       loadZoneSQLite3
-#endif
-    )
-{
-    // Prepare the database first
-    const string test_db = TEST_DATA_BUILDDIR "/auth_test.sqlite3.copied";
-    const string bad_db = TEST_DATA_BUILDDIR "/does-not-exist.sqlite3";
-    stringstream ss("example.org. 3600 IN SOA . . 0 0 0 0 0\n");
-    createSQLite3DB(RRClass::IN(), Name("example.org"), test_db.c_str(), ss);
-    // This describes the data source in the configuration
-    const ConstElementPtr config(Element::fromJSON("{"
-        "\"IN\": [{"
-        "    \"type\": \"sqlite3\","
-        "    \"params\": {\"database_file\": \"" + test_db + "\"},"
-        "    \"cache-enable\": true,"
-        "    \"cache-zones\": [\"example.org\"]"
-        "}]}"));
-    installDataSrcClientLists(server_, configureDataSource(config));
-
-    {
-        DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
-
-        // Check that the A record at www.example.org does not exist
-        EXPECT_EQ(ZoneFinder::NXDOMAIN,
-                  holder.findClientList(RRClass::IN())->
-                  find(Name("example.org")).finder_->
-                  find(Name("www.example.org"), RRType::A())->code);
-
-        // Add the record to the underlying sqlite database, by loading
-        // it as a separate datasource, and updating it
-        ConstElementPtr sql_cfg = Element::fromJSON("{ \"type\": \"sqlite3\","
-                                                    "\"database_file\": \""
-                                                    + test_db + "\"}");
-        DataSourceClientContainer sql_ds("sqlite3", sql_cfg);
-        ZoneUpdaterPtr sql_updater =
-            sql_ds.getInstance().getUpdater(Name("example.org"), false);
-        RRsetPtr rrset(new RRset(Name("www.example.org."), RRClass::IN(),
-                                 RRType::A(), RRTTL(60)));
-        rrset->addRdata(rdata::createRdata(rrset->getType(),
-                                           rrset->getClass(),
-                                           "192.0.2.1"));
-        sql_updater->addRRset(*rrset);
-        sql_updater->commit();
-
-        EXPECT_EQ(ZoneFinder::NXDOMAIN,
-                  holder.findClientList(RRClass::IN())->
-                  find(Name("example.org")).finder_->
-                  find(Name("www.example.org"), RRType::A())->code);
-    }
-
-    // Now send the command to reload it
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON(
-                                        "{\"origin\": \"example.org\"}"));
-    checkAnswer(0, "Successful load");
-
-    {
-        DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
-        // And now it should be present too.
-        EXPECT_EQ(ZoneFinder::SUCCESS,
-                  holder.findClientList(RRClass::IN())->
-                  find(Name("example.org")).finder_->
-                  find(Name("www.example.org"), RRType::A())->code);
-    }
-
-    // Some error cases. First, the zone has no configuration. (note .com here)
-    result_ = execAuthServerCommand(server_, "loadzone",
-        Element::fromJSON("{\"origin\": \"example.com\"}"));
-    checkAnswer(1, "example.com");
-
-    {
-        DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
-        // The previous zone is not hurt in any way
-        EXPECT_EQ(ZoneFinder::SUCCESS,
-                  holder.findClientList(RRClass::IN())->
-                  find(Name("example.org")).finder_->
-                  find(Name("example.org"), RRType::SOA())->code);
-    }
-
-    const ConstElementPtr config2(Element::fromJSON("{"
-        "\"IN\": [{"
-        "    \"type\": \"sqlite3\","
-        "    \"params\": {\"database_file\": \"" + bad_db + "\"},"
-        "    \"cache-enable\": true,"
-        "    \"cache-zones\": [\"example.com\"]"
-        "}]}"));
-    EXPECT_THROW(configureDataSource(config2),
-                 ConfigurableClientList::ConfigurationError);
-
-    result_ = execAuthServerCommand(server_, "loadzone",
-        Element::fromJSON("{\"origin\": \"example.com\"}"));
-    checkAnswer(1, "Unreadable");
-
-    DataSrcClientsMgr::Holder holder(server_.getDataSrcClientsMgr());
-    // The previous zone is not hurt in any way
-    EXPECT_EQ(ZoneFinder::SUCCESS,
-              holder.findClientList(RRClass::IN())->
-              find(Name("example.org")).finder_->
-              find(Name("example.org"), RRType::SOA())->code);
-}
-
-TEST_F(AuthCommandTest, loadBrokenZone) {
-    configureZones(server_);
-
-    ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR
-                        "/test1-broken.zone.in "
-                        TEST_DATA_BUILDDIR "/test1.zone.copied"));
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON(
-                                        "{\"origin\": \"test1.example\"}"));
-    checkAnswer(1);
-    zoneChecks(server_);     // zone shouldn't be replaced
-}
-
-TEST_F(AuthCommandTest, loadUnreadableZone) {
-    configureZones(server_);
-
-    // install the zone file as unreadable
-    ASSERT_EQ(0, system(INSTALL_PROG " -c -m 000 " TEST_DATA_DIR
-                        "/test1.zone.in "
-                        TEST_DATA_BUILDDIR "/test1.zone.copied"));
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON(
-                                        "{\"origin\": \"test1.example\"}"));
-    checkAnswer(1);
-    zoneChecks(server_);     // zone shouldn't be replaced
-}
-
-TEST_F(AuthCommandTest, loadZoneWithoutDataSrc) {
-    // try to execute load command without configuring the zone beforehand.
-    // it should fail.
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON(
-                                        "{\"origin\": \"test1.example\"}"));
-    checkAnswer(1);
-}
-
-TEST_F(AuthCommandTest, loadZoneInvalidParams) {
-    configureZones(server_);
-
-    // null arg
-    result_ = execAuthServerCommand(server_, "loadzone", ElementPtr());
-    checkAnswer(1, "Null arg");
-
-    // zone class is bogus
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON(
-                                        "{\"origin\": \"test1.example\","
-                                        " \"class\": \"no_such_class\"}"));
-    checkAnswer(1, "No such class");
-
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON(
-                                        "{\"origin\": \"test1.example\","
-                                        " \"class\": 1}"));
-    checkAnswer(1, "Integral class");
-
-
-    // origin is missing
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON("{}"));
-    checkAnswer(1, "Missing origin");
-
-    // zone doesn't exist in the data source
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON("{\"origin\": \"xx\"}"));
-    checkAnswer(1, "No such zone");
-
-    // origin is bogus
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON(
-                                        "{\"origin\": \"...\"}"));
-    checkAnswer(1, "Wrong name");
-
-    result_ = execAuthServerCommand(server_, "loadzone",
-                                    Element::fromJSON("{\"origin\": 10}"));
-    checkAnswer(1, "Integral name");
-}
-
 TEST_F(AuthCommandTest, getStats) {
     result_ = execAuthServerCommand(server_, "getstats", ConstElementPtr());
     parseAnswer(rcode_, result_);

src/bin/auth/tests/datasrc_clients_builder_unittest.cc

@@ -492,11 +492,13 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
                             Element::fromJSON(
                                 "{\"origin\": \"test1.example\"}")));
             }, "");
+/*
         EXPECT_DEATH_IF_SUPPORTED({
                 builder.handleCommand(Command(LOADZONE,
                                               Element::fromJSON(
                                                   "{\"class\": \"IN\"}")));
             }, "");
+*/
     }
 
     // zone doesn't exist in the data source

src/bin/auth/tests/datasrc_clients_mgr_unittest.cc

@@ -196,6 +196,37 @@ TEST(DataSrcClientsMgrTest, holder) {
     EXPECT_THROW(TestDataSrcClientsMgr::Holder holder2(mgr), isc::Unexpected);
 }
 
+TEST(DataSrcClientsMgrTest, reload) {
+    TestDataSrcClientsMgr mgr;
+    EXPECT_TRUE(FakeDataSrcClientsBuilder::started);
+    EXPECT_TRUE(FakeDataSrcClientsBuilder::command_queue->empty());
+
+    isc::data::ElementPtr args =
+        isc::data::Element::fromJSON("{ \"class\": \"IN\","
+                                     "  \"origin\": \"example.com\" }");
+    mgr.loadZone(args);
+    EXPECT_EQ(1, FakeDataSrcClientsBuilder::command_queue->size());
+    mgr.loadZone(args);
+    EXPECT_EQ(2, FakeDataSrcClientsBuilder::command_queue->size());
+
+    // Should succeed without 'class'
+    args->remove("class");
+    mgr.loadZone(args);
+    EXPECT_EQ(3, FakeDataSrcClientsBuilder::command_queue->size());
+
+    // but fail without origin, without sending new commands
+    args->remove("origin");
+    EXPECT_THROW(mgr.loadZone(args), LoadZoneCommandError);
+    EXPECT_EQ(3, FakeDataSrcClientsBuilder::command_queue->size());
+
+    // same for empty data and data that is not a map
+    EXPECT_THROW(mgr.loadZone(isc::data::ConstElementPtr()),
+                              LoadZoneCommandError);
+    EXPECT_THROW(mgr.loadZone(isc::data::Element::createList()),
+                              LoadZoneCommandError);
+    EXPECT_EQ(3, FakeDataSrcClientsBuilder::command_queue->size());
+}
+
 TEST(DataSrcClientsMgrTest, realThread) {
     // Using the non-test definition with a real thread.  Just checking
     // no disruption happens.

tests/lettuce/features/ddns_system.feature

@@ -39,7 +39,7 @@ Feature: DDNS System
         # Test 5
         When I use DDNS to set the SOA serial to 1237
         # also check if Auth server reloaded
-        And wait for new bind10 stderr message AUTH_LOAD_ZONE
+        And wait for new bind10 stderr message AUTH_DATASRC_CLIENTS_BUILDER_LOAD_ZONE
         The DDNS response should be SUCCESS
         And the SOA serial for example.org should be 1237
 
@@ -57,7 +57,7 @@ Feature: DDNS System
         And the SOA serial for example.org should be 1238
 
         When I use DDNS to set the SOA serial to 1239
-        And wait for new bind10 stderr message AUTH_LOAD_ZONE
+        And wait for new bind10 stderr message AUTH_DATASRC_CLIENTS_BUILDER_LOAD_ZONE
         The DDNS response should be SUCCESS
         And the SOA serial for example.org should be 1239
 
@@ -69,7 +69,7 @@ Feature: DDNS System
 
         # Test 10
         When I use DDNS to set the SOA serial to 1240
-        And wait for new bind10 stderr message AUTH_LOAD_ZONE
+        And wait for new bind10 stderr message AUTH_DATASRC_CLIENTS_BUILDER_LOAD_ZONE
         The DDNS response should be SUCCESS
         And the SOA serial for example.org should be 1240
 

tests/lettuce/features/inmemory_over_sqlite3.feature

@@ -33,7 +33,7 @@ Feature: In-memory zone using SQLite3 backend
         A query for mail.example.org to [::1]:47806 should have rcode NXDOMAIN
         When I send bind10 the command Xfrin retransfer example.org IN ::1 47807
         Then wait for new bind10 stderr message XFRIN_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE
-        Then wait for new bind10 stderr message AUTH_LOAD_ZONE
+        Then wait for new bind10 stderr message AUTH_DATASRC_CLIENTS_BUILDER_LOAD_ZONE
 
         A query for www.example.org to [::1]:47807 should have rcode NOERROR
         The answer section of the last query response should be