Merge lp:~james-page/charms/trusty/rabbitmq-server/status-check into lp:~openstack-charmers-archive/charms/trusty/rabbitmq-server/next

[James Page]

Add basic use of status feature in Juju

This includes:

1) Evaluation of current rabbitmq state on the server using 'status'
2) Evaluation of cluster status once peers appear on the cluster relation
3) status set when installing packages and when configuring mirroring (long ops)

I also found a race in leader-settings-changed - it can fire prior to installation of rabbitmq in config-changed, so added a basic defer until installed check.

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #11153 rabbitmq-server-next for james-page mp273037
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11153/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10358 rabbitmq-server-next for james-page mp273037
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10358/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6934 rabbitmq-server-next for james-page mp273037
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 124
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12634394/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6934/

[David Ames (thedac)]

I really like the workgroup status approach in this MP.

I also like your solution to https://bugs.launchpad.net/charms/+source/rabbitmq-server/+bug/1501048 better than mine.
mkdir is imported in hooks/rabbitmq_server_relations.py but is never used. I suspect you used os.path.exists instead.

I also really like breaking out clustered() as a cached function on its own. However, one of the bugs I am fighting (https://bugs.launchpad.net/charms/+source/rabbitmq-server/+bug/1500204) is a direct result of checking for more than one running node.

if len(running_nodes()) > 1:

Consider, the 3rd, 4th and 5th nodes to attempt clustering. There will already by 2+ running nodes and they will assume incorrectly they are already clustered.

In my testing I have removed this check entirely with some success. I would be interested in finding a more robust clustered check.

I plan to merge your MP into my branch and do some more testing and see if I can come up with a clustered check that works in all cases.

review: Needs Fixing

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6937 rabbitmq-server-next for james-page mp273037
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
2015-10-01 21:19:58,241 publish_amqp_message_by_unit DEBUG: Publishing message to test queue:
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12634746/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6937/

[David Ames (thedac)]

> However, one of the bugs I am fighting
> (https://bugs.launchpad.net/charms/+source/rabbitmq-server/+bug/1500204) is a
> direct result of checking for more than one running node.
>
> if len(running_nodes()) > 1:
>
> Consider, the 3rd, 4th and 5th nodes to attempt clustering. There will already
> by 2+ running nodes and they will assume incorrectly they are already
> clustered.

Ignore this. I see what it is supposed to do. As the local unit will only ever see 1 if not clustered or more if clustered.

> I plan to merge your MP into my branch and do some more testing and see if I
> can come up with a clustered check that works in all cases.

I am testing a combination of my MP which ignores min-cluster-size when leadership election is available and your MP now. I'll see if any more race conditions reveal themselves.

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6938 rabbitmq-server-next for james-page mp273037
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12635100/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6938/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #11156 rabbitmq-server-next for james-page mp273037
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11156/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10364 rabbitmq-server-next for james-page mp273037
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10364/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6970 rabbitmq-server-next for james-page mp273037
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12638811/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6970/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #11169 rabbitmq-server-next for james-page mp273037
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11169/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10372 rabbitmq-server-next for james-page mp273037
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10372/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6992 rabbitmq-server-next for james-page mp273037
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 124
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12640571/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6992/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #7002 rabbitmq-server-next for james-page mp273037
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12641390/
Build: http://10.245.162.77:8080/job/charm_amulet_test/7002/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #7009 rabbitmq-server-next for james-page mp273037
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12642068/
Build: http://10.245.162.77:8080/job/charm_amulet_test/7009/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #7025 rabbitmq-server-next for james-page mp273037
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12644560/
Build: http://10.245.162.77:8080/job/charm_amulet_test/7025/

[James Page (james-page)]

review: Needs Resubmitting

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10521 rabbitmq-server-next for james-page mp273037
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10521/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #11327 rabbitmq-server-next for james-page mp273037
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11327/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #7112 rabbitmq-server-next for james-page mp273037
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/7112/

[David Ames (thedac)]

Apporved

review: Approve

=== modified file 'hooks/rabbit_utils.py'
--- hooks/rabbit_utils.py	2015-09-23 15:53:31 +0000
+++ hooks/rabbit_utils.py	2015-10-01 09:52:45 +0000
@@ -23,7 +23,9 @@
     related_units,
     log, ERROR,
     INFO,
-    service_name
+    service_name,
+    status_set,
+    cached
 )
 
 from charmhelpers.core.host import (
@@ -193,6 +195,11 @@
     subprocess.check_call(cmd)
 
 
+@cached
+def caching_cmp_pkgrevno(package, revno, pkgcache=None):
+    return cmp_pkgrevno(package, revno, pkgcache)
+
+
 def set_ha_mode(vhost, mode, params=None, sync_mode='automatic'):
     """Valid mode values:
 
@@ -212,7 +219,7 @@
                       http://www.rabbitmq.com./ha.html#eager-synchronisation
     """
 
-    if cmp_pkgrevno('rabbitmq-server', '3.0.0') < 0:
+    if caching_cmp_pkgrevno('rabbitmq-server', '3.0.0') < 0:
         log(("Mirroring queues cannot be enabled, only supported "
              "in rabbitmq-server >= 3.0"), level='WARN')
         log(("More information at http://www.rabbitmq.com/blog/"
@@ -266,6 +273,11 @@
              "2012/11/19/breaking-things-with-rabbitmq-3-0"), level='INFO')
         return
 
+    if enable:
+        status_set('active', 'Enabling queue mirroring')
+    else:
+        status_set('active', 'Disabling queue mirroring')
+
     for vhost in list_vhosts():
         if enable:
             set_ha_mode(vhost, 'all')
@@ -284,19 +296,8 @@
         cluster_cmd = 'join_cluster'
     else:
         cluster_cmd = 'cluster'
-    out = subprocess.check_output([RABBITMQ_CTL, 'cluster_status'])
-    log('cluster status is %s' % str(out))
-
-    # check if node is already clustered
-    total_nodes = 1
-    running_nodes = []
-    m = re.search("\{running_nodes,\[(.*?)\]\}", out.strip(), re.DOTALL)
-    if m is not None:
-        running_nodes = m.group(1).split(',')
-        running_nodes = [x.replace("'", '') for x in running_nodes]
-        total_nodes = len(running_nodes)
-
-    if total_nodes > 1:
+
+    if clustered():
         log('Node is already clustered, skipping')
         return False
 
@@ -324,10 +325,11 @@
         return False
 
     # iterate over all the nodes, join to the first available
+    active_nodes = running_nodes()
     num_tries = 0
     for node in available_nodes:
         log('Clustering with remote rabbit host (%s).' % node)
-        if node in running_nodes:
+        if node in active_nodes:
             log('Host already clustered with %s.' % node)
             return False
 
@@ -600,3 +602,52 @@
     for v in restart_map().values():
         _services = _services + v
     return list(set(_services))
+
+
+@cached
+def running_nodes():
+    ''' Determine the current set of running rabbitmq-units in the cluster '''
+    out = subprocess.check_output([RABBITMQ_CTL, 'cluster_status'])
+
+    running_nodes = []
+    m = re.search("\{running_nodes,\[(.*?)\]\}", out.strip(), re.DOTALL)
+    if m is not None:
+        running_nodes = m.group(1).split(',')
+        running_nodes = [x.replace("'", '').strip() for x in running_nodes]
+
+    return running_nodes
+
+
+@cached
+def clustered():
+    ''' Determine whether local rabbitmq-server is clustered '''
+    if len(running_nodes()) > 1:
+        return True
+    else:
+        return False
+
+
+def assess_status():
+    ''' Assess the status for the current running unit '''
+    # NOTE: ensure rabbitmq is actually installed before doing
+    #       any checks
+    if os.path.exists(RABBITMQ_CTL):
+        # Clustering Check
+        peer_ids = relation_ids('cluster')
+        if peer_ids and len(related_units(peer_ids[0])):
+            if not clustered():
+                status_set('waiting',
+                           'Unit has peers, but RabbitMQ not clustered')
+                return
+        # General status check
+        status_cmd = ['rabbitmqctl', 'status']
+        ret = subprocess.call(status_cmd)
+        if ret > 0:
+            status_set('blocked', 'RabbitMQ server is not running')
+        else:
+            if clustered():
+                status_set('active', 'Unit is ready and clustered')
+            else:
+                status_set('active', 'Unit is ready')
+    else:
+        status_set('waiting', 'RabbitMQ is not yet installed')

=== modified file 'hooks/rabbitmq_server_relations.py'
--- hooks/rabbitmq_server_relations.py	2015-09-23 13:16:23 +0000
+++ hooks/rabbitmq_server_relations.py	2015-10-01 09:52:45 +0000
@@ -65,6 +65,7 @@
     UnregisteredHookError,
     is_leader,
     charm_dir,
+    status_set,
 )
 from charmhelpers.core.host import (
     cmp_pkgrevno,
@@ -73,6 +74,7 @@
     service_stop,
     service_restart,
     write_file,
+    mkdir,
 )
 from charmhelpers.contrib.charmsupport import nrpe
 
@@ -644,6 +646,7 @@
         '/etc/default/rabbitmq-server')
     # Install packages to ensure any changes to source
     # result in an upgrade if applicable.
+    status_set('maintenance', 'Installing/upgrading RabbitMQ packages')
     apt_install(rabbit.PACKAGES, fatal=True)
 
     open_port(5672)
@@ -688,6 +691,9 @@
 
 @hooks.hook('leader-settings-changed')
 def leader_settings_changed():
+    if not os.path.exists(rabbit.RABBITMQ_CTL):
+        log('Deferring cookie configuration, RabbitMQ not yet installed')
+        return
     # Get cookie from leader, update cookie locally and
     # force cluster-relation-changed hooks to run on peers
     cookie = leader_get(attribute='cookie')
@@ -716,5 +722,6 @@
 if __name__ == '__main__':
     try:
         hooks.execute(sys.argv)
+        rabbit.assess_status()
     except UnregisteredHookError as e:
         log('Unknown hook {} - skipping.'.format(e))

=== modified file 'unit_tests/test_rabbit_utils.py'
--- unit_tests/test_rabbit_utils.py	2015-09-24 21:49:44 +0000
+++ unit_tests/test_rabbit_utils.py	2015-10-01 09:52:45 +0000
@@ -4,8 +4,19 @@
 import tempfile
 import sys
 import collections
-
-import rabbit_utils
+from functools import wraps
+
+
+with mock.patch('charmhelpers.core.hookenv.cached') as cached:
+    def passthrough(func):
+        @wraps(func)
+        def wrapper(*args, **kwargs):
+            return func(*args, **kwargs)
+        wrapper._wrapped = func
+        return wrapper
+    cached.side_effect = passthrough
+    import rabbit_utils
+
 sys.modules['MySQLdb'] = mock.Mock()
 
 
@@ -41,6 +52,23 @@
         self.assertTrue(log.called)
 
 
+RABBITMQCTL_CLUSTERSTATUS_RUNNING = """Cluster status of node 'rabbit@juju-devel3-machine-19' ...
+[{nodes,[{disc,['rabbit@juju-devel3-machine-14',
+                'rabbit@juju-devel3-machine-19']}]},
+ {running_nodes,['rabbit@juju-devel3-machine-14',
+                 'rabbit@juju-devel3-machine-19']},
+ {cluster_name,<<"[e-mail]">>},
+ {partitions,[]}]
+ """
+
+RABBITMQCTL_CLUSTERSTATUS_SOLO = """Cluster status of node 'rabbit@juju-devel3-machine-14' ...
+[{nodes,[{disc,['rabbit@juju-devel3-machine-14']}]},
+ {running_nodes,['rabbit@juju-devel3-machine-14']},
+ {cluster_name,<<"[e-mail]">>},
+ {partitions,[]}]
+ """
+
+
 class UtilsTests(unittest.TestCase):
     def setUp(self):
         super(UtilsTests, self).setUp()
@@ -102,3 +130,30 @@
         self.assertEqual(lines[0], "#somedata\n")
         self.assertEqual(lines[1], "%s %s\n" % (map.items()[0]))
         self.assertEqual(lines[4], "%s %s\n" % (map.items()[3]))
+
+    @mock.patch('rabbit_utils.running_nodes')
+    def test_not_clustered(self, mock_running_nodes):
+        mock_running_nodes.return_value = []
+        self.assertFalse(rabbit_utils.clustered())
+
+    @mock.patch('rabbit_utils.running_nodes')
+    def test_clustered(self, mock_running_nodes):
+        mock_running_nodes.return_value = ['a', 'b']
+        self.assertTrue(rabbit_utils.clustered())
+
+    @mock.patch('rabbit_utils.subprocess')
+    def test_running_nodes(self, mock_subprocess):
+        '''Ensure cluster_status can be parsed for a clustered deployment'''
+        mock_subprocess.check_output.return_value = \
+            RABBITMQCTL_CLUSTERSTATUS_RUNNING
+        self.assertEqual(rabbit_utils.running_nodes(),
+                         ['rabbit@juju-devel3-machine-14',
+                          'rabbit@juju-devel3-machine-19'])
+
+    @mock.patch('rabbit_utils.subprocess')
+    def test_running_nodes_solo(self, mock_subprocess):
+        '''Ensure cluster_status can be parsed for a single unit deployment'''
+        mock_subprocess.check_output.return_value = \
+            RABBITMQCTL_CLUSTERSTATUS_SOLO
+        self.assertEqual(rabbit_utils.running_nodes(),
+                         ['rabbit@juju-devel3-machine-14'])