Merge lp:~hopem/charms/trusty/cinder/ensure-svcs-restart-after-db-init into lp:~openstack-charmers-archive/charms/trusty/cinder/next

Proposed by Edward Hope-Morley
Status: Merged
Approved by: Billy Olsen
Approved revision: 97
Merged at revision: 95
Proposed branch: lp:~hopem/charms/trusty/cinder/ensure-svcs-restart-after-db-init
Merge into: lp:~openstack-charmers-archive/charms/trusty/cinder/next
Diff against target: 235 lines (+95/-3)
4 files modified
hooks/cinder_hooks.py (+9/-2)
hooks/cinder_utils.py (+46/-0)
unit_tests/test_cinder_utils.py (+37/-1)
unit_tests/test_cluster_hooks.py (+3/-0)
To merge this branch: bzr merge lp:~hopem/charms/trusty/cinder/ensure-svcs-restart-after-db-init
Reviewer Review Type Date Requested Status
Billy Olsen Approve
OpenStack Charmers Pending
James Page Pending
Review via email: mp+260821@code.launchpad.net

This proposal supersedes a proposal from 2015-05-22.

To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_lint_check #4734 cinder-next for hopem mp259709
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/11272876/
Build: http://10.245.162.77:8080/job/charm_lint_check/4734/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_unit_test #4414 cinder-next for hopem mp259709
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4414/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_amulet_test #4271 cinder-next for hopem mp259709
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4271/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_lint_check #4740 cinder-next for hopem mp259709
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/4740/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_unit_test #4420 cinder-next for hopem mp259709
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4420/

Revision history for this message
James Page (james-page) : Posted in a previous version of this proposal
review: Needs Fixing
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_amulet_test #4276 cinder-next for hopem mp259709
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4276/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_lint_check #4742 cinder-next for hopem mp259984
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/4742/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_unit_test #4422 cinder-next for hopem mp259984
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4422/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal

charm_amulet_test #4278 cinder-next for hopem mp259984
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4278/

Revision history for this message
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal

A couple of minor nits. Functionally it works, but would like to see the nits fixed.

review: Needs Fixing
Revision history for this message
Edward Hope-Morley (hopem) wrote : Posted in a previous version of this proposal

Ok reworked this and seems to behave much nicer (crucially, less noise since echoes cannot trigger restarts)

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #4943 cinder-next for hopem mp260821
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/4943/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #4623 cinder-next for hopem mp260821
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4623/

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #4394 cinder-next for hopem mp260821
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4394/

Revision history for this message
Billy Olsen (billy-olsen) wrote :

LGTM, Approved.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/cinder_hooks.py'
2--- hooks/cinder_hooks.py 2015-04-30 17:03:20 +0000
3+++ hooks/cinder_hooks.py 2015-06-02 12:20:54 +0000
4@@ -24,7 +24,8 @@
5 CINDER_CONF,
6 CINDER_API_CONF,
7 ceph_config_file,
8- setup_ipv6
9+ setup_ipv6,
10+ check_db_initialised,
11 )
12
13 from charmhelpers.core.hookenv import (
14@@ -40,7 +41,7 @@
15 unit_get,
16 log,
17 ERROR,
18- INFO
19+ INFO,
20 )
21
22 from charmhelpers.fetch import (
23@@ -365,6 +366,11 @@
24 relation_id=relation_id,
25 relation_settings={'{}-address'.format(addr_type): address}
26 )
27+
28+ # Only do if this is fired by cluster rel
29+ if not relation_id:
30+ check_db_initialised()
31+
32 if config('prefer-ipv6'):
33 private_addr = get_ipv6_addr(exc_list=[config('vip')])[0]
34 relation_set(relation_id=relation_id,
35@@ -375,6 +381,7 @@
36 'cluster-relation-departed')
37 @restart_on_change(restart_map(), stopstart=True)
38 def cluster_changed():
39+ check_db_initialised()
40 CONFIGS.write_all()
41
42
43
44=== modified file 'hooks/cinder_utils.py'
45--- hooks/cinder_utils.py 2015-05-12 20:43:48 +0000
46+++ hooks/cinder_utils.py 2015-06-02 12:20:54 +0000
47@@ -1,6 +1,7 @@
48 import os
49 import shutil
50 import subprocess
51+import uuid
52
53 from collections import OrderedDict
54 from copy import copy
55@@ -12,8 +13,12 @@
56 from charmhelpers.core.hookenv import (
57 charm_dir,
58 config,
59+ local_unit,
60+ relation_get,
61+ relation_set,
62 relation_ids,
63 log,
64+ DEBUG,
65 service_name
66 )
67
68@@ -127,6 +132,9 @@
69 # Cluster resource used to determine leadership when hacluster'd
70 CLUSTER_RES = 'grp_cinder_vips'
71
72+CINDER_DB_INIT_RKEY = 'cinder-db-initialised'
73+CINDER_DB_INIT_ECHO_RKEY = 'cinder-db-initialised-echo'
74+
75
76 class CinderCharmError(Exception):
77 pass
78@@ -305,6 +313,15 @@
79 return OrderedDict(_map)
80
81
82+def enabled_services():
83+ m = restart_map()
84+ svcs = set()
85+ for t in m.iteritems():
86+ svcs.update(t[1])
87+
88+ return list(svcs)
89+
90+
91 def services():
92 ''' Returns a list of services associate with this charm '''
93 _services = []
94@@ -451,10 +468,39 @@
95 return ('/dev/{}'.format(block_device), 0)
96
97
98+def check_db_initialised():
99+ """Check if we have received db init'd notify and restart services if we
100+ have not already.
101+ """
102+ settings = relation_get() or {}
103+ if settings:
104+ init_id = settings.get(CINDER_DB_INIT_RKEY)
105+ echoed_init_id = relation_get(unit=local_unit(),
106+ attribute=CINDER_DB_INIT_ECHO_RKEY)
107+ if (init_id and init_id != echoed_init_id and
108+ local_unit() not in init_id):
109+ log("Restarting cinder services following db initialisation",
110+ level=DEBUG)
111+ for svc in enabled_services():
112+ service_restart(svc)
113+
114+ # Set echo
115+ relation_set(**{CINDER_DB_INIT_ECHO_RKEY: init_id})
116+
117+
118 def migrate_database():
119 'Runs cinder-manage to initialize a new database or migrate existing'
120 cmd = ['cinder-manage', 'db', 'sync']
121 subprocess.check_call(cmd)
122+ # Notify peers so that services get restarted
123+ log("Notifying peer(s) that db is initialised and restarting services",
124+ level=DEBUG)
125+ for r_id in relation_ids('cluster'):
126+ for svc in enabled_services():
127+ service_restart(svc)
128+
129+ id = "%s-%s" % (local_unit(), uuid.uuid4())
130+ relation_set(relation_id=r_id, **{CINDER_DB_INIT_RKEY: id})
131
132
133 def set_ceph_env_variables(service):
134
135=== modified file 'unit_tests/test_cinder_utils.py'
136--- unit_tests/test_cinder_utils.py 2015-05-12 20:43:48 +0000
137+++ unit_tests/test_cinder_utils.py 2015-06-02 12:20:54 +0000
138@@ -14,10 +14,14 @@
139 # helpers.core.hookenv
140 'config',
141 'log',
142+ 'relation_get',
143+ 'relation_set',
144+ 'local_unit',
145 # helpers.core.host
146 'mounts',
147 'umount',
148 'mkdir',
149+ 'service_restart',
150 # ceph utils
151 # storage_utils
152 'create_lvm_physical_volume',
153@@ -26,6 +30,7 @@
154 'is_lvm_physical_volume',
155 'list_lvm_volume_group',
156 'relation_ids',
157+ 'relation_set',
158 'remove_lvm_physical_volume',
159 'ensure_loopback_device',
160 'is_block_device',
161@@ -406,11 +411,19 @@
162 cinder_utils.extend_lvm_volume_group('test', '/dev/sdb')
163 _call.assert_called_with(['vgextend', 'test', '/dev/sdb'])
164
165- def test_migrate_database(self):
166+ @patch.object(cinder_utils, 'local_unit', lambda *args: 'unit/0')
167+ @patch.object(cinder_utils, 'uuid')
168+ def test_migrate_database(self, mock_uuid):
169 'It migrates database with cinder-manage'
170+ uuid = 'a-great-uuid'
171+ mock_uuid.uuid4.return_value = uuid
172+ rid = 'cluster:0'
173+ self.relation_ids.return_value = [rid]
174+ args = {'cinder-db-initialised': "unit/0-%s" % uuid}
175 with patch('subprocess.check_call') as check_call:
176 cinder_utils.migrate_database()
177 check_call.assert_called_with(['cinder-manage', 'db', 'sync'])
178+ self.relation_set.assert_called_with(relation_id=rid, **args)
179
180 @patch('os.path.exists')
181 def test_register_configs_apache(self, exists):
182@@ -673,3 +686,26 @@
183 call('cinder-scheduler'),
184 ]
185 self.assertEquals(service_restart.call_args_list, expected)
186+
187+ @patch.object(cinder_utils, 'local_unit', lambda *args: 'unit/0')
188+ def test_check_db_initialised_by_self(self):
189+ self.relation_get.return_value = {}
190+ cinder_utils.check_db_initialised()
191+ self.assertFalse(self.relation_set.called)
192+
193+ self.relation_get.return_value = {'cinder-db-initialised':
194+ 'unit/0-1234'}
195+ cinder_utils.check_db_initialised()
196+ self.assertFalse(self.relation_set.called)
197+
198+ @patch.object(cinder_utils, 'local_unit', lambda *args: 'unit/0')
199+ def test_check_db_initialised(self):
200+ self.relation_get.return_value = {}
201+ cinder_utils.check_db_initialised()
202+ self.assertFalse(self.relation_set.called)
203+
204+ self.relation_get.return_value = {'cinder-db-initialised':
205+ 'unit/1-1234'}
206+ cinder_utils.check_db_initialised()
207+ calls = [call(**{'cinder-db-initialised-echo': 'unit/1-1234'})]
208+ self.relation_set.assert_has_calls(calls)
209
210=== modified file 'unit_tests/test_cluster_hooks.py'
211--- unit_tests/test_cluster_hooks.py 2015-01-22 16:51:07 +0000
212+++ unit_tests/test_cluster_hooks.py 2015-06-02 12:20:54 +0000
213@@ -63,6 +63,7 @@
214 super(TestClusterHooks, self).setUp(hooks, TO_PATCH)
215 self.config.side_effect = self.test_config.get
216
217+ @patch.object(hooks, 'check_db_initialised', lambda *args, **kwargs: None)
218 @patch('charmhelpers.core.host.service')
219 @patch('charmhelpers.core.host.file_hash')
220 def test_cluster_hook(self, file_hash, service):
221@@ -88,12 +89,14 @@
222 call('start', 'apache2')]
223 self.assertEquals(ex, service.call_args_list)
224
225+ @patch.object(hooks, 'check_db_initialised', lambda *args, **kwargs: None)
226 def test_cluster_joined_hook(self):
227 self.config.side_effect = self.test_config.get
228 self.get_address_in_network.return_value = None
229 hooks.hooks.execute(['hooks/cluster-relation-joined'])
230 self.assertFalse(self.relation_set.called)
231
232+ @patch.object(hooks, 'check_db_initialised', lambda *args, **kwargs: None)
233 def test_cluster_joined_hook_multinet(self):
234 self.config.side_effect = self.test_config.get
235 self.get_address_in_network.side_effect = [

Subscribers

People subscribed via source and target branches