Merge lp:~johnsca/charms/trusty/cloudfoundry/better-basic-reconciler-status into lp:~cf-charmers/charms/trusty/cloudfoundry/trunk

[Cory Johns]

Since the monit status is so unreliable, switch the basic health check to use the self-reported Juju status until we have something that more deeply introspects.

[Cory Johns (johnsca)]

Basically, relies on the extended hookenv.juju_status() report (which accounts for data_ready), if available, and the basic Juju status if not.

Should give us a much more accurate approximation of when the system is up, although there may still be a slight delay (due to, e.g., `monit restart all` taking some time).

[Benjamin Saller (bcsaller)]

This looks like a good intermediate step. Thanks for doing this.

review: Approve

[Whit Morriss (whitmo)]

This look good to me.

I think the next step might be for the service framework to leave some breadcrumbs ala the status spec. Basically a list of incomplete relationships from the perspective of the service definition.

review: Approve

[lp:~johnsca/charms/trusty/cloudfoundry/better-basic-reconciler-status]

164. By Cory Johns on 2014-11-20

Only log "Incomplete requirement" for reqs that are blocking

[Cory Johns (johnsca)]

Whit,

The extended status is stored in $CHARM_DIR/juju_status.json on each unit, and is retrieved as part of the health hook that the reconciler calls. It might be nice for this to be pushed up to the reconciler instead of pulled, but I don't think it's all that urgent.

[Benjamin Saller (bcsaller)]

still LGTM, thanks :)

review: Approve

=== modified file 'charmhelpers/core/services/base.py'
--- charmhelpers/core/services/base.py	2014-10-31 16:11:51 +0000
+++ charmhelpers/core/services/base.py	2014-11-20 17:56:06 +0000
@@ -297,6 +297,8 @@
                     if getattr(req, 'manual', False):
                         manual = True
                     if hasattr(req, 'name'):
+                        hookenv.log('Incomplete requirement: %s' % req.name,
+                                    hookenv.DEBUG)
                         blockers.append(req.name)
         hookenv.juju_status(status, manual=manual, blockers=blockers)
 

=== modified file 'charmhelpers/core/services/helpers.py'
--- charmhelpers/core/services/helpers.py	2014-10-31 13:20:03 +0000
+++ charmhelpers/core/services/helpers.py	2014-11-20 17:56:06 +0000
@@ -45,10 +45,7 @@
         """
         Returns True if all of the `required_keys` are available from any units.
         """
-        ready = len(self.get(self.name, [])) > 0
-        if not ready:
-            hookenv.log('Incomplete relation: {}'.format(self.__class__.__name__), hookenv.DEBUG)
-        return ready
+        return len(self.get(self.name, [])) > 0
 
     def _is_ready(self, unit_data):
         """

=== modified file 'cloudfoundry/health_checks.py'
--- cloudfoundry/health_checks.py	2014-08-24 21:36:50 +0000
+++ cloudfoundry/health_checks.py	2014-11-20 17:56:06 +0000
@@ -1,3 +1,4 @@
+from charmhelpers.core import hookenv
 from cloudfoundry import tasks
 
 
@@ -17,3 +18,21 @@
                     message='not all services running',
                     data={'services': summary})
     return result
+
+
+def status(service):
+    result = {
+        'name': 'monit_summary',
+        'health': 'pass',
+        'message': None,
+        'data': {},
+    }
+    status = hookenv.juju_status()
+    if status['status'] == 'error':
+        return dict(result, health='fail', message=status['message'])
+    elif status['status'] == 'blocked' and status['manual']:
+        return dict(result, health='fail', message='Blocked: %s' % status['blockers'])
+    elif status['status'] == 'up':
+        return result
+    else:
+        return dict(result, health='warn', message='Working (%s)' % status['status'])

=== modified file 'cloudfoundry/jobs.py'
--- cloudfoundry/jobs.py	2014-10-03 15:47:17 +0000
+++ cloudfoundry/jobs.py	2014-11-20 17:56:06 +0000
@@ -68,7 +68,7 @@
     service_def = service_data[charm_name]
     results = []
     health = 'pass'
-    checks = service_def.get('health', []) + [health_checks.monit_summary]
+    checks = service_def.get('health', []) + [health_checks.status]
     for health_check in checks:
         result = health_check(service_def)
         if result['health'] == 'fail':

=== modified file 'reconciler/app.py'
--- reconciler/app.py	2014-11-14 17:00:08 +0000
+++ reconciler/app.py	2014-11-20 17:56:06 +0000
@@ -83,13 +83,14 @@
         units = service.get('Units', {}) or {}
         for unit_name, unit in units.iteritems():
             unit_addr = unit.get('PublicAddress')
+            unit_state = unit.get('AgentState')
             if unit_addr:
                 loop = tornado.ioloop.IOLoop.instance()
                 loop.add_callback(check_health, service_name,
-                                  unit_name, unit_addr)
-
-
-def check_health(service_name, unit_name, unit_addr):
+                                  unit_name, unit_addr, unit_state)
+
+
+def check_health(service_name, unit_name, unit_addr, unit_state):
     service = health.setdefault(service_name, {
         'name': service_name,
         'health': 'unknown',
@@ -118,8 +119,13 @@
             unit['health'] = 'fail'
             unit['state'] = {'message': 'Unable to parse health: {}'.format(output)}
     except subprocess.CalledProcessError as e:
-        unit['health'] = 'warn'
         unit['state'] = {'message': 'Unable to retrieve health: {}'.format(e.output)}
+        if unit_state == 'started':
+            unit['health'] = 'pass'
+        elif unit_state == 'error':
+            unit['health'] = 'fail'
+        else:
+            unit['health'] = 'warn'
 
     units_fail = [u['health'] == 'fail' for u in service['units'].values()]
     units_not_pass = [u['health'] != 'pass' for u in service['units'].values()]