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

Proposed by Guillermo Gonzalez on 2015-10-30
Status: Merged
Approved by: Guillermo Gonzalez on 2015-10-30
Approved revision: 34
Merged at revision: 33
Proposed branch: lp:~verterok/charms/trusty/tanuki-spec-manager/be-more-gentle-on-db-relation-changes
Merge into: lp:~tanuki/charms/trusty/tanuki-spec-manager/trunk
Diff against target: 125 lines (+37/-17)
2 files modified
hooks/actions.py (+33/-17)
hooks/services.py (+4/-0)
To merge this branch: bzr merge lp:~verterok/charms/trusty/tanuki-spec-manager/be-more-gentle-on-db-relation-changes
Reviewer Review Type Date Requested Status
Celso Providelo (community) 2015-10-30 Approve on 2015-10-30
Review via email: mp+276254@code.launchpad.net

Commit message

- 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

Description of the change

- 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

To post a comment you must log in.
Celso Providelo (cprov) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
11 host,11 host,
12)12)
13from charmhelpers.core.services import helpers13from charmhelpers.core.services import helpers
14from charmhelpers.core.services.base import service_restart
14from charmhelpers.core.templating import render15from charmhelpers.core.templating import render
15from charmhelpers.payload import (16from charmhelpers.payload import (
16 archive,17 archive,
@@ -19,6 +20,7 @@
1920
20from charmhelpers.contrib.charmsupport import nrpe21from charmhelpers.contrib.charmsupport import nrpe
2122
23
22REQUIRED_PACKAGES = [24REQUIRED_PACKAGES = [
23 'python-virtualenv', 'python3-dev', 'libpq5'25 'python-virtualenv', 'python3-dev', 'libpq5'
24]26]
@@ -34,6 +36,29 @@
34ETC_DIR = os.path.join(BASE_DIR, 'etc')36ETC_DIR = os.path.join(BASE_DIR, 'etc')
3537
3638
39def restart(service_name):
40 """Restart the service, only if the config changed"""
41 current_config_hash = get_config_file_hash()
42 if current_config_hash != config['current_config_hash']:
43 service_restart(service_name)
44
45
46def get_config_file_hash():
47 from hashlib import md5
48 config_file = os.path.join(SERVICE_DIR, 'service.conf')
49 if not os.path.exists(config_file):
50 return None
51 with open(config_file, 'r') as fd:
52 h = md5(fd.read())
53 return h.hexdigest()
54
55
56def refresh_config_hash(service_name):
57 current_config_hash = get_config_file_hash()
58 config['current_config_hash'] = current_config_hash
59 config.save()
60
61
37def log_start(service_name):62def log_start(service_name):
38 hookenv.log('spec-manager starting')63 hookenv.log('spec-manager starting')
3964
@@ -149,7 +174,6 @@
149 db_config_created = False174 db_config_created = False
150 db_config_path = os.path.join(ETC_DIR, 'db.yaml')175 db_config_path = os.path.join(ETC_DIR, 'db.yaml')
151 db_schema_config_path = os.path.join(ETC_DIR, 'db-schema.yaml')176 db_schema_config_path = os.path.join(ETC_DIR, 'db-schema.yaml')
152 db_admin_config_created = False
153 db_admin_config_path = os.path.join(ETC_DIR, 'db-admin.yaml')177 db_admin_config_path = os.path.join(ETC_DIR, 'db-admin.yaml')
154 if db_rels:178 if db_rels:
155 # use the master/standalone server179 # use the master/standalone server
@@ -180,12 +204,7 @@
180 with open(config_file, 'w') as fp:204 with open(config_file, 'w') as fp:
181 parser.write(fp)205 parser.write(fp)
182 fp.flush()206 fp.flush()
183 # cleanup if nothing was created207
184 if not db_config_created:
185 if os.path.exists(db_config_path):
186 os.remove(db_config_path)
187 if os.path.exists(db_schema_config_path):
188 os.remove(db_schema_config_path)
189 db_admin_rels = hookenv.relations_of_type('db-admin')208 db_admin_rels = hookenv.relations_of_type('db-admin')
190 if db_admin_rels and db_config_created:209 if db_admin_rels and db_config_created:
191 # use the master/standalone server210 # use the master/standalone server
@@ -196,13 +215,6 @@
196 content = yaml.dump(db_rel)215 content = yaml.dump(db_rel)
197 host.write_file(db_admin_config_path, content,216 host.write_file(db_admin_config_path, content,
198 owner='root', group='root', perms=0o400)217 owner='root', group='root', perms=0o400)
199 db_admin_config_created = True
200
201 # cleanup if nothing was updated
202 if not db_config_created and os.path.exists(db_config_path):
203 os.remove(db_config_path)
204 if not db_admin_config_created and os.path.exists(db_admin_config_path):
205 os.remove(db_admin_config_path)
206218
207219
208class PostgresqlRelation(helpers.RelationContext):220class PostgresqlRelation(helpers.RelationContext):
@@ -274,10 +286,14 @@
274 config = hookenv.config()286 config = hookenv.config()
275 checks = []287 checks = []
276 db_rels = hookenv.relations_of_type('db')288 db_rels = hookenv.relations_of_type('db')
277 required_data = ('port', 'host', 'user', 'password', 'database')289 required_data = set(['port', 'host', 'user', 'password', 'database'])
278 for server in db_rels:290 for server in db_rels:
279 if len(required_data) != len([k for k in required_data if k in server]) \291 if not required_data.issubset(set(server.keys())):
280 and server['database'] == config['db_name']:292 hookenv.log('ConnCheckRelation: skipping db relation as '
293 'required data is not complete')
294 continue
295 elif 'database' in server and \
296 server['database'] != config['db_name']:
281 continue297 continue
282 db_check = {'type': 'postgres',298 db_check = {'type': 'postgres',
283 'host': server['host'],299 'host': server['host'],
284300
=== 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 @@
18 'required_data': [config,18 'required_data': [config,
19 actions.PostgresqlRelation(required=True)],19 actions.PostgresqlRelation(required=True)],
20 'data_ready': [20 'data_ready': [
21 actions.refresh_config_hash,
21 actions.ensure_directories,22 actions.ensure_directories,
22 actions.get_cloud_service_from_tarball,23 actions.get_cloud_service_from_tarball,
23 actions.install_python_packages,24 actions.install_python_packages,
@@ -32,6 +33,9 @@
32 actions.log_start,33 actions.log_start,
33 actions.nrpe_external_master_relation,34 actions.nrpe_external_master_relation,
34 ],35 ],
36 'start': [
37 actions.restart
38 ],
35 },39 },
36 ])40 ])
37 manager.manage()41 manager.manage()

Subscribers

People subscribed via source and target branches