Merge ~paulgear/charm-openstack-service-checks:master into ~canonical-bootstack/charm-openstack-service-checks:master

[Paul Gear]

Correct state management so that install isn't called on every hook

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Stuart Bishop (stub)]

Mostly fine, but one bug commented on inline (the restart handler can run much too early).

triggers is an alternative approach to some of this, but docs recommend things the way you have (and I can argue it both ways). https://charmsreactive.readthedocs.io/en/latest/triggers.html#coupling-flags-with-triggers

review: Approve

[Stuart Bishop (stub)]

yup

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 4c0cf281240f1a87278e2b8846f7ea352bb90997

diff --git a/reactive/service_checks.py b/reactive/service_checks.py
index 3081354..8341324 100644
--- a/reactive/service_checks.py
+++ b/reactive/service_checks.py
@@ -29,17 +29,12 @@ install_packages = ['nagios-nrpe-server', 'python-openstackclient']
 
 
 @when_not('os-service-checks.installed')
-def set_install_service_checks():
-    set_state('os-service-checks.do-install')
-
-
-@when('os-service-checks.do-install')
 def install_service_checks():
     hookenv.status_set('maintenance', 'Installing software')
     apt_update()
     apt_install(install_packages)
     set_state('os-service-checks.installed')
-    set_state('os-service-checks.do-check-reconfig')
+    remove_state('os-service-checks.configured')
     hookenv.status_set('active', 'Ready')
 # setup openstack user
 
@@ -54,7 +49,7 @@ def configure_keystone_username(keystone):
 def save_creds(keystone):
     creds = get_creds(keystone)
     unitdata.kv().set('keystone-relation-creds', creds)
-    set_state('os-service-checks.do-reconfig')
+    remove_state('os-service-checks.configured')
 
 
 def get_creds(keystone):
@@ -117,7 +112,6 @@ def get_credentials():
     else:
         kv = unitdata.kv()
         creds = kv.get('keystone-relation-creds')
-    set_state('os-service-checks.do-reconfig')
     return creds
 
 
@@ -166,10 +160,11 @@ def render_checks():
 
 @when('nrpe-external-master.available')
 def nrpe_connected(nem):
-    set_state('os-service-checks.do-reconfig')
+    remove_state('os-service-checks.configured')
 
 
-@when('os-service-checks.do-reconfig')
+@when('os-service-checks.installed')
+@when_not('os-service-checks.configured')
 def render_config():
     creds = get_credentials()
     if not creds:
@@ -182,15 +177,17 @@ def render_config():
     render_checks()
     if config.get('trusted_ssl_ca', None):
         fix_ssl()
-    set_state('os-service-checks.do-restart')
+    set_state('os-service-checks.configured')
+    remove_state('os-service-checks.started')
 
 
-@when('os-service-checks.do-restart')
+@when('os-service-checks.configured')
+@when_not('os-service-checks.started')
 def do_restart():
     hookenv.log('Reloading nagios-nrpe-server')
     host.service_restart('nagios-nrpe-server')
     hookenv.status_set('active', 'Ready')
-    remove_state('os-service-checks.do-restart')
+    set_state('os-service-checks.started')
 
 
 def fix_ssl():