Merge lp:~gnuoy/charms/trusty/percona-cluster/sstpasswd2 into lp:~le-charmers/charms/trusty/percona-cluster/leadership-election

Proposed by Liam Young
Status: Rejected
Rejected by: Liam Young
Proposed branch: lp:~gnuoy/charms/trusty/percona-cluster/sstpasswd2
Merge into: lp:~le-charmers/charms/trusty/percona-cluster/leadership-election
Diff against target: 217 lines (+78/-10) (has conflicts)
3 files modified
hooks/charmhelpers/contrib/database/mysql.py (+21/-6)
hooks/percona_hooks.py (+54/-2)
hooks/percona_utils.py (+3/-2)
Text conflict in hooks/percona_hooks.py
To merge this branch: bzr merge lp:~gnuoy/charms/trusty/percona-cluster/sstpasswd2
Reviewer Review Type Date Requested Status
Edward Hope-Morley Pending
Review via email: mp+258979@code.launchpad.net

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

To post a comment you must log in.
Revision history for this message
Edward Hope-Morley (hopem) wrote : Posted in a previous version of this proposal

I'm surprised this has not given is more issue in production. So, unfortunately the way you are doing things here will break the case where no password is provided in config.yaml sst-passwd and is therefore auto-generated and shared on the peer relation. A better option might be to add a new param to get_mysl_password() e.g. create_if_none=False so that we check the peer rel and only create a new one if create_if_none if True which we would be the case only if we were the leader. FTR get_mysql_password() is also called from install() and config_changed() so will need protection there too.

review: Needs Fixing

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'hooks/charmhelpers/contrib/database/mysql.py'
--- hooks/charmhelpers/contrib/database/mysql.py 2015-05-11 10:23:34 +0000
+++ hooks/charmhelpers/contrib/database/mysql.py 2015-05-13 10:25:03 +0000
@@ -134,6 +134,14 @@
134 finally:134 finally:
135 cursor.close()135 cursor.close()
136136
137 def wipe_disk_passwords(self):
138 """Remove all passwords stored on disk"""
139 dirname = os.path.dirname(self.root_passwd_file_template)
140 path = os.path.join(dirname, '*.passwd')
141 for f in glob.glob(path):
142 log("Removing password file {}".format(f), level=DEBUG)
143 os.unlink(f)
144
137 def migrate_passwords_to_peer_relation(self, excludes=None):145 def migrate_passwords_to_peer_relation(self, excludes=None):
138 """Migrate any passwords storage on disk to cluster peer relation."""146 """Migrate any passwords storage on disk to cluster peer relation."""
139 dirname = os.path.dirname(self.root_passwd_file_template)147 dirname = os.path.dirname(self.root_passwd_file_template)
@@ -156,7 +164,7 @@
156 # NOTE cluster relation not yet ready - skip for now164 # NOTE cluster relation not yet ready - skip for now
157 pass165 pass
158166
159 def get_mysql_password_on_disk(self, username=None, password=None):167 def get_mysql_password_on_disk(self, username=None, password=None, create_if_none=True):
160 """Retrieve, generate or store a mysql password for the provided168 """Retrieve, generate or store a mysql password for the provided
161 username on disk."""169 username on disk."""
162 if username:170 if username:
@@ -170,7 +178,7 @@
170 log("Using existing password file '%s'" % passwd_file, level=DEBUG)178 log("Using existing password file '%s'" % passwd_file, level=DEBUG)
171 with open(passwd_file, 'r') as passwd:179 with open(passwd_file, 'r') as passwd:
172 _password = passwd.read().strip()180 _password = passwd.read().strip()
173 else:181 elif create_if_none:
174 log("Generating new password file '%s'" % passwd_file, level=DEBUG)182 log("Generating new password file '%s'" % passwd_file, level=DEBUG)
175 if not os.path.isdir(os.path.dirname(passwd_file)):183 if not os.path.isdir(os.path.dirname(passwd_file)):
176 # NOTE: need to ensure this is not mysql root dir (which needs184 # NOTE: need to ensure this is not mysql root dir (which needs
@@ -207,7 +215,8 @@
207 for key in keys:215 for key in keys:
208 yield key216 yield key
209217
210 def get_mysql_password(self, username=None, password=None):218 def get_mysql_password(self, username=None, password=None,
219 create_if_none=True):
211 """Retrieve, generate or store a mysql password for the provided220 """Retrieve, generate or store a mysql password for the provided
212 username using peer relation cluster."""221 username using peer relation cluster."""
213 excludes = []222 excludes = []
@@ -227,12 +236,18 @@
227 # cluster relation is not yet started; use on-disk236 # cluster relation is not yet started; use on-disk
228 _password = None237 _password = None
229238
230 # If none available, generate new one239 # If none available, check if one is on disk, if not
240 # get_mysql_password_on_disk will generate a new one if create_if_none
241 # is true
231 if not _password:242 if not _password:
232 _password = self.get_mysql_password_on_disk(username, password)243 _password = self.get_mysql_password_on_disk(
244 username,
245 password,
246 create_if_none=create_if_none,
247 )
233248
234 # Put on wire if required249 # Put on wire if required
235 if self.migrate_passwd_to_peer_relation:250 if _password and self.migrate_passwd_to_peer_relation:
236 self.migrate_passwords_to_peer_relation(excludes=excludes)251 self.migrate_passwords_to_peer_relation(excludes=excludes)
237252
238 return _password253 return _password
239254
=== modified file 'hooks/charmhelpers/contrib/peerstorage/__init__.py'
=== modified file 'hooks/charmhelpers/core/hookenv.py'
=== modified file 'hooks/percona_hooks.py'
--- hooks/percona_hooks.py 2015-05-11 10:23:34 +0000
+++ hooks/percona_hooks.py 2015-05-13 10:25:03 +0000
@@ -61,6 +61,13 @@
61 is_clustered,61 is_clustered,
62 oldest_peer,62 oldest_peer,
63 peer_units,63 peer_units,
64<<<<<<< TREE
65=======
66 oldest_peer,
67 is_elected_leader,
68 is_clustered,
69 is_leader,
70>>>>>>> MERGE-SOURCE
64)71)
65from charmhelpers.payload.execd import execd_preinstall72from charmhelpers.payload.execd import execd_preinstall
66from charmhelpers.contrib.network.ip import (73from charmhelpers.contrib.network.ip import (
@@ -94,6 +101,13 @@
94 add_source(config('source'), config('key'))101 add_source(config('source'), config('key'))
95102
96 configure_mysql_root_password(config('root-password'))103 configure_mysql_root_password(config('root-password'))
104<<<<<<< TREE
105=======
106 db_helper = get_db_helper(migrate_passwd_to_peer_relation=False)
107 cfg_passwd = config('sst-password')
108 mysql_password = db_helper.get_mysql_password(username='sstuser',
109 password=cfg_passwd)
110>>>>>>> MERGE-SOURCE
97 # Render base configuration (no cluster)111 # Render base configuration (no cluster)
98 render_config()112 render_config()
99 apt_update(fatal=True)113 apt_update(fatal=True)
@@ -105,6 +119,15 @@
105 if not os.path.exists(os.path.dirname(MY_CNF)):119 if not os.path.exists(os.path.dirname(MY_CNF)):
106 os.makedirs(os.path.dirname(MY_CNF))120 os.makedirs(os.path.dirname(MY_CNF))
107121
122<<<<<<< TREE
123=======
124 if not mysql_password:
125 db_helper = get_db_helper(migrate_passwd_to_peer_relation=False)
126 cfg_passwd = config('sst-password')
127 mysql_password = db_helper.get_mysql_password(username='sstuser',
128 password=cfg_passwd)
129
130>>>>>>> MERGE-SOURCE
108 context = {131 context = {
109 'cluster_name': 'juju_cluster',132 'cluster_name': 'juju_cluster',
110 'private_address': get_host_ip(),133 'private_address': get_host_ip(),
@@ -141,7 +164,9 @@
141 clustered = len(hosts) > 1164 clustered = len(hosts) > 1
142 pre_hash = file_hash(MY_CNF)165 pre_hash = file_hash(MY_CNF)
143 render_config(clustered, hosts)166 render_config(clustered, hosts)
167 db_helper = get_db_helper()
144 if file_hash(MY_CNF) != pre_hash:168 if file_hash(MY_CNF) != pre_hash:
169<<<<<<< TREE
145 try:170 try:
146 # NOTE(jamespage): try with leadership election171 # NOTE(jamespage): try with leadership election
147 if clustered and not is_leader() and not seeded():172 if clustered and not is_leader() and not seeded():
@@ -162,6 +187,18 @@
162 # Restart with new configuration187 # Restart with new configuration
163 service_restart('mysql')188 service_restart('mysql')
164189
190=======
191 oldest = oldest_peer(peer_units())
192 if clustered and not oldest and not seeded():
193 # Bootstrap node into seeded cluster
194 service_restart('mysql')
195 mark_seeded()
196 # Bootstrapping from existing cluster invalidates passwords
197 db_helper.wipe_disk_passwords()
198 elif not clustered:
199 # Restart with new configuration
200 service_restart('mysql')
201>>>>>>> MERGE-SOURCE
165 # Notify any changes to the access network202 # Notify any changes to the access network
166 for r_id in relation_ids('shared-db'):203 for r_id in relation_ids('shared-db'):
167 for unit in related_units(r_id):204 for unit in related_units(r_id):
@@ -210,8 +247,15 @@
210@hooks.hook('db-relation-changed')247@hooks.hook('db-relation-changed')
211@hooks.hook('db-admin-relation-changed')248@hooks.hook('db-admin-relation-changed')
212def db_changed(relation_id=None, unit=None, admin=None):249def db_changed(relation_id=None, unit=None, admin=None):
250<<<<<<< TREE
213 if not is_elected_leader(LEADER_RES):251 if not is_elected_leader(LEADER_RES):
214 log('Not leader of service, deferring db-changed to leader')252 log('Not leader of service, deferring db-changed to leader')
253=======
254 if not is_elected_leader(LEADER_RES):
255 log('Service is peered, clearing db relation'
256 ' as this service unit is not the leader')
257 relation_clear(relation_id)
258>>>>>>> MERGE-SOURCE
215 return259 return
216260
217 if is_clustered():261 if is_clustered():
@@ -280,10 +324,14 @@
280# TODO: This could be a hook common between mysql and percona-cluster324# TODO: This could be a hook common between mysql and percona-cluster
281@hooks.hook('shared-db-relation-changed')325@hooks.hook('shared-db-relation-changed')
282def shared_db_changed(relation_id=None, unit=None):326def shared_db_changed(relation_id=None, unit=None):
327<<<<<<< TREE
283 if not is_elected_leader(LEADER_RES):328 if not is_elected_leader(LEADER_RES):
284 # NOTE(jamespage): relation level data candidate329 # NOTE(jamespage): relation level data candidate
285 log('Service is peered, clearing shared-db relation'330 log('Service is peered, clearing shared-db relation'
286 ' as this service unit is not the leader')331 ' as this service unit is not the leader')
332=======
333 if not is_elected_leader(LEADER_RES):
334>>>>>>> MERGE-SOURCE
287 relation_clear(relation_id)335 relation_clear(relation_id)
288 # Each unit needs to set the db information otherwise if the unit336 # Each unit needs to set the db information otherwise if the unit
289 # with the info dies the settings die with it Bug# 1355848337 # with the info dies the settings die with it Bug# 1355848
@@ -417,13 +465,17 @@
417465
418 resources = {'res_mysql_vip': res_mysql_vip,466 resources = {'res_mysql_vip': res_mysql_vip,
419 'res_mysql_monitor': 'ocf:percona:mysql_monitor'}467 'res_mysql_monitor': 'ocf:percona:mysql_monitor'}
420 db_helper = get_db_helper()468 if is_elected_leader(LEADER_RES):
469 migrate_passwd=True
470 else:
471 migrate_passwd=False
472 db_helper = get_db_helper(migrate_passwd_to_peer_relation=migrate_passwd)
421 cfg_passwd = config('sst-password')473 cfg_passwd = config('sst-password')
422 sstpsswd = db_helper.get_mysql_password(username='sstuser',474 sstpsswd = db_helper.get_mysql_password(username='sstuser',
423 password=cfg_passwd)475 password=cfg_passwd)
424 resource_params = {'res_mysql_vip': vip_params,476 resource_params = {'res_mysql_vip': vip_params,
425 'res_mysql_monitor':477 'res_mysql_monitor':
426 RES_MONITOR_PARAMS % {'sstpass': sstpsswd}}478 RES_MONITOR_PARAMS % {'sstpass': sstpsswd}}
427 groups = {'grp_percona_cluster': 'res_mysql_vip'}479 groups = {'grp_percona_cluster': 'res_mysql_vip'}
428480
429 clones = {'cl_mysql_monitor': 'res_mysql_monitor meta interleave=true'}481 clones = {'cl_mysql_monitor': 'res_mysql_monitor meta interleave=true'}
430482
=== modified file 'hooks/percona_utils.py'
--- hooks/percona_utils.py 2015-03-09 14:02:03 +0000
+++ hooks/percona_utils.py 2015-05-13 10:25:03 +0000
@@ -135,11 +135,12 @@
135 "BY '{}'")135 "BY '{}'")
136136
137137
138def get_db_helper():138def get_db_helper(migrate_passwd_to_peer_relation=True):
139 return MySQLHelper(rpasswdf_template='/var/lib/charm/%s/mysql.passwd' %139 return MySQLHelper(rpasswdf_template='/var/lib/charm/%s/mysql.passwd' %
140 (service_name()),140 (service_name()),
141 upasswdf_template='/var/lib/charm/%s/mysql-{}.passwd' %141 upasswdf_template='/var/lib/charm/%s/mysql-{}.passwd' %
142 (service_name()))142 (service_name()),
143 migrate_passwd_to_peer_relation=migrate_passwd_to_peer_relation)
143144
144145
145def configure_sstuser(sst_password):146def configure_sstuser(sst_password):

Subscribers

People subscribed via source and target branches