[hopem,r=gnuoy]
Fixes db migration (keystone-manage db-sync) races but preventing database access/usage until the database is ready and has been initialised.
This commit is contained in:
Edward Hope-Morley
commit
466c9f6c6f
@ -4,7 +4,6 @@ import json
|
||||
import os
|
||||
import stat
|
||||
import sys
|
||||
import time
|
||||
|
||||
from subprocess import check_call
|
||||
|
||||
@ -72,6 +71,7 @@ from keystone_utils import (
|
||||
is_ssl_cert_master,
|
||||
is_db_ready,
|
||||
clear_ssl_synced_units,
|
||||
is_db_initialised,
|
||||
)
|
||||
|
||||
from charmhelpers.contrib.hahelpers.cluster import (
|
||||
@ -198,17 +198,18 @@ def update_all_identity_relation_units(check_db_ready=True):
|
||||
level=INFO)
|
||||
return
|
||||
|
||||
try:
|
||||
migrate_database()
|
||||
except Exception as exc:
|
||||
log("Database initialisation failed (%s) - db not ready?" % (exc),
|
||||
level=WARNING)
|
||||
else:
|
||||
if not is_db_initialised():
|
||||
log("Database not yet initialised - deferring identity-relation "
|
||||
"updates", level=INFO)
|
||||
return
|
||||
|
||||
if is_elected_leader(CLUSTER_RES):
|
||||
ensure_initial_admin(config)
|
||||
log('Firing identity_changed hook for all related services.')
|
||||
for rid in relation_ids('identity-service'):
|
||||
for unit in related_units(rid):
|
||||
identity_changed(relation_id=rid, remote_unit=unit)
|
||||
|
||||
log('Firing identity_changed hook for all related services.')
|
||||
for rid in relation_ids('identity-service'):
|
||||
for unit in related_units(rid):
|
||||
identity_changed(relation_id=rid, remote_unit=unit)
|
||||
|
||||
|
||||
@synchronize_ca_if_changed(force=True)
|
||||
@ -233,8 +234,9 @@ def db_changed():
|
||||
level=INFO)
|
||||
return
|
||||
|
||||
migrate_database()
|
||||
# Ensure any existing service entries are updated in the
|
||||
# new database backend
|
||||
# new database backend. Also avoid duplicate db ready check.
|
||||
update_all_identity_relation_units(check_db_ready=False)
|
||||
|
||||
|
||||
@ -247,9 +249,15 @@ def pgsql_db_changed():
|
||||
else:
|
||||
CONFIGS.write(KEYSTONE_CONF)
|
||||
if is_elected_leader(CLUSTER_RES):
|
||||
if not is_db_ready(use_current_context=True):
|
||||
log('Allowed_units list provided and this unit not present',
|
||||
level=INFO)
|
||||
return
|
||||
|
||||
migrate_database()
|
||||
# Ensure any existing service entries are updated in the
|
||||
# new database backend
|
||||
update_all_identity_relation_units()
|
||||
# new database backend. Also avoid duplicate db ready check.
|
||||
update_all_identity_relation_units(check_db_ready=False)
|
||||
|
||||
|
||||
@hooks.hook('identity-service-relation-changed')
|
||||
@ -265,6 +273,11 @@ def identity_changed(relation_id=None, remote_unit=None):
|
||||
"ready - deferring until db ready", level=WARNING)
|
||||
return
|
||||
|
||||
if not is_db_initialised():
|
||||
log("Database not yet initialised - deferring identity-relation "
|
||||
"updates", level=INFO)
|
||||
return
|
||||
|
||||
add_service_to_keystone(relation_id, remote_unit)
|
||||
settings = relation_get(rid=relation_id, unit=remote_unit)
|
||||
service = settings.get('service', None)
|
||||
@ -394,7 +407,7 @@ def cluster_changed():
|
||||
# NOTE(jamespage) re-echo passwords for peer storage
|
||||
echo_whitelist, overrides = \
|
||||
apply_echo_filters(settings, ['_passwd', 'identity-service:',
|
||||
'ssl-cert-master'])
|
||||
'ssl-cert-master', 'db-initialised'])
|
||||
log("Peer echo overrides: %s" % (overrides), level=DEBUG)
|
||||
relation_set(**overrides)
|
||||
if echo_whitelist:
|
||||
@ -487,15 +500,8 @@ def ha_changed():
|
||||
|
||||
clustered = relation_get('clustered')
|
||||
if clustered and is_elected_leader(CLUSTER_RES):
|
||||
if not is_db_ready():
|
||||
log('Allowed_units list provided and this unit not present',
|
||||
level=INFO)
|
||||
return
|
||||
|
||||
ensure_initial_admin(config)
|
||||
log('Cluster configured, notifying other services and updating '
|
||||
'keystone endpoint configuration')
|
||||
|
||||
update_all_identity_relation_units()
|
||||
|
||||
|
||||
@ -546,7 +552,6 @@ def upgrade_charm():
|
||||
if is_elected_leader(CLUSTER_RES):
|
||||
log('Cluster leader - ensuring endpoint configuration is up to '
|
||||
'date', level=DEBUG)
|
||||
time.sleep(10)
|
||||
update_all_identity_relation_units()
|
||||
|
||||
|
||||
|
||||
@ -314,7 +314,32 @@ def do_openstack_upgrade(configs):
|
||||
configs.write_all()
|
||||
|
||||
if is_elected_leader(CLUSTER_RES):
|
||||
migrate_database()
|
||||
if is_db_ready():
|
||||
migrate_database()
|
||||
else:
|
||||
log("Database not ready - deferring to shared-db relation",
|
||||
level=INFO)
|
||||
return
|
||||
|
||||
|
||||
def set_db_initialised():
|
||||
for rid in relation_ids('cluster'):
|
||||
relation_set(relation_settings={'db-initialised': 'True'},
|
||||
relation_id=rid)
|
||||
|
||||
|
||||
def is_db_initialised():
|
||||
for rid in relation_ids('cluster'):
|
||||
units = related_units(rid) + [local_unit()]
|
||||
for unit in units:
|
||||
db_initialised = relation_get(attribute='db-initialised',
|
||||
unit=unit, rid=rid)
|
||||
if db_initialised:
|
||||
log("Database is initialised", level=DEBUG)
|
||||
return True
|
||||
|
||||
log("Database is NOT initialised", level=DEBUG)
|
||||
return False
|
||||
|
||||
|
||||
def migrate_database():
|
||||
@ -328,10 +353,11 @@ def migrate_database():
|
||||
subprocess.check_output(cmd)
|
||||
service_start('keystone')
|
||||
time.sleep(10)
|
||||
|
||||
set_db_initialised()
|
||||
|
||||
# OLD
|
||||
|
||||
|
||||
def get_local_endpoint():
|
||||
"""Returns the URL for the local end-point bypassing haproxy/ssl"""
|
||||
if config('prefer-ipv6'):
|
||||
|
||||
@ -63,7 +63,6 @@ TO_PATCH = [
|
||||
'execd_preinstall',
|
||||
'mkdir',
|
||||
'os',
|
||||
'time',
|
||||
# ip
|
||||
'get_iface_for_address',
|
||||
'get_netmask_for_address',
|
||||
@ -203,6 +202,7 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
configs.write = MagicMock()
|
||||
hooks.pgsql_db_changed()
|
||||
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'is_db_ready')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@ -210,7 +210,9 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
@patch.object(hooks, 'identity_changed')
|
||||
def test_db_changed_allowed(self, identity_changed, configs,
|
||||
mock_ensure_ssl_cert_master,
|
||||
mock_log, mock_is_db_ready):
|
||||
mock_log, mock_is_db_ready,
|
||||
mock_is_db_initialised):
|
||||
mock_is_db_initialised.return_value = True
|
||||
mock_is_db_ready.return_value = True
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
self.relation_ids.return_value = ['identity-service:0']
|
||||
@ -247,12 +249,14 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'is_db_ready')
|
||||
@patch.object(hooks, 'CONFIGS')
|
||||
@patch.object(hooks, 'identity_changed')
|
||||
def test_postgresql_db_changed(self, identity_changed, configs,
|
||||
mock_is_db_ready,
|
||||
mock_is_db_ready, mock_is_db_initialised,
|
||||
mock_ensure_ssl_cert_master, mock_log):
|
||||
mock_is_db_initialised.return_value = True
|
||||
mock_is_db_ready.return_value = True
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
self.relation_ids.return_value = ['identity-service:0']
|
||||
@ -269,6 +273,7 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'is_db_ready')
|
||||
@patch.object(hooks, 'peer_units')
|
||||
@patch.object(hooks, 'ensure_permissions')
|
||||
@ -283,8 +288,9 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
self, configure_https, identity_changed,
|
||||
configs, get_homedir, ensure_user, cluster_joined,
|
||||
admin_relation_changed, ensure_permissions, mock_peer_units,
|
||||
mock_is_db_ready,
|
||||
mock_is_db_ready, mock_is_db_initialised,
|
||||
mock_ensure_ssl_cert_master, mock_log):
|
||||
mock_is_db_initialised.return_value = True
|
||||
mock_is_db_ready.return_value = True
|
||||
self.openstack_upgrade_available.return_value = False
|
||||
self.is_elected_leader.return_value = True
|
||||
@ -302,7 +308,6 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
configure_https.assert_called_with()
|
||||
self.assertTrue(configs.write_all.called)
|
||||
|
||||
self.migrate_database.assert_called_with()
|
||||
self.assertTrue(self.ensure_initial_admin.called)
|
||||
self.log.assert_called_with(
|
||||
'Firing identity_changed hook for all related services.')
|
||||
@ -343,6 +348,7 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'is_db_ready')
|
||||
@patch.object(hooks, 'peer_units')
|
||||
@patch.object(hooks, 'ensure_permissions')
|
||||
@ -361,9 +367,11 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
ensure_permissions,
|
||||
mock_peer_units,
|
||||
mock_is_db_ready,
|
||||
mock_is_db_initialised,
|
||||
mock_ensure_ssl_cert_master,
|
||||
mock_log):
|
||||
mock_is_db_ready.return_value = True
|
||||
mock_is_db_initialised.return_value = True
|
||||
self.openstack_upgrade_available.return_value = True
|
||||
self.is_elected_leader.return_value = True
|
||||
# avoid having to mock syncer
|
||||
@ -382,7 +390,6 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
configure_https.assert_called_with()
|
||||
self.assertTrue(configs.write_all.called)
|
||||
|
||||
self.migrate_database.assert_called_with()
|
||||
self.assertTrue(self.ensure_initial_admin.called)
|
||||
self.log.assert_called_with(
|
||||
'Firing identity_changed hook for all related services.')
|
||||
@ -391,6 +398,7 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
remote_unit='unit/0')
|
||||
admin_relation_changed.assert_called_with('identity-service:0')
|
||||
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'is_db_ready')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@ -398,7 +406,9 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
@patch.object(hooks, 'send_notifications')
|
||||
def test_identity_changed_leader(self, mock_send_notifications,
|
||||
mock_hashlib, mock_ensure_ssl_cert_master,
|
||||
mock_log, mock_is_db_ready):
|
||||
mock_log, mock_is_db_ready,
|
||||
mock_is_db_initialised):
|
||||
mock_is_db_initialised.return_value = True
|
||||
mock_is_db_ready.return_value = True
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
hooks.identity_changed(
|
||||
@ -557,13 +567,16 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'is_db_ready')
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'identity_changed')
|
||||
@patch.object(hooks, 'CONFIGS')
|
||||
def test_ha_relation_changed_clustered_leader(self, configs,
|
||||
identity_changed,
|
||||
mock_is_db_initialised,
|
||||
mock_is_db_ready,
|
||||
mock_ensure_ssl_cert_master,
|
||||
mock_log):
|
||||
mock_is_db_initialised.return_value = True
|
||||
mock_is_db_ready.return_value = True
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
self.relation_get.return_value = True
|
||||
@ -610,6 +623,8 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
cmd = ['a2dissite', 'openstack_https_frontend']
|
||||
self.check_call.assert_called_with(cmd)
|
||||
|
||||
@patch.object(hooks, 'is_db_ready')
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.relation_ids')
|
||||
@patch('keystone_utils.is_elected_leader')
|
||||
@ -623,7 +638,11 @@ class KeystoneRelationTests(CharmTestCase):
|
||||
mock_ensure_ssl_cert_master,
|
||||
mock_is_elected_leader,
|
||||
mock_relation_ids,
|
||||
mock_log):
|
||||
mock_log,
|
||||
mock_is_db_ready,
|
||||
mock_is_db_initialised):
|
||||
mock_is_db_initialised.return_value = True
|
||||
mock_is_db_ready.return_value = True
|
||||
mock_is_elected_leader.return_value = False
|
||||
mock_relation_ids.return_value = []
|
||||
mock_ensure_ssl_cert_master.return_value = True
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user