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
1=== modified file 'hooks/actions.py'
2--- hooks/actions.py 2015-10-15 18:29:30 +0000
3+++ hooks/actions.py 2015-10-30 12:58:21 +0000
4@@ -11,6 +11,7 @@
5 host,
6 )
7 from charmhelpers.core.services import helpers
8+from charmhelpers.core.services.base import service_restart
9 from charmhelpers.core.templating import render
10 from charmhelpers.payload import (
11 archive,
12@@ -19,6 +20,7 @@
13
14 from charmhelpers.contrib.charmsupport import nrpe
15
16+
17 REQUIRED_PACKAGES = [
18 'python-virtualenv', 'python3-dev', 'libpq5'
19 ]
20@@ -34,6 +36,29 @@
21 ETC_DIR = os.path.join(BASE_DIR, 'etc')
22
23
24+def restart(service_name):
25+ """Restart the service, only if the config changed"""
26+ current_config_hash = get_config_file_hash()
27+ if current_config_hash != config['current_config_hash']:
28+ service_restart(service_name)
29+
30+
31+def get_config_file_hash():
32+ from hashlib import md5
33+ config_file = os.path.join(SERVICE_DIR, 'service.conf')
34+ if not os.path.exists(config_file):
35+ return None
36+ with open(config_file, 'r') as fd:
37+ h = md5(fd.read())
38+ return h.hexdigest()
39+
40+
41+def refresh_config_hash(service_name):
42+ current_config_hash = get_config_file_hash()
43+ config['current_config_hash'] = current_config_hash
44+ config.save()
45+
46+
47 def log_start(service_name):
48 hookenv.log('spec-manager starting')
49
50@@ -149,7 +174,6 @@
51 db_config_created = False
52 db_config_path = os.path.join(ETC_DIR, 'db.yaml')
53 db_schema_config_path = os.path.join(ETC_DIR, 'db-schema.yaml')
54- db_admin_config_created = False
55 db_admin_config_path = os.path.join(ETC_DIR, 'db-admin.yaml')
56 if db_rels:
57 # use the master/standalone server
58@@ -180,12 +204,7 @@
59 with open(config_file, 'w') as fp:
60 parser.write(fp)
61 fp.flush()
62- # cleanup if nothing was created
63- if not db_config_created:
64- if os.path.exists(db_config_path):
65- os.remove(db_config_path)
66- if os.path.exists(db_schema_config_path):
67- os.remove(db_schema_config_path)
68+
69 db_admin_rels = hookenv.relations_of_type('db-admin')
70 if db_admin_rels and db_config_created:
71 # use the master/standalone server
72@@ -196,13 +215,6 @@
73 content = yaml.dump(db_rel)
74 host.write_file(db_admin_config_path, content,
75 owner='root', group='root', perms=0o400)
76- db_admin_config_created = True
77-
78- # cleanup if nothing was updated
79- if not db_config_created and os.path.exists(db_config_path):
80- os.remove(db_config_path)
81- if not db_admin_config_created and os.path.exists(db_admin_config_path):
82- os.remove(db_admin_config_path)
83
84
85 class PostgresqlRelation(helpers.RelationContext):
86@@ -274,10 +286,14 @@
87 config = hookenv.config()
88 checks = []
89 db_rels = hookenv.relations_of_type('db')
90- required_data = ('port', 'host', 'user', 'password', 'database')
91+ required_data = set(['port', 'host', 'user', 'password', 'database'])
92 for server in db_rels:
93- if len(required_data) != len([k for k in required_data if k in server]) \
94- and server['database'] == config['db_name']:
95+ if not required_data.issubset(set(server.keys())):
96+ hookenv.log('ConnCheckRelation: skipping db relation as '
97+ 'required data is not complete')
98+ continue
99+ elif 'database' in server and \
100+ server['database'] != config['db_name']:
101 continue
102 db_check = {'type': 'postgres',
103 'host': server['host'],
104
105=== modified file 'hooks/services.py'
106--- hooks/services.py 2015-09-14 19:48:15 +0000
107+++ hooks/services.py 2015-10-30 12:58:21 +0000
108@@ -18,6 +18,7 @@
109 'required_data': [config,
110 actions.PostgresqlRelation(required=True)],
111 'data_ready': [
112+ actions.refresh_config_hash,
113 actions.ensure_directories,
114 actions.get_cloud_service_from_tarball,
115 actions.install_python_packages,
116@@ -32,6 +33,9 @@
117 actions.log_start,
118 actions.nrpe_external_master_relation,
119 ],
120+ 'start': [
121+ actions.restart
122+ ],
123 },
124 ])
125 manager.manage()

Subscribers

People subscribed via source and target branches