Introduce new Daemon.setup_logging() method

This makes things 'just work' without too much boiler plate.
Also, succubus itself relies on self.logger to exist, but leaves
it to the user to actually set it up.

README.rst

@@ -36,13 +36,15 @@ Examples
     class MyDaemon(Daemon):
         def run(self):
             """Overwrite the run function of the Daemon class"""
+            while True:
+                time.sleep(1)
+                self.logger.warn('Hello world')
+
+        def setup_logging(self):
             # TODO: don't log to /tmp except for example code
             handler = WatchedFileHandler('/tmp/succubus.log')
             self.logger = logging.getLogger('succubus')
             self.logger.addHandler(handler)
-            while True:
-                time.sleep(1)
-                self.logger.warn('Hello world')
 
 
     def main():

src/integrationtest/python/logging_daemon.py

@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+"""A daemon that relies on self.logger to be usable"""
+from __future__ import print_function, absolute_import, division
+
+import os
+import sys
+import time
+
+from succubus import Daemon
+
+
+class MyDaemon(Daemon):
+    def run(self):
+        while True:
+            self.logger.error("succubus test running")
+            time.sleep(1)
+
+
+def main():
+    daemon = MyDaemon(pid_file=os.environ['PID_FILE'])
+    sys.exit(daemon.action())
+
+
+if __name__ == '__main__':
+    main()

src/integrationtest/python/logging_daemon_tests.py

@@ -0,0 +1,34 @@
+#!/usr/bin/env python
+from __future__ import print_function, absolute_import, division
+
+import os
+import shutil
+import subprocess
+import tempfile
+import time
+import unittest2
+
+class LoggingDaemonTests(unittest2.TestCase):
+    def setUp(self):
+        self.temp_dir = tempfile.mkdtemp(prefix="succubus-test")
+        self.pid_file = os.path.join(self.temp_dir, 'succubus.pid')
+        os.environ['PID_FILE'] = self.pid_file
+
+    def tearDown(self):
+        shutil.rmtree(self.temp_dir)
+
+    def test_daemon_start_status_stop(self):
+        daemon = "./src/integrationtest/python/logging_daemon.py"
+        subprocess.check_call([daemon, "start"])
+
+        time.sleep(0.3)
+        # Daemon must be running.
+        subprocess.check_call([daemon, "status"])
+        subprocess.check_call([daemon, "stop"])
+
+        # Daemon must be stopped as soon as "stop" returns.
+        self.assertRaises(Exception, subprocess.check_call, [daemon, "status"])
+
+
+if __name__ == "__main__":
+    unittest2.main()

src/main/python/succubus/daemonize.py

@@ -3,6 +3,8 @@
 
 from __future__ import print_function, absolute_import, division
 
+from logging.handlers import SysLogHandler
+import logging
 import os
 import signal
 import sys
@@ -86,6 +88,22 @@ def load_configuration(self):
         """Set up self.config if needed"""
         pass
 
+    def setup_logging(self):
+        """Set up self.logger
+
+        This function is called after load_configuration() and after changing
+        to new user/group IDs (if configured). Logging to syslog using the
+        root logger is configured by default, you can override this method if
+        you want something else.
+        """
+        self.logger = logging.getLogger()
+
+        if os.path.exists('/dev/log'):
+            handler = SysLogHandler('/dev/log')
+        else:
+            handler = SysLogHandler()
+        self.logger.addHandler(handler)
+
     def shutdown(self):
         """Clean up when daemon is about to terminate
 
@@ -169,6 +187,9 @@ def start(self):
             return 0
         self.set_gid()
         self.set_uid()
+        # Create log files (if configured) with the new user/group. Creating
+        # them as root would allow symlink exploits.
+        self.setup_logging()
         # Create pid file with new user/group. This ensures we will be able
         # to delete the file when shutting down.
         self.daemonize()

src/unittest/python/daemonize_tests.py

@@ -252,3 +252,53 @@ def test_status_handles_missing_process(self):
 
         daemon._already_running.assert_called_once_with()
         self.assertEqual(retval, 3)
+
+class TestSetupLogging(TestCase):
+    @patch("succubus.daemonize.SysLogHandler")
+    @patch("succubus.daemonize.logging")
+    def test_logger_exists_and_has_a_handler(self, mock_logging, mock_sysloghandler):
+        daemon = Daemon(pid_file="foo")
+
+        daemon.setup_logging()
+
+        self.assertTrue(daemon.logger.handlers)
+
+    @patch("succubus.daemonize.SysLogHandler")
+    @patch("succubus.daemonize.logging")
+    @patch("succubus.daemonize.os")
+    def test_uses_dev_log_if_available(self, mock_os, mock_logging, mock_sysloghandler):
+        daemon = Daemon(pid_file="foo")
+        mock_os.path.exists.return_value = True
+
+        daemon.setup_logging()
+
+        mock_sysloghandler.assert_called_with('/dev/log')
+
+    @patch("succubus.daemonize.SysLogHandler")
+    @patch("succubus.daemonize.logging")
+    @patch("succubus.daemonize.os")
+    def test_uses_defaults_if_dev_log_unavailable(self, mock_os, mock_logging, mock_sysloghandler):
+        daemon = Daemon(pid_file="foo")
+        mock_os.path.exists.return_value = False
+
+        daemon.setup_logging()
+
+        # SysLogHandler defaults to using UDP to localhost:514
+        mock_sysloghandler.assert_called_with()
+
+    def test_setup_logging_called_at_right_time(self):
+        # daemon.setup_logging must be called after daemon.set_uid/set_gid
+        # and before daemon.daemonize.
+        daemon = Daemon(pid_file="foo")
+        daemon._already_running = lambda: False
+        daemon.set_uid = Mock()
+        daemon.set_gid = Mock()
+        daemon.setup_logging = Mock()
+        daemon.setup_logging.side_effect = ZeroDivisionError
+        daemon.daemonize = Mock()
+
+        self.assertRaises(ZeroDivisionError, daemon.start)
+
+        daemon.set_uid.assert_called()
+        daemon.set_gid.assert_called()
+        daemon.daemonize.assert_not_called()