Browse Source

Merge #1924

Provide a "message undeliverable" error by the msgq daemon in case the
sender declares it expects an answer and there's no recipient to send it
to.

Conflicts:
	src/bin/bindctl/run_bindctl.sh.in
	src/bin/sysinfo/run_sysinfo.sh.in
	src/lib/python/isc/cc/Makefile.am
	src/lib/python/isc/cc/session.py
Michal 'vorner' Vaner 12 years ago
parent
commit
6bd562f85c

+ 2 - 1
configure.ac

@@ -306,7 +306,7 @@ AC_SUBST(PYTHON_LOGMSGPKG_DIR)
 # lib/dns/python/.libs is necessary because __init__.py of isc package
 # lib/dns/python/.libs is necessary because __init__.py of isc package
 # automatically imports isc.datasrc, which then requires the DNS loadable
 # automatically imports isc.datasrc, which then requires the DNS loadable
 # module.  #2145 should eliminate the need for it.
 # module.  #2145 should eliminate the need for it.
-COMMON_PYTHON_PATH="\$(abs_top_builddir)/src/lib/python/isc/log_messages:\$(abs_top_srcdir)/src/lib/python:\$(abs_top_builddir)/src/lib/python:\$(abs_top_builddir)/src/lib/dns/python/.libs"
+COMMON_PYTHON_PATH="\$(abs_top_builddir)/src/lib/python/isc/log_messages:\$(abs_top_builddir)/src/lib/python/isc/cc:\$(abs_top_srcdir)/src/lib/python:\$(abs_top_builddir)/src/lib/python:\$(abs_top_builddir)/src/lib/dns/python/.libs"
 AC_SUBST(COMMON_PYTHON_PATH)
 AC_SUBST(COMMON_PYTHON_PATH)
 
 
 # Check for python development environments
 # Check for python development environments
@@ -1225,6 +1225,7 @@ AC_CONFIG_FILES([Makefile
                  src/lib/python/isc/datasrc/tests/Makefile
                  src/lib/python/isc/datasrc/tests/Makefile
                  src/lib/python/isc/dns/Makefile
                  src/lib/python/isc/dns/Makefile
                  src/lib/python/isc/cc/Makefile
                  src/lib/python/isc/cc/Makefile
+                 src/lib/python/isc/cc/cc_generated/Makefile
                  src/lib/python/isc/cc/tests/Makefile
                  src/lib/python/isc/cc/tests/Makefile
                  src/lib/python/isc/config/Makefile
                  src/lib/python/isc/config/Makefile
                  src/lib/python/isc/config/tests/Makefile
                  src/lib/python/isc/config/tests/Makefile

+ 1 - 1
src/bin/bind10/run_bind10.sh.in

@@ -23,7 +23,7 @@ BIND10_PATH=@abs_top_builddir@/src/bin/bind10
 PATH=@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/resolver:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:@abs_top_builddir@/src/bin/ddns:@abs_top_builddir@/src/bin/dhcp6:@abs_top_builddir@/src/bin/sockcreator:$PATH
 PATH=@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/resolver:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:@abs_top_builddir@/src/bin/ddns:@abs_top_builddir@/src/bin/dhcp6:@abs_top_builddir@/src/bin/sockcreator:$PATH
 export PATH
 export PATH
 
 
-PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/python/isc/config:@abs_top_builddir@/src/lib/python/isc/acl/.libs:@abs_top_builddir@/src/lib/python/isc/datasrc/.libs
+PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/python/isc/config:@abs_top_builddir@/src/lib/python/isc/acl/.libs:@abs_top_builddir@/src/lib/python/isc/datasrc/.libs
 export PYTHONPATH
 export PYTHONPATH
 
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # If necessary (rare cases), explicitly specify paths to dynamic libraries

+ 1 - 4
src/bin/bindctl/run_bindctl.sh.in

@@ -20,10 +20,7 @@ export PYTHON_EXEC
 
 
 BINDCTL_PATH=@abs_top_builddir@/src/bin/bindctl
 BINDCTL_PATH=@abs_top_builddir@/src/bin/bindctl
 
 
-# Note: lib/dns/python/.libs is necessary because __init__.py of isc package
-# automatically imports isc.datasrc, which then requires the DNS loadable
-# module.  #2145 should eliminate the need for it.
-PYTHONPATH=@abs_top_srcdir@/src/bin:@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/bin:@abs_top_srcdir@/src/lib/python
+PYTHONPATH=@abs_top_srcdir@/src/bin:@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/bin:@abs_top_srcdir@/src/lib/python
 export PYTHONPATH
 export PYTHONPATH
 
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # If necessary (rare cases), explicitly specify paths to dynamic libraries

+ 1 - 1
src/bin/cmdctl/run_b10-cmdctl.sh.in

@@ -19,7 +19,7 @@ PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
 export PYTHON_EXEC
 export PYTHON_EXEC
 
 
 CMD_CTRLD_PATH=@abs_top_builddir@/src/bin/cmdctl
 CMD_CTRLD_PATH=@abs_top_builddir@/src/bin/cmdctl
-PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/python/isc/config:@abs_top_builddir@/src/lib/python/isc/acl/.libs:@abs_top_builddir@/src/lib/python/isc/datasrc/.libs
+PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/python/isc/config:@abs_top_builddir@/src/lib/python/isc/acl/.libs:@abs_top_builddir@/src/lib/python/isc/datasrc/.libs
 export PYTHONPATH
 export PYTHONPATH
 
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # If necessary (rare cases), explicitly specify paths to dynamic libraries

+ 1 - 1
src/bin/dbutil/run_dbutil.sh.in

@@ -23,7 +23,7 @@ DBUTIL_PATH=@abs_top_builddir@/src/bin/dbutil
 # Note: lib/dns/python/.libs is necessary because __init__.py of isc package
 # Note: lib/dns/python/.libs is necessary because __init__.py of isc package
 # automatically imports isc.datasrc, which then requires the DNS loadable
 # automatically imports isc.datasrc, which then requires the DNS loadable
 # module.  #2145 should eliminate the need for it.
 # module.  #2145 should eliminate the need for it.
-PYTHONPATH=@abs_top_srcdir@/src/bin:@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/bin:@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs
+PYTHONPATH=@abs_top_srcdir@/src/bin:@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/bin:@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs
 export PYTHONPATH
 export PYTHONPATH
 
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # If necessary (rare cases), explicitly specify paths to dynamic libraries

+ 1 - 1
src/bin/loadzone/run_loadzone.sh.in

@@ -18,7 +18,7 @@
 PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
 PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
 export PYTHON_EXEC
 export PYTHON_EXEC
 
 
-PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python:@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs
+PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_builddir@/src/lib/python:@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs
 export PYTHONPATH
 export PYTHONPATH
 
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # If necessary (rare cases), explicitly specify paths to dynamic libraries

+ 58 - 8
src/bin/msgq/msgq.py.in

@@ -33,6 +33,7 @@ import threading
 import isc.config.ccsession
 import isc.config.ccsession
 from optparse import OptionParser, OptionValueError
 from optparse import OptionParser, OptionValueError
 import isc.util.process
 import isc.util.process
+from isc.cc.proto_defs import *
 import isc.log
 import isc.log
 from isc.log_messages.msgq_messages import *
 from isc.log_messages.msgq_messages import *
 
 
@@ -503,6 +504,15 @@ class MsgQ:
             sock.setblocking(1)
             sock.setblocking(1)
 
 
     def send_prepared_msg(self, sock, msg):
     def send_prepared_msg(self, sock, msg):
+        '''
+        Add a message to the queue. If there's nothing waiting, try
+        to send it right away.
+
+        Return if the socket is still alive. It can return false if the
+        socket dies (for example due to EPIPE in the attempt to send).
+        Returning true does not guarantee the message will be delivered,
+        but returning false means it won't.
+        '''
         # Try to send the data, but only if there's nothing waiting
         # Try to send the data, but only if there's nothing waiting
         fileno = sock.fileno()
         fileno = sock.fileno()
         if fileno in self.sendbuffs:
         if fileno in self.sendbuffs:
@@ -511,7 +521,7 @@ class MsgQ:
             amount_sent = self._send_data(sock, msg)
             amount_sent = self._send_data(sock, msg)
             if amount_sent is None:
             if amount_sent is None:
                 # Socket has been killed, drop the send
                 # Socket has been killed, drop the send
-                return
+                return False
 
 
         # Still something to send, add it to outgoing queue
         # Still something to send, add it to outgoing queue
         if amount_sent < len(msg):
         if amount_sent < len(msg):
@@ -521,7 +531,7 @@ class MsgQ:
                 (last_sent, buff) = self.sendbuffs[fileno]
                 (last_sent, buff) = self.sendbuffs[fileno]
                 if now - last_sent > 0.1:
                 if now - last_sent > 0.1:
                     self.kill_socket(fileno, sock)
                     self.kill_socket(fileno, sock)
-                    return
+                    return False
                 buff += msg
                 buff += msg
             else:
             else:
                 buff = msg[amount_sent:]
                 buff = msg[amount_sent:]
@@ -532,6 +542,7 @@ class MsgQ:
                 else:
                 else:
                     self.add_kqueue_socket(sock, True)
                     self.add_kqueue_socket(sock, True)
             self.sendbuffs[fileno] = (last_sent, buff)
             self.sendbuffs[fileno] = (last_sent, buff)
+        return True
 
 
     def __process_write(self, fileno):
     def __process_write(self, fileno):
         # Try to send some data from the buffer
         # Try to send some data from the buffer
@@ -566,26 +577,65 @@ class MsgQ:
         self.sendmsg(sock, { "type" : "getlname" }, { "lname" : lname })
         self.sendmsg(sock, { "type" : "getlname" }, { "lname" : lname })
 
 
     def process_command_send(self, sock, routing, data):
     def process_command_send(self, sock, routing, data):
-        group = routing["group"]
-        instance = routing["instance"]
-        to = routing["to"]
+        group = routing[CC_HEADER_GROUP]
+        instance = routing[CC_HEADER_INSTANCE]
+        to = routing[CC_HEADER_TO]
         if group == None or instance == None:
         if group == None or instance == None:
+            # FIXME: Should we log them instead?
             return  # ignore invalid packets entirely
             return  # ignore invalid packets entirely
 
 
-        if to == "*":
+        if to == CC_TO_WILDCARD:
             sockets = self.subs.find(group, instance)
             sockets = self.subs.find(group, instance)
         else:
         else:
             if to in self.lnames:
             if to in self.lnames:
                 sockets = [ self.lnames[to] ]
                 sockets = [ self.lnames[to] ]
             else:
             else:
-                return # recipient doesn't exist
+                sockets = []
 
 
         msg = self.preparemsg(routing, data)
         msg = self.preparemsg(routing, data)
 
 
         if sock in sockets:
         if sock in sockets:
+            # Don't bounce to self
             sockets.remove(sock)
             sockets.remove(sock)
+
+        has_recipient = False
         for socket in sockets:
         for socket in sockets:
-            self.send_prepared_msg(socket, msg)
+            if self.send_prepared_msg(socket, msg):
+                has_recipient = True
+        if not has_recipient and routing.get(CC_HEADER_WANT_ANSWER) and \
+            CC_HEADER_REPLY not in routing:
+            # We have no recipients. But the sender insists on a reply
+            # (and the message isn't a reply itself). We need to send
+            # an error to satisfy the request, since there's nobody
+            # else who can.
+            #
+            # We omit the replies on purpose. The recipient might generate
+            # the response by copying and mangling the header of incoming
+            # message (just like we do below) and would include the want_answer
+            # by accident. And we want to avoid loops of errors. Also, it
+            # is unclear if the knowledge of undeliverable reply would be
+            # of any use to the sender, and it should be much rarer situation.
+
+            # The real errors would be positive, 1 most probably. We use
+            # negative errors for delivery errors to distinguish them a
+            # little. We probably should have a way to provide more data
+            # in the error message.
+            payload = isc.config.ccsession.create_answer(CC_REPLY_NO_RECPT,
+                                                         "No such recipient")
+            # We create the header based on the current one. But we don't
+            # want to mangle it for the caller, so we get a copy. A shallow
+            # one should be enough, we modify the dict only.
+            header = routing.copy()
+            header[CC_HEADER_REPLY] = routing[CC_HEADER_SEQ]
+            # Dummy lname not assigned to clients
+            header[CC_HEADER_FROM] = "msgq"
+            header[CC_HEADER_TO] = routing[CC_HEADER_FROM]
+            # We keep the seq as it is. We don't need to track the message
+            # and we will not confuse the sender. The sender would use an
+            # unique id for each message, so we won't return one twice to it.
+            errmsg = self.preparemsg(header, payload)
+            # Send it back.
+            self.send_prepared_msg(sock, errmsg)
 
 
     def process_command_subscribe(self, sock, routing, data):
     def process_command_subscribe(self, sock, routing, data):
         group = routing["group"]
         group = routing["group"]

+ 1 - 1
src/bin/msgq/run_msgq.sh.in

@@ -20,7 +20,7 @@ export PYTHON_EXEC
 
 
 MYPATH_PATH=@abs_top_builddir@/src/bin/msgq
 MYPATH_PATH=@abs_top_builddir@/src/bin/msgq
 
 
-PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/log/.libs
+PYTHONPATH=@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/log/.libs
 export PYTHONPATH
 export PYTHONPATH
 
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # If necessary (rare cases), explicitly specify paths to dynamic libraries

+ 189 - 5
src/bin/msgq/tests/msgq_test.py

@@ -12,6 +12,8 @@ import threading
 import isc.cc
 import isc.cc
 import collections
 import collections
 import isc.log
 import isc.log
+import struct
+import json
 
 
 #
 #
 # Currently only the subscription part and some sending is implemented...
 # Currently only the subscription part and some sending is implemented...
@@ -141,6 +143,150 @@ class TestSubscriptionManager(unittest.TestCase):
         self.sm.subscribe('ConfigManager', '*', 's3')
         self.sm.subscribe('ConfigManager', '*', 's3')
         self.assertEqual(1, self.__cfgmgr_ready_called)
         self.assertEqual(1, self.__cfgmgr_ready_called)
 
 
+class MsgQTest(unittest.TestCase):
+    """
+    Tests for the behaviour of MsgQ. This is for the core of MsgQ, other
+    subsystems are in separate test fixtures.
+    """
+    def setUp(self):
+        self.__msgq = MsgQ()
+
+    def parse_msg(self, msg):
+        """
+        Parse a binary representation of message to the routing header and the
+        data payload. It assumes the message is correctly encoded and the
+        payload is not omitted. It'd probably throw in other cases, but we
+        don't use it in such situations in this test.
+        """
+        (length, header_len) = struct.unpack('>IH', msg[:6])
+        header = json.loads(msg[6:6 + header_len].decode('utf-8'))
+        data = json.loads(msg[6 + header_len:].decode('utf-8'))
+        return (header, data)
+
+    def test_undeliverable_errors(self):
+        """
+        Send several packets through the MsgQ and check it generates
+        undeliverable notifications under the correct circumstances.
+
+        The test is not exhaustive as it doesn't test all combination
+        of existence of the recipient, addressing schemes, want_answer
+        header and the reply header. It is not needed, these should
+        be mostly independant. That means, for example, if the message
+        is a reply and there's no recipient to send it to, the error
+        would not be generated no matter if we addressed the recipient
+        by lname or group. If we included everything, the test would
+        have too many scenarios with little benefit.
+        """
+        self.__sent_messages = []
+        def fake_send_prepared_msg(socket, msg):
+            self.__sent_messages.append((socket, msg))
+            return True
+        self.__msgq.send_prepared_msg = fake_send_prepared_msg
+        # These would be real sockets in the MsgQ, but we pass them as
+        # parameters only, so we don't need them to be. We use simple
+        # integers to tell one from another.
+        sender = 1
+        recipient = 2
+        another_recipiet = 3
+        # The routing headers and data to test with.
+        routing = {
+            'to': '*',
+            'from': 'sender',
+            'group': 'group',
+            'instance': '*',
+            'seq': 42
+        }
+        data = {
+            "data": "Just some data"
+        }
+
+        # Some common checking patterns
+        def check_error():
+            self.assertEqual(1, len(self.__sent_messages))
+            self.assertEqual(1, self.__sent_messages[0][0])
+            self.assertEqual(({
+                                  'group': 'group',
+                                  'instance': '*',
+                                  'reply': 42,
+                                  'seq': 42,
+                                  'from': 'msgq',
+                                  'to': 'sender',
+                                  'want_answer': True
+                              }, {'result': [-1, "No such recipient"]}),
+                              self.parse_msg(self.__sent_messages[0][1]))
+            self.__sent_messages = []
+
+        def check_no_message():
+            self.assertEqual([], self.__sent_messages)
+
+        def check_delivered(rcpt_socket=recipient):
+            self.assertEqual(1, len(self.__sent_messages))
+            self.assertEqual(rcpt_socket, self.__sent_messages[0][0])
+            self.assertEqual((routing, data),
+                             self.parse_msg(self.__sent_messages[0][1]))
+            self.__sent_messages = []
+
+        # Send the message. No recipient, but errors are not requested,
+        # so none is generated.
+        self.__msgq.process_command_send(sender, routing, data)
+        check_no_message()
+
+        # It should act the same if we explicitly say we do not want replies.
+        routing["want_answer"] = False
+        self.__msgq.process_command_send(sender, routing, data)
+        check_no_message()
+
+        # Ask for errors if it can't be delivered.
+        routing["want_answer"] = True
+        self.__msgq.process_command_send(sender, routing, data)
+        check_error()
+
+        # If the message is a reply itself, we never generate the errors
+        routing["reply"] = 3
+        self.__msgq.process_command_send(sender, routing, data)
+        check_no_message()
+
+        # If there are recipients (but no "reply" header), the error should not
+        # be sent and the message should get delivered.
+        del routing["reply"]
+        self.__msgq.subs.find = lambda group, instance: [recipient]
+        self.__msgq.process_command_send(sender, routing, data)
+        check_delivered()
+
+        # When we send a direct message and the recipient is not there, we get
+        # the error too
+        routing["to"] = "lname"
+        self.__msgq.process_command_send(sender, routing, data)
+        check_error()
+
+        # But when the recipient is there, it is delivered and no error is
+        # generated.
+        self.__msgq.lnames["lname"] = recipient
+        self.__msgq.process_command_send(sender, routing, data)
+        check_delivered()
+
+        # If an attempt to send fails, consider it no recipient.
+        def fail_send_prepared_msg(socket, msg):
+            '''
+            Pretend sending a message failed. After one call, return to the
+            usual mock, so the errors or other messages can be sent.
+            '''
+            self.__msgq.send_prepared_msg = fake_send_prepared_msg
+            return False
+
+        self.__msgq.send_prepared_msg = fail_send_prepared_msg
+        self.__msgq.process_command_send(sender, routing, data)
+        check_error()
+
+        # But if there are more recipients and only one fails, it should
+        # be delivered to the other and not considered an error
+        self.__msgq.send_prepared_msg = fail_send_prepared_msg
+        routing["to"] = '*'
+        self.__msgq.subs.find = lambda group, instance: [recipient,
+                                                         another_recipiet]
+        self.__msgq.process_command_send(sender, routing, data)
+        check_delivered(rcpt_socket=another_recipiet)
+
 class DummySocket:
 class DummySocket:
     """
     """
     Dummy socket class.
     Dummy socket class.
@@ -245,17 +391,27 @@ class SendNonblock(unittest.TestCase):
             self.assertEqual(0, status,
             self.assertEqual(0, status,
                 "The task did not complete successfully in time")
                 "The task did not complete successfully in time")
 
 
+    def get_msgq_with_sockets(self):
+        '''
+        Create a message queue and prepare it for use with a socket pair.
+        The write end is put into the message queue, so we can check it.
+        It returns (msgq, read_end, write_end). It is expected the sockets
+        are closed by the caller afterwards.
+        '''
+        msgq = MsgQ()
+        # We do only partial setup, so we don't create the listening socket
+        msgq.setup_poller()
+        (read, write) = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM)
+        msgq.register_socket(write)
+        return (msgq, read, write)
+
     def infinite_sender(self, sender):
     def infinite_sender(self, sender):
         """
         """
         Sends data until an exception happens. socket.error is caught,
         Sends data until an exception happens. socket.error is caught,
         as it means the socket got closed. Sender is called to actually
         as it means the socket got closed. Sender is called to actually
         send the data.
         send the data.
         """
         """
-        msgq = MsgQ()
-        # We do only partial setup, so we don't create the listening socket
-        msgq.setup_poller()
-        (read, write) = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM)
-        msgq.register_socket(write)
+        (msgq, read, write) = self.get_msgq_with_sockets()
         # Keep sending while it is not closed by the msgq
         # Keep sending while it is not closed by the msgq
         try:
         try:
             while True:
             while True:
@@ -291,6 +447,34 @@ class SendNonblock(unittest.TestCase):
         self.terminate_check(lambda: self.infinite_sender(
         self.terminate_check(lambda: self.infinite_sender(
             lambda msgq, socket: msgq.send_prepared_msg(socket, data)))
             lambda msgq, socket: msgq.send_prepared_msg(socket, data)))
 
 
+    def test_sendprepared_success(self):
+        '''
+        Test the send_prepared_msg returns success when queueing messages.
+        It does so on the first attempt (when it actually tries to send
+        something to the socket) and on any attempt that follows and the
+        buffer is already full.
+        '''
+        (msgq, read, write) = self.get_msgq_with_sockets()
+        # Now keep sending until we fill in something into the internal
+        # buffer.
+        while not write.fileno() in msgq.sendbuffs:
+            self.assertTrue(msgq.send_prepared_msg(write, b'data'))
+        read.close()
+        write.close()
+
+    def test_sendprepared_epipe(self):
+        '''
+        Test the send_prepared_msg returns false when we try to queue a
+        message and the other side is not there any more. It should be done
+        with EPIPE, so not a fatal error.
+        '''
+        (msgq, read, write) = self.get_msgq_with_sockets()
+        # Close one end. It should make a EPIPE on the other.
+        read.close()
+        # Now it should soft-fail
+        self.assertFalse(msgq.send_prepared_msg(write, b'data'))
+        write.close()
+
     def send_many(self, data):
     def send_many(self, data):
         """
         """
         Tries that sending a command many times and getting an answer works.
         Tries that sending a command many times and getting an answer works.

+ 8 - 2
src/lib/cc/Makefile.am

@@ -24,9 +24,12 @@ lib_LTLIBRARIES = libb10-cc.la
 libb10_cc_la_SOURCES = data.cc data.h session.cc session.h
 libb10_cc_la_SOURCES = data.cc data.h session.cc session.h
 libb10_cc_la_SOURCES += logger.cc logger.h
 libb10_cc_la_SOURCES += logger.cc logger.h
 nodist_libb10_cc_la_SOURCES = cc_messages.cc cc_messages.h
 nodist_libb10_cc_la_SOURCES = cc_messages.cc cc_messages.h
+libb10_cc_la_SOURCES += proto_defs.cc
+nodist_libb10_cc_la_SOURCES += proto_defs.h
 libb10_cc_la_LIBADD = $(top_builddir)/src/lib/log/libb10-log.la
 libb10_cc_la_LIBADD = $(top_builddir)/src/lib/log/libb10-log.la
 
 
-CLEANFILES = *.gcno *.gcda session_config.h cc_messages.cc cc_messages.h
+CLEANFILES = *.gcno *.gcda session_config.h cc_messages.cc cc_messages.h \
+	proto_defs.h
 
 
 session_config.h: session_config.h.pre
 session_config.h: session_config.h.pre
 	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" session_config.h.pre >$@
 	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" session_config.h.pre >$@
@@ -34,6 +37,9 @@ session_config.h: session_config.h.pre
 cc_messages.cc cc_messages.h: cc_messages.mes
 cc_messages.cc cc_messages.h: cc_messages.mes
 	$(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/lib/cc/cc_messages.mes
 	$(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/lib/cc/cc_messages.mes
 
 
-BUILT_SOURCES = session_config.h cc_messages.cc cc_messages.h
+BUILT_SOURCES = session_config.h cc_messages.cc cc_messages.h proto_defs.h
+
+proto_defs.h: $(top_srcdir)/src/lib/util/python/const2hdr.py proto_defs.cc
+	$(PYTHON) $(top_srcdir)/src/lib/util/python/const2hdr.py $(srcdir)/proto_defs.cc $@
 
 
 EXTRA_DIST = cc_messages.mes
 EXTRA_DIST = cc_messages.mes

+ 44 - 0
src/lib/cc/proto_defs.cc

@@ -0,0 +1,44 @@
+// Copyright (C) 2013  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.
+
+#include <cc/proto_defs.h>
+
+namespace isc {
+namespace cc {
+
+// Aside from defining the values for the C++ library, this file is also
+// used as direct input of the generator of the python counterpart. Please,
+// keep the syntax here simple and check the generated file
+// (lib/python/isc/cc/proto_defs.py) is correct and sane.
+
+// The constants used in the CC protocol
+// First the header names
+const char* const CC_HEADER_TYPE = "type";
+const char* const CC_HEADER_FROM = "from";
+const char* const CC_HEADER_TO = "to";
+const char* const CC_HEADER_GROUP = "group";
+const char* const CC_HEADER_INSTANCE = "instance";
+const char* const CC_HEADER_SEQ = "seq";
+const char* const CC_HEADER_WANT_ANSWER = "want_answer";
+const char* const CC_HEADER_REPLY = "reply";
+// The commands in the "type" header
+const char* const CC_COMMAND_SEND = "send";
+// The wildcards of some headers
+const char* const CC_TO_WILDCARD = "*";
+const char* const CC_INSTANCE_WILDCARD = "*";
+// Reply codes
+const int CC_REPLY_NO_RECPT = -1;
+
+}
+}

+ 20 - 19
src/lib/cc/session.cc

@@ -30,6 +30,9 @@
 #include <asio/deadline_timer.hpp>
 #include <asio/deadline_timer.hpp>
 #include <asio/system_error.hpp>
 #include <asio/system_error.hpp>
 
 
+#include <cc/data.h>
+#include <cc/session.h>
+
 #include <cstdio>
 #include <cstdio>
 #include <vector>
 #include <vector>
 #include <iostream>
 #include <iostream>
@@ -44,9 +47,6 @@
 
 
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
-#include <cc/data.h>
-#include <cc/session.h>
-
 using namespace std;
 using namespace std;
 using namespace isc::cc;
 using namespace isc::cc;
 using namespace isc::data;
 using namespace isc::data;
@@ -344,8 +344,8 @@ Session::establish(const char* socket_file) {
 // prefix.
 // prefix.
 //
 //
 void
 void
-Session::sendmsg(ConstElementPtr msg) {
-    std::string header_wire = msg->toWire();
+Session::sendmsg(ConstElementPtr header) {
+    std::string header_wire = header->toWire();
     unsigned int length = 2 + header_wire.length();
     unsigned int length = 2 + header_wire.length();
     unsigned int length_net = htonl(length);
     unsigned int length_net = htonl(length);
     unsigned short header_length = header_wire.length();
     unsigned short header_length = header_wire.length();
@@ -357,9 +357,9 @@ Session::sendmsg(ConstElementPtr msg) {
 }
 }
 
 
 void
 void
-Session::sendmsg(ConstElementPtr env, ConstElementPtr msg) {
-    std::string header_wire = env->toWire();
-    std::string body_wire = msg->toWire();
+Session::sendmsg(ConstElementPtr header, ConstElementPtr payload) {
+    std::string header_wire = header->toWire();
+    std::string body_wire = payload->toWire();
     unsigned int length = 2 + header_wire.length() + body_wire.length();
     unsigned int length = 2 + header_wire.length() + body_wire.length();
     unsigned int length_net = htonl(length);
     unsigned int length_net = htonl(length);
     unsigned short header_length = header_wire.length();
     unsigned short header_length = header_wire.length();
@@ -474,20 +474,21 @@ Session::unsubscribe(std::string group, std::string instance) {
 
 
 int
 int
 Session::group_sendmsg(ConstElementPtr msg, std::string group,
 Session::group_sendmsg(ConstElementPtr msg, std::string group,
-                       std::string instance, std::string to)
+                       std::string instance, std::string to, bool want_answer)
 {
 {
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, CC_GROUP_SEND).arg(msg->str()).
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, CC_GROUP_SEND).arg(msg->str()).
         arg(group);
         arg(group);
     ElementPtr env = Element::createMap();
     ElementPtr env = Element::createMap();
-    long int nseq = ++impl_->sequence_;
-    
-    env->set("type", Element::create("send"));
-    env->set("from", Element::create(impl_->lname_));
-    env->set("to", Element::create(to));
-    env->set("group", Element::create(group));
-    env->set("instance", Element::create(instance));
-    env->set("seq", Element::create(nseq));
-    //env->set("msg", Element::create(msg->toWire()));
+    const long int nseq = ++impl_->sequence_;
+
+    env->set(CC_HEADER_TYPE,
+             Element::create(CC_COMMAND_SEND));
+    env->set(CC_HEADER_FROM, Element::create(impl_->lname_));
+    env->set(CC_HEADER_TO, Element::create(to));
+    env->set(CC_HEADER_GROUP, Element::create(group));
+    env->set(CC_HEADER_INSTANCE, Element::create(instance));
+    env->set(CC_HEADER_SEQ, Element::create(nseq));
+    env->set(CC_HEADER_WANT_ANSWER, Element::create(want_answer));
 
 
     sendmsg(env, msg);
     sendmsg(env, msg);
     return (nseq);
     return (nseq);
@@ -514,7 +515,7 @@ Session::reply(ConstElementPtr envelope, ConstElementPtr newmsg) {
         arg(newmsg->str());
         arg(newmsg->str());
     ElementPtr env = Element::createMap();
     ElementPtr env = Element::createMap();
     long int nseq = ++impl_->sequence_;
     long int nseq = ++impl_->sequence_;
-    
+
     env->set("type", Element::create("send"));
     env->set("type", Element::create("send"));
     env->set("from", Element::create(impl_->lname_));
     env->set("from", Element::create(impl_->lname_));
     env->set("to", Element::create(envelope->get("from")->stringValue()));
     env->set("to", Element::create(envelope->get("from")->stringValue()));

+ 35 - 11
src/lib/cc/session.h

@@ -15,14 +15,15 @@
 #ifndef ISC_SESSION_H
 #ifndef ISC_SESSION_H
 #define ISC_SESSION_H 1
 #define ISC_SESSION_H 1
 
 
-#include <string>
-
-#include <boost/function.hpp>
+#include <cc/data.h>
+#include <cc/session_config.h>
+#include <cc/proto_defs.h>
 
 
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
-#include <cc/data.h>
-#include <cc/session_config.h>
+#include <string>
+
+#include <boost/function.hpp>
 
 
 namespace asio {
 namespace asio {
 class io_service;
 class io_service;
@@ -81,8 +82,10 @@ namespace isc {
             virtual void disconnect() = 0;
             virtual void disconnect() = 0;
             virtual int group_sendmsg(isc::data::ConstElementPtr msg,
             virtual int group_sendmsg(isc::data::ConstElementPtr msg,
                                       std::string group,
                                       std::string group,
-                                      std::string instance = "*",
-                                      std::string to = "*") = 0;
+                                      std::string instance =
+                                          CC_INSTANCE_WILDCARD,
+                                      std::string to = CC_TO_WILDCARD,
+                                      bool want_answer = false) = 0;
             virtual bool group_recvmsg(isc::data::ConstElementPtr& envelope,
             virtual bool group_recvmsg(isc::data::ConstElementPtr& envelope,
                                        isc::data::ConstElementPtr& msg,
                                        isc::data::ConstElementPtr& msg,
                                        bool nonblock = true,
                                        bool nonblock = true,
@@ -128,10 +131,26 @@ namespace isc {
                                    std::string instance = "*");
                                    std::string instance = "*");
             virtual void unsubscribe(std::string group,
             virtual void unsubscribe(std::string group,
                              std::string instance = "*");
                              std::string instance = "*");
+
+            /// \brief Send a message to a group.
+            ///
+            /// \todo Can someone explain how the group, instance and to work?
+            ///     What is the desired semantics here?
+            /// \param msg The message to send.
+            /// \param group Part of addressing.
+            /// \param instance Part of addressing.
+            /// \param to Part of addressing.
+            /// \param want_answer Require an answer? If it is true and there's
+            ///     no recipient, the message queue answers by an error
+            ///     instead.
+            /// \return The squence number of the message sent. It can be used
+            ///     to wait for an answer by group_recvmsg.
             virtual int group_sendmsg(isc::data::ConstElementPtr msg,
             virtual int group_sendmsg(isc::data::ConstElementPtr msg,
                                       std::string group,
                                       std::string group,
                                       std::string instance = "*",
                                       std::string instance = "*",
-                                      std::string to = "*");
+                                      std::string to = "*",
+                                      bool want_answer = false);
+
             virtual bool group_recvmsg(isc::data::ConstElementPtr& envelope,
             virtual bool group_recvmsg(isc::data::ConstElementPtr& envelope,
                                        isc::data::ConstElementPtr& msg,
                                        isc::data::ConstElementPtr& msg,
                                        bool nonblock = true,
                                        bool nonblock = true,
@@ -147,9 +166,14 @@ namespace isc {
             /// @param returns socket descriptor used for session connection
             /// @param returns socket descriptor used for session connection
             virtual int getSocketDesc() const;
             virtual int getSocketDesc() const;
     private:
     private:
-            void sendmsg(isc::data::ConstElementPtr msg);
-            void sendmsg(isc::data::ConstElementPtr env,
-                         isc::data::ConstElementPtr msg);
+            // The following two methods are virtual to allow tests steal and
+            // replace them. It is not expected to be specialized by a derived
+            // class. Actually, it is not expected to inherit from this class
+            // to begin with.
+            virtual void sendmsg(isc::data::ConstElementPtr header);
+            virtual void sendmsg(isc::data::ConstElementPtr header,
+                                 isc::data::ConstElementPtr payload);
+
             bool recvmsg(isc::data::ConstElementPtr& msg,
             bool recvmsg(isc::data::ConstElementPtr& msg,
                          bool nonblock = true,
                          bool nonblock = true,
                          int seq = -1);
                          int seq = -1);

+ 129 - 16
src/lib/cc/tests/session_unittests.cc

@@ -19,16 +19,28 @@
 // XXX: the ASIO header must be included before others.  See session.cc.
 // XXX: the ASIO header must be included before others.  See session.cc.
 #include <asio.hpp>
 #include <asio.hpp>
 
 
+#include <cc/session.h>
+#include <cc/data.h>
+#include <cc/tests/session_unittests_config.h>
+
 #include <gtest/gtest.h>
 #include <gtest/gtest.h>
 #include <boost/bind.hpp>
 #include <boost/bind.hpp>
 
 
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
-#include <cc/session.h>
-#include <cc/data.h>
-#include <session_unittests_config.h>
+#include <utility>
+#include <list>
+#include <string>
+#include <iostream>
 
 
 using namespace isc::cc;
 using namespace isc::cc;
+using std::pair;
+using std::list;
+using std::string;
+using isc::data::ConstElementPtr;
+using isc::data::Element;
+
+namespace {
 
 
 TEST(AsioSession, establish) {
 TEST(AsioSession, establish) {
     asio::io_service io_service_;
     asio::io_service io_service_;
@@ -50,7 +62,6 @@ TEST(AsioSession, establish) {
                        "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
                        "/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/aaaaaaaaaa/"
                   ), isc::cc::SessionError
                   ), isc::cc::SessionError
     );
     );
-                  
 }
 }
 
 
 // This class sets up a domain socket for the session to connect to
 // This class sets up a domain socket for the session to connect to
@@ -70,18 +81,16 @@ public:
                                boost::bind(&TestDomainSocket::acceptHandler,
                                boost::bind(&TestDomainSocket::acceptHandler,
                                            this, _1));
                                            this, _1));
     }
     }
-    
+
     ~TestDomainSocket() {
     ~TestDomainSocket() {
         socket_.close();
         socket_.close();
         unlink(BIND10_TEST_SOCKET_FILE);
         unlink(BIND10_TEST_SOCKET_FILE);
     }
     }
 
 
-    void
-    acceptHandler(const asio::error_code&) const {
+    void acceptHandler(const asio::error_code&) const {
     }
     }
 
 
-    void
-    sendmsg(isc::data::ElementPtr& env, isc::data::ElementPtr& msg) {
+    void sendmsg(isc::data::ElementPtr& env, isc::data::ElementPtr& msg) {
         const std::string header_wire = env->toWire();
         const std::string header_wire = env->toWire();
         const std::string body_wire = msg->toWire();
         const std::string body_wire = msg->toWire();
         const unsigned int length = 2 + header_wire.length() +
         const unsigned int length = 2 + header_wire.length() +
@@ -89,7 +98,7 @@ public:
         const unsigned int length_net = htonl(length);
         const unsigned int length_net = htonl(length);
         const unsigned short header_length = header_wire.length();
         const unsigned short header_length = header_wire.length();
         const unsigned short header_length_net = htons(header_length);
         const unsigned short header_length_net = htons(header_length);
-    
+
         socket_.send(asio::buffer(&length_net, sizeof(length_net)));
         socket_.send(asio::buffer(&length_net, sizeof(length_net)));
         socket_.send(asio::buffer(&header_length_net,
         socket_.send(asio::buffer(&header_length_net,
                                   sizeof(header_length_net)));
                                   sizeof(header_length_net)));
@@ -97,8 +106,7 @@ public:
         socket_.send(asio::buffer(body_wire.data(), body_wire.length()));
         socket_.send(asio::buffer(body_wire.data(), body_wire.length()));
     }
     }
 
 
-    void
-    sendLname() {
+    void sendLname() {
         isc::data::ElementPtr lname_answer1 =
         isc::data::ElementPtr lname_answer1 =
             isc::data::Element::fromJSON("{ \"type\": \"lname\" }");
             isc::data::Element::fromJSON("{ \"type\": \"lname\" }");
         isc::data::ElementPtr lname_answer2 =
         isc::data::ElementPtr lname_answer2 =
@@ -106,13 +114,12 @@ public:
         sendmsg(lname_answer1, lname_answer2);
         sendmsg(lname_answer1, lname_answer2);
     }
     }
 
 
-    void
-    setSendLname() {
+    void setSendLname() {
         // ignore whatever data we get, send back an lname
         // ignore whatever data we get, send back an lname
         asio::async_read(socket_,  asio::buffer(data_buf, 0),
         asio::async_read(socket_,  asio::buffer(data_buf, 0),
                          boost::bind(&TestDomainSocket::sendLname, this));
                          boost::bind(&TestDomainSocket::sendLname, this));
     }
     }
-    
+
 private:
 private:
     asio::io_service& io_service_;
     asio::io_service& io_service_;
     asio::local::stream_protocol::endpoint ep_;
     asio::local::stream_protocol::endpoint ep_;
@@ -121,6 +128,38 @@ private:
     char data_buf[1024];
     char data_buf[1024];
 };
 };
 
 
+/// \brief Pair holding header and data of a message sent over the connection.
+typedef pair<ConstElementPtr, ConstElementPtr> SentMessage;
+
+// We specialize the tested class a little. We replace some low-level
+// methods so we can examine the rest without relying on real network IO
+class TestSession : public Session {
+public:
+    TestSession(asio::io_service& ioservice) :
+        Session(ioservice)
+    {}
+    // Get first message previously sent by sendmsg and remove it from the
+    // buffer. Expects there's at leas one message in the buffer.
+    SentMessage getSentMessage() {
+        assert(!sent_messages_.empty());
+        const SentMessage result(sent_messages_.front());
+        sent_messages_.pop_front();
+        return (result);
+    }
+private:
+    // Override the sendmsg. They are not sent over the real connection, but
+    // stored locally and can be extracted by getSentMessage()
+    virtual void sendmsg(ConstElementPtr header) {
+        sendmsg(header, ConstElementPtr(new isc::data::NullElement));
+    }
+    virtual void sendmsg(ConstElementPtr header, ConstElementPtr payload) {
+        sent_messages_.push_back(SentMessage(header, payload));
+    }
+
+    // The sendmsg stores data here.
+    list<SentMessage> sent_messages_;
+};
+
 class SessionTest : public ::testing::Test {
 class SessionTest : public ::testing::Test {
 protected:
 protected:
     SessionTest() : sess(my_io_service), work(my_io_service) {
     SessionTest() : sess(my_io_service), work(my_io_service) {
@@ -134,6 +173,26 @@ protected:
         delete tds;
         delete tds;
     }
     }
 
 
+    // Check the session sent a message with the given header. The
+    // message is hardcoded.
+    void checkSentMessage(const string& expected_hdr, const char* description)
+    {
+        SCOPED_TRACE(description);
+        const SentMessage& msg(sess.getSentMessage());
+        elementsEqual(expected_hdr, msg.first);
+        elementsEqual("{\"test\": 42}", msg.second);
+    }
+
+private:
+    // Check two elements are equal
+    void elementsEqual(const string& expected,
+                       const ConstElementPtr& actual) const
+    {
+        EXPECT_TRUE(Element::fromJSON(expected)->equals(*actual)) <<
+            "Elements differ, expected: " << expected <<
+            ", got: " << actual->toWire();
+    }
+
 public:
 public:
     // used in the handler test
     // used in the handler test
     // This handler first reads (and ignores) whatever message caused
     // This handler first reads (and ignores) whatever message caused
@@ -156,7 +215,7 @@ public:
 protected:
 protected:
     asio::io_service my_io_service;
     asio::io_service my_io_service;
     TestDomainSocket* tds;
     TestDomainSocket* tds;
-    Session sess;
+    TestSession sess;
     // Keep run() from stopping right away by informing it it has work to do
     // Keep run() from stopping right away by informing it it has work to do
     asio::io_service::work work;
     asio::io_service::work work;
 };
 };
@@ -249,3 +308,57 @@ TEST_F(SessionTest, get_socket_descr) {
     // expect actual socket handle to be returned, not 0
     // expect actual socket handle to be returned, not 0
     EXPECT_LT(0, socket);
     EXPECT_LT(0, socket);
 }
 }
+
+// Test the group_sendmsg sends the correct data.
+TEST_F(SessionTest, group_sendmsg) {
+    // Connect (to set the lname, so we can see it sets the from)
+    tds->setSendLname();
+    sess.establish(BIND10_TEST_SOCKET_FILE);
+    // Eat the "get_lname" message, so it doesn't confuse the
+    // test below.
+    sess.getSentMessage();
+
+    const ConstElementPtr msg(Element::fromJSON("{\"test\": 42}"));
+    sess.group_sendmsg(msg, "group");
+    checkSentMessage("{"
+                     "   \"from\": \"foobar\","
+                     "   \"group\": \"group\","
+                     "   \"instance\": \"*\","
+                     "   \"seq\": 0,"
+                     "   \"to\": \"*\","
+                     "   \"type\": \"send\","
+                     "   \"want_answer\": False"
+                     "}", "No instance");
+    sess.group_sendmsg(msg, "group", "instance", "recipient");
+    checkSentMessage("{"
+                     "   \"from\": \"foobar\","
+                     "   \"group\": \"group\","
+                     "   \"instance\": \"instance\","
+                     "   \"seq\": 1,"
+                     "   \"to\": \"recipient\","
+                     "   \"type\": \"send\","
+                     "   \"want_answer\": False"
+                     "}", "With instance");
+    sess.group_sendmsg(msg, "group", "*", "*", true);
+    checkSentMessage("{"
+                     "   \"from\": \"foobar\","
+                     "   \"group\": \"group\","
+                     "   \"instance\": \"*\","
+                     "   \"seq\": 2,"
+                     "   \"to\": \"*\","
+                     "   \"type\": \"send\","
+                     "   \"want_answer\": True"
+                     "}", "Want answer");
+    sess.group_sendmsg(msg, "group", "*", "*", false);
+    checkSentMessage("{"
+                     "   \"from\": \"foobar\","
+                     "   \"group\": \"group\","
+                     "   \"instance\": \"*\","
+                     "   \"seq\": 3,"
+                     "   \"to\": \"*\","
+                     "   \"type\": \"send\","
+                     "   \"want_answer\": False"
+                     "}", "Doesn't want answer");
+}
+
+}

+ 1 - 1
src/lib/config/tests/fake_session.cc

@@ -183,7 +183,7 @@ FakeSession::unsubscribe(std::string group, std::string instance) {
 
 
 int
 int
 FakeSession::group_sendmsg(ConstElementPtr msg, std::string group,
 FakeSession::group_sendmsg(ConstElementPtr msg, std::string group,
-                           std::string to, std::string)
+                           std::string to, std::string, bool)
 {
 {
     if (throw_on_send_) {
     if (throw_on_send_) {
         isc_throw(Exception, "Throw on send is set in FakeSession");
         isc_throw(Exception, "Throw on send is set in FakeSession");

+ 2 - 1
src/lib/config/tests/fake_session.h

@@ -61,7 +61,8 @@ public:
     virtual int group_sendmsg(isc::data::ConstElementPtr msg,
     virtual int group_sendmsg(isc::data::ConstElementPtr msg,
                               std::string group,
                               std::string group,
                               std::string instance = "*",
                               std::string instance = "*",
-                              std::string to = "*");
+                              std::string to = "*",
+                              bool want_answer = false);
     virtual bool group_recvmsg(isc::data::ConstElementPtr& envelope,
     virtual bool group_recvmsg(isc::data::ConstElementPtr& envelope,
                                isc::data::ConstElementPtr& msg,
                                isc::data::ConstElementPtr& msg,
                                bool nonblock = true,
                                bool nonblock = true,

+ 1 - 1
src/lib/python/isc/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = datasrc cc config dns log net notify util testutils acl bind10
+SUBDIRS = datasrc util cc config dns log net notify testutils acl bind10
 SUBDIRS += xfrin log_messages server_common ddns sysinfo statistics
 SUBDIRS += xfrin log_messages server_common ddns sysinfo statistics
 
 
 python_PYTHON = __init__.py
 python_PYTHON = __init__.py

+ 2 - 2
src/lib/python/isc/cc/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = . tests
+SUBDIRS = . cc_generated tests
 
 
 python_PYTHON =	__init__.py data.py session.py message.py logger.py
 python_PYTHON =	__init__.py data.py session.py message.py logger.py
 BUILT_SOURCES = $(PYTHON_LOGMSGPKG_DIR)/work/pycc_messages.py
 BUILT_SOURCES = $(PYTHON_LOGMSGPKG_DIR)/work/pycc_messages.py
@@ -8,7 +8,7 @@ pylogmessagedir = $(pyexecdir)/isc/log_messages/
 CLEANFILES = $(PYTHON_LOGMSGPKG_DIR)/work/pycc_messages.py
 CLEANFILES = $(PYTHON_LOGMSGPKG_DIR)/work/pycc_messages.py
 CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/pycc_messages.pyc
 CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/pycc_messages.pyc
 
 
-EXTRA_DIST = pycc_messages.mes
+EXTRA_DIST = pycc_messages.mes proto_defs.py
 
 
 # Define rule to build logging source files from message file
 # Define rule to build logging source files from message file
 $(PYTHON_LOGMSGPKG_DIR)/work/pycc_messages.py: pycc_messages.mes
 $(PYTHON_LOGMSGPKG_DIR)/work/pycc_messages.py: pycc_messages.mes

+ 30 - 0
src/lib/python/isc/cc/cc_generated/Makefile.am

@@ -0,0 +1,30 @@
+# This makefile is a hack to enable tests to run with one module generated
+# while the rest is just used. The generated file is created under build dir,
+# not the src dir, which means it is not found when these are different.
+#
+# We have a forwarder module in the src dir and build the real one in different
+# location. This is similar to what happens in log_messages/work. We can't
+# reuse the name `work`, since it would collide, so we use less generic name.
+
+nodist_python_PYTHON = proto_defs.py
+BUILT_SOURCES = proto_defs.py __init__.py
+noinst_SCRIPTS = __init__.py
+
+proto_defs.py: $(top_srcdir)/src/lib/cc/proto_defs.cc \
+	$(top_srcdir)/src/lib/util/python/pythonize_constants.py
+	$(PYTHON) $(top_srcdir)/src/lib/util/python/pythonize_constants.py \
+		$(top_srcdir)/src/lib/cc/proto_defs.cc $@
+
+# We need to create an __init__.py, so it is recognized as module.
+# But it may be empty.
+__init__.py:
+	touch $@
+
+pythondir = $(pyexecdir)/isc/cc
+
+CLEANDIRS = __pycache__
+
+CLEANFILES = proto_defs.py __init__.py
+
+clean-local:
+	rm -rf $(CLEANDIRS)

+ 2 - 0
src/lib/python/isc/cc/proto_defs.py

@@ -0,0 +1,2 @@
+# Forwarder module. Look into cc_generated/Makefile.am for details.
+from cc_generated.proto_defs import *

+ 31 - 10
src/lib/python/isc/cc/session.py

@@ -25,6 +25,7 @@ import isc.cc.message
 import isc.log
 import isc.log
 from isc.cc.logger import logger
 from isc.cc.logger import logger
 from isc.log_messages.pycc_messages import *
 from isc.log_messages.pycc_messages import *
+from isc.cc.proto_defs import *
 
 
 class ProtocolError(Exception): pass
 class ProtocolError(Exception): pass
 class NetworkError(Exception): pass
 class NetworkError(Exception): pass
@@ -33,7 +34,7 @@ class SessionTimeout(Exception): pass
 
 
 class Session:
 class Session:
     MSGQ_DEFAULT_TIMEOUT = 4000
     MSGQ_DEFAULT_TIMEOUT = 4000
-    
+
     def __init__(self, socket_file=None):
     def __init__(self, socket_file=None):
         self._socket = None
         self._socket = None
         self._lname = None
         self._lname = None
@@ -164,7 +165,7 @@ class Session:
         if len(data) == 0: # server closed connection
         if len(data) == 0: # server closed connection
             raise ProtocolError("Read of 0 bytes: connection closed")
             raise ProtocolError("Read of 0 bytes: connection closed")
         return data
         return data
-        
+
     def _receive_len_data(self):
     def _receive_len_data(self):
         """Reads self._recv_len_size bytes of data from the socket into
         """Reads self._recv_len_size bytes of data from the socket into
            self._recv_len_data
            self._recv_len_data
@@ -208,7 +209,7 @@ class Session:
             # they may never both be non-zero (we are either starting
             # they may never both be non-zero (we are either starting
             # a full read, or continuing one of the reads
             # a full read, or continuing one of the reads
             assert self._recv_size == 0 or self._recv_len_size == 0
             assert self._recv_size == 0 or self._recv_len_size == 0
-            
+
             if self._recv_size == 0:
             if self._recv_size == 0:
                 if self._recv_len_size == 0:
                 if self._recv_len_size == 0:
                     # both zero, start a new full read
                     # both zero, start a new full read
@@ -261,15 +262,35 @@ class Session:
             "instance": instance,
             "instance": instance,
         })
         })
 
 
-    def group_sendmsg(self, msg, group, instance = "*", to = "*"):
+    def group_sendmsg(self, msg, group, instance=CC_INSTANCE_WILDCARD,
+                      to=CC_TO_WILDCARD, want_answer=False):
+        '''
+        Send a message over the CC session.
+
+        Parameters:
+        - msg The message to send, encoded as python structures (dicts,
+          lists, etc).
+        - group The recipient group to send to.
+        - instance Instance in the group.
+        - to Direct recipient (overrides the above, should contain the
+          lname of the recipient).
+        - want_answer If an answer is requested. If there's no recipient
+          and this is true, the message queue would send an error message
+          instead of the answer.
+
+        Return:
+          A sequence number that can be used to wait for an answer
+          (see group_recvmsg).
+        '''
         seq = self._next_sequence()
         seq = self._next_sequence()
         self.sendmsg({
         self.sendmsg({
-            "type": "send",
-            "from": self._lname,
-            "to": to,
-            "group": group,
-            "instance": instance,
-            "seq": seq,
+            CC_HEADER_TYPE: CC_COMMAND_SEND,
+            CC_HEADER_FROM: self._lname,
+            CC_HEADER_TO: to,
+            CC_HEADER_GROUP: group,
+            CC_HEADER_INSTANCE: instance,
+            CC_HEADER_SEQ: seq,
+            CC_HEADER_WANT_ANSWER: want_answer
         }, isc.cc.message.to_wire(msg))
         }, isc.cc.message.to_wire(msg))
         return seq
         return seq
 
 

+ 32 - 16
src/lib/python/isc/cc/tests/session_test.py

@@ -377,22 +377,38 @@ class testSession(unittest.TestCase):
         sess = MySession()
         sess = MySession()
         self.assertEqual(sess._sequence, 1)
         self.assertEqual(sess._sequence, 1)
 
 
-        sess.group_sendmsg({ 'hello': 'a' }, "my_group")
-        sent = sess._socket.readsentmsg_parsed()
-        self.assertEqual(sent, ({"from": "test_name", "seq": 2, "to": "*",
-                                 "instance": "*", "group": "my_group",
-                                 "type": "send"}, {"hello": "a"}))
-        self.assertEqual(sess._sequence, 2)
-
-        sess.group_sendmsg({ 'hello': 'a' }, "my_group", "my_instance")
-        sent = sess._socket.readsentmsg_parsed()
-        self.assertEqual(sent, ({"from": "test_name", "seq": 3, "to": "*", "instance": "my_instance", "group": "my_group", "type": "send"}, {"hello": "a"}))
-        self.assertEqual(sess._sequence, 3)
-
-        sess.group_sendmsg({ 'hello': 'a' }, "your_group", "your_instance")
-        sent = sess._socket.readsentmsg_parsed()
-        self.assertEqual(sent, ({"from": "test_name", "seq": 4, "to": "*", "instance": "your_instance", "group": "your_group", "type": "send"}, {"hello": "a"}))
-        self.assertEqual(sess._sequence, 4)
+        msg = { "hello": "a" }
+
+        def check_sent(additional_headers, sequence):
+            sent = sess._socket.readsentmsg_parsed()
+            headers = dict({"from": "test_name",
+                            "seq": sequence,
+                            "to": "*",
+                            "type": "send"},
+                           **additional_headers)
+            self.assertEqual(sent, (headers, msg))
+            self.assertEqual(sess._sequence, sequence)
+
+        sess.group_sendmsg(msg, "my_group")
+        check_sent({"instance": "*", "group": "my_group",
+                    "want_answer": False}, 2)
+
+        sess.group_sendmsg(msg, "my_group", "my_instance")
+        check_sent({"instance": "my_instance", "group": "my_group",
+                    "want_answer": False}, 3)
+
+        sess.group_sendmsg(msg, "your_group", "your_instance")
+        check_sent({"instance": "your_instance", "group": "your_group",
+                    "want_answer": False}, 4)
+
+        # Test the optional want_answer parameter
+        sess.group_sendmsg(msg, "group", want_answer=True)
+        check_sent({"instance": "*", "group": "group", "want_answer": True}, 5)
+
+
+        sess.group_sendmsg(msg, "group", want_answer=False)
+        check_sent({"instance": "*", "group": "group", "want_answer": False},
+                   6)
 
 
     def test_group_recvmsg(self):
     def test_group_recvmsg(self):
         # must this one do anything except not return messages with
         # must this one do anything except not return messages with

+ 1 - 1
src/lib/testutils/mockups.h

@@ -48,7 +48,7 @@ public:
     virtual void disconnect() {}
     virtual void disconnect() {}
 
 
     virtual int group_sendmsg(isc::data::ConstElementPtr msg, std::string group,
     virtual int group_sendmsg(isc::data::ConstElementPtr msg, std::string group,
-                              std::string, std::string)
+                              std::string, std::string, bool)
     {
     {
         if (!send_ok_) {
         if (!send_ok_) {
             isc_throw(isc::cc::SessionError,
             isc_throw(isc::cc::SessionError,

+ 0 - 1
src/lib/util/Makefile.am

@@ -30,7 +30,6 @@ libb10_util_la_SOURCES += random/qid_gen.h random/qid_gen.cc
 libb10_util_la_SOURCES += random/random_number_generator.h
 libb10_util_la_SOURCES += random/random_number_generator.h
 
 
 EXTRA_DIST = python/pycppwrapper_util.h
 EXTRA_DIST = python/pycppwrapper_util.h
-
 libb10_util_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 libb10_util_la_LIBADD = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
 CLEANFILES = *.gcno *.gcda
 CLEANFILES = *.gcno *.gcda
 
 

+ 3 - 1
src/lib/util/python/Makefile.am

@@ -1 +1,3 @@
-noinst_SCRIPTS = gen_wiredata.py mkpywrapper.py
+noinst_SCRIPTS = gen_wiredata.py mkpywrapper.py const2hdr.py \
+	pythonize_constants.py
+EXTRA_DIST = const2hdr.py pythonize_constants.py

+ 65 - 0
src/lib/util/python/const2hdr.py

@@ -0,0 +1,65 @@
+# Copyright (C) 2013  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+'''
+The script takes a C++ file with constant definitions and creates a
+header file for the constants. It, however, does not understand C++
+syntax, it only does some heuristics to guess what looks like
+a constant and strips the values.
+
+The purpose is just to save some work with keeping both the source and
+header. The source syntax must be limited already, because it's used to
+generate the python module (by the
+lib/python/isc/util/pythonize_constants.py script).
+'''
+
+import sys
+import re
+
+if len(sys.argv) != 3:
+    sys.stderr.write("Usage: python3 ./const2hdr.py input.cc output.h\n")
+    sys.exit(1)
+
+[filename_in, filename_out] = sys.argv[1:3]
+
+preproc = re.compile('^#')
+constant = re.compile('^([a-zA-Z].*?[a-zA-Z_0-9]+)\\s*=.*;')
+
+with open(filename_in) as file_in, open(filename_out, "w") as file_out:
+    file_out.write("// This file is generated from " + filename_in + "\n" +
+                   "// by the const2hdr.py script.\n" +
+                   "// Do not edit, all changes will be lost.\n\n")
+    for line in file_in:
+        if preproc.match(line):
+            # There's only one preprocessor line in the .cc file. We abuse
+            # that to position the top part of the header.
+            file_out.write("#ifndef BIND10_COMMON_DEFS_H\n" +
+                           "#define BIND10_COMMON_DEFS_H\n" +
+                           "\n" +
+                           "// \\file " + filename_out + "\n" +
+'''// \\brief Common shared constants\n
+// This file contains common definitions of constasts used across the sources.
+// It includes, but is not limited to the definitions of messages sent from
+// one process to another. Since the names should be self-explanatory and
+// the variables here are used mostly to synchronize the same values across
+// multiple programs, separate documentation for each variable is not provided.
+''')
+            continue
+        # Extract the constant. Remove the values and add "extern"
+        line = constant.sub('extern \\1;', line)
+
+        file_out.write(line)
+
+    file_out.write("#endif\n")

+ 57 - 0
src/lib/util/python/pythonize_constants.py

@@ -0,0 +1,57 @@
+# Copyright (C) 2013  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and 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 INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM 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.
+
+'''
+This script takes a C++ file with constants and converts it to a python
+module. However, the syntax it parses is very limited (it doesn't understand
+C++ at all, it just looks for lines containing the equal sign and strips
+what it thinks might be type).
+
+The purpose is to keep the same values of constants in C++ and python. This
+saves the work of keeping the constants in sync manually and is less error
+prone.
+'''
+
+import sys
+import re
+
+if len(sys.argv) != 3:
+    sys.stderr.write("Usage: python3 ./pythonize_constants.py input.cc output.py\n")
+    sys.exit(1)
+
+[filename_in, filename_out] = sys.argv[1:3]
+
+# Ignore preprocessor, namespaces and the ends of namespaces.
+ignore = re.compile('^(#|namespace|})')
+comment = re.compile('^//(.*)')
+constant = re.compile('^[a-zA-Z].*?([a-zA-Z_0-9]+\\s*=.*);')
+
+with open(filename_in) as file_in, open(filename_out, "w") as file_out:
+    file_out.write("# This file is generated from " + filename_in + "\n" +
+                   "# by the pythonize_constants.py script.\n" +
+                   "# Do not edit, all changes will be lost.\n\n")
+    for line in file_in:
+        if ignore.match(line):
+            continue
+        # Mangle comments to be python-like
+        line = comment.sub('#\\1', line)
+        # Extract the constant.
+
+        # TODO: We may want to do something with the true vs. True and
+        # NULL vs. None and such. Left out for now, since none are in
+        # the input file currently.
+        line = constant.sub('\\1', line)
+
+        file_out.write(line)

+ 1 - 1
tests/lettuce/setup_intree_bind10.sh.in

@@ -23,7 +23,7 @@ BIND10_PATH=@abs_top_builddir@/src/bin/bind10
 PATH=@abs_top_builddir@/src/bin/bind10:@abs_top_builddir@/src/bin/bindctl:@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/resolver:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:@abs_top_builddir@/src/bin/ddns:@abs_top_builddir@/src/bin/dhcp6:@abs_top_builddir@/src/bin/sockcreator:$PATH
 PATH=@abs_top_builddir@/src/bin/bind10:@abs_top_builddir@/src/bin/bindctl:@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/resolver:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:@abs_top_builddir@/src/bin/ddns:@abs_top_builddir@/src/bin/dhcp6:@abs_top_builddir@/src/bin/sockcreator:$PATH
 export PATH
 export PATH
 
 
-PYTHONPATH=@abs_top_builddir@/src/bin:@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/python/isc/config:@abs_top_builddir@/src/lib/python/isc/acl/.libs:@abs_top_builddir@/src/lib/python/isc/datasrc/.libs:$PYTHONPATH
+PYTHONPATH=@abs_top_builddir@/src/bin:@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python/isc/cc:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/log/.libs:@abs_top_builddir@/src/lib/util/io/.libs:@abs_top_builddir@/src/lib/python/isc/config:@abs_top_builddir@/src/lib/python/isc/acl/.libs:@abs_top_builddir@/src/lib/python/isc/datasrc/.libs:$PYTHONPATH
 export PYTHONPATH
 export PYTHONPATH
 
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # If necessary (rare cases), explicitly specify paths to dynamic libraries