Merge lp:~doanac/ubuntu-ci-services-itself/rabbit-queue-status into lp:ubuntu-ci-services-itself

[Andy Doan]

run_worker: add minimal monitoring for workers

We've had some periodic issues where a run_worker script
failes to come online.

This adds a simple check for each of our services that use
rabbitmq workers. It checks queue information via the rabbitmq
web API to see how many consumers are subscribed to the queue. This
will let us know when a run_worker script isn't running.

We've had a few bugs lately where one or more of our rabbit-workers weren't online. Its a hard situation to detect, and requires poking around via "juju-ssh". This adds a simple status check to each of our services using rabbit workers. They check for the "consumer count" of a queue which essentially indicates whether or not its corresponding runner is online.

In addition to unit test cases, I ran some test cases in the cloud with:

turning rabbit off yields a webui page that looks like:

 | imagebuild-restish/0 | rabbit configured: true
 | | workers-online: unable to check

turning off one of the workers yields (with "workers-online" highlighted in red):

 | imagebuild-restish/0 | rabbit configured: true
 | | workers-online: 0

and when everything is online:

 | imagebuild-restish/0 | rabbit configured: true
 | | workers-online: 1

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:333
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/322/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/322/rebuild

review: Needs Fixing (continuous-integration)

[lp:~doanac/ubuntu-ci-services-itself/rabbit-queue-status]

334. By Andy Doan on 2014-03-07

fix broken test case

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:334
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/333/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/333/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:334
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/334/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/334/rebuild

review: Approve (continuous-integration)

[Vincent Ladeuil (vila)]

62 + '''checks if there any workers attached to the queue.'''

'are' missing ?

66 + # we already report rabbit isn't configured, so no sense adding
67 + # another failure for this

Let's have a common way to report that error then, in the most obscure cases, it's better to have *some* output than none. I know this will never happen... but I know that if it happens, we'll be glad to have an error message ;)

review: Approve

[lp:~doanac/ubuntu-ci-services-itself/rabbit-queue-status]

335. By Andy Doan on 2014-03-10

grammar mistake

[Andy Doan (doanac)]

On 03/09/2014 04:43 AM, Vincent Ladeuil wrote:
> 62 + '''checks if there any workers attached to the queue.'''
>
> 'are' missing ?

fixed in revno(335)

> 66 + # we already report rabbit isn't configured, so no sense adding
> 67 + # another failure for this
>
> Let's have a common way to report that error then, in the most obscure cases, it's better to have*some* output than none. I know this will never happen... but I know that if it happens, we'll be glad to have an error message;)

We are via the "add_rabbit_configured" in that object. My point was
we'll be reporting a "rabbit isn't configured" message already so
showing another error is just piling on extra stuff that might make it
more confusing to diagnose.

[Andy Doan (doanac)]

self-acking since vila already acked.

review: Approve

[Vincent Ladeuil (vila)]

>>>>> Andy Doan <email address hidden> writes:

    > On 03/09/2014 04:43 AM, Vincent Ladeuil wrote:
    >> 62 + '''checks if there any workers attached to the queue.'''
    >>
    >> 'are' missing ?

    > fixed in revno(335)

    >> 66 + # we already report rabbit isn't configured, so no sense adding
    >> 67 + # another failure for this
    >>
    >> Let's have a common way to report that error then, in the most obscure cases, it's better to have*some* output than none. I know this will never happen... but I know that if it happens, we'll be glad to have an error message;)

    > We are via the "add_rabbit_configured" in that object. My point was
    > we'll be reporting a "rabbit isn't configured" message already so
    > showing another error is just piling on extra stuff that might make it
    > more confusing to diagnose.

Ok, I was mistaken then, I thought the previous error was fatal and that
this one could only trigger if the config existed before and disappeared
for unknown reasons.

=== modified file 'branch-source-builder/bsbuilder/resources/v1.py'
--- branch-source-builder/bsbuilder/resources/v1.py	2014-03-06 19:04:41 +0000
+++ branch-source-builder/bsbuilder/resources/v1.py	2014-03-10 01:19:47 +0000
@@ -21,11 +21,13 @@
 from ci_utils import amqp_utils, json_status, restish_utils
 
 log = logging.getLogger(__name__)
+WORKER_QUEUE = 'bsbuilder'
 
 
 def _status():
     status = json_status.JSONStatus()
     status.add_rabbit_configured()
+    status.add_rabbit_worker_health(WORKER_QUEUE)
     return restish_utils.json_ok(status.results)
 
 
@@ -43,7 +45,7 @@
         log.error('Unable to notify progress trigger, aborting build_source')
         return http.service_unavailable(body=r)
 
-    r = amqp_utils.send('bsbuilder', json.dumps(params))
+    r = amqp_utils.send(WORKER_QUEUE, json.dumps(params))
     if r:
         # send only returns something if it an error message
         r = http.service_unavailable(body=r)

=== modified file 'branch-source-builder/bsbuilder/tests/test_v1.py'
--- branch-source-builder/bsbuilder/tests/test_v1.py	2014-03-06 19:04:41 +0000
+++ branch-source-builder/bsbuilder/tests/test_v1.py	2014-03-10 01:19:47 +0000
@@ -41,7 +41,9 @@
         }]
         self.assertListEqual(expected, data)
 
-        get_config.return_value = {'foo': 'bar'}
+        config = mock.Mock()
+        config.AMQP_HOST = 'bar'
+        get_config.return_value = config
         resp = self.app.get('/api/v1/status', status=200)
         data = json.loads(resp.body)
         self.assertEqual(True, data[0]['value'])

=== modified file 'ci-utils/ci_utils/json_status.py'
--- ci-utils/ci_utils/json_status.py	2014-03-06 15:45:17 +0000
+++ ci-utils/ci_utils/json_status.py	2014-03-10 01:19:47 +0000
@@ -13,6 +13,9 @@
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import json
+import urllib2
+
 
 class JSONStatus(object):
     def __init__(self):
@@ -47,3 +50,35 @@
             self.add_okay('rabbit configured', True)
         else:
             self.add_fail('rabbit configured', False)
+
+    def add_rabbit_worker_health(self, queue):
+        '''checks if there are any workers attached to the queue.'''
+        from ci_utils import amqp_utils
+        config = amqp_utils.get_config()
+        if config is None:
+            # we already report rabbit isn't configured, so no sense adding
+            # another failure for this
+            return
+
+        mgr = urllib2.HTTPPasswordMgrWithDefaultRealm()
+        base = 'http://{}:55672'.format(config.AMQP_HOST)
+        # NOTE: juju leaves this guest/guest and doesn't seem to expose
+        # anything in the relation to override it
+        mgr.add_password(None, base, 'guest', 'guest')
+        handler = urllib2.HTTPBasicAuthHandler(mgr)
+
+        try:
+            resp = urllib2.build_opener(handler).open(base + '/api/queues')
+            content = resp.read()
+            data = json.loads(content)
+            consumers = None
+            for q in data:
+                if q['name'] == queue:
+                    consumers = q['consumers']
+                    break
+            if consumers is None:
+                self.add_fail('workers-online', 'no queue defined')
+            else:
+                self.add_true_false('workers-online', consumers, consumers > 0)
+        except:
+            self.add_fail('workers-online', 'unable to check')

=== added file 'ci-utils/ci_utils/tests/test_json_status.py'
--- ci-utils/ci_utils/tests/test_json_status.py	1970-01-01 00:00:00 +0000
+++ ci-utils/ci_utils/tests/test_json_status.py	2014-03-10 01:19:47 +0000
@@ -0,0 +1,96 @@
+# Ubuntu CI Engine
+# Copyright 2014 Canonical Ltd.
+
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License version 3, as
+# published by the Free Software Foundation.
+
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU Affero General Public License for more details.
+
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import json
+import unittest
+
+import mock
+
+from ci_utils.json_status import JSONStatus
+
+
+class TestRabbitStatus(unittest.TestCase):
+    @mock.patch('ci_utils.amqp_utils.get_config')
+    def testNoConfig(self, get_config):
+        get_config.return_value = None
+        status = JSONStatus()
+        status.add_rabbit_worker_health('foo')
+        self.assertEqual([], status.results)
+
+    @mock.patch('ci_utils.amqp_utils.get_config')
+    @mock.patch('urllib2.build_opener')
+    def testCantReach(self, opener, get_config):
+        opener.side_effect = RuntimeError('foo')
+        status = JSONStatus()
+        status.add_rabbit_worker_health('foo')
+        expected = {
+            'status': 'fail',
+            'value': 'unable to check',
+            'label': 'workers-online'
+        }
+        self.assertEqual([expected], status.results)
+
+    @mock.patch('ci_utils.amqp_utils.get_config')
+    @mock.patch('urllib2.build_opener')
+    def _testRabbitHealth(self, queue, data, opener, get_config):
+        fdfake = mock.Mock()
+        fdfake.read.return_value = json.dumps(data)
+        con = mock.Mock()
+        con.open.return_value = fdfake
+        opener.return_value = con
+        status = JSONStatus()
+        status.add_rabbit_worker_health(queue)
+        return status.results
+
+    def testWorkers(self):
+        data = [
+            {'name': 'bla', 'consumers': 2},
+            {'name': 'foo', 'consumers': 1},
+            {'name': 'bar', 'consumers': 1},
+        ]
+        expected = {
+            'status': 'okay',
+            'value': 1,
+            'label': 'workers-online'
+        }
+        results = self._testRabbitHealth('foo', data)
+        self.assertEqual([expected], results)
+
+    def testNoWorkers(self):
+        data = [
+            {'name': 'bla', 'consumers': 2},
+            {'name': 'foo', 'consumers': 0},
+            {'name': 'bar', 'consumers': 1},
+        ]
+        expected = {
+            'status': 'fail',
+            'value': 0,
+            'label': 'workers-online'
+        }
+        results = self._testRabbitHealth('foo', data)
+        self.assertEqual([expected], results)
+
+    def testNoQueue(self):
+        data = [
+            {'name': 'bla', 'consumers': 2},
+            {'name': 'bar', 'consumers': 1},
+        ]
+        expected = {
+            'status': 'fail',
+            'value': 'no queue defined',
+            'label': 'workers-online'
+        }
+        results = self._testRabbitHealth('foo', data)
+        self.assertEqual([expected], results)

=== modified file 'image-builder/imagebuilder/resources/v1.py'
--- image-builder/imagebuilder/resources/v1.py	2014-03-06 19:04:41 +0000
+++ image-builder/imagebuilder/resources/v1.py	2014-03-10 01:19:47 +0000
@@ -21,6 +21,7 @@
 from ci_utils import amqp_utils, json_status, restish_utils
 
 log = logging.getLogger(__name__)
+WORKER_QUEUE = 'imagebuilder'
 
 
 def _build_image(base_image, ppa_list, package_list, progress_trigger,
@@ -39,7 +40,7 @@
         log.error('Unable to notify progress trigger, aborting build_image')
         return http.service_unavailable(body=r)
 
-    r = amqp_utils.send('imagebuilder', json.dumps(params))
+    r = amqp_utils.send(WORKER_QUEUE, json.dumps(params))
     if r:
         r = http.service_unavailable(body=r)
     return r
@@ -48,6 +49,7 @@
 def _status():
     status = json_status.JSONStatus()
     status.add_rabbit_configured()
+    status.add_rabbit_worker_health(WORKER_QUEUE)
     return restish_utils.json_ok(status.results)
 
 

=== modified file 'juju-deployer/branch-source-builder.yaml.tmpl'
--- juju-deployer/branch-source-builder.yaml.tmpl	2014-03-06 22:18:04 +0000
+++ juju-deployer/branch-source-builder.yaml.tmpl	2014-03-10 01:19:47 +0000
@@ -37,6 +37,8 @@
         rabbit:
             branch: lp:~canonical-ci-engineering/charms/precise/ubuntu-ci-services-itself/rabbitmq-server@46
             charm: rabbitmq
+            options:
+                management_plugin: true
     relations:
         - [bsb-restish, bsb-gunicorn]
         - [bsb-worker, rabbit]

=== modified file 'juju-deployer/image-builder.yaml.tmpl'
--- juju-deployer/image-builder.yaml.tmpl	2014-03-06 22:18:04 +0000
+++ juju-deployer/image-builder.yaml.tmpl	2014-03-10 01:19:47 +0000
@@ -38,6 +38,8 @@
         rabbit:
             branch: lp:~canonical-ci-engineering/charms/precise/ubuntu-ci-services-itself/rabbitmq-server@46
             charm: rabbitmq
+            options:
+                management_plugin: true
     relations:
         - ["imagebuild-restish:wsgi", "imagebuild-gunicorn:wsgi-file"]
         - ["imagebuild-worker:amqp", "rabbit:amqp"]

=== modified file 'juju-deployer/test-runner.yaml.tmpl'
--- juju-deployer/test-runner.yaml.tmpl	2014-03-06 14:13:05 +0000
+++ juju-deployer/test-runner.yaml.tmpl	2014-03-10 01:19:47 +0000
@@ -45,6 +45,8 @@
         rabbit:
             branch: lp:~canonical-ci-engineering/charms/precise/ubuntu-ci-services-itself/rabbitmq-server@46
             charm: rabbitmq
+            options:
+                management_plugin: true
     relations:
         # Relations should be explicit as amulet can't infer them otherwise
         # even if there is a single one

=== modified file 'test_runner/tstrun/resources/v1.py'
--- test_runner/tstrun/resources/v1.py	2014-03-04 22:48:57 +0000
+++ test_runner/tstrun/resources/v1.py	2014-03-10 01:19:47 +0000
@@ -34,6 +34,7 @@
 def _status():
     status = json_status.JSONStatus()
     status.add_rabbit_configured()
+    status.add_rabbit_worker_health(tstrun.internal_rabbit_queue)
     return restish_utils.json_ok(status.results)