diff --git a/daemons/ganeti-confd b/daemons/ganeti-confd
index 958e1a133e50948250b38f899628b0d668e72846..3a685c78e2682eeda7eb46d0e26664b5df003ca2 100755
--- a/daemons/ganeti-confd
+++ b/daemons/ganeti-confd
@@ -117,7 +117,8 @@ class ConfdConfigurationReloader(object):
self.inotify_handler = asyncnotifier.SingleFileEventHandler(self.wm,
self.OnInotify,
cfg_file)
- self.notifier = asyncnotifier.AsyncNotifier(self.wm, self.inotify_handler)
+ notifier_class = asyncnotifier.ErrorLoggingAsyncNotifier
+ self.notifier = notifier_class(self.wm, self.inotify_handler)
self.timer_handle = None
self._EnableTimer()
diff --git a/lib/asyncnotifier.py b/lib/asyncnotifier.py
index 421e476dba9de643556d934ccc7482e6e0952fb1..e60c26087557a0c96cd067418b3c9ab696b9dd83 100644
--- a/lib/asyncnotifier.py
+++ b/lib/asyncnotifier.py
@@ -31,6 +31,7 @@ try:
except ImportError:
import pyinotify
+from ganeti import daemon
from ganeti import errors
# We contributed the AsyncNotifier class back to python-pyinotify, and it's
@@ -65,6 +66,16 @@ class AsyncNotifier(asyncore.file_dispatcher):
self.notifier.process_events()
+class ErrorLoggingAsyncNotifier(AsyncNotifier,
+ daemon.GanetiBaseAsyncoreDispatcher):
+ """An asyncnotifier that can survive errors in the callbacks.
+
+ We define this as a separate class, since we don't want to make AsyncNotifier
+ diverge from what we contributed upstream.
+
+ """
+
+
class SingleFileEventHandler(pyinotify.ProcessEvent):
"""Handle modify events for a single file.
@@ -122,14 +133,7 @@ class SingleFileEventHandler(pyinotify.ProcessEvent):
# by the callback by calling "enable" again on us.
logging.debug("Received 'ignored' inotify event for %s", event.path)
self.watch_handle = None
-
- try:
- self.callback(False)
- except: # pylint: disable-msg=W0702
- # we need to catch any exception here, log it, but proceed, because even
- # if we failed handling a single request, we still want our daemon to
- # proceed.
- logging.error("Unexpected exception", exc_info=True)
+ self.callback(False)
# pylint: disable-msg=C0103
# this overrides a method in pyinotify.ProcessEvent
@@ -139,14 +143,7 @@ class SingleFileEventHandler(pyinotify.ProcessEvent):
# replacing any file with a new one, at filesystem level, rather than
# actually changing it. (see utils.WriteFile)
logging.debug("Received 'modify' inotify event for %s", event.path)
-
- try:
- self.callback(True)
- except: # pylint: disable-msg=W0702
- # we need to catch any exception here, log it, but proceed, because even
- # if we failed handling a single request, we still want our daemon to
- # proceed.
- logging.error("Unexpected exception", exc_info=True)
+ self.callback(True)
def process_default(self, event):
logging.error("Received unhandled inotify event: %s", event)
diff --git a/lib/daemon.py b/lib/daemon.py
index 8a057a0f3cf83b08a9cc8a3817815a7f6c1367fb..21aab9227c42a49c3542421535d9ad31c3a3b719 100644
--- a/lib/daemon.py
+++ b/lib/daemon.py
@@ -72,7 +72,26 @@ class AsyncoreScheduler(sched.scheduler):
sched.scheduler.__init__(self, timefunc, AsyncoreDelayFunction)
-class AsyncUDPSocket(asyncore.dispatcher):
+class GanetiBaseAsyncoreDispatcher(asyncore.dispatcher):
+ """Base Ganeti Asyncore Dispacher
+
+ """
+ # this method is overriding an asyncore.dispatcher method
+ def handle_error(self):
+ """Log an error in handling any request, and proceed.
+
+ """
+ logging.exception("Error while handling asyncore request")
+
+ # this method is overriding an asyncore.dispatcher method
+ def writable(self):
+ """Most of the time we don't want to check for writability.
+
+ """
+ return False
+
+
+class AsyncUDPSocket(GanetiBaseAsyncoreDispatcher):
"""An improved asyncore udp socket.
"""
@@ -80,7 +99,7 @@ class AsyncUDPSocket(asyncore.dispatcher):
"""Constructor for AsyncUDPSocket
"""
- asyncore.dispatcher.__init__(self)
+ GanetiBaseAsyncoreDispatcher.__init__(self)
self._out_queue = []
self.create_socket(socket.AF_INET, socket.SOCK_DGRAM)
@@ -119,13 +138,6 @@ class AsyncUDPSocket(asyncore.dispatcher):
utils.IgnoreSignals(self.sendto, payload, 0, (ip, port))
self._out_queue.pop(0)
- # this method is overriding an asyncore.dispatcher method
- def handle_error(self):
- """Log an error in handling any request, and proceed.
-
- """
- logging.exception("Error while handling asyncore request")
-
def enqueue_send(self, ip, port, payload):
"""Enqueue a datagram to be sent when possible
diff --git a/test/ganeti.asyncnotifier_unittest.py b/test/ganeti.asyncnotifier_unittest.py
index b379ba1efff8d77f459938d8a8f4f6552be3c995..51b3a82c470003b542a15e023980d20efec090ad 100755
--- a/test/ganeti.asyncnotifier_unittest.py
+++ b/test/ganeti.asyncnotifier_unittest.py
@@ -34,14 +34,27 @@ except ImportError:
from ganeti import asyncnotifier
from ganeti import daemon
from ganeti import utils
+from ganeti import errors
import testutils
+class _MyErrorLoggingAsyncNotifier(asyncnotifier.ErrorLoggingAsyncNotifier):
+ def __init__(self, *args, **kwargs):
+ asyncnotifier.ErrorLoggingAsyncNotifier.__init__(self, *args, **kwargs)
+ self.error_count = 0
+
+ def handle_error(self):
+ self.error_count += 1
+ # We should also terminate while handling an error, so that any unexpected
+ # error is registered and can be checked.
+ os.kill(os.getpid(), signal.SIGTERM)
+
+
class TestSingleFileEventHandler(testutils.GanetiTestCase):
"""Test daemon.Mainloop"""
- NOTIFIERS = [NOTIFIER_TERM, NOTIFIER_NORM] = range(2)
+ NOTIFIERS = [NOTIFIER_TERM, NOTIFIER_NORM, NOTIFIER_ERR] = range(3)
def setUp(self):
testutils.GanetiTestCase.setUp(self)
@@ -57,8 +70,8 @@ class TestSingleFileEventHandler(testutils.GanetiTestCase):
self.cbk[i],
self.chk_files[i])
for i in range(len(self.NOTIFIERS))]
- self.notifiers = [asyncnotifier.AsyncNotifier(self.wms[i],
- self.ihandler[i])
+ self.notifiers = [_MyErrorLoggingAsyncNotifier(self.wms[i],
+ self.ihandler[i])
for i in range(len(self.NOTIFIERS))]
# TERM notifier is enabled by default, as we use it to get out of the loop
self.ihandler[self.NOTIFIER_TERM].enable()
@@ -73,12 +86,16 @@ class TestSingleFileEventHandler(testutils.GanetiTestCase):
self.notified[self.i] = True
if self.i == self.testobj.NOTIFIER_TERM:
os.kill(os.getpid(), signal.SIGTERM)
+ elif self.i == self.testobj.NOTIFIER_ERR:
+ raise errors.GenericError("an error")
def testReplace(self):
utils.WriteFile(self.chk_files[self.NOTIFIER_TERM], data="dummy")
self.mainloop.Run()
self.assert_(self.notified[self.NOTIFIER_TERM])
self.assert_(not self.notified[self.NOTIFIER_NORM])
+ self.assertEquals(self.notifiers[self.NOTIFIER_TERM].error_count, 0)
+ self.assertEquals(self.notifiers[self.NOTIFIER_NORM].error_count, 0)
def testEnableDisable(self):
self.ihandler[self.NOTIFIER_TERM].enable()
@@ -91,6 +108,8 @@ class TestSingleFileEventHandler(testutils.GanetiTestCase):
self.mainloop.Run()
self.assert_(self.notified[self.NOTIFIER_TERM])
self.assert_(not self.notified[self.NOTIFIER_NORM])
+ self.assertEquals(self.notifiers[self.NOTIFIER_TERM].error_count, 0)
+ self.assertEquals(self.notifiers[self.NOTIFIER_NORM].error_count, 0)
def testDoubleEnable(self):
self.ihandler[self.NOTIFIER_TERM].enable()
@@ -99,6 +118,8 @@ class TestSingleFileEventHandler(testutils.GanetiTestCase):
self.mainloop.Run()
self.assert_(self.notified[self.NOTIFIER_TERM])
self.assert_(not self.notified[self.NOTIFIER_NORM])
+ self.assertEquals(self.notifiers[self.NOTIFIER_TERM].error_count, 0)
+ self.assertEquals(self.notifiers[self.NOTIFIER_NORM].error_count, 0)
def testDefaultDisabled(self):
utils.WriteFile(self.chk_files[self.NOTIFIER_NORM], data="dummy")
@@ -107,6 +128,8 @@ class TestSingleFileEventHandler(testutils.GanetiTestCase):
self.assert_(self.notified[self.NOTIFIER_TERM])
# NORM notifier is disabled by default
self.assert_(not self.notified[self.NOTIFIER_NORM])
+ self.assertEquals(self.notifiers[self.NOTIFIER_TERM].error_count, 0)
+ self.assertEquals(self.notifiers[self.NOTIFIER_NORM].error_count, 0)
def testBothEnabled(self):
self.ihandler[self.NOTIFIER_NORM].enable()
@@ -115,6 +138,17 @@ class TestSingleFileEventHandler(testutils.GanetiTestCase):
self.mainloop.Run()
self.assert_(self.notified[self.NOTIFIER_TERM])
self.assert_(self.notified[self.NOTIFIER_NORM])
+ self.assertEquals(self.notifiers[self.NOTIFIER_TERM].error_count, 0)
+ self.assertEquals(self.notifiers[self.NOTIFIER_NORM].error_count, 0)
+
+ def testError(self):
+ self.ihandler[self.NOTIFIER_ERR].enable()
+ utils.WriteFile(self.chk_files[self.NOTIFIER_ERR], data="dummy")
+ self.mainloop.Run()
+ self.assert_(self.notified[self.NOTIFIER_ERR])
+ self.assertEquals(self.notifiers[self.NOTIFIER_ERR].error_count, 1)
+ self.assertEquals(self.notifiers[self.NOTIFIER_NORM].error_count, 0)
+ self.assertEquals(self.notifiers[self.NOTIFIER_TERM].error_count, 0)
if __name__ == "__main__":