Mark Bergsma has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/405298 )
Change subject: Stop pybal on all failures to properly parse 'monitors'
......................................................................
Stop pybal on all failures to properly parse 'monitors'
When a monitor module failed to import, pybal was made to exit in order
to prevent it from silently doing nothing. However, a failure parsing
the 'monitors' configuration option would only log an error and still
not have the desired effect. Stop pybal in more cases when it fails to
interpret 'monitors'.
Also complete unit testing coverage of createMonitoringInstances
Change-Id: I804af7081f645f4d2e90499a2394c40712a52241
---
M pybal/server.py
M pybal/test/test_server.py
2 files changed, 46 insertions(+), 34 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/debs/pybal
refs/changes/98/405298/1
diff --git a/pybal/server.py b/pybal/server.py
index 38a9224..3622322 100644
--- a/pybal/server.py
+++ b/pybal/server.py
@@ -181,37 +181,41 @@
lvsservice = self.lvsservice
try:
- monitorlist = eval(lvsservice.configuration['monitors'])
- except KeyError:
- log.critical(
- "LVS service {} does not have a 'monitors' configuration
option set.".format(
- lvsservice.name)
- )
- raise
+ try:
+ monitorlist = eval(lvsservice.configuration['monitors'])
+ except KeyError:
+ log.critical(
+ "LVS service {} does not have a 'monitors' configuration
option set.".format(
+ lvsservice.name)
+ )
+ raise
- if type(monitorlist) != list:
- msg = "option 'monitors' in LVS service section {} is not a python
list"
- log.err(msg.format(lvsservice.name))
- else:
- for monitorname in monitorlist:
- try:
- monitormodule = importlib.import_module(
- "pybal.monitors.{}".format(monitorname.lower()))
- except ImportError:
- log.err("Monitor {} does not exist".format(monitorname))
- except Exception:
- log.critical("Cannot import
pybal.monitors.{}".format(monitorname))
- # An exception was raised importing the given monitor
- # module. Instead of just logging the problem, stop PyBal
- # as the admin might think everything is fine and all
- # checks are green, while in fact no check is being
- # performed.
- reactor.stop()
- else:
- monitorclass = getattr(monitormodule, monitorname +
'MonitoringProtocol')
- monitor = monitorclass(coordinator, self,
lvsservice.configuration)
- self.addMonitor(monitor)
- monitor.run()
+ if not isinstance(monitorlist, list):
+ msg = "option 'monitors' in LVS service section {} is not a
python list"
+ log.critical(msg.format(lvsservice.name))
+ raise Exception(msg.format(lvsservice.name))
+ else:
+ for monitorname in monitorlist:
+ try:
+ monitormodule = importlib.import_module(
+ "pybal.monitors.{}".format(monitorname.lower()))
+ except ImportError:
+ log.err("Monitor {} does not
exist".format(monitorname))
+ except Exception:
+ log.critical("Cannot import
pybal.monitors.{}".format(monitorname))
+ raise
+ else:
+ monitorclass = getattr(monitormodule, monitorname +
'MonitoringProtocol')
+ monitor = monitorclass(coordinator, self,
lvsservice.configuration)
+ self.addMonitor(monitor)
+ monitor.run()
+ except Exception:
+ # An exception was raised importing a monitor
+ # module. Instead of just logging the problem, stop PyBal
+ # as the admin might think everything is fine and all
+ # checks are green, while in fact no check is being
+ # performed.
+ reactor.stop()
def calcStatus(self):
"""AND quantification of monitor.up over all monitoring instances of a
single Server"""
diff --git a/pybal/test/test_server.py b/pybal/test/test_server.py
index 1df0121..017b3fa 100644
--- a/pybal/test/test_server.py
+++ b/pybal/test/test_server.py
@@ -124,10 +124,6 @@
self.assertFalse(self.server.ready)
def testCreateMonitoringInstances(self):
- assert 'monitors' not in self.config
- self.assertRaises(KeyError,
- self.server.createMonitoringInstances, self.mockCoordinator)
-
self.config['monitors'] = "[ \"NonexistentMonitor\" ]"
self.server.createMonitoringInstances(self.mockCoordinator)
@@ -145,6 +141,18 @@
self.server.createMonitoringInstances(self.mockCoordinator)
self.assertTrue(mock_reactor.assert_called)
+ @mock.patch('twisted.internet.reactor.stop')
+ def testCreateMonitoringInstanceFailure(self, mock_reactor):
+ assert 'monitors' not in self.config
+ self.server.createMonitoringInstances(self.mockCoordinator)
+ mock_reactor.assert_called()
+ mock_reactor.mock_reset()
+
+ # Test a non-list in the configuration
+ self.config['monitors'] = "(1,2)"
+ self.server.createMonitoringInstances(self.mockCoordinator)
+ mock_reactor.assert_called()
+
def testCalcStatus(self):
self.mockMonitor.up = True
self.assertTrue(self.server.calcStatus())
--
To view, visit https://gerrit.wikimedia.org/r/405298
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I804af7081f645f4d2e90499a2394c40712a52241
Gerrit-PatchSet: 1
Gerrit-Project: operations/debs/pybal
Gerrit-Branch: master
Gerrit-Owner: Mark Bergsma <m...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
