Merge lp:~paulgear/charms/trusty/grafana/layer-grafana-reactive-states-fix into lp:~canonical-is-sa/charms/trusty/grafana/layer-grafana

[Paul Gear]

[lp:~paulgear/charms/trusty/grafana/layer-grafana-reactive-states-fix]

43. By Paul Gear on 2016-07-20

Latest stable

44. By Paul Gear on 2016-07-20

Add repo to keep charm build happy

45. By Paul Gear on 2016-07-20

Revert repo change; contra https://packagecloud.io/grafana/stable/install#manual, trusty does not exist

46. By Paul Gear on 2016-07-20

Add upgrade charm hook

47. By Paul Gear on 2016-07-20

Prevent race in calling add_backup_api_keys()

48. By Paul Gear on 2016-07-20

Clarify reason for commented-out state

[Nick Moffitt (nick-moffitt)]

I've been thinking out loud a lot about this merge while in the office, and it boils down to basically one thing that's sorely needed when using the reactive model:

  Locking semantics.

The (now deprecated, I hope) only_once decorator is the most obvious example of this. Unfortunately the implementation doesn't fit our needs, but the desire it expresses is a good one. What you've gone through and done for a lot of this is to add "When not Y" to a lot of "When X" conditions. Again, this ensures that the non-idempotent steps are only run once.

I have always fought for idempotent state-enforcing changes, but accept that those aren't always possible. I'm not sure what the Right Thing is, but this change is definitely not Wrong!

review: Approve

[lp:~paulgear/charms/trusty/grafana/layer-grafana-reactive-states-fix]

49. By Paul Gear on 2016-07-21

Update nagios service check per charm helpers MP

50. By Paul Gear on 2016-07-21

Run init script directly

[Stuart Bishop (stub)]

review: Approve

=== modified file 'README.md'
--- README.md	2016-05-13 09:43:01 +0000
+++ README.md	2016-07-21 02:47:52 +0000
@@ -1,6 +1,6 @@
 #Overview
 
-This charm provides the latest version of Grafana.
+This charm provides the latest stable version of Grafana.
 
 #Usage
 
@@ -9,7 +9,7 @@
 
 Above will automatcially configure prometheus as grafana datasource
 
-If admin password is not set using configuration option it is autogenated.
+If admin password is not set using configuration option it is autogenerated.
 To retried autogenerated password run:
     juju run --service grafana "scripts/get_admin_password"
 

=== modified file 'layer.yaml'
--- layer.yaml	2016-04-29 22:11:08 +0000
+++ layer.yaml	2016-07-21 02:47:52 +0000
@@ -1,2 +1,3 @@
 includes: ['layer:basic', 'interface:nrpe-external-master', 'interface:grafana-source', 'interface:http']
 ignore: ['.*.swp' ]
+repo: bzr+ssh://bazaar.launchpad.net/~canonical-is-sa/charms/trusty/grafana/layer-grafana/

=== modified file 'reactive/grafana.py'
--- reactive/grafana.py	2016-07-14 15:07:36 +0000
+++ reactive/grafana.py	2016-07-21 02:47:52 +0000
@@ -7,12 +7,11 @@
 import json
 import subprocess
 import base64
-from time import sleep
 from charmhelpers import fetch
 from charmhelpers.core import host, hookenv, unitdata
 from charmhelpers.core.templating import render
 from charmhelpers.contrib.charmsupport import nrpe
-from charms.reactive import when, when_not, set_state, only_once
+from charms.reactive import hook, remove_state, set_state, when, when_not
 from charms.reactive.helpers import any_file_changed, data_changed
 
 
@@ -39,9 +38,11 @@
 DASHBOARDS_BACKUP_CRON_TMPL = 'juju-dashboards-backup.j2'
 
 
+@when_not('grafana.installed')
 def install_packages():
     config = hookenv.config()
     install_opts = ('install_sources', 'install_keys')
+    hookenv.status_set('maintenance', 'Installing grafana')
     if config.changed('install_file') and config.get('install_file', False):
         hookenv.status_set('maintenance', 'Installing deb pkgs')
         fetch.apt_install(GRAFANA_DEPS)
@@ -56,7 +57,25 @@
         packages = ['grafana']
         fetch.configure_sources(update=True)
         fetch.apt_install(packages)
-    hookenv.status_set('maintenance', 'Waiting for start')
+    set_state('grafana.installed')
+    hookenv.status_set('active', 'Completed installing grafana')
+
+
+@hook('upgrade-charm')
+def upgrade_charm():
+    hookenv.status_set('maintenance', 'Forcing package update and reconfiguration on upgrade-charm')
+    remove_state('grafana.installed')
+    remove_state('grafana.configured')
+
+
+@hook('config.changed')
+def config_changed():
+    remove_state('grafana.configured')
+    config = hookenv.config()
+    if config.changed('dashboards_backup_schedule') or config.changed('dashboards_backup_dir'):
+        remove_state('grafana.backup.configured')
+    if config.changed('admin_password') or config.changed('nagios_context'):
+        remove_state('grafana.admin_password.set')
 
 
 def check_ports(new_port):
@@ -97,8 +116,8 @@
         if select_query('SELECT id FROM api_key WHERE org_id=? AND name=?', [org_id, name]):
             hookenv.log('API key {} in org {} already exists, skipping'.format(name, org_id))
             continue
-        j = {'n':  name,
-             'k':  passwd,
+        j = {'n': name,
+             'k': passwd,
              'id': org_id}
         encoded = base64.b64encode(json.dumps(j).encode('ascii')).decode('ascii')
         stmt = 'INSERT INTO api_key (org_id, name, key, role, created, updated)' + \
@@ -119,10 +138,10 @@
     return kv.get('grafana.dashboards_backup_keys')
 
 
-@when_not('grafana.started')
+@when('grafana.installed')
+@when_not('grafana.configured')
 def setup_grafana():
     hookenv.status_set('maintenance', 'Configuring grafana')
-    install_packages()
     config = hookenv.config()
     settings = {'config': config}
     render(source=GRAFANA_INI_TMPL,
@@ -131,7 +150,23 @@
            owner='root', group='grafana',
            perms=0o640,
            )
+    check_ports(config.get('port'))
+    set_state('grafana.configured')
+    remove_state('grafana.started')
+    hookenv.status_set('active', 'Completed configuring grafana')
+
+
+@when('grafana.started')
+@when_not('grafana.backup.configured')
+def setup_backup_shedule():
+    # copy script, create cronjob, ensure directory exists
+
+    # XXX: If you add any dependencies on config items here,
+    #      be sure to update config_changed() accordingly!
+
+    config = hookenv.config()
     if config.get('dashboards_backup_schedule', False):
+        hookenv.status_set('maintenance', 'Configuring grafana dashboard backup')
         hookenv.log('Setting up dashboards backup job...')
         host.rsync('files/dashboards_backup', '/usr/local/bin/dashboards_backup')
         host.mkdir(config.get('dashboards_backup_dir'))
@@ -144,37 +179,29 @@
                owner='root', group='root',
                perms=0o640,
                )
-        # copy script, create cronjob, ensure directory exists
-    check_ports(config.get('port'))
-    set_state('grafana.start')
-    hookenv.status_set('active', 'Ready')
-
-
-@when('grafana.started')
-def check_config():
-    if data_changed('grafana.config', hookenv.config()):
-        setup_grafana()  # reconfigure and restart
-    db_init()
-
-
-@when('grafana.start')
+        hookenv.status_set('active', 'Completed configuring grafana dashboard backup')
+    set_state('grafana.backup.configured')
+
+
+@when('grafana.configured')
+@when_not('grafana.started')
 def restart_grafana():
     if not host.service_running(SVCNAME):
-        hookenv.log('Starting {}...'.format(SVCNAME))
+        msg = 'Starting {}'.format(SVCNAME)
+        hookenv.status_set('maintenance', msg)
+        hookenv.log(msg)
         host.service_start(SVCNAME)
     elif any_file_changed([GRAFANA_INI]):
-        hookenv.log('Restarting {}, config file changed...'.format(SVCNAME))
+        msg = 'Restarting {}'.format(SVCNAME)
+        hookenv.log(msg)
+        hookenv.status_set('maintenance', msg)
         host.service_restart(SVCNAME)
     hookenv.status_set('active', 'Ready')
     set_state('grafana.started')
-
-
-@only_once
-def db_init():
-    sleep(10)
-    check_adminuser()
-
-
+    hookenv.status_set('active', 'Started {}'.format(SVCNAME))
+
+
+@when('grafana.started')
 @when('nrpe-external-master.available')
 def update_nrpe_config(svc):
     # python-dbus is used by check_upstart_job
@@ -185,6 +212,12 @@
     nrpe.add_init_service_checks(nrpe_setup, SVCNAME, current_unit)
     nrpe_setup.write()
 
+    # XXX: Update this when https://code.launchpad.net/~paulgear/charm-helpers/nrpe-service-immediate-check/+merge/300682 is merged.
+    output = open('/var/lib/nagios/service-check-grafana.txt', 'w')
+    cmd = '/usr/local/lib/nagios/plugins/check_exit_status.pl -s /etc/init.d/grafana'
+    ret = subprocess.call(cmd.split(), stdout=output, stderr=subprocess.STDOUT)
+    hookenv.log('{} returned {}'.format(cmd, ret))
+
 
 @when_not('nrpe-external-master.available')
 def wipe_nrpe_checks():
@@ -326,6 +359,8 @@
     return (stmt, values)
 
 
+@when('grafana.started')
+@when_not('grafana.admin_password.set')
 def check_adminuser():
     """
     CREATE TABLE `user` (
@@ -347,6 +382,10 @@
     );
     INSERT INTO "user" VALUES(1,0,'admin','[e-mail]','BootStack Team','309bc4e78bc60d02dc0371d9e9fa6bf9a809d5dc25c745b9e3f85c3ed49c6feccd4ffc96d1db922f4297663a209e93f7f2b6','LZeJ3nSdrC','hseJcLcnPN','',1,1,0,'light','2016-01-22 12:00:08','2016-01-22 12:02:13');
     """
+
+    # XXX: If you add any dependencies on config items here,
+    #      be sure to update config_changed() accordingly!
+
     config = hookenv.config()
     passwd = config.get('admin_password', False)
     if not passwd:
@@ -380,6 +419,7 @@
     except sqlite3.OperationalError as e:
         hookenv.log('check_adminuser::sqlite3.OperationError: {}'.format(e))
         return
+    set_state('grafana.admin_password.set')
 
 
 def hpwgen(passwd, salt):