From: JINMEI Tatuya Date: Tue, 5 Jun 2012 07:31:40 +0000 (-0700) Subject: [1513] added notification from ddns to xfrout about zone update. X-Git-Tag: trac2351_base~226^2~59^2~7^2~1^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=31d7b2369b79150c8d8db697e489216aca50d2fc;p=thirdparty%2Fkea.git [1513] added notification from ddns to xfrout about zone update. there's not directly related change: rename the internal SessionError to InternalError, because the former could be confused with the same name of exception defined in isc.cc. --- diff --git a/src/bin/ddns/ddns.py.in b/src/bin/ddns/ddns.py.in index 4901c16d4a..1a4746d3e6 100755 --- a/src/bin/ddns/ddns.py.in +++ b/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: " + - str(tsig_error)) + raise InternalError("Failed to verify request's TSIG: " + + str(tsig_error)) return tsig_ctx def handle_request(self, req_session): @@ -339,13 +343,13 @@ 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 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: ' + - str(self.__request_msg.get_opcode())) + raise InternalError('Update request has unexpected 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) @@ -372,8 +376,11 @@ class DDNSServer: else: msg.to_wire(self.__response_renderer) - return self.__send_response(sock, self.__response_renderer.get_data(), - remote_addr) + 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. @@ -401,6 +408,42 @@ class DDNSServer: 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. diff --git a/src/bin/ddns/ddns_messages.mes b/src/bin/ddns/ddns_messages.mes index 65961346eb..2a0a0afc19 100644 --- a/src/bin/ddns/ddns_messages.mes +++ b/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_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. + +% 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. diff --git a/src/bin/ddns/tests/ddns_test.py b/src/bin/ddns/tests/ddns_test.py index 2e7e2c651c..6135ee98d8 100755 --- a/src/bin/ddns/tests/ddns_test.py +++ b/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): @@ -573,11 +613,11 @@ def create_msg(opcode=Opcode.UPDATE(), zones=[TEST_ZONE_RECORD], prereq=[], class TestDDNSession(unittest.TestCase): def setUp(self): - cc_session = MyCCSession() - self.assertFalse(cc_session._started) + self.__cc_session = MyCCSession() + 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': , + # '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