BIND 10 trac2676, updated. 773d25dc86682595c2af983b7c349deea9abb0b6 [2676] Fix FIXME

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Feb 19 08:56:24 UTC 2013


The branch, trac2676 has been updated
       via  773d25dc86682595c2af983b7c349deea9abb0b6 (commit)
       via  f29f0653f27ce5e551039d544b07acefd6e1275e (commit)
       via  b459be530b5b159af7de708fe653f5b500a9745f (commit)
       via  3d8d88cb37a6423b9f1b969e8ab3342e8116cf83 (commit)
       via  12027383143fceba912caded72cdef8451a37021 (commit)
      from  ab159f505de08729862904b1412d9dede8138013 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 773d25dc86682595c2af983b7c349deea9abb0b6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Feb 19 09:53:01 2013 +0100

    [2676] Fix FIXME
    
    According to the notes in the review, we need to wait for the answer.

commit f29f0653f27ce5e551039d544b07acefd6e1275e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Feb 19 09:50:15 2013 +0100

    [2676] Reuse constants for remote loadzone command

commit b459be530b5b159af7de708fe653f5b500a9745f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Feb 19 09:38:41 2013 +0100

    [2676] Remove the notion of timeouts from the message docs
    
    Remove the part of the DDNS_UPDATE_NOTIFY_FAIL message description,
    complaining about how bad the timeouts are. The timeouts should be
    solved by now and should happen only in really rare circumstances. If
    the other module is not there, it should return error right away.

commit 3d8d88cb37a6423b9f1b969e8ab3342e8116cf83
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Feb 19 09:30:31 2013 +0100

    [2676] Reorder and tweak exception to be closer to original
    
    In case the auth wasn't there before the refactoring, it did a session
    timeout, which resulted in DDNS_(START|STOP)_FORWARDER_ERROR log
    message. So we handle the RPCRecipientMissing in the same branch (and
    list above the RPCError, since it is a subclass of that).

commit 12027383143fceba912caded72cdef8451a37021
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Feb 19 09:24:42 2013 +0100

    [2676] (minor) Comment tweaks

-----------------------------------------------------------------------

Summary of changes:
 src/bin/cmdctl/cmdctl.py.in                        |    2 +-
 src/bin/ddns/ddns.py.in                            |   21 ++++++----
 src/bin/ddns/ddns_messages.mes                     |   42 ++++++++------------
 .../isc/config/tests/unittest_fakesession.py       |    2 +-
 src/lib/python/isc/server_common/auth_command.py   |   14 ++++---
 5 files changed, 40 insertions(+), 41 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/cmdctl/cmdctl.py.in b/src/bin/cmdctl/cmdctl.py.in
index 6468016..457873b 100755
--- a/src/bin/cmdctl/cmdctl.py.in
+++ b/src/bin/cmdctl/cmdctl.py.in
@@ -433,7 +433,7 @@ class CommandControl():
             # from the module one (due to threads and blocking), and
             # because the plain cc session does not have the high-level
             # rpc-call method, we use the low-level way and create the
-            # command ourself.
+            # command ourselves.
             msg = ccsession.create_command(command_name, params)
             seq = self._cc.group_sendmsg(msg, module_name, want_answer=True)
             logger.debug(DBG_CMDCTL_MESSAGING, CMDCTL_COMMAND_SENT,
diff --git a/src/bin/ddns/ddns.py.in b/src/bin/ddns/ddns.py.in
index e8cff14..0eff02c 100755
--- a/src/bin/ddns/ddns.py.in
+++ b/src/bin/ddns/ddns.py.in
@@ -32,6 +32,8 @@ import isc.util.cio.socketsession
 import isc.server_common.tsig_keyring
 from isc.server_common.dns_tcp import DNSTCPContext
 from isc.datasrc import DataSourceClient
+from isc.server_common.auth_command import AUTH_LOADZONE_COMMAND, \
+    auth_loadzone_params
 import select
 import time
 import errno
@@ -544,10 +546,11 @@ class DDNSServer:
         '''Notify auth that DDNS Update messages can now be forwarded'''
         try:
             self._cc.rpc_call("start_ddns_forwarder", AUTH_MODULE_NAME)
+        except (SessionTimeout, SessionError, ProtocolError,
+            RPCRecipientMissing) as ex:
+            logger.error(DDNS_START_FORWARDER_FAIL, ex)
         except RPCError as e:
             logger.error(DDNS_START_FORWARDER_ERROR, e)
-        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.
@@ -555,15 +558,16 @@ class DDNSServer:
         '''
         try:
             self._cc.rpc_call("stop_ddns_forwarder", AUTH_MODULE_NAME)
+        except (SessionTimeout, SessionError, ProtocolError,
+            RPCRecipientMissing) as ex:
+            logger.error(DDNS_STOP_FORWARDER_FAIL, ex)
         except RPCError as e:
             logger.error(DDNS_STOP_FORWARDER_ERROR, e)
-        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.'''
-        param = {'origin': zname.to_text(), 'class': zclass.to_text()}
-        self.__notify_update(AUTH_MODULE_NAME, 'loadzone', param, zname,
+        self.__notify_update(AUTH_MODULE_NAME, AUTH_LOADZONE_COMMAND,
+                             auth_loadzone_params(zname, zclass), zname,
                              zclass)
 
     def __notify_xfrout(self, zname, zclass):
@@ -592,8 +596,9 @@ class DDNSServer:
 
         '''
         try:
-            # FIXME? Do we really need to wait for the answer here? Shouldn't
-            # this be some kind of notification instead?
+            # FIXME? Is really rpc_call the correct one? What if there are more
+            # than one recipient of the given kind? What if none? We need to
+            # think of some kind of notification/broadcast mechanism.
             self._cc.rpc_call(command, modname, params=params)
             logger.debug(TRACE_BASIC, DDNS_UPDATE_NOTIFY, modname,
                          ZoneFormatter(zname, zclass))
diff --git a/src/bin/ddns/ddns_messages.mes b/src/bin/ddns/ddns_messages.mes
index cdc7b4d..42b1276 100644
--- a/src/bin/ddns/ddns_messages.mes
+++ b/src/bin/ddns/ddns_messages.mes
@@ -253,29 +253,19 @@ notify messages to secondary servers.
 b10-ddns has made updates to a zone based on an update request and
 tried to notify an external component of the updates, but the
 notification fails.  One possible cause of this is that the external
-component is not really running and it times out in waiting for the
-response, although it will be less likely to happen in practice
-because these components will normally be configured to run when the
-server provides the authoritative DNS service; ddns is rather optional
-among them.  If this happens, however, it will suspend b10-ddns for a
-few seconds during which it cannot handle new requests (some may be
-delayed, some may be dropped, depending on the volume of the incoming
-requests).  This is obviously bad, and if this error happens due to
-this reason, the administrator should make sure the component in
-question should be configured to run.  For a longer term, b10-ddns
-should be more robust about this case such as by making this
-notification asynchronously and/or detecting the existence of the
-external components to avoid hopeless notification in the first place.
-Severity of this error for the receiving components depends on the
-type of the component.  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.
+component is not really running, although it will be less likely to
+happen in practice because these components will normally be
+configured to run when the server provides the authoritative DNS
+service; ddns is rather optional among them. Severity of this error
+for the receiving components depends on the type of the component.  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.
diff --git a/src/lib/python/isc/config/tests/unittest_fakesession.py b/src/lib/python/isc/config/tests/unittest_fakesession.py
index 84be1fe..7043683 100644
--- a/src/lib/python/isc/config/tests/unittest_fakesession.py
+++ b/src/lib/python/isc/config/tests/unittest_fakesession.py
@@ -28,7 +28,7 @@ class WouldBlockForever(Exception):
 class FakeModuleCCSession:
     def __init__(self):
         self.subscriptions = {}
-        # each entry is of the form [ channel, instance, message ]
+        # each entry is of the form [ channel, instance, message, want_answer ]
         self.message_queue = []
         self._socket = "ok we just need something not-None here atm"
         # if self.timeout is set to anything other than 0, and
diff --git a/src/lib/python/isc/server_common/auth_command.py b/src/lib/python/isc/server_common/auth_command.py
index 493d5fb..5b7635a 100644
--- a/src/lib/python/isc/server_common/auth_command.py
+++ b/src/lib/python/isc/server_common/auth_command.py
@@ -22,6 +22,13 @@ from isc.log_messages.server_common_messages import *
 from isc.server_common.logger import logger
 
 AUTH_MODULE_NAME = 'Auth'
+AUTH_LOADZONE_COMMAND = 'loadzone'
+
+def auth_loadzone_params(zone_name, zone_class):
+    return {
+        "origin": zone_name.to_text(),
+        "class": zone_class.to_text()
+    }
 
 def auth_loadzone_command(module_cc, zone_name, zone_class):
     '''Create a 'loadzone' command with a given zone for Auth server.
@@ -50,8 +57,5 @@ def auth_loadzone_command(module_cc, zone_name, zone_class):
     # to notification-driven approach, at which point the function would
     # be changed a lot.
 
-    param = {
-        "origin": zone_name.to_text(),
-        "class": zone_class.to_text()
-    }
-    return create_command("loadzone", param)
+    return create_command(AUTH_LOADZONE_COMMAND,
+                          auth_loadzone_params(zone_name, zone_class))



More information about the bind10-changes mailing list