[hopem, r=gnuoy] Fix upgrade breakage whereby if upgrading from
version of charm that did not support db-initialised peer setting db ops get stuck waiting infinitely for db to be intialised. Closes-Bug: 1519035
This commit is contained in:
Liam Young
commit
42119dc093
|
|
@ -271,6 +271,33 @@ def update_all_identity_relation_units_force_sync():
|
|||
update_all_identity_relation_units()
|
||||
|
||||
|
||||
def leader_init_db_if_ready(use_current_context=False):
|
||||
""" Initialise the keystone db if it is ready and mark it as initialised.
|
||||
|
||||
NOTE: this must be idempotent.
|
||||
"""
|
||||
if not is_elected_leader(CLUSTER_RES):
|
||||
log("Not leader - skipping db init", level=DEBUG)
|
||||
return
|
||||
|
||||
if is_db_initialised():
|
||||
log("Database already initialised - skipping db init", level=DEBUG)
|
||||
return
|
||||
|
||||
# Bugs 1353135 & 1187508. Dbs can appear to be ready before the
|
||||
# units acl entry has been added. So, if the db supports passing
|
||||
# a list of permitted units then check if we're in the list.
|
||||
if not is_db_ready(use_current_context=use_current_context):
|
||||
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. Also avoid duplicate db ready check.
|
||||
update_all_identity_relation_units(check_db_ready=False)
|
||||
|
||||
|
||||
@hooks.hook('shared-db-relation-changed')
|
||||
@restart_on_change(restart_map())
|
||||
@synchronize_ca_if_changed()
|
||||
|
|
@ -279,19 +306,7 @@ def db_changed():
|
|||
log('shared-db relation incomplete. Peer not ready?')
|
||||
else:
|
||||
CONFIGS.write(KEYSTONE_CONF)
|
||||
if is_elected_leader(CLUSTER_RES):
|
||||
# Bugs 1353135 & 1187508. Dbs can appear to be ready before the
|
||||
# units acl entry has been added. So, if the db supports passing
|
||||
# a list of permitted units then check if we're in the list.
|
||||
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. Also avoid duplicate db ready check.
|
||||
update_all_identity_relation_units(check_db_ready=False)
|
||||
leader_init_db_if_ready(use_current_context=True)
|
||||
|
||||
|
||||
@hooks.hook('pgsql-db-relation-changed')
|
||||
|
|
@ -302,16 +317,7 @@ def pgsql_db_changed():
|
|||
log('pgsql-db relation incomplete. Peer not ready?')
|
||||
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. Also avoid duplicate db ready check.
|
||||
update_all_identity_relation_units(check_db_ready=False)
|
||||
leader_init_db_if_ready(use_current_context=True)
|
||||
|
||||
|
||||
@hooks.hook('identity-service-relation-changed')
|
||||
|
|
@ -612,6 +618,10 @@ def upgrade_charm():
|
|||
ensure_ssl_dirs()
|
||||
|
||||
CONFIGS.write_all()
|
||||
|
||||
# See LP bug 1519035
|
||||
leader_init_db_if_ready()
|
||||
|
||||
update_nrpe_config()
|
||||
|
||||
if is_elected_leader(CLUSTER_RES):
|
||||
|
|
|
|||
|
|
@ -60,7 +60,6 @@ TO_PATCH = [
|
|||
'synchronize_ca_if_changed',
|
||||
'update_nrpe_config',
|
||||
'ensure_ssl_dirs',
|
||||
'is_db_initialised',
|
||||
'is_db_ready',
|
||||
# other
|
||||
'check_call',
|
||||
|
|
@ -246,14 +245,30 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
configs.write = MagicMock()
|
||||
hooks.pgsql_db_changed()
|
||||
|
||||
@patch('keystone_utils.relation_ids')
|
||||
@patch('keystone_utils.peer_retrieve')
|
||||
@patch('keystone_utils.peer_store')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'CONFIGS')
|
||||
@patch.object(hooks, 'identity_changed')
|
||||
def test_db_changed_allowed(self, identity_changed, configs,
|
||||
mock_ensure_ssl_cert_master,
|
||||
mock_log):
|
||||
self.is_db_initialised.return_value = True
|
||||
mock_ensure_ssl_cert_master, mock_log,
|
||||
mock_peer_store,
|
||||
mock_peer_retrieve, mock_relation_ids):
|
||||
mock_relation_ids.return_value = ['peer/0']
|
||||
peer_settings = {}
|
||||
|
||||
def fake_peer_store(key, val):
|
||||
peer_settings[key] = val
|
||||
|
||||
def fake_migrate():
|
||||
fake_peer_store('db-initialised', 'True')
|
||||
|
||||
self.migrate_database.side_effect = fake_migrate
|
||||
mock_peer_store.side_effect = fake_peer_store
|
||||
mock_peer_retrieve.side_effect = lambda key: peer_settings.get(key)
|
||||
|
||||
self.is_db_ready.return_value = True
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
self.relation_ids.return_value = ['identity-service:0']
|
||||
|
|
@ -268,12 +283,15 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
relation_id='identity-service:0',
|
||||
remote_unit='unit/0')
|
||||
|
||||
@patch('keystone_utils.relation_ids')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'CONFIGS')
|
||||
@patch.object(hooks, 'identity_changed')
|
||||
def test_db_changed_not_allowed(self, identity_changed, configs,
|
||||
mock_ensure_ssl_cert_master, mock_log):
|
||||
mock_ensure_ssl_cert_master, mock_log,
|
||||
mock_relation_ids):
|
||||
mock_relation_ids.return_value = []
|
||||
self.is_db_ready.return_value = False
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
self.relation_ids.return_value = ['identity-service:0']
|
||||
|
|
@ -286,13 +304,31 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
self.assertFalse(self.ensure_initial_admin.called)
|
||||
self.assertFalse(identity_changed.called)
|
||||
|
||||
@patch('keystone_utils.relation_ids')
|
||||
@patch('keystone_utils.peer_retrieve')
|
||||
@patch('keystone_utils.peer_store')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'CONFIGS')
|
||||
@patch.object(hooks, 'identity_changed')
|
||||
def test_postgresql_db_changed(self, identity_changed, configs,
|
||||
mock_ensure_ssl_cert_master, mock_log):
|
||||
self.is_db_initialised.return_value = True
|
||||
mock_ensure_ssl_cert_master, mock_log,
|
||||
mock_peer_store, mock_peer_retrieve,
|
||||
mock_relation_ids):
|
||||
mock_relation_ids.return_value = ['peer/0']
|
||||
|
||||
peer_settings = {}
|
||||
|
||||
def fake_peer_store(key, val):
|
||||
peer_settings[key] = val
|
||||
|
||||
def fake_migrate():
|
||||
fake_peer_store('db-initialised', 'True')
|
||||
|
||||
self.migrate_database.side_effect = fake_migrate
|
||||
mock_peer_store.side_effect = fake_peer_store
|
||||
mock_peer_retrieve.side_effect = lambda key: peer_settings.get(key)
|
||||
|
||||
self.is_db_ready.return_value = True
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
self.relation_ids.return_value = ['identity-service:0']
|
||||
|
|
@ -307,6 +343,7 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
relation_id='identity-service:0',
|
||||
remote_unit='unit/0')
|
||||
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'git_install_requested')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
|
|
@ -340,11 +377,12 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
mock_ensure_pki_dir_permissions,
|
||||
mock_ensure_ssl_dirs,
|
||||
mock_ensure_ssl_cert_master,
|
||||
mock_log, git_requested):
|
||||
mock_log, git_requested,
|
||||
mock_is_db_initialised):
|
||||
git_requested.return_value = False
|
||||
mock_is_pki_enabled.return_value = True
|
||||
mock_is_ssl_cert_master.return_value = True
|
||||
self.is_db_initialised.return_value = True
|
||||
mock_is_db_initialised.return_value = True
|
||||
self.is_db_ready.return_value = True
|
||||
self.openstack_upgrade_available.return_value = False
|
||||
self.is_elected_leader.return_value = True
|
||||
|
|
@ -421,6 +459,7 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
self.assertFalse(self.ensure_initial_admin.called)
|
||||
self.assertFalse(identity_changed.called)
|
||||
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'git_install_requested')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
|
|
@ -453,12 +492,13 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
mock_ensure_pki_permissions,
|
||||
mock_ensure_ssl_dirs,
|
||||
mock_ensure_ssl_cert_master,
|
||||
mock_log, git_requested):
|
||||
mock_log, git_requested,
|
||||
mock_is_db_initialised):
|
||||
git_requested.return_value = False
|
||||
mock_is_pki_enabled.return_value = True
|
||||
mock_is_ssl_cert_master.return_value = True
|
||||
self.is_db_ready.return_value = True
|
||||
self.is_db_initialised.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
|
||||
|
|
@ -544,6 +584,7 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
self.assertFalse(self.openstack_upgrade_available.called)
|
||||
self.assertFalse(self.do_openstack_upgrade_reexec.called)
|
||||
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch.object(hooks, 'git_install_requested')
|
||||
@patch.object(hooks, 'config_value_changed')
|
||||
@patch.object(hooks, 'ensure_ssl_dir')
|
||||
|
|
@ -562,7 +603,8 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
is_pki, config_https,
|
||||
ensure_ssl_dir,
|
||||
config_value_changed,
|
||||
git_requested):
|
||||
git_requested,
|
||||
mock_db_init):
|
||||
ensure_ssl_cert.return_value = False
|
||||
is_pki.return_value = False
|
||||
peer_units.return_value = []
|
||||
|
|
@ -575,14 +617,15 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
|
||||
self.assertFalse(self.do_openstack_upgrade_reexec.called)
|
||||
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'hashlib')
|
||||
@patch.object(hooks, 'send_notifications')
|
||||
def test_identity_changed_leader(self, mock_send_notifications,
|
||||
mock_hashlib, mock_ensure_ssl_cert_master,
|
||||
mock_log):
|
||||
self.is_db_initialised.return_value = True
|
||||
mock_log, mock_is_db_initialised):
|
||||
mock_is_db_initialised.return_value = True
|
||||
self.is_db_ready.return_value = True
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
hooks.identity_changed(
|
||||
|
|
@ -784,6 +827,7 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
self.assertTrue(configs.write_all.called)
|
||||
self.assertFalse(mock_synchronize_ca.called)
|
||||
|
||||
@patch.object(hooks, 'is_db_initialised')
|
||||
@patch('keystone_utils.log')
|
||||
@patch('keystone_utils.ensure_ssl_cert_master')
|
||||
@patch.object(hooks, 'identity_changed')
|
||||
|
|
@ -791,8 +835,9 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
def test_ha_relation_changed_clustered_leader(self, configs,
|
||||
identity_changed,
|
||||
mock_ensure_ssl_cert_master,
|
||||
mock_log):
|
||||
self.is_db_initialised.return_value = True
|
||||
mock_log,
|
||||
mock_is_db_initialised):
|
||||
mock_is_db_initialised.return_value = True
|
||||
self.is_db_ready.return_value = True
|
||||
mock_ensure_ssl_cert_master.return_value = False
|
||||
self.relation_get.return_value = True
|
||||
|
|
@ -906,5 +951,5 @@ class KeystoneRelationTests(CharmTestCase):
|
|||
ssh_authorized_peers.assert_called_with(
|
||||
user=self.ssh_user, group='juju_keystone',
|
||||
peer_interface='cluster', ensure_local_user=True)
|
||||
self.assertFalse(self.log.called)
|
||||
self.assertTrue(self.log.called)
|
||||
self.assertFalse(self.ensure_initial_admin.called)
|
||||
|
|
|
|||
Loading…
Reference in New Issue