diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py index 9ffe9d25..34672425 100644 --- a/hooks/keystone_utils.py +++ b/hooks/keystone_utils.py @@ -72,6 +72,7 @@ from charmhelpers.core.decorators import ( from charmhelpers.core.hookenv import ( config, + is_leader, leader_get, leader_set, log, @@ -105,8 +106,6 @@ from charmhelpers.core.host import ( from charmhelpers.contrib.peerstorage import ( peer_store_and_set, - peer_store, - peer_retrieve, ) import keystone_context @@ -1035,18 +1034,12 @@ def store_data(backing_file, data): def get_admin_passwd(user=None): passwd = config("admin-password") if passwd and passwd.lower() != "none": - # Previous charm versions did not always store on leader setting so do - # this now to avoid an initial update on install/upgrade - if (is_elected_leader(CLUSTER_RES) and - peer_retrieve('{}_passwd'.format(user)) is None): - set_admin_passwd(passwd, user=user) - return passwd _migrate_admin_password() - passwd = peer_retrieve('{}_passwd'.format(user)) + passwd = leader_get('{}_passwd'.format(user)) - if not passwd and is_elected_leader(CLUSTER_RES): + if not passwd and is_leader(): log("Generating new passwd for user: %s" % config("admin-user")) cmd = ['pwgen', '-c', '16', '1'] @@ -1059,7 +1052,7 @@ def set_admin_passwd(passwd, user=None): if user is None: user = 'admin' - peer_store('{}_passwd'.format(user), passwd) + leader_set({'{}_passwd'.format(user): passwd}) def get_api_version(): @@ -1228,29 +1221,28 @@ def load_stored_passwords(path=SERVICE_PASSWD_PATH): def _migrate_admin_password(): - """Migrate on-disk admin passwords to peer storage""" - if os.path.exists(STORED_PASSWD): - log('Migrating on-disk stored passwords to peer storage') + """Migrate on-disk admin passwords to leader storage""" + if is_leader() and os.path.exists(STORED_PASSWD): + log('Migrating on-disk stored passwords to leader storage') with open(STORED_PASSWD) as fd: - peer_store("admin_passwd", fd.readline().strip('\n')) + leader_set({"admin_passwd": fd.readline().strip('\n')}) os.unlink(STORED_PASSWD) def _migrate_service_passwords(): - """Migrate on-disk service passwords to peer storage""" - if os.path.exists(SERVICE_PASSWD_PATH): - log('Migrating on-disk stored passwords to peer storage') + """Migrate on-disk service passwords to leader storage""" + if is_leader() and os.path.exists(SERVICE_PASSWD_PATH): + log('Migrating on-disk stored passwords to leader storage') creds = load_stored_passwords() for k, v in creds.iteritems(): - peer_store(key="{}_passwd".format(k), value=v) + leader_set({"{}_passwd".format(k): v}) os.unlink(SERVICE_PASSWD_PATH) def get_service_password(service_username): _migrate_service_passwords() - peer_key = "{}_passwd".format(service_username) - passwd = peer_retrieve(peer_key) + passwd = leader_get("{}_passwd".format(service_username)) if passwd is None: passwd = pwgen(length=64) @@ -1258,13 +1250,11 @@ def get_service_password(service_username): def set_service_password(passwd, user): - peer_key = "{}_passwd".format(user) - peer_store(key=peer_key, value=passwd) + leader_set({"{}_passwd".format(user): passwd}) def is_password_changed(username, passwd): - peer_key = "{}_passwd".format(username) - _passwd = peer_retrieve(peer_key) + _passwd = leader_get("{}_passwd".format(username)) return (_passwd is None or passwd != _passwd) diff --git a/unit_tests/test_keystone_hooks.py b/unit_tests/test_keystone_hooks.py index 4c518675..720c37d4 100644 --- a/unit_tests/test_keystone_hooks.py +++ b/unit_tests/test_keystone_hooks.py @@ -631,12 +631,10 @@ class KeystoneRelationTests(CharmTestCase): self.assertTrue(self.update_dns_ha_resource_params.called) self.relation_set.assert_called_with(**args) - @patch.object(utils, 'peer_retrieve') @patch('keystone_utils.log') @patch.object(hooks, 'CONFIGS') def test_ha_relation_changed_not_clustered_not_leader(self, configs, - mock_log, - mock_peer_retrieve): + mock_log): self.relation_get.return_value = False hooks.ha_changed() @@ -842,7 +840,6 @@ class KeystoneRelationTests(CharmTestCase): # Still updates relations self.assertTrue(self.relation_ids.called) - @patch.object(utils, 'peer_retrieve') @patch.object(hooks, 'update_all_identity_relation_units') @patch.object(utils, 'os_release') @patch('keystone_utils.log') @@ -850,11 +847,10 @@ class KeystoneRelationTests(CharmTestCase): def test_upgrade_charm_not_leader(self, mock_relation_ids, mock_log, - os_release, update, mock_peer_retrieve): + os_release, update): os_release.return_value = 'havana' self.filter_installed_packages.return_value = [] - mock_peer_retrieve.return_value = 'true' self.is_elected_leader.return_value = False hooks.upgrade_charm() self.assertTrue(self.apt_install.called) diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py index bd210c59..86b0e346 100644 --- a/unit_tests/test_keystone_utils.py +++ b/unit_tests/test_keystone_utils.py @@ -60,7 +60,6 @@ TO_PATCH = [ 'local_unit', 'related_units', 'https', - 'peer_store', 'mkdir', 'write_file', # generic @@ -291,6 +290,7 @@ class TestKeystoneUtils(CharmTestCase): self.peer_store_and_set.assert_called_with(relation_id=relation_id, **relation_data) + @patch.object(utils, 'leader_set') @patch.object(utils, 'leader_get') @patch.object(utils, 'get_api_version') @patch.object(utils, 'create_user') @@ -301,7 +301,7 @@ class TestKeystoneUtils(CharmTestCase): def test_add_service_to_keystone_no_clustered_no_https_complete_values( self, KeystoneManager, add_endpoint, ensure_valid_service, _resolve_address, create_user, get_api_version, leader_get, - test_api_version=2): + leader_set, test_api_version=2): get_api_version.return_value = test_api_version leader_get.return_value = None relation_id = 'identity-service:0' @@ -395,6 +395,8 @@ class TestKeystoneUtils(CharmTestCase): test_add_service_to_keystone_no_clustered_no_https_complete_values( test_api_version=3) + @patch.object(utils, 'leader_set') + @patch.object(utils, 'is_leader') @patch.object(utils, 'leader_get') @patch('charmhelpers.contrib.openstack.ip.config') @patch.object(utils, 'ensure_valid_service') @@ -402,7 +404,7 @@ class TestKeystoneUtils(CharmTestCase): @patch.object(utils, 'get_manager') def test_add_service_to_keystone_nosubset( self, KeystoneManager, add_endpoint, ensure_valid_service, - ip_config, leader_get): + ip_config, leader_get, is_leader, leader_set): relation_id = 'identity-service:0' remote_unit = 'unit/0' @@ -666,22 +668,16 @@ class TestKeystoneUtils(CharmTestCase): mock_relation_set.assert_called_once_with(relation_id=relation_id, relation_settings=settings) - @patch.object(utils, 'is_elected_leader') - @patch.object(utils, 'peer_retrieve') - @patch.object(utils, 'peer_store') - def test_get_admin_passwd_pwd_set(self, mock_peer_store, - mock_peer_retrieve, - mock_is_elected_leader): - mock_peer_retrieve.return_value = None + def test_get_admin_passwd_pwd_set(self): self.test_config.set('admin-password', 'supersecret') - mock_is_elected_leader.return_value = True self.assertEqual(utils.get_admin_passwd(), 'supersecret') - mock_peer_store.assert_called_once_with('admin_passwd', 'supersecret') - @patch.object(utils, 'peer_retrieve') + @patch.object(utils, 'is_leader') + @patch.object(utils, 'leader_get') @patch('os.path.isfile') - def test_get_admin_passwd_genpass(self, isfile, peer_retrieve): - peer_retrieve.return_value = 'supersecretgen' + def test_get_admin_passwd_genpass(self, isfile, leader_get, is_leader): + is_leader.return_value = True + leader_get.return_value = 'supersecretgen' self.test_config.set('admin-password', '') isfile.return_value = False self.subprocess.check_output.return_value = 'supersecretgen'