From: JINMEI Tatuya <jinmei@isc.org>
Date: Wed, 13 Jun 2012 21:22:49 +0000 (-0700)
Subject: [2020] updated comments about zonemgr config updates re. zone class and errors.
X-Git-Tag: trac2351_base~97^2~9^2~5^2~2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f5abe6318c5b84383cde4f74e79a6de0cfca87e4;p=thirdparty%2Fkea.git

[2020] updated comments about zonemgr config updates re. zone class and errors.
---

diff --git a/src/bin/ddns/ddns.py.in b/src/bin/ddns/ddns.py.in
index 29da937e23..3e0172586d 100755
--- a/src/bin/ddns/ddns.py.in
+++ b/src/bin/ddns/ddns.py.in
@@ -309,16 +309,17 @@ class DDNSServer:
         new_secondary_zones = set()
         try:
             # Parse the new config and build a new list of secondary zones.
-            # Note that validation should have been done by zonemgr, so
-            # the following shouldn't fail in theory.  But the configuration
-            # interface is quite complicated and there may be a hole, so
-            # we'll perform minimal defense ourselves.
+            # Unforutnately, in the current implementation, even an observer
+            # module needs to perform full validation.  This should be changed
+            # so that only post-validation (done by the main module) config is
+            # delevered to observer modules, but until it's supported we need
+            # to protect ourselves.
             for zone_spec in sec_zones:
                 zname = Name(zone_spec['name'])
-                # class is optional per spec.  ideally this should be merged
-                # within the config module, but it's not really clear if we
-                # can assume that due to its complexity - so we don't rely on
-                # it.
+                # class has the default value in case it's unspecified.
+                # ideally this should be merged within the config module, but
+                # the current implementation doesn't esnure that, so we need to
+                # subsitute it ourselves.
                 if 'class' in zone_spec:
                     zclass = RRClass(zone_spec['class'])
                 else:
diff --git a/src/bin/ddns/ddns_messages.mes b/src/bin/ddns/ddns_messages.mes
index de83487c2b..7718c4d2b5 100644
--- a/src/bin/ddns/ddns_messages.mes
+++ b/src/bin/ddns/ddns_messages.mes
@@ -162,15 +162,21 @@ zones is logged.
 % DDNS_SECONDARY_ZONES_UPDATE_FAIL failed to update secondary zone list: %1
 An error message.  b10-ddns was notified of updates to a list of
 secondary zones from b10-zonemgr and tried to update its own internal
-copy of the list, but it failed.  This can happen only if the
-configuration contains an error, but such a configuration should have
-been rejected by b10-zonemgr first and shouldn't be delivered to
-b10-ddns, so this should basically be an internal bug.  It's advisable
-to submit a bug report if you ever see this message.  Also, while
-b10-ddns still keeps running with the previous configuration when this
-error happens, it's possible that the entire system is in an
-inconsistent state.  So it's probably better to restart bind10, or at
-least restart b10-ddns.
+copy of the list, but it failed.  This can happen if the configuration
+contains an error, and b10-zonemgr should also reject that update.
+Unfortunately, in the current implementation there is no way to ensure
+that both zonemgr and ddns have consistent information when an update
+contains an error; further, as of this writing zonemgr has a bug that
+it could partially update the list of secondary zones if part of the
+list has an error (see Trac ticket #2038).  b10-ddns still keeps
+running with the previous configuration, but it's strongly advisable
+to check log messages from zonemgr, and if it indicates there can be
+inconsistent state, it's better to restart the entire BIND 10 system
+(just restarting b10-ddns wouldn't be enough, because zonemgr can have
+partially updated configuration due to bug #2038).  The log message
+contains an error description, but it's intentionally kept simple as
+it's primarily a matter of zonemgr.  To know the details of the error,
+log messages of zonemgr should be consulted.
 
 % DDNS_SESSION session arrived on file descriptor %1
 A debug message, informing there's some activity on the given file descriptor.
diff --git a/src/bin/ddns/tests/ddns_test.py b/src/bin/ddns/tests/ddns_test.py
index 4468315dd8..b5208c2b3d 100755
--- a/src/bin/ddns/tests/ddns_test.py
+++ b/src/bin/ddns/tests/ddns_test.py
@@ -499,7 +499,8 @@ class TestDDNSServer(unittest.TestCase):
         self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
                          self.ddns_server._secondary_zones)
 
-        # Similar to the above, but the optional 'class' is missing.
+        # Similar to the above, but 'class' is unspecified.  The default value
+        # should be used.
         self.__cc_session._zonemgr_config = {'secondary_zones': [
                 {'name': TEST_ZONE_NAME_STR}]}
         self.__cc_session.add_remote_config_by_name('Zonemgr')