BIND 10 trac1259, updated. 9e17bd49b426ffba00312cf90ec80d178a20b964 [1259] Better names
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Oct 3 13:14:59 UTC 2011
The branch, trac1259 has been updated
via 9e17bd49b426ffba00312cf90ec80d178a20b964 (commit)
via 519720d9c6eb354a2e31089f1c7b8fd0760053f9 (commit)
via c6babcd3e44bc42fdb090d3a4837848d8c7c149c (commit)
via d36eda71276b43e4281ae53fd558155725f4d4eb (commit)
via 32f075fa288dc5ea049cbf72657386889144bd12 (commit)
via 471edfd7a86d91f04536bc7c7fb42ad7239e1731 (commit)
from 26691e282b76d74959e63524b280e77b09ac89df (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 9e17bd49b426ffba00312cf90ec80d178a20b964
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 3 15:13:51 2011 +0200
[1259] Better names
commit 519720d9c6eb354a2e31089f1c7b8fd0760053f9
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 3 15:11:01 2011 +0200
[1259] Log when TTLs don't match
commit c6babcd3e44bc42fdb090d3a4837848d8c7c149c
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 3 11:40:46 2011 +0200
[1259] Check the thing is not used after exception
As it would not be safe anyway.
commit d36eda71276b43e4281ae53fd558155725f4d4eb
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 3 11:19:11 2011 +0200
[1259] Check the class
If it corresponds to the updater. This would be a bug, but we better
detect it sooner.
commit 32f075fa288dc5ea049cbf72657386889144bd12
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 3 11:04:07 2011 +0200
[1259] Rename parameter to ds_client
Because datasource could represent something bigger behind the scenes.
commit 471edfd7a86d91f04536bc7c7fb42ad7239e1731
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon Oct 3 11:02:27 2011 +0200
[1259] Use a constant for 100 updates
-----------------------------------------------------------------------
Summary of changes:
src/lib/python/isc/Makefile.am | 2 +-
src/lib/python/isc/log_messages/Makefile.am | 2 +
.../python/isc/log_messages/libxfrin_messages.py | 1 +
src/lib/python/isc/xfrin/Makefile.am | 11 ++
src/lib/python/isc/xfrin/diff.py | 84 +++++++++++----
.../lib/python/isc/xfrin/libxfrin_messages.mes | 18 ++--
src/lib/python/isc/xfrin/tests/diff_tests.py | 109 +++++++++++++++++++-
7 files changed, 190 insertions(+), 37 deletions(-)
create mode 100644 src/lib/python/isc/log_messages/libxfrin_messages.py
copy tests/system/bindctl/setup.sh => src/lib/python/isc/xfrin/libxfrin_messages.mes (64%)
mode change 100755 => 100644
-----------------------------------------------------------------------
diff --git a/src/lib/python/isc/Makefile.am b/src/lib/python/isc/Makefile.am
index a9cc0e1..a3e74c5 100644
--- a/src/lib/python/isc/Makefile.am
+++ b/src/lib/python/isc/Makefile.am
@@ -1,5 +1,5 @@
SUBDIRS = datasrc cc config dns log net notify util testutils acl bind10
-SUBDIRS += log_messages xfrin
+SUBDIRS += xfrin log_messages
python_PYTHON = __init__.py
diff --git a/src/lib/python/isc/log_messages/Makefile.am b/src/lib/python/isc/log_messages/Makefile.am
index b9bc4c8..30f8374 100644
--- a/src/lib/python/isc/log_messages/Makefile.am
+++ b/src/lib/python/isc/log_messages/Makefile.am
@@ -11,6 +11,7 @@ EXTRA_DIST += zonemgr_messages.py
EXTRA_DIST += cfgmgr_messages.py
EXTRA_DIST += config_messages.py
EXTRA_DIST += notify_out_messages.py
+EXTRA_DIST += libxfrin_messages.py
CLEANFILES = __init__.pyc
CLEANFILES += bind10_messages.pyc
@@ -23,6 +24,7 @@ CLEANFILES += zonemgr_messages.pyc
CLEANFILES += cfgmgr_messages.pyc
CLEANFILES += config_messages.pyc
CLEANFILES += notify_out_messages.pyc
+CLEANFILES += libxfrin_messages.pyc
CLEANDIRS = __pycache__
diff --git a/src/lib/python/isc/log_messages/libxfrin_messages.py b/src/lib/python/isc/log_messages/libxfrin_messages.py
new file mode 100644
index 0000000..74da329
--- /dev/null
+++ b/src/lib/python/isc/log_messages/libxfrin_messages.py
@@ -0,0 +1 @@
+from work.libxfrin_messages import *
diff --git a/src/lib/python/isc/xfrin/Makefile.am b/src/lib/python/isc/xfrin/Makefile.am
index 8b45348..d31acb6 100644
--- a/src/lib/python/isc/xfrin/Makefile.am
+++ b/src/lib/python/isc/xfrin/Makefile.am
@@ -1,6 +1,17 @@
SUBDIRS = . tests
python_PYTHON = __init__.py diff.py
+BUILT_SOURCES = $(PYTHON_LOGMSGPKG_DIR)/work/libxfrin_messages.py
+nodist_pylogmessage_PYTHON = $(PYTHON_LOGMSGPKG_DIR)/work/libxfrin_messages.py
+pylogmessagedir = $(pyexecdir)/isc/log_messages/
+
+CLEANFILES = $(PYTHON_LOGMSGPKG_DIR)/work/libxfrin_messages.py
+CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/libxfrin_messages.pyc
+
+# Define rule to build logging source files from message file
+$(PYTHON_LOGMSGPKG_DIR)/work/libxfrin_messages.py: libxfrin_messages.mes
+ $(top_builddir)/src/lib/log/compiler/message \
+ -d $(PYTHON_LOGMSGPKG_DIR)/work -p $(srcdir)/libxfrin_messages.mes
pythondir = $(pyexecdir)/isc/xfrin
diff --git a/src/lib/python/isc/xfrin/diff.py b/src/lib/python/isc/xfrin/diff.py
index 6c05401..001c215 100644
--- a/src/lib/python/isc/xfrin/diff.py
+++ b/src/lib/python/isc/xfrin/diff.py
@@ -19,6 +19,8 @@ it to the datasource.
"""
import isc.dns
+import isc.log
+from isc.log_messages.libxfrin_messages import *
class NoSuchZone(Exception):
"""
@@ -26,6 +28,19 @@ class NoSuchZone(Exception):
"""
pass
+"""
+This is the amount of changes we accumulate before calling Diff.apply
+automatically.
+
+The number 100 is just taken from Bind9. We don't know the rationale
+for exactly this amount, but we think it is just some randomly chosen
+number.
+"""
+# If changing this, modify the tests accordingly as well.
+DIFF_APPLY_TRESHOLD = 100
+
+logger = isc.log.Logger('libxfrin')
+
class Diff:
"""
The class represents a diff against current state of datasource on
@@ -40,34 +55,35 @@ class Diff:
the changes to underlying data source right away, but keeps them for
a while.
"""
- def __init__(self, datasource, zone):
+ def __init__(self, ds_client, zone):
"""
Initializes the diff to a ready state. It checks the zone exists
in the datasource and if not, NoSuchZone is raised. This also creates
a transaction in the data source.
- The datasource is the one containing the zone. Zone is isc.dns.Name
- object representing the name of the zone (its apex).
+ The ds_client is the datasource client containing the zone. Zone is
+ isc.dns.Name object representing the name of the zone (its apex).
You can also expect isc.datasrc.Error or isc.datasrc.NotImplemented
exceptions.
"""
- self.__updater = datasource.get_updater(zone, False)
+ self.__updater = ds_client.get_updater(zone, False)
if self.__updater is None:
# The no such zone case
raise NoSuchZone("Zone " + str(zone) +
" does not exist in the data source " +
- str(datasource))
+ str(ds_client))
self.__buffer = []
def __check_commited(self):
"""
- This checks if the diff is already commited. If it is, it raises
- ValueError. This check is for methods that need to work only on
+ This checks if the diff is already commited or broken. If it is, it
+ raises ValueError. This check is for methods that need to work only on
yet uncommited diffs.
"""
if self.__updater is None:
- raise ValueError("The diff is already commited, you come late")
+ raise ValueError("The diff is already commited or it has raised " +
+ "an exception, you come late")
def __data_common(self, rr, operation):
"""
@@ -80,8 +96,12 @@ class Diff:
if rr.get_rdata_count() != 1:
raise ValueError('The rrset must contain exactly 1 Rdata, but ' +
'it holds ' + str(rr.get_rdata_count()))
+ if rr.get_class() != self.__updater.get_class():
+ raise ValueError("The rrset's class " + str(rr.get_class()) +
+ " does not match updater's " +
+ str(self.__updater.get_class()))
self.__buffer.append((operation, rr))
- if len(self.__buffer) >= 100:
+ if len(self.__buffer) >= DIFF_APPLY_TRESHOLD:
# Time to auto-apply, so the data don't accumulate too much
self.apply()
@@ -92,6 +112,9 @@ class Diff:
The rr is of isc.dns.RRset type and it must contain only one RR.
If this is not the case or if the diff was already commited, this
raises the ValueError exception.
+
+ The rr class must match the one of the datasource client. If
+ it does not, ValueError is raised.
"""
self.__data_common(rr, 'add')
@@ -102,6 +125,9 @@ class Diff:
The rr is of isc.dns.RRset type and it must contain only one RR.
If this is not the case or if the diff was already commited, this
raises the ValueError exception.
+
+ The rr class must match the one of the datasource client. If
+ it does not, ValueError is raised.
"""
self.__data_common(rr, 'remove')
@@ -128,6 +154,9 @@ class Diff:
rrset.get_class(),
rrset.get_type(),
rrset.get_ttl())))
+ if rrset.get_ttl() != buf[-1][1].get_ttl():
+ logger.warn(LIBXFRIN_DIFFERENT_TTL, rrset.get_ttl(),
+ buf[-1][1].get_ttl())
for rdatum in rrset.get_rdata():
buf[-1][1].add_rdata(rdatum)
self.__buffer = buf
@@ -151,15 +180,21 @@ class Diff:
self.__check_commited()
# First, compact the data
self.compact()
- # Then pass the data inside the data source
- for (operation, rrset) in self.__buffer:
- if operation == 'add':
- self.__updater.add_rrset(rrset)
- elif operation == 'remove':
- self.__updater.remove_rrset(rrset)
- else:
- raise ValueError('Unknown operation ' + operation)
- # As everything is already in, drop the buffer
+ try:
+ # Then pass the data inside the data source
+ for (operation, rrset) in self.__buffer:
+ if operation == 'add':
+ self.__updater.add_rrset(rrset)
+ elif operation == 'remove':
+ self.__updater.remove_rrset(rrset)
+ else:
+ raise ValueError('Unknown operation ' + operation)
+ # As everything is already in, drop the buffer
+ except:
+ # If there's a problem, we can't continue.
+ self.__updater = None
+ raise
+
self.__buffer = []
def commit(self):
@@ -174,10 +209,15 @@ class Diff:
# Push the data inside the data source
self.apply()
# Make sure they are visible.
- self.__updater.commit()
- # Remove the updater. That will free some resources for one, but
- # mark this object as already commited, so we can check
- self.__updater = None
+ try:
+ self.__updater.commit()
+ finally:
+ # Remove the updater. That will free some resources for one, but
+ # mark this object as already commited, so we can check
+
+ # We remove it even in case the commit failed, as that makes us
+ # unusable.
+ self.__updater = None
def get_buffer(self):
"""
diff --git a/src/lib/python/isc/xfrin/libxfrin_messages.mes b/src/lib/python/isc/xfrin/libxfrin_messages.mes
new file mode 100644
index 0000000..8bfb7b1
--- /dev/null
+++ b/src/lib/python/isc/xfrin/libxfrin_messages.mes
@@ -0,0 +1,22 @@
+# Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC")
+#
+# Permission to use, copy, modify, and/or distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+# REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+# AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+# LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+# OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+# PERFORMANCE OF THIS SOFTWARE.
+
+# No namespace declaration - these constants go in the global namespace
+# of the libxfrin_messages python module.
+
+% LIBXFRIN_DIFFERENT_TTL multiple data with different TTLs (%1, %2) on %3/%4
+The xfrin module received an update containing multiple rdata changes for the
+same RRset. But the TTLs of these don't match each other. The data will be put
+into the underlying data source. What happens with the TTLs is up to the
+data source.
diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py
index 963b16d..d431f42 100644
--- a/src/lib/python/isc/xfrin/tests/diff_tests.py
+++ b/src/lib/python/isc/xfrin/tests/diff_tests.py
@@ -18,6 +18,13 @@ import unittest
from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata
from isc.xfrin.diff import Diff, NoSuchZone
+class TestError(Exception):
+ """
+ Just to have something to be raised during the tests.
+ Not used outside.
+ """
+ pass
+
class DiffTest(unittest.TestCase):
"""
Tests for the isc.xfrin.diff.Diff class.
@@ -37,6 +44,8 @@ class DiffTest(unittest.TestCase):
self.__data_operations = []
self.__apply_called = False
self.__commit_called = False
+ self.__broken_called = False
+ self.__warn_called = False
# Some common values
self.__rrclass = RRClass.IN()
self.__type = RRType.A()
@@ -73,6 +82,22 @@ class DiffTest(unittest.TestCase):
"""
self.__apply_called = True
+ def __broken_operation(self, *args):
+ """
+ This can be used whenever an operation should fail. It raises TestError.
+ It should take whatever amount of parameters needed, so it can be put
+ quite anywhere.
+ """
+ self.__broken_called = True
+ raise TestError("Test error")
+
+ def warn(self, *args):
+ """
+ This is for checking the warn function was called, we replace the logger
+ in the tested module.
+ """
+ self.__warn_called = True
+
def commit(self):
"""
This is part of pretending to be a zone updater. This notes the commit
@@ -94,6 +119,13 @@ class DiffTest(unittest.TestCase):
"""
self.__data_operations.append(('remove', rrset))
+ def get_class(self):
+ """
+ This one is part of pretending to be a zone updater. It returns
+ the IN class.
+ """
+ return self.__rrclass
+
def get_updater(self, zone_name, replace):
"""
This one pretends this is the data source client and serves
@@ -128,7 +160,7 @@ class DiffTest(unittest.TestCase):
self.assertRaises(NoSuchZone, Diff, self, Name('none.example.org.'))
self.assertTrue(self.__updater_requested)
- def __data_common(self, diff, method, name):
+ def __data_common(self, diff, method, operation):
"""
Common part of test for test_add and test_remove.
"""
@@ -137,10 +169,10 @@ class DiffTest(unittest.TestCase):
self.assertRaises(ValueError, method, self.__rrset_multi)
# They were not added
self.assertEqual([], diff.get_buffer())
- # Add some proper data
+ # Put some proper data into the diff
method(self.__rrset1)
method(self.__rrset2)
- dlist = [(name, self.__rrset1), (name, self.__rrset2)]
+ dlist = [(operation, self.__rrset1), (operation, self.__rrset2)]
self.assertEqual(dlist, diff.get_buffer())
# Check the data are not destroyed by raising an exception because of
# bad data
@@ -322,6 +354,77 @@ class DiffTest(unittest.TestCase):
diff.compact()
check()
+ def test_wrong_class(self):
+ """
+ Test a wrong class of rrset is rejected.
+ """
+ diff = Diff(self, Name('example.org.'))
+ rrset = RRset(Name('a.example.org.'), RRClass.CH(), RRType.NS(),
+ self.__ttl)
+ rrset.add_rdata(Rdata(RRType.NS(), RRClass.CH(), 'ns.example.org.'))
+ self.assertRaises(ValueError, diff.add_data, rrset)
+ self.assertRaises(ValueError, diff.remove_data, rrset)
+
+ def __do_raise_test(self):
+ """
+ Do a raise test. Expects that one of the operations is exchanged for
+ broken version.
+ """
+ diff = Diff(self, Name('example.org.'))
+ diff.add_data(self.__rrset1)
+ diff.remove_data(self.__rrset2)
+ self.assertRaises(TestError, diff.commit)
+ self.assertTrue(self.__broken_called)
+ self.assertRaises(ValueError, diff.add_data, self.__rrset1)
+ self.assertRaises(ValueError, diff.remove_data, self.__rrset2)
+ self.assertRaises(ValueError, diff.commit)
+ self.assertRaises(ValueError, diff.apply)
+
+ def test_raise_add(self):
+ """
+ Test the exception from add_rrset is propagated and the diff can't be
+ used afterwards.
+ """
+ self.add_rrset = self.__broken_operation
+ self.__do_raise_test()
+
+ def test_raise_remove(self):
+ """
+ Test the exception from remove_rrset is propagated and the diff can't be
+ used afterwards.
+ """
+ self.remove_rrset = self.__broken_operation
+ self.__do_raise_test()
+
+ def test_raise_commit(self):
+ """
+ Test the exception from updater's commit gets propagated and it can't be
+ used afterwards.
+ """
+ self.commit = self.__broken_operation
+ self.__do_raise_test()
+
+ def test_ttl(self):
+ """
+ Test the TTL handling. A warn function should have been called if they
+ differ, but that's all, it should not crash or raise.
+ """
+ orig_logger = isc.xfrin.diff.logger
+ try:
+ isc.xfrin.diff.logger = self
+ diff = Diff(self, Name('example.org.'))
+ diff.add_data(self.__rrset1)
+ rrset2 = RRset(Name('a.example.org.'), self.__rrclass,
+ self.__type, RRTTL(120))
+ rrset2.add_rdata(Rdata(self.__type, self.__rrclass, '192.10.2.2'))
+ diff.add_data(rrset2)
+ # They should get compacted together and complain.
+ diff.compact()
+ self.assertEqual(1, len(diff.get_buffer()))
+ self.assertTrue(self.__warn_called)
+ finally:
+ isc.xfrin.diff.logger = orig_logger
+
if __name__ == "__main__":
isc.log.init("bind10")
unittest.main()
More information about the bind10-changes
mailing list