[master] Merge branch 'trac1513'

src/bin/ddns/ddns.py.in

@@ -23,11 +23,12 @@ import bind10_config
 from isc.dns import *
 import isc.ddns.session
 from isc.ddns.zone_config import ZoneConfig
-from isc.ddns.logger import ClientFormatter
+from isc.ddns.logger import ClientFormatter, ZoneFormatter
 from isc.config.ccsession import *
-from isc.cc import SessionError, SessionTimeout
+from isc.cc import SessionError, SessionTimeout, ProtocolError
 import isc.util.process
 import isc.util.cio.socketsession
+from isc.notify.notify_out import ZONE_NEW_DATA_READY_CMD
 import isc.server_common.tsig_keyring
 from isc.datasrc import DataSourceClient
 import select
@@ -79,6 +80,9 @@ AUTH_SPECFILE_LOCATION = AUTH_SPECFILE_PATH + '/auth.spec'
 
 isc.util.process.rename()
 
+# Cooperating modules
+XFROUT_MODULE_NAME = 'Xfrout'
+
 class DDNSConfigError(Exception):
     '''An exception indicating an error in updating ddns configuration.
 
@@ -193,7 +197,7 @@ class DDNSServer:
         # DDNS Protocol handling class.
         self._UpdateSessionClass = isc.ddns.session.UpdateSession
 
-    class SessionError(Exception):
+    class InternalError(Exception):
         '''Exception for internal errors in an update session.
 
         This exception is expected to be caught within the server class,
@@ -301,8 +305,8 @@ class DDNSServer:
                                isc.server_common.tsig_keyring.get_keyring())
         tsig_error = tsig_ctx.verify(tsig_record, req_data)
         if tsig_error != TSIGError.NOERROR:
-            raise SessionError("Failed to verify request's TSIG: " +
+            raise self.InternalError("Failed to verify request's TSIG: " +
-                               str(tsig_error))
+                                     str(tsig_error))
         return tsig_ctx
 
     def handle_request(self, req_session):
@@ -339,13 +343,14 @@ class DDNSServer:
         # as an internal error and don't bother to respond.
         try:
             if sock.proto == socket.IPPROTO_TCP:
-                raise SessionError('TCP requests are not yet supported')
+                raise self.InternalError('TCP requests are not yet supported')
             self.__request_msg.clear(Message.PARSE)
             # specify PRESERVE_ORDER as we need to handle each RR separately.
             self.__request_msg.from_wire(req_data, Message.PRESERVE_ORDER)
             if self.__request_msg.get_opcode() != Opcode.UPDATE():
-                raise SessionError('Update request has unexpected opcode: ' +
+                raise self.InternalError('Update request has unexpected '
-                                   str(self.__request_msg.get_opcode()))
+                                         'opcode: ' +
+                                         str(self.__request_msg.get_opcode()))
             tsig_ctx = self.__check_request_tsig(self.__request_msg, req_data)
         except Exception as ex:
             logger.error(DDNS_REQUEST_PARSE_FAIL, ex)
@@ -371,15 +376,75 @@ class DDNSServer:
             msg.to_wire(self.__response_renderer, tsig_ctx)
         else:
             msg.to_wire(self.__response_renderer)
+
+        ret = self.__send_response(sock, self.__response_renderer.get_data(),
+                                   remote_addr)
+        if result == isc.ddns.session.UPDATE_SUCCESS:
+            self.__notify_update(zname, zclass)
+        return ret
+
+    def __send_response(self, sock, data, dest):
+        '''Send DDNS response to the client.
+
+        Right now, this is a straightforward subroutine of handle_request(),
+        but is intended to be extended evetually so that it can handle more
+        comlicated operations for TCP (which requires asynchronous write).
+        Further, when we support multiple requests over a single TCP
+        connection, this method may even be shared by multiple methods.
+
+        Parameters:
+        sock: (python socket) the socket to which the response should be sent.
+        data: (binary) the response data
+        dest: (python socket address) the destion address to which the response
+          should be sent.
+
+        Return: True if the send operation succeds; otherwise False.
+
+        '''
         try:
-            sock.sendto(self.__response_renderer.get_data(), remote_addr)
+            sock.sendto(data, dest)
         except socket.error as ex:
-            logger.error(DDNS_RESPONSE_SOCKET_ERROR,
+            logger.error(DDNS_RESPONSE_SOCKET_ERROR, ClientFormatter(dest), ex)
-                         ClientFormatter(remote_addr), ex)
             return False
 
         return True
 
+    def __notify_update(self, zname, zclass):
+        '''Notify other modules of the update.
+
+        Note that we use blocking communication here.  While the internal
+        communication bus is generally expected to be pretty responsive and
+        error free, notable delay can still occur, and in worse cases timeouts
+        or connection reset can happen.  In these cases, even if the trouble
+        is temporary, the update service will be suspended for a while.
+        For a longer term we'll need to switch to asynchronous communication,
+        but for now we rely on the blocking operation.
+
+        Note also that we directly refer to the "protected" member of
+        ccsession (_cc._session) rather than creating a separate channel.
+        It's probably not the best practice, but hopefully we can introduce
+        a cleaner way when we support asynchronous communication.
+        At the moment we prefer the brevity with the use of internal channel
+        of the cc session.
+
+        '''
+        param = {'zone_name': zname.to_text(), 'zone_class': zclass.to_text()}
+        msg = create_command(ZONE_NEW_DATA_READY_CMD, param)
+        modname = XFROUT_MODULE_NAME
+        try:
+            seq = self._cc._session.group_sendmsg(msg, modname)
+            answer, _ = self._cc._session.group_recvmsg(False, seq)
+            rcode, error_msg = parse_answer(answer)
+        except (SessionTimeout, SessionError, ProtocolError) as ex:
+            rcode = 1
+            error_msg = str(ex)
+        if rcode == 0:
+            logger.debug(TRACE_BASIC, DDNS_UPDATE_NOTIFY, modname,
+                         ZoneFormatter(zname, zclass))
+        else:
+            logger.error(DDNS_UPDATE_NOTIFY_FAIL, modname,
+                         ZoneFormatter(zname, zclass), error_msg)
+
     def handle_session(self, fileno):
         """
         Handle incoming session on the socket with given fileno.

src/bin/ddns/ddns_messages.mes

@@ -112,3 +112,26 @@ process will now shut down.
 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
 normal circumstances. The exception type and message are printed.
+
+% DDNS_UPDATE_NOTIFY notified %1 of updates to %2
+Debug message.  b10-ddns has made updates to a zone based on an update
+request and has successfully notified an external module of the updates.
+The notified module will use that information for updating its own
+state or any necessary protocol action such as zone reloading or sending
+notify messages to secondary servers.
+
+% DDNS_UPDATE_NOTIFY_FAIL failed to notify %1 of updates to %2: %3
+b10-ddns has made updates to a zone based on an update request and
+tried to notify an external module of the updates, but the
+notification fails.  Severity of this effect depends on the type of
+the module.  If it's b10-xfrout, this means DNS notify messages won't
+be sent to secondary servers of the zone.  It's suboptimal, but not
+necessarily critical as the secondary servers will try to check the
+zone's status periodically.  If it's b10-auth and the notification was
+needed to have it reload the corresponding zone, it's more serious
+because b10-auth won't be able to serve the new version of the zone
+unless some explicit recovery action is taken.  So the administrator
+needs to examine this message and takes an appropriate action.  In
+either case, this notification is generally expected to succeed; so
+the fact it fails itself means there's something wrong in the BIND 10
+system, and it would be advisable to check other log messages.

src/bin/ddns/tests/ddns_test.py

@@ -19,7 +19,9 @@ from isc.ddns.session import *
 from isc.dns import *
 from isc.acl.acl import ACCEPT
 import isc.util.cio.socketsession
+from isc.cc.session import SessionTimeout, SessionError, ProtocolError
 from isc.datasrc import DataSourceClient
+from isc.config.ccsession import create_answer
 import ddns
 import errno
 import os
@@ -147,6 +149,10 @@ class FakeKeyringModule:
 
 class MyCCSession(isc.config.ConfigData):
     '''Fake session with minimal interface compliance.'''
+
+    # faked CC sequence used in group_send/recvmsg
+    FAKE_SEQUENCE = 53
+
     def __init__(self):
         module_spec = isc.config.module_spec_from_file(
             ddns.SPECFILE_LOCATION)
@@ -155,6 +161,14 @@ class MyCCSession(isc.config.ConfigData):
         self._stopped = False
         # Used as the return value of get_remote_config_value.  Customizable.
         self.auth_db_file = READ_ZONE_DB_FILE
+        # faked cc channel, providing group_send/recvmsg itself.  The following
+        # attributes are for inspection/customization in tests.
+        self._session = self
+        self._sent_msg = []
+        self._recvmsg_called = 0
+        self._answer_code = 0   # code used in answer returned via recvmsg
+        self._sendmsg_exception = None # will be raised from sendmsg if !None
+        self._recvmsg_exception = None # will be raised from recvmsg if !None
 
     def start(self):
         '''Called by DDNSServer initialization, but not used in tests'''
@@ -177,6 +191,32 @@ class MyCCSession(isc.config.ConfigData):
         if module_name == "Auth" and item == "database_file":
             return self.auth_db_file, False
 
+    def group_sendmsg(self, msg, group):
+        # remember the passed parameter, and return dummy sequence
+        self._sent_msg.append((msg, group))
+        if self._sendmsg_exception is not None:
+            raise self._sendmsg_exception
+        return self.FAKE_SEQUENCE
+
+    def group_recvmsg(self, nonblock, seq):
+        self._recvmsg_called += 1
+        if seq != self.FAKE_SEQUENCE:
+            raise RuntimeError('unexpected CC sequence: ' + str(seq))
+        if self._recvmsg_exception is not None:
+            raise self._recvmsg_exception
+        if self._answer_code is 0:
+            return create_answer(0), None
+        else:
+            return create_answer(self._answer_code, "dummy error value"), None
+
+    def clear_msg(self):
+        '''Clear instrumental attributes related session messages.'''
+        self._sent_msg = []
+        self._recvmsg_called = 0
+        self._answer_code = 0
+        self._sendmsg_exception = None
+        self._recvmsg_exception = None
+
 class MyDDNSServer():
     '''Fake DDNS server used to test the main() function'''
     def __init__(self):
@@ -571,13 +611,13 @@ def create_msg(opcode=Opcode.UPDATE(), zones=[TEST_ZONE_RECORD], prereq=[],
     return renderer.get_data()
 
 
-class TestDDNSession(unittest.TestCase):
+class TestDDNSSession(unittest.TestCase):
     def setUp(self):
-        cc_session = MyCCSession()
+        self.__cc_session = MyCCSession()
-        self.assertFalse(cc_session._started)
+        self.assertFalse(self.__cc_session._started)
         self.orig_tsig_keyring = isc.server_common.tsig_keyring
         isc.server_common.tsig_keyring = FakeKeyringModule()
-        self.server = ddns.DDNSServer(cc_session)
+        self.server = ddns.DDNSServer(self.__cc_session)
         self.server._UpdateSessionClass = self.__fake_session_creator
         self.__faked_result = UPDATE_SUCCESS # will be returned by fake session
         self.__sock = FakeSocket(-1)
@@ -705,6 +745,76 @@ class TestDDNSession(unittest.TestCase):
         num_rrsets = len(self.__req_message.get_section(SECTION_PREREQUISITE))
         self.assertEqual(2, num_rrsets)
 
+    def check_session_msg(self, result, expect_recv=1):
+        '''Check post update communication with other modules.'''
+        # iff the update succeeds, b10-ddns should tell interested other
+        # modules the information about the update zone in the form of
+        # {'command': ['notify', {'zone_name': <updated_zone_name>,
+        #                         'zone_class', <updated_zone_class>}]}
+        # and expect an answer by calling group_recvmsg().
+        #
+        # expect_recv indicates the expected number of calls to
+        # group_recvmsg(), which is normally 1, but can be 0 if send fails.
+        if result == UPDATE_SUCCESS:
+            self.assertEqual(1, len(self.__cc_session._sent_msg))
+            self.assertEqual(expect_recv, self.__cc_session._recvmsg_called)
+            sent_msg, sent_group = self.__cc_session._sent_msg[0]
+            sent_cmd = sent_msg['command']
+            self.assertEqual('Xfrout', sent_group)
+            self.assertEqual('notify', sent_cmd[0])
+            self.assertEqual(2, len(sent_cmd[1]))
+            self.assertEqual(TEST_ZONE_NAME.to_text(), sent_cmd[1]['zone_name'])
+            self.assertEqual(TEST_RRCLASS.to_text(), sent_cmd[1]['zone_class'])
+        else:
+            # for other result cases neither send nor recvmsg should be called.
+            self.assertEqual([], self.__cc_session._sent_msg)
+            self.assertEqual(0, self.__cc_session._recvmsg_called)
+
+    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]:
+            self.__cc_session.clear_msg()
+            self.check_session(result=r)
+            self.check_session_msg(r)
+
+        # Return an error from the remote module, which should be just ignored.
+        self.__cc_session.clear_msg()
+        self.__cc_session._answer_code = 1
+        self.check_session()
+        self.check_session_msg(UPDATE_SUCCESS)
+
+        # raise some exceptions from the faked session.  Expected ones are
+        # simply (logged and) ignored
+        self.__cc_session.clear_msg()
+        self.__cc_session._recvmsg_exception = SessionTimeout('dummy timeout')
+        self.check_session()
+        self.check_session_msg(UPDATE_SUCCESS)
+
+        self.__cc_session.clear_msg()
+        self.__cc_session._recvmsg_exception = SessionError('dummy error')
+        self.check_session()
+        self.check_session_msg(UPDATE_SUCCESS)
+
+        self.__cc_session.clear_msg()
+        self.__cc_session._recvmsg_exception = ProtocolError('dummy perror')
+        self.check_session()
+        self.check_session_msg(UPDATE_SUCCESS)
+
+        # Similar to the previous cases, but sendmsg() raises, so there should
+        # be no call to recvmsg().
+        self.__cc_session.clear_msg()
+        self.__cc_session._sendmsg_exception = SessionError('send error')
+        self.check_session()
+        self.check_session_msg(UPDATE_SUCCESS, expect_recv=0)
+
+        # Unexpected exception will be propagated (and will terminate the
+        # server)
+        self.__cc_session.clear_msg()
+        self.__cc_session._sendmsg_exception = RuntimeError('unexpected')
+        self.assertRaises(RuntimeError, self.check_session)
+
     def test_session_with_config(self):
         '''Check a session with more relistic config setups