Giuseppe Lavagetto has submitted this change and it was merged.
Change subject: Re-think the alerts instrumentation
......................................................................
Re-think the alerts instrumentation
In order to avoid edge cases and race conditions, we just check for each
pool that is configured if any server is pooled while marked to be down.
This is bot inexpensive and much safer
Change-Id: I3167e877842847cace43d3dd907470ade2b67ff1
---
M pybal/instrumentation.py
M pybal/pybal.py
M pybal/test/test_instrumentation.py
3 files changed, 18 insertions(+), 43 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, approved
jenkins-bot: Verified
diff --git a/pybal/instrumentation.py b/pybal/instrumentation.py
index 56263d1..21b5e16 100644
--- a/pybal/instrumentation.py
+++ b/pybal/instrumentation.py
@@ -54,24 +54,21 @@
alerting_services = {}
isLeaf = True
- @classmethod
- def addAlert(cls, name, msg):
- cls.alerting_services[name] = msg
-
- @classmethod
- def delAlert(cls, name):
- if name in cls.alerting_services:
- del cls.alerting_services[name]
-
def render_GET(self, request):
- if not len(self.alerting_services):
+ critPools = {}
+ for pool, crd in PoolsRoot._pools.items():
+ pooledDown = len(crd.pooledDownServers)
+ if pooledDown:
+ slist = ", ".join(crd.pooledDownServers)
+ critPools[pool] = "Servers %s are marked down but pooled" %
slist
+ if critPools == {}:
return "OK"
else:
if wantJson(request):
- return json.dump(self.alerting_services)
+ return json.dumps(critPools)
else:
return "; ".join(["%s - %s" % (k, v)
- for k, v in self.alerting_services.items()])
+ for k, v in critPools.items()])
class PoolsRoot(Resource):
diff --git a/pybal/pybal.py b/pybal/pybal.py
index 4f7863b..cbffd3a 100755
--- a/pybal/pybal.py
+++ b/pybal/pybal.py
@@ -364,7 +364,6 @@
self.pooledDownServers.add(server)
msg = "Could not depool server " \
"{} because of too many down!".format(server.host)
- instrumentation.Alerts.addAlert(self.lvsservice.name, msg)
log.error(msg, system=self.lvsservice.name)
def repool(self, server):
@@ -387,10 +386,6 @@
# See if we can depool any servers that could not be depooled before
while len(self.pooledDownServers) > 0 and self.canDepool():
self.depool(self.pooledDownServers.pop())
-
- # See if we can clear the alert
- if len(self.pooledDownServers) == 0:
- instrumentation.Alerts.delAlert(self.lvsservice.name)
def canDepool(self):
"""Returns a boolean denoting whether another server can be depooled"""
diff --git a/pybal/test/test_instrumentation.py
b/pybal/test/test_instrumentation.py
index 632c352..4c1d16a 100644
--- a/pybal/test/test_instrumentation.py
+++ b/pybal/test/test_instrumentation.py
@@ -48,6 +48,7 @@
self.coordinators.append(coord)
PoolsRoot.addPool('test_pool0', self.coordinators[0])
+
class Resp404TestCase(WebBaseTestCase):
"""Test case for `pybal.instrumentation.Resp404`"""
@@ -79,33 +80,15 @@
class AlertsTestCase(WebBaseTestCase):
"""Test case for `pybal.instrumentation.Alerts`"""
- def test_addAlert(self):
- """
- Test case for `Alerts.addAlert`
- """
- Alerts.addAlert('test', 'test_msg')
- self.assertEquals(Alerts.alerting_services, {'test': 'test_msg'})
+ def test_render(self):
+ """Test case for `PoolsRoot.render_GET`"""
+ crd = self.coordinators[0]
+ r = Alerts()
+ self.assertEquals("OK", r.render_GET(self.request))
+ crd.pooledDownServers = ['mw1001', 'mw1002']
+ self.assertEquals('{"test_pool0": "Servers mw1001, mw1002 are marked
down but pooled"}',
+ r.render_GET(self.request))
- def test_delAlert(self):
- """
- Test case for `Alerts.delAlert`
- """
- try:
- Alerts.delAlert('inexistent')
- except Exception:
- self.fail("An exception was raised when deleting an alert")
- Alerts.addAlert('test', 'test_msg')
- Alerts.delAlert('test')
- self.assertEquals(Alerts.alerting_services, {})
-
- def test_render(self):
- """Test case for `PoolsRoot.render_GET`"""
- r = Alerts()
- self.assertEquals("OK", r.render_GET(self.request))
- Alerts.addAlert('test', 'test_msg')
- Alerts.addAlert('test1', 'test_msg1')
- self.assertEquals("test - test_msg; test1 - test_msg1",
- r.render_GET(self.request))
class PoolsRootTestCase(WebBaseTestCase):
"""Test case for `pybal.instrumentation.PoolsRoot`"""
--
To view, visit https://gerrit.wikimedia.org/r/261183
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3167e877842847cace43d3dd907470ade2b67ff1
Gerrit-PatchSet: 2
Gerrit-Project: operations/debs/pybal
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Lavagetto <[email protected]>
Gerrit-Reviewer: Giuseppe Lavagetto <[email protected]>
Gerrit-Reviewer: Mark Bergsma <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
