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

Proposed by Paul Gear on 2016-07-20
Status: Merged
Approved by: Nick Moffitt on 2016-07-20
Approved revision: 48
Merged at revision: 41
Proposed branch: lp:~paulgear/charms/trusty/grafana/layer-grafana-reactive-states-fix
Merge into: lp:~canonical-is-sa/charms/trusty/grafana/layer-grafana
Diff against target: 228 lines (+74/-33)
3 files modified
README.md (+2/-2)
layer.yaml (+1/-0)
reactive/grafana.py (+71/-31)
To merge this branch: bzr merge lp:~paulgear/charms/trusty/grafana/layer-grafana-reactive-states-fix
Reviewer Review Type Date Requested Status
Stuart Bishop Approve on 2016-07-21
Nick Moffitt 2016-07-20 Approve on 2016-07-20
Review via email: mp+300547@code.launchpad.net
To post a comment you must log in.
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) wrote :

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
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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README.md'
2--- README.md 2016-05-13 09:43:01 +0000
3+++ README.md 2016-07-21 02:47:52 +0000
4@@ -1,6 +1,6 @@
5 #Overview
6
7-This charm provides the latest version of Grafana.
8+This charm provides the latest stable version of Grafana.
9
10 #Usage
11
12@@ -9,7 +9,7 @@
13
14 Above will automatcially configure prometheus as grafana datasource
15
16-If admin password is not set using configuration option it is autogenated.
17+If admin password is not set using configuration option it is autogenerated.
18 To retried autogenerated password run:
19 juju run --service grafana "scripts/get_admin_password"
20
21
22=== modified file 'layer.yaml'
23--- layer.yaml 2016-04-29 22:11:08 +0000
24+++ layer.yaml 2016-07-21 02:47:52 +0000
25@@ -1,2 +1,3 @@
26 includes: ['layer:basic', 'interface:nrpe-external-master', 'interface:grafana-source', 'interface:http']
27 ignore: ['.*.swp' ]
28+repo: bzr+ssh://bazaar.launchpad.net/~canonical-is-sa/charms/trusty/grafana/layer-grafana/
29
30=== modified file 'reactive/grafana.py'
31--- reactive/grafana.py 2016-07-14 15:07:36 +0000
32+++ reactive/grafana.py 2016-07-21 02:47:52 +0000
33@@ -7,12 +7,11 @@
34 import json
35 import subprocess
36 import base64
37-from time import sleep
38 from charmhelpers import fetch
39 from charmhelpers.core import host, hookenv, unitdata
40 from charmhelpers.core.templating import render
41 from charmhelpers.contrib.charmsupport import nrpe
42-from charms.reactive import when, when_not, set_state, only_once
43+from charms.reactive import hook, remove_state, set_state, when, when_not
44 from charms.reactive.helpers import any_file_changed, data_changed
45
46
47@@ -39,9 +38,11 @@
48 DASHBOARDS_BACKUP_CRON_TMPL = 'juju-dashboards-backup.j2'
49
50
51+@when_not('grafana.installed')
52 def install_packages():
53 config = hookenv.config()
54 install_opts = ('install_sources', 'install_keys')
55+ hookenv.status_set('maintenance', 'Installing grafana')
56 if config.changed('install_file') and config.get('install_file', False):
57 hookenv.status_set('maintenance', 'Installing deb pkgs')
58 fetch.apt_install(GRAFANA_DEPS)
59@@ -56,7 +57,25 @@
60 packages = ['grafana']
61 fetch.configure_sources(update=True)
62 fetch.apt_install(packages)
63- hookenv.status_set('maintenance', 'Waiting for start')
64+ set_state('grafana.installed')
65+ hookenv.status_set('active', 'Completed installing grafana')
66+
67+
68+@hook('upgrade-charm')
69+def upgrade_charm():
70+ hookenv.status_set('maintenance', 'Forcing package update and reconfiguration on upgrade-charm')
71+ remove_state('grafana.installed')
72+ remove_state('grafana.configured')
73+
74+
75+@hook('config.changed')
76+def config_changed():
77+ remove_state('grafana.configured')
78+ config = hookenv.config()
79+ if config.changed('dashboards_backup_schedule') or config.changed('dashboards_backup_dir'):
80+ remove_state('grafana.backup.configured')
81+ if config.changed('admin_password') or config.changed('nagios_context'):
82+ remove_state('grafana.admin_password.set')
83
84
85 def check_ports(new_port):
86@@ -97,8 +116,8 @@
87 if select_query('SELECT id FROM api_key WHERE org_id=? AND name=?', [org_id, name]):
88 hookenv.log('API key {} in org {} already exists, skipping'.format(name, org_id))
89 continue
90- j = {'n': name,
91- 'k': passwd,
92+ j = {'n': name,
93+ 'k': passwd,
94 'id': org_id}
95 encoded = base64.b64encode(json.dumps(j).encode('ascii')).decode('ascii')
96 stmt = 'INSERT INTO api_key (org_id, name, key, role, created, updated)' + \
97@@ -119,10 +138,10 @@
98 return kv.get('grafana.dashboards_backup_keys')
99
100
101-@when_not('grafana.started')
102+@when('grafana.installed')
103+@when_not('grafana.configured')
104 def setup_grafana():
105 hookenv.status_set('maintenance', 'Configuring grafana')
106- install_packages()
107 config = hookenv.config()
108 settings = {'config': config}
109 render(source=GRAFANA_INI_TMPL,
110@@ -131,7 +150,23 @@
111 owner='root', group='grafana',
112 perms=0o640,
113 )
114+ check_ports(config.get('port'))
115+ set_state('grafana.configured')
116+ remove_state('grafana.started')
117+ hookenv.status_set('active', 'Completed configuring grafana')
118+
119+
120+@when('grafana.started')
121+@when_not('grafana.backup.configured')
122+def setup_backup_shedule():
123+ # copy script, create cronjob, ensure directory exists
124+
125+ # XXX: If you add any dependencies on config items here,
126+ # be sure to update config_changed() accordingly!
127+
128+ config = hookenv.config()
129 if config.get('dashboards_backup_schedule', False):
130+ hookenv.status_set('maintenance', 'Configuring grafana dashboard backup')
131 hookenv.log('Setting up dashboards backup job...')
132 host.rsync('files/dashboards_backup', '/usr/local/bin/dashboards_backup')
133 host.mkdir(config.get('dashboards_backup_dir'))
134@@ -144,37 +179,29 @@
135 owner='root', group='root',
136 perms=0o640,
137 )
138- # copy script, create cronjob, ensure directory exists
139- check_ports(config.get('port'))
140- set_state('grafana.start')
141- hookenv.status_set('active', 'Ready')
142-
143-
144-@when('grafana.started')
145-def check_config():
146- if data_changed('grafana.config', hookenv.config()):
147- setup_grafana() # reconfigure and restart
148- db_init()
149-
150-
151-@when('grafana.start')
152+ hookenv.status_set('active', 'Completed configuring grafana dashboard backup')
153+ set_state('grafana.backup.configured')
154+
155+
156+@when('grafana.configured')
157+@when_not('grafana.started')
158 def restart_grafana():
159 if not host.service_running(SVCNAME):
160- hookenv.log('Starting {}...'.format(SVCNAME))
161+ msg = 'Starting {}'.format(SVCNAME)
162+ hookenv.status_set('maintenance', msg)
163+ hookenv.log(msg)
164 host.service_start(SVCNAME)
165 elif any_file_changed([GRAFANA_INI]):
166- hookenv.log('Restarting {}, config file changed...'.format(SVCNAME))
167+ msg = 'Restarting {}'.format(SVCNAME)
168+ hookenv.log(msg)
169+ hookenv.status_set('maintenance', msg)
170 host.service_restart(SVCNAME)
171 hookenv.status_set('active', 'Ready')
172 set_state('grafana.started')
173-
174-
175-@only_once
176-def db_init():
177- sleep(10)
178- check_adminuser()
179-
180-
181+ hookenv.status_set('active', 'Started {}'.format(SVCNAME))
182+
183+
184+@when('grafana.started')
185 @when('nrpe-external-master.available')
186 def update_nrpe_config(svc):
187 # python-dbus is used by check_upstart_job
188@@ -185,6 +212,12 @@
189 nrpe.add_init_service_checks(nrpe_setup, SVCNAME, current_unit)
190 nrpe_setup.write()
191
192+ # XXX: Update this when https://code.launchpad.net/~paulgear/charm-helpers/nrpe-service-immediate-check/+merge/300682 is merged.
193+ output = open('/var/lib/nagios/service-check-grafana.txt', 'w')
194+ cmd = '/usr/local/lib/nagios/plugins/check_exit_status.pl -s /etc/init.d/grafana'
195+ ret = subprocess.call(cmd.split(), stdout=output, stderr=subprocess.STDOUT)
196+ hookenv.log('{} returned {}'.format(cmd, ret))
197+
198
199 @when_not('nrpe-external-master.available')
200 def wipe_nrpe_checks():
201@@ -326,6 +359,8 @@
202 return (stmt, values)
203
204
205+@when('grafana.started')
206+@when_not('grafana.admin_password.set')
207 def check_adminuser():
208 """
209 CREATE TABLE `user` (
210@@ -347,6 +382,10 @@
211 );
212 INSERT INTO "user" VALUES(1,0,'admin','root+bootstack-ps45@canonical.com','BootStack Team','309bc4e78bc60d02dc0371d9e9fa6bf9a809d5dc25c745b9e3f85c3ed49c6feccd4ffc96d1db922f4297663a209e93f7f2b6','LZeJ3nSdrC','hseJcLcnPN','',1,1,0,'light','2016-01-22 12:00:08','2016-01-22 12:02:13');
213 """
214+
215+ # XXX: If you add any dependencies on config items here,
216+ # be sure to update config_changed() accordingly!
217+
218 config = hookenv.config()
219 passwd = config.get('admin_password', False)
220 if not passwd:
221@@ -380,6 +419,7 @@
222 except sqlite3.OperationalError as e:
223 hookenv.log('check_adminuser::sqlite3.OperationError: {}'.format(e))
224 return
225+ set_state('grafana.admin_password.set')
226
227
228 def hpwgen(passwd, salt):

Subscribers

People subscribed via source and target branches

to all changes: