[master] Merge branch 'trac1986'

src/bin/auth/auth.spec.pre.in

@@ -131,6 +131,16 @@
             "item_optional": true, "item_default": "memory"
           }
         ]
+      },
+      {
+        "command_name": "start_ddns_forwarder",
+        "command_description": "(Re)start internal forwarding of DDNS Update messages. This is automatically called if b10-ddns is started, and is not expected to be called by administrators; it will be removed as a public command in the future.",
+        "command_args": []
+      },
+      {
+        "command_name": "stop_ddns_forwarder",
+        "command_description": "Stop internal forwarding of DDNS Update messages. This is automatically called if b10-ddns is stopped, and is not expected to be called by administrators; it will be removed as a public command in the future.",
+        "command_args": []
       }
     ],
     "statistics": [

src/bin/auth/auth_messages.mes

@@ -110,9 +110,6 @@ look into the cause and address the issue.  The log message includes
 the client's address (and port), and the error message sent from the
 lower layer that detects the failure.
 
-% AUTH_RECEIVED_NOTIFY received incoming NOTIFY for zone name %1, zone class %2
-This is a debug message reporting that an incoming NOTIFY was received.
-
 % AUTH_NOTIFY_QUESTIONS invalid number of questions (%1) in incoming NOTIFY
 This debug message is logged by the authoritative server when it receives
 a NOTIFY packet that contains zero or more than one question. (A valid
@@ -169,6 +166,9 @@ bug ticket for this issue.
 This is a debug message issued when the authoritative server has received
 a command on the command channel.
 
+% AUTH_RECEIVED_NOTIFY received incoming NOTIFY for zone name %1, zone class %2
+This is a debug message reporting that an incoming NOTIFY was received.
+
 % AUTH_RECEIVED_SENDSTATS command 'sendstats' received
 This is a debug message issued when the authoritative server has received
 a command from the statistics module to send it data. The 'sendstats'
@@ -235,6 +235,13 @@ This is a debug message indicating that the authoritative server has
 found that the data source it is loading is an SQLite3 data source,
 so no further validation is needed.
 
+% AUTH_START_DDNS_FORWARDER DDNS UPDATE handling started
+This is a debug message indicating that b10-auth has received a message
+that it should internally forward UPDATE message to b10-ddns. When b10-ddns
+is not running, b10-auth will respond to UPDATE requests with rcode NOTIMP.
+When b10-ddns is running, b10-ddns will handle and respond to the UPDATE
+message.
+
 % AUTH_STATS_CHANNEL_CREATED STATS session channel created
 This is a debug message indicating that the authoritative server has
 created a channel to the statistics process.  It is issued during server
@@ -266,6 +273,15 @@ This is a debug message indicating that the statistics timer has been
 enabled and that the authoritative server will produce statistics data
 at the specified interval.
 
+% AUTH_STOP_DDNS_FORWARDER DDNS UPDATE handling stopped
+This is a debug message indicating that b10-auth has received a message
+that it should stop internally forwarding UPDATE message to b10-ddns.
+b10-auth will no longer forward UPDATE messages to b10-ddns, but will
+respond itself with error code NOTIMP.
+This message is also logged when the forwarding is restarted (for instance
+if b10-ddns is restarted and the internal connection needs to be created
+again), in which case it should be followed by AUTH_START_DDNS_FORWARDER.
+
 % AUTH_UNSUPPORTED_OPCODE unsupported opcode: %1
 This is a debug message, produced when a received DNS packet being
 processed by the authoritative server has been found to contain an

src/bin/auth/auth_srv.cc

@@ -58,6 +58,7 @@
 
 #include <boost/bind.hpp>
 #include <boost/lexical_cast.hpp>
+#include <boost/scoped_ptr.hpp>
 
 #include <algorithm>
 #include <cassert>
@@ -270,6 +271,14 @@ public:
     /// isc:config::ModuleSpec::validateStatistics.
     void registerStatisticsValidator();
 
+    /// Socket session forwarder for dynamic update requests
+    BaseSocketSessionForwarder& ddns_base_forwarder_;
+
+    /// Holder for the DDNS Forwarder, which is used to send
+    /// DDNS messages to b10-ddns, but can be set to empty if
+    /// b10-ddns is not running
+    boost::scoped_ptr<SocketSessionForwarderHolder> ddns_forwarder_;
+
     /// \brief Resume the server
     ///
     /// This is a wrapper call for DNSServer::resume(done), if 'done' is true,
@@ -285,6 +294,7 @@ public:
     void resumeServer(isc::asiodns::DNSServer* server,
                       isc::dns::Message& message,
                       bool done);
+
 private:
     std::string db_file_;
 
@@ -297,9 +307,6 @@ private:
     bool xfrout_connected_;
     AbstractXfroutClient& xfrout_client_;
 
-    // Socket session forwarder for dynamic update requests
-    SocketSessionForwarderHolder ddns_forwarder_;
-
     /// Increment query counter
     void incCounter(const int protocol);
 
@@ -318,9 +325,10 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache,
     statistics_timer_(io_service_),
     counters_(),
     keyring_(NULL),
+    ddns_base_forwarder_(ddns_forwarder),
+    ddns_forwarder_(NULL),
     xfrout_connected_(false),
-    xfrout_client_(xfrout_client),
-    ddns_forwarder_("update", ddns_forwarder)
+    xfrout_client_(xfrout_client)
 {
     // cur_datasrc_ is automatically initialized by the default constructor,
     // effectively being an empty (sqlite) data source.  once ccsession is up
@@ -456,7 +464,7 @@ makeErrorMessage(MessageRenderer& renderer, Message& message,
     for_each(questions.begin(), questions.end(), QuestionInserter(message));
 
     message.setRcode(rcode);
-    
+
     RendererHolder holder(renderer, &buffer);
     if (tsig_context.get() != NULL) {
         message.toWire(renderer, *tsig_context);
@@ -653,7 +661,12 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
             send_answer = impl_->processNotify(io_message, message, buffer,
                                                tsig_context);
         } else if (opcode == Opcode::UPDATE()) {
-            send_answer = impl_->processUpdate(io_message);
+            if (impl_->ddns_forwarder_) {
+                send_answer = impl_->processUpdate(io_message);
+            } else {
+                makeErrorMessage(impl_->renderer_, message, buffer,
+                                 Rcode::NOTIMP(), tsig_context);
+            }
         } else if (opcode != Opcode::QUERY()) {
             LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_UNSUPPORTED_OPCODE)
                       .arg(message.getOpcode().toText());
@@ -880,7 +893,7 @@ AuthSrvImpl::processUpdate(const IOMessage& io_message) {
     // Push the update request to a separate process via the forwarder.
     // On successful push, the request shouldn't be responded from b10-auth,
     // so we return false.
-    ddns_forwarder_.push(io_message);
+    ddns_forwarder_->push(io_message);
     return (false);
 }
 
@@ -1027,3 +1040,20 @@ void
 AuthSrv::setTSIGKeyRing(const boost::shared_ptr<TSIGKeyRing>* keyring) {
     impl_->keyring_ = keyring;
 }
+
+void
+AuthSrv::createDDNSForwarder() {
+    LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_START_DDNS_FORWARDER);
+    impl_->ddns_forwarder_.reset(
+        new SocketSessionForwarderHolder("update", impl_->ddns_base_forwarder_));
+}
+
+void
+AuthSrv::destroyDDNSForwarder() {
+    if (impl_->ddns_forwarder_) {
+        LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_STOP_DDNS_FORWARDER);
+        impl_->ddns_forwarder_.reset();
+    }
+}
+
+

src/bin/auth/auth_srv.h

@@ -111,7 +111,7 @@ public:
     /// This method should never throw an exception.
     void stop();
 
-    /// \brief Process an incoming DNS message, then signal 'server' to resume 
+    /// \brief Process an incoming DNS message, then signal 'server' to resume
     ///
     /// A DNS query (or other message) has been received by a \c DNSServer
     /// object.  Find an answer, then post the \c DNSServer object on the
@@ -355,13 +355,13 @@ public:
     bool submitStatistics() const;
 
     /// \brief Get the value of counter in the AuthCounters.
-    /// 
+    ///
     /// This function calls AuthCounters::getCounter() and
     /// returns its return value.
     ///
     /// This function never throws an exception as far as
     /// AuthCounters::getCounter() doesn't throw.
-    /// 
+    ///
     /// Note: Currently this function is for testing purpose only.
     ///
     /// \param type Type of a counter to get the value of
@@ -418,6 +418,25 @@ public:
     void setTSIGKeyRing(const boost::shared_ptr<isc::dns::TSIGKeyRing>*
                         keyring);
 
+    /// \brief Create the internal forwarder for DDNS update messages
+    ///
+    /// Until this method is called (it is called when the
+    /// start_ddns_forwarder command is sent to b10-auth), b10-auth will
+    /// respond to UPDATE messages with a NOTIMP rcode.
+    /// If the internal forwarder was already created, it is destroyed and
+    /// created again. This is useful for instance when b10-ddns is shut
+    /// down and restarted.
+    void createDDNSForwarder();
+
+    /// \brief Destroy the internal forwarder for DDNS update messages
+    ///
+    /// After this method has been called (it is called when the
+    /// stop_ddns_forwarder command is sent to b10-auth), DDNS Update
+    /// messages are no longer forwarded internally, but b10-auth will
+    /// immediately respond with a NOTIMP rcode.
+    /// If there was no forwarder yet, this method does nothing.
+    void destroyDDNSForwarder();
+
 private:
     AuthSrvImpl* impl_;
     isc::asiolink::SimpleCallback* checkin_;
@@ -428,6 +447,6 @@ private:
 
 #endif // __AUTH_SRV_H
 
-// Local Variables: 
+// Local Variables:
 // mode: c++
-// End: 
+// End:

src/bin/auth/command.cc

@@ -141,6 +141,20 @@ public:
     }
 };
 
+class StartDDNSForwarderCommand : public AuthCommand {
+public:
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+        server.createDDNSForwarder();
+    }
+};
+
+class StopDDNSForwarderCommand : public AuthCommand {
+public:
+    virtual void exec(AuthSrv& server, isc::data::ConstElementPtr) {
+        server.destroyDDNSForwarder();
+    }
+};
+
 // Handle the "loadzone" command.
 class LoadZoneCommand : public AuthCommand {
 public:
@@ -309,6 +323,10 @@ createAuthCommand(const string& command_id) {
         return (new SendStatsCommand());
     } else if (command_id == "loadzone") {
         return (new LoadZoneCommand());
+    } else if (command_id == "start_ddns_forwarder") {
+        return (new StartDDNSForwarderCommand());
+    } else if (command_id == "stop_ddns_forwarder") {
+        return (new StopDDNSForwarderCommand());
     } else if (false && command_id == "_throw_exception") {
         // This is for testing purpose only and should not appear in the
         // actual configuration syntax.

src/bin/auth/common.cc

@@ -57,3 +57,4 @@ getDDNSSocketPath() {
 }
 
 const char* const AUTH_NAME = "b10-auth";
+const char* const AUTH_STARTED_NOTIFICATION = "auth_started";

src/bin/auth/common.h

@@ -57,6 +57,11 @@ std::string getDDNSSocketPath();
 /// This is currently b10-auth, but it can be changed easily in one place.
 extern const char* const AUTH_NAME;
 
+/// \brief Notification string that is used to inform auth is starting
+///
+/// This is sent to interested modules (currently only b10-ddns)
+extern const char* const AUTH_STARTED_NOTIFICATION;
+
 #endif // __COMMON_H
 
 // Local Variables:

src/bin/auth/main.cc

@@ -210,6 +210,12 @@ main(int argc, char* argv[]) {
 
         // Successfully initialized.
         LOG_INFO(auth_logger, AUTH_SERVER_STARTED);
+
+        // Ping any interested module that (a new) auth is up
+        // Currently, only the DDNS module is notified, but we could consider
+        // make an announcement channel for these (one-way) messages
+        cc_session->group_sendmsg(
+            isc::config::createCommand(AUTH_STARTED_NOTIFICATION), "DDNS");
         io_service.run();
 
     } catch (const std::exception& ex) {

src/bin/auth/tests/auth_srv_unittest.cc

@@ -31,6 +31,7 @@
 
 #include <datasrc/memory_datasrc.h>
 #include <auth/auth_srv.h>
+#include <auth/command.h>
 #include <auth/common.h>
 #include <auth/statistics.h>
 
@@ -99,6 +100,7 @@ protected:
         server.setDNSService(dnss_);
         server.setXfrinSession(&notify_session);
         server.setStatisticsSession(&statistics_session);
+        server.createDDNSForwarder();
     }
 
     ~AuthSrvTest() {
@@ -106,6 +108,7 @@ protected:
         // type information may be lost if the message is cleared
         // automatically later, so as a precaution we do it now.
         parse_message->clear(Message::PARSE);
+        server.destroyDDNSForwarder();
     }
 
     virtual void processMessage() {
@@ -1625,6 +1628,7 @@ TEST_F(AuthSrvTest, DDNSForwardPushFail) {
 
 TEST_F(AuthSrvTest, DDNSForwardClose) {
     scoped_ptr<AuthSrv> tmp_server(new AuthSrv(true, xfrout, ddns_forwarder));
+    tmp_server->createDDNSForwarder();
     UnitTestUtil::createRequestMessage(request_message, Opcode::UPDATE(),
                                        default_qid, Name("example.com"),
                                        RRClass::IN(), RRType::SOA());
@@ -1639,4 +1643,85 @@ TEST_F(AuthSrvTest, DDNSForwardClose) {
     EXPECT_FALSE(ddns_forwarder.isConnected());
 }
 
+namespace {
+    // Send a basic command without arguments, and check the response has
+    // result code 0
+    void sendSimpleCommand(AuthSrv& server, const std::string& command) {
+        ConstElementPtr response = execAuthServerCommand(server, command,
+                                                         ConstElementPtr());
+        int command_result = -1;
+        isc::config::parseAnswer(command_result, response);
+        EXPECT_EQ(0, command_result);
+    }
+} // end anonymous namespace
+
+TEST_F(AuthSrvTest, DDNSForwardCreateDestroy) {
+    // Test that AuthSrv returns NOTIMP before ddns forwarder is created,
+    // that the ddns_forwarder is connected when the 'start_ddns_forwarder'
+    // command has been sent, and that it is no longer connected and auth
+    // returns NOTIMP after the stop_ddns_forwarding command is sent.
+    scoped_ptr<AuthSrv> tmp_server(new AuthSrv(true, xfrout, ddns_forwarder));
+
+    // Prepare update message to send
+    UnitTestUtil::createRequestMessage(request_message, Opcode::UPDATE(),
+                                       default_qid, Name("example.com"),
+                                       RRClass::IN(), RRType::SOA());
+    createRequestPacket(request_message, IPPROTO_UDP);
+
+    // before creating forwarder. isConnected() should be false and
+    // rcode to UPDATE should be NOTIMP
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOTIMP(),
+                Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
+
+    // now create forwarder
+    sendSimpleCommand(*tmp_server, "start_ddns_forwarder");
+
+    // our mock does not respond, and since auth is supposed to send it on,
+    // there should now be no result when an UPDATE is sent
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(dnsserv.hasAnswer());
+    EXPECT_TRUE(ddns_forwarder.isConnected());
+
+    // If we send a start again, the connection should be recreated,
+    // visible because isConnected() reports false until an actual message
+    // has been forwarded
+    sendSimpleCommand(*tmp_server, "start_ddns_forwarder");
+
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(dnsserv.hasAnswer());
+    EXPECT_TRUE(ddns_forwarder.isConnected());
+
+    // Now tell it to stop forwarder, should respond with NOTIMP again
+    sendSimpleCommand(*tmp_server, "stop_ddns_forwarder");
+
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOTIMP(),
+                Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
+
+    // Sending stop again should make no difference
+    sendSimpleCommand(*tmp_server, "stop_ddns_forwarder");
+
+    parse_message->clear(Message::PARSE);
+    tmp_server->processMessage(*io_message, *parse_message, *response_obuffer,
+                               &dnsserv);
+    EXPECT_FALSE(ddns_forwarder.isConnected());
+    EXPECT_TRUE(dnsserv.hasAnswer());
+    headerCheck(*parse_message, default_qid, Rcode::NOTIMP(),
+                Opcode::UPDATE().getCode(), QR_FLAG, 0, 0, 0, 0);
+}
+
 }

src/bin/ddns/ddns.py.in

@@ -228,6 +228,9 @@ class DDNSServer:
         # Outstanding TCP context: fileno=>(context_obj, dst)
         self._tcp_ctxs = {}
 
+        # Notify Auth server that DDNS update messages can now be forwarded
+        self.__notify_start_forwarder()
+
     class InternalError(Exception):
         '''Exception for internal errors in an update session.
 
@@ -274,6 +277,9 @@ class DDNSServer:
             logger.info(DDNS_RECEIVED_SHUTDOWN_COMMAND)
             self.trigger_shutdown()
             answer = create_answer(0)
+        elif cmd == "auth_started":
+            self.__notify_start_forwarder()
+            answer = None
         else:
             answer = create_answer(1, "Unknown command: " + str(cmd))
         return answer
@@ -378,6 +384,8 @@ class DDNSServer:
         Do NOT call this to initialize shutdown, use trigger_shutdown().
 
         '''
+        # tell Auth not to forward UPDATE messages anymore
+        self.__notify_stop_forwarder()
         # tell the ModuleCCSession to send a message that this module is
         # stopping.
         self._cc.send_stopping()
@@ -533,6 +541,32 @@ class DDNSServer:
 
         return True
 
+    def __notify_start_forwarder(self):
+        '''Notify auth that DDNS Update messages can now be forwarded'''
+        try:
+            seq = self._cc._session.group_sendmsg(create_command(
+                    "start_ddns_forwarder"), AUTH_MODULE_NAME)
+            answer, _ = self._cc._session.group_recvmsg(False, seq)
+            rcode, error_msg = parse_answer(answer)
+            if rcode != 0:
+                logger.error(DDNS_START_FORWARDER_ERROR, error_msg)
+        except (SessionTimeout, SessionError, ProtocolError) as ex:
+            logger.error(DDNS_START_FORWARDER_FAIL, ex)
+
+    def __notify_stop_forwarder(self):
+        '''Notify auth that DDNS Update messages should no longer be forwarded.
+
+        '''
+        try:
+            seq = self._cc._session.group_sendmsg(create_command(
+                    "stop_ddns_forwarder"), AUTH_MODULE_NAME)
+            answer, _ = self._cc._session.group_recvmsg(False, seq)
+            rcode, error_msg = parse_answer(answer)
+            if rcode != 0:
+                logger.error(DDNS_STOP_FORWARDER_ERROR, error_msg)
+        except (SessionTimeout, SessionError, ProtocolError) as ex:
+            logger.error(DDNS_STOP_FORWARDER_FAIL, ex)
+
     def __notify_auth(self, zname, zclass):
         '''Notify auth of the update, if necessary.'''
         msg = auth_loadzone_command(self._cc, zname, zclass)

src/bin/ddns/ddns_messages.mes

@@ -192,6 +192,25 @@ be completed, after which the process will exit.
 The ddns process has successfully started and is now ready to receive commands
 and updates.
 
+% DDNS_START_FORWARDER_ERROR Error from b10-auth when requesting DDNS UPDATE forwarding: %1
+There was an error response from b10-auth to the command to start
+forwarding DDNS UPDATE messages to b10-ddns.
+It is almost certain that DDNS UPDATE messages are not handled now, and that
+they will be responded to with a NOTIMP error code, even though b10-ddns is
+running.
+The error message is printed, and additional information may be found in
+the b10-auth log output. Since this is an error that is sent by the
+b10-auth module, it should have more information as to what the actual
+problem was.
+
+% DDNS_START_FORWARDER_FAIL Error sending request for DDNS UPDATE forwarding to b10-auth: %1
+There was an error when attempting to send b10-auth the request to forward
+DDNS UPDATE messages to the b10-ddns module. This points to an internal
+problem using the inter-module messaging system.
+This needs to be inspected, as it is almost certain that DDNS UPDATE messages
+are not handled now.
+The specific error is printed in the log message.
+
 % DDNS_STOPPED ddns server has stopped
 The ddns process has successfully stopped and is no longer listening for or
 handling commands or updates, and will now exit.
@@ -200,6 +219,24 @@ handling commands or updates, and will now exit.
 There was a keyboard interrupt signal to stop the ddns process. The
 process will now shut down.
 
+% DDNS_STOP_FORWARDER_ERROR Error from b10-auth when requesting to stop DDNS UPDATE forwarding: %1
+There was an error response from b10-auth to the command to stop
+forwarding DDNS UPDATE messages to b10-ddns.
+This specific error may not be fatal; instead of returning NOTIMP for future
+DDNS UPDATE messages, it will return SERVFAIL. However, this does point to
+an underlying problem in the messaging system, and should be inspected.
+The error message is printed, and additional information may be found in
+the b10-auth log output.
+
+% DDNS_STOP_FORWARDER_FAIL Error sending request to stop DDNS UPDATE forwarding to b10-auth: %1
+There was an error when attempting to send b10-auth the request to stop
+forwarding DDNS UPDATE messages to the b10-ddns module. This points to an
+internal problem using the inter-module messaging system.
+This specific error may not be fatal; instead of returning NOTIMP for future
+DDNS UPDATE messages, it will return SERVFAIL. However, this does point to
+an underlying problem in the messaging system, and should be inspected.
+The specific error is printed in the log message.
+
 % DDNS_UNCAUGHT_EXCEPTION uncaught exception of type %1: %2
 The b10-ddns process encountered an uncaught exception and will now shut
 down. This is indicative of a programming error and should not happen under

src/bin/ddns/tests/ddns_test.py

@@ -918,6 +918,10 @@ class TestDDNSSession(unittest.TestCase):
         self.orig_tsig_keyring = isc.server_common.tsig_keyring
         isc.server_common.tsig_keyring = FakeKeyringModule()
         self.server = ddns.DDNSServer(self.__cc_session)
+        # Check that start_ddns_forwarder has been called upon
+        # initialization (before we do anything else that might
+        # cause messages to be sent)
+        self.check_session_start_forwarder_called()
         self.server._UpdateSessionClass = self.__fake_session_creator
         self.__faked_result = UPDATE_SUCCESS # will be returned by fake session
         self.__sock = FakeSocket(-1)
@@ -1179,8 +1183,33 @@ class TestDDNSSession(unittest.TestCase):
             self.assertEqual([], self.__cc_session._sent_msg)
             self.assertEqual(0, self.__cc_session._recvmsg_called)
 
+    def check_session_start_forwarder_called(self):
+        '''Check that the command 'start_ddns_forwarder' has been called
+           This test removes said message from the sent message queue.
+        '''
+        sent_msg, sent_group = self.__cc_session._sent_msg.pop(0)
+        sent_cmd = sent_msg['command']
+        self.assertEqual('Auth', sent_group)
+        self.assertEqual('start_ddns_forwarder', sent_cmd[0])
+        self.assertEqual(1, len(sent_cmd))
+        self.assertEqual(1, self.__cc_session._recvmsg_called)
+        # reset it for other tests
+        self.__cc_session._recvmsg_called = 0
+
+    def check_session_stop_forwarder_called(self):
+        '''Check that the command 'stop_ddns_forwarder' has been called
+           This test removes said message from the sent message queue.
+        '''
+        # check the last message sent
+        sent_msg, sent_group = self.__cc_session._sent_msg.pop()
+        sent_cmd = sent_msg['command']
+        self.assertEqual('Auth', sent_group)
+        self.assertEqual('stop_ddns_forwarder', sent_cmd[0])
+        self.assertEqual(1, len(sent_cmd))
+
     def test_session_msg(self):
         '''Test post update communication with other modules.'''
+
         # Normal cases, confirming communication takes place iff update
         # succeeds
         for r in [UPDATE_SUCCESS, UPDATE_ERROR, UPDATE_DROP]:
@@ -1224,8 +1253,14 @@ class TestDDNSSession(unittest.TestCase):
         self.__cc_session._sendmsg_exception = RuntimeError('unexpected')
         self.assertRaises(RuntimeError, self.check_session)
 
+    def test_session_shutdown_cleanup(self):
+        '''Test that the stop forwarding message is sent'''
+        self.server.shutdown_cleanup()
+        self.check_session_stop_forwarder_called()
+
     def test_session_msg_for_auth(self):
         '''Test post update communication with other modules including Auth.'''
+
         # Let the CC session return in-memory config with sqlite3 backend.
         # (The default case was covered by other tests.)
         self.__cc_session.auth_datasources = \
@@ -1270,6 +1305,34 @@ class TestDDNSSession(unittest.TestCase):
         # check the result
         self.check_session(UPDATE_DROP)
 
+    def test_session_start_stop_forwarder_failures(self):
+        '''Check that we don't crash if the server reports an error
+           setting up or closing down the DDNS UPDATE message forwarder,
+           or if there is an exception from the message queue.'''
+        self.__cc_session._answer_code = 1
+        self.server._DDNSServer__notify_start_forwarder()
+        self.server._DDNSServer__notify_stop_forwarder()
+
+        for exc in [ SessionError("sessionerror"),
+                     SessionTimeout("sessiontimeout"),
+                     ProtocolError("protocolerror") ]:
+            self.__cc_session._recvmsg_exception = exc
+            self.server._DDNSServer__notify_start_forwarder()
+            self.server._DDNSServer__notify_stop_forwarder()
+            self.__cc_session._recvmsg_exception = None
+
+            self.__cc_session._sendmsg_exception = exc
+            self.server._DDNSServer__notify_start_forwarder()
+            self.server._DDNSServer__notify_stop_forwarder()
+            self.__cc_session._recvmsg_exception = None
+
+    def test_session_auth_started(self):
+        '''Check that 'start_ddns_forwarder' is sent (again) when the
+           notification 'auth_started' is received'''
+        # auth_started message should trigger it again
+        answer = self.server.command_handler('auth_started', None)
+        self.check_session_start_forwarder_called()
+
 class TestMain(unittest.TestCase):
     def setUp(self):
         self._server = MyDDNSServer()

tests/lettuce/features/ddns_system.feature

@@ -13,9 +13,8 @@ Feature: DDNS System
 
         # Test 1
         When I use DDNS to set the SOA serial to 1235
-        # Note: test spec says refused here, system returns SERVFAIL
-        #The DDNS response should be REFUSED
-        The DDNS response should be SERVFAIL
+        # Note: test spec says refused here, system returns NOTIMP
+        The DDNS response should be NOTIMP
         And the SOA serial for example.org should be 1234
 
         # Test 2
@@ -53,15 +52,14 @@ Feature: DDNS System
         And wait for new bind10 stderr message DDNS_STARTED
 
         # Test 8
-        # Known issue: after shutdown, first new attempt results in SERVFAIL
         When I use DDNS to set the SOA serial to 1238
-        The DDNS response should be SERVFAIL
-        And the SOA serial for example.org should be 1237
+        The DDNS response should be SUCCESS
+        And the SOA serial for example.org should be 1238
 
-        When I use DDNS to set the SOA serial to 1238
+        When I use DDNS to set the SOA serial to 1239
         And wait for new bind10 stderr message AUTH_LOAD_ZONE
         The DDNS response should be SUCCESS
-        And the SOA serial for example.org should be 1238
+        And the SOA serial for example.org should be 1239
 
         # Test 9
         When I send bind10 the command Auth shutdown
@@ -70,10 +68,10 @@ Feature: DDNS System
         And wait for new bind10 stderr message AUTH_SERVER_STARTED
 
         # Test 10
-        When I use DDNS to set the SOA serial to 1239
+        When I use DDNS to set the SOA serial to 1240
         And wait for new bind10 stderr message AUTH_LOAD_ZONE
         The DDNS response should be SUCCESS
-        And the SOA serial for example.org should be 1239
+        And the SOA serial for example.org should be 1240
 
         # Test 11
         When I configure BIND10 to stop running DDNS
@@ -82,10 +80,10 @@ Feature: DDNS System
         bind10 module DDNS should not be running
 
         # Test 12
-        When I use DDNS to set the SOA serial to 1240
+        When I use DDNS to set the SOA serial to 1241
         # should this be REFUSED again?
-        The DDNS response should be SERVFAIL
-        And the SOA serial for example.org should be 1239
+        The DDNS response should be NOTIMP
+        And the SOA serial for example.org should be 1240
 
     Scenario: ACL
         Given I have bind10 running with configuration ddns/ddns.config