From 10774ff1f65ceee525fe5c0eae0588c2bb63f2b1 Mon Sep 17 00:00:00 2001 From: James Page Date: Tue, 4 Feb 2025 09:00:40 +0000 Subject: [PATCH] Improve robustness of bootstrap process Its possible that the bootstrap process might fail part way through; refactor the leader_init_db_if_ready function to not just exit early is the db is initialized as the keystone bootstrap process might have also failed for some reason (like DNS slowness). Change-Id: I6b2d7c2fde8bcffdb061746480ded4a410a67315 Closes-Bug: 2089695 --- hooks/keystone_hooks.py | 13 +++++----- unit_tests/test_keystone_hooks.py | 42 ++++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/hooks/keystone_hooks.py b/hooks/keystone_hooks.py index 0150c932..c1347a6a 100755 --- a/hooks/keystone_hooks.py +++ b/hooks/keystone_hooks.py @@ -109,6 +109,7 @@ from keystone_utils import ( setup_ipv6, send_notifications, is_db_initialised, + is_bootstrapped, is_service_present, delete_service_entry, assess_status, @@ -379,11 +380,6 @@ def leader_init_db_if_ready(use_current_context=False): log("Not leader - skipping db init", level=DEBUG) return - if is_db_initialised(): - log("Database already initialised - skipping db init", level=DEBUG) - update_all_identity_relation_units() - 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. @@ -392,8 +388,11 @@ def leader_init_db_if_ready(use_current_context=False): level=INFO) return - migrate_database() - bootstrap_keystone(configs=CONFIGS) + if not is_db_initialised(): + migrate_database() + if not is_bootstrapped(): + bootstrap_keystone(configs=CONFIGS) + ensure_initial_admin(config) if CompareOpenStackReleases( os_release('keystone')) >= 'liberty': diff --git a/unit_tests/test_keystone_hooks.py b/unit_tests/test_keystone_hooks.py index e1fcfe73..fab936a1 100644 --- a/unit_tests/test_keystone_hooks.py +++ b/unit_tests/test_keystone_hooks.py @@ -83,6 +83,7 @@ TO_PATCH = [ 'openstack_upgrade_available', 'save_script_rc', 'migrate_database', + 'bootstrap_keystone', 'ensure_initial_admin', 'add_service_to_keystone', 'update_nrpe_config', @@ -627,6 +628,7 @@ class KeystoneRelationTests(CharmTestCase): cmd = ['a2dissite', 'openstack_https_frontend'] self.check_call.assert_called_with(cmd) + @patch.object(hooks, 'is_bootstrapped') @patch.object(utils, 'get_subordinate_release_packages') @patch.object(hooks, 'bootstrap_keystone') @patch.object(hooks, @@ -650,8 +652,11 @@ class KeystoneRelationTests(CharmTestCase): mock_maybe_do_policyd_overrides, mock_protect_service_accounts, mock_bootstrap_keystone, - mock_get_subordinate_release_packages): + mock_get_subordinate_release_packages, + mock_is_bootstrapped): os_release.return_value = 'havana' + self.os_release.return_value = 'havana' + mock_is_bootstrapped.return_value = True mock_is_db_initialised.return_value = True mock_is_db_ready.return_value = True mock_relation_ids.return_value = [] @@ -672,6 +677,7 @@ class KeystoneRelationTests(CharmTestCase): ANY, "keystone", restart_handler=ANY) mock_protect_service_accounts.assert_called_once_with() + @patch.object(hooks, 'is_bootstrapped') @patch.object(utils, 'get_subordinate_release_packages') @patch.object(hooks, 'bootstrap_keystone') @patch.object(hooks, @@ -696,10 +702,13 @@ class KeystoneRelationTests(CharmTestCase): mock_maybe_do_policyd_overrides, mock_protect_service_accounts, mock_bootstrap_keystone, - mock_get_subordinate_release_packages): + mock_get_subordinate_release_packages, + mock_is_bootstrapped): os_release.return_value = 'havana' + self.os_release.return_value = 'havana' mock_is_db_initialised.return_value = True mock_is_db_ready.return_value = True + mock_is_bootstrapped.return_value = True mock_relation_ids.return_value = [] self.remove_old_packages.return_value = True self.services.return_value = ['apache2'] @@ -718,14 +727,17 @@ class KeystoneRelationTests(CharmTestCase): ANY, "keystone", restart_handler=ANY) mock_protect_service_accounts.assert_called_once_with() + @patch.object(hooks, 'is_bootstrapped') @patch.object(hooks, 'bootstrap_keystone') @patch.object(hooks, 'update_all_identity_relation_units') @patch.object(hooks, 'is_db_initialised') def test_leader_init_db_if_ready(self, is_db_initialized, - update, mock_bootstrap_keystone): + update, mock_bootstrap_keystone, + is_boostrapped): """ Verify leader initilaizes db """ self.is_elected_leader.return_value = True is_db_initialized.return_value = False + is_boostrapped.return_value = False self.is_db_ready.return_value = True self.os_release.return_value = 'mitaka' hooks.leader_init_db_if_ready() @@ -745,16 +757,34 @@ class KeystoneRelationTests(CharmTestCase): self.assertFalse(self.migrate_database.called) self.assertFalse(update.called) + @patch.object(hooks, 'is_bootstrapped') @patch.object(hooks, 'update_all_identity_relation_units') @patch.object(hooks, 'is_db_initialised') - def test_leader_init_db_not_initilaized(self, is_db_initialized, update): + def test_leader_init_db_not_initilaized(self, is_db_initialized, update, + is_bootstrapped): """ Verify leader does not initilaize db when already initialized """ + self.os_release.return_value = 'havana' self.is_elected_leader.return_value = True is_db_initialized.return_value = True + is_bootstrapped.return_value = True hooks.leader_init_db_if_ready() - self.log.assert_called_with('Database already initialised - skipping ' - 'db init', level='DEBUG') self.assertFalse(self.migrate_database.called) + self.assertFalse(self.bootstrap_keystone.called) + self.assertTrue(update.called) + + @patch.object(hooks, 'is_bootstrapped') + @patch.object(hooks, 'update_all_identity_relation_units') + @patch.object(hooks, 'is_db_initialised') + def test_leader_init_db_not_initilaized_not_bootstrapped( + self, is_db_initialized, update, is_bootstrapped): + """ Verify leader does not initilaize db when already initialized """ + self.os_release.return_value = 'havana' + self.is_elected_leader.return_value = True + is_db_initialized.return_value = True + is_bootstrapped.return_value = False + hooks.leader_init_db_if_ready() + self.assertFalse(self.migrate_database.called) + self.assertTrue(self.bootstrap_keystone.called) self.assertTrue(update.called) @patch.object(hooks, 'update_all_identity_relation_units')