Merge lp:~verterok/charms/trusty/tanuki-spec-manager/be-more-gentle-on-db-relation-changes into lp:~tanuki/charms/trusty/tanuki-spec-manager/trunk

[Guillermo Gonzalez]

- improve ConnCheckRelation._get_postgresql_checks method to not explode on missing required_data (seems that we can get a missing database key "sometimes" :/ )
- add config file (service.conf) checksum verification in custom restart function, to avoid pointless restarts

- improve ConnCheckRelation._get_postgresql_checks method to not explode on missing required_data (seems that we can get a missing database key "sometimes" :/ )
- add config file (service.conf) checksum verification in custom restart function, to avoid pointless restarts

[Celso Providelo (cprov)]

Guillermo,

Thanks for working on this.

Preserving old config and preventing restart on insufficient DB-information looks sane and appropriate for this.

We can explore this behaviour in staging (by poking pgsql weirdly) and verify spec-manager stands up until it's back.

review: Approve

=== modified file 'hooks/actions.py'
--- hooks/actions.py	2015-10-15 18:29:30 +0000
+++ hooks/actions.py	2015-10-30 12:58:21 +0000
@@ -11,6 +11,7 @@
     host,
 )
 from charmhelpers.core.services import helpers
+from charmhelpers.core.services.base import service_restart
 from charmhelpers.core.templating import render
 from charmhelpers.payload import (
     archive,
@@ -19,6 +20,7 @@
 
 from charmhelpers.contrib.charmsupport import nrpe
 
+
 REQUIRED_PACKAGES = [
     'python-virtualenv', 'python3-dev', 'libpq5'
 ]
@@ -34,6 +36,29 @@
 ETC_DIR = os.path.join(BASE_DIR, 'etc')
 
 
+def restart(service_name):
+    """Restart the service, only if the config changed"""
+    current_config_hash = get_config_file_hash()
+    if current_config_hash != config['current_config_hash']:
+        service_restart(service_name)
+
+
+def get_config_file_hash():
+    from hashlib import md5
+    config_file = os.path.join(SERVICE_DIR, 'service.conf')
+    if not os.path.exists(config_file):
+        return None
+    with open(config_file, 'r') as fd:
+        h = md5(fd.read())
+        return h.hexdigest()
+
+
+def refresh_config_hash(service_name):
+    current_config_hash = get_config_file_hash()
+    config['current_config_hash'] = current_config_hash
+    config.save()
+
+
 def log_start(service_name):
     hookenv.log('spec-manager starting')
 
@@ -149,7 +174,6 @@
     db_config_created = False
     db_config_path = os.path.join(ETC_DIR, 'db.yaml')
     db_schema_config_path = os.path.join(ETC_DIR, 'db-schema.yaml')
-    db_admin_config_created = False
     db_admin_config_path = os.path.join(ETC_DIR, 'db-admin.yaml')
     if db_rels:
         # use the master/standalone server
@@ -180,12 +204,7 @@
                 with open(config_file, 'w') as fp:
                     parser.write(fp)
                     fp.flush()
-    # cleanup if nothing was created
-    if not db_config_created:
-        if os.path.exists(db_config_path):
-            os.remove(db_config_path)
-        if os.path.exists(db_schema_config_path):
-            os.remove(db_schema_config_path)
+
     db_admin_rels = hookenv.relations_of_type('db-admin')
     if db_admin_rels and db_config_created:
         # use the master/standalone server
@@ -196,13 +215,6 @@
                 content = yaml.dump(db_rel)
                 host.write_file(db_admin_config_path, content,
                                 owner='root', group='root', perms=0o400)
-                db_admin_config_created = True
-
-    # cleanup if nothing was updated
-    if not db_config_created and os.path.exists(db_config_path):
-        os.remove(db_config_path)
-    if not db_admin_config_created and os.path.exists(db_admin_config_path):
-        os.remove(db_admin_config_path)
 
 
 class PostgresqlRelation(helpers.RelationContext):
@@ -274,10 +286,14 @@
         config = hookenv.config()
         checks = []
         db_rels = hookenv.relations_of_type('db')
-        required_data = ('port', 'host', 'user', 'password', 'database')
+        required_data = set(['port', 'host', 'user', 'password', 'database'])
         for server in db_rels:
-            if len(required_data) != len([k for k in required_data if k in server]) \
-                    and server['database'] == config['db_name']:
+            if not required_data.issubset(set(server.keys())):
+                hookenv.log('ConnCheckRelation: skipping db relation as '
+                            'required data is not complete')
+                continue
+            elif 'database' in server and \
+                    server['database'] != config['db_name']:
                 continue
             db_check = {'type': 'postgres',
                         'host': server['host'],
 
=== modified file 'hooks/services.py'
--- hooks/services.py	2015-09-14 19:48:15 +0000
+++ hooks/services.py	2015-10-30 12:58:21 +0000
@@ -18,6 +18,7 @@
             'required_data': [config,
                               actions.PostgresqlRelation(required=True)],
             'data_ready': [
+                actions.refresh_config_hash,
                 actions.ensure_directories,
                 actions.get_cloud_service_from_tarball,
                 actions.install_python_packages,
@@ -32,6 +33,9 @@
                 actions.log_start,
                 actions.nrpe_external_master_relation,
             ],
+            'start': [
+                actions.restart
+            ],
         },
     ])
     manager.manage()