From ca8e1e9d5b519d9f324b348a094358fa17c23329 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Tue, 21 Jan 2020 14:23:13 -0600
Subject: [PATCH 01/18] import functional tests keystone-ldap and zaza-ify

---
 zaza/openstack/charm_tests/keystone/tests.py | 96 ++++++++++++++++++--
 1 file changed, 87 insertions(+), 9 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 66727cc..1d3a459 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -145,11 +145,11 @@ class CharmOperationTest(BaseKeystoneTest):
                 if unit_repo != lead_repo:
                     raise zaza_exceptions.KeystoneKeyRepositoryError(
                         'expect: "{}" actual({}): "{}"'
-                        .format(pprint.pformat(lead_repo), unit.entity_id,
-                                pprint.pformat(unit_repo)))
+                        ''.format(pprint.pformat(lead_repo), unit.entity_id,
+                                  pprint.pformat(unit_repo)))
                 logging.info('"{}" == "{}"'
-                             .format(pprint.pformat(unit_repo),
-                                     pprint.pformat(lead_repo)))
+                             ''.format(pprint.pformat(unit_repo),
+                                       pprint.pformat(lead_repo)))
 
 
 class AuthenticationAuthorizationTest(BaseKeystoneTest):
@@ -212,7 +212,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest):
     def test_end_user_domain_admin_access(self):
         """Verify that end-user domain admin does not have elevated privileges.
 
-        In additon to validating that the `policy.json` is written and the
+        In addition to validating that the `policy.json` is written and the
         service is restarted on config-changed, the test validates that our
         `policy.json` is correct.
 
@@ -257,8 +257,9 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest):
                         'allowed when it should not be.')
         logging.info('OK')
 
-    def test_end_user_acccess_and_token(self):
-        """Verify regular end-user access resources and validate token data.
+    def test_end_user_access_and_token(self):
+        """
+        Verify regular end-user access resources and validate token data.
 
         In effect this also validates user creation, presence of standard
         roles (`_member_`, `Member`), effect of policy and configuration
@@ -279,12 +280,12 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest):
                 if len(token) != 32:
                     raise zaza_exceptions.KeystoneWrongTokenProvider(
                         'We expected a UUID token and got this: "{}"'
-                        .format(token))
+                        ''.format(token))
             else:
                 if len(token) < 180:
                     raise zaza_exceptions.KeystoneWrongTokenProvider(
                         'We expected a Fernet token and got this: "{}"'
-                        .format(token))
+                        ''.format(token))
             logging.info('token: "{}"'.format(pprint.pformat(token)))
 
             if (openstack_utils.get_os_release() <
@@ -371,3 +372,80 @@ class SecurityTests(BaseKeystoneTest):
             expected_passes,
             expected_failures,
             expected_to_pass=False)
+
+
+class LdapTests(BaseKeystoneTest):
+    """Keystone ldap tests tests."""
+
+    @classmethod
+    def setUpClass(cls):
+        """Run class setup for running Keystone ldap-tests."""
+        super(LdapTests, cls).setUpClass()
+
+    def _get_ldap_config(self):
+        ldap_ips = zaza.model.get_app_ips("ldap-server")
+        ldap_server_ip = ldap_ips[0]
+
+        keystone_ldap_config = {
+            'ldap-server': "ldap://{}".format(ldap_server_ip),
+            'ldap-user': 'cn=admin,dc=test,dc=com',
+            'ldap-password': 'crapper',
+            'ldap-suffix': 'dc=test,dc=com',
+            'domain-name': 'userdomain',
+        }
+        if all(keystone_ldap_config.values()):
+            self.ldap_configured = True
+            return keystone_ldap_config
+        else:
+            # NOTE(jamespage): Use mock values to check deployment only
+            #                  as no test fixture has been supplied
+            self.ldap_configured = False
+            return {
+                'ldap-server': 'myserver',
+                'ldap-user': 'myuser',
+                'ldap-password': 'mypassword',
+                'ldap-suffix': 'mysuffix',
+                'domain-name': 'userdomain',
+            }
+
+    def _find_keystone_v3_user(self, username, domain):
+        """Find a user within a specified keystone v3 domain."""
+        client = self.admin_keystone_client
+        domain_users = client.users.list(
+            domain=client.domains.find(name=domain).id
+        )
+        usernames = []
+        for user in domain_users:
+            usernames.append(user.name)
+            if username.lower() == user.name.lower():
+                return user
+        logging.debug("User {} was not in these users: {}. Returning None."
+                      "".format(username, usernames))
+        return None
+
+    def test_100_keystone_ldap_users(self):
+        """Validate basic functionality of keystone API with ldap."""
+        zaza.model.set_application_config('keystone-ldap',
+                                          self._get_ldap_config())
+
+        if not self.ldap_configured:
+            msg = 'Skipping API tests as no LDAP test fixture'
+            logging.info(msg)
+            return
+
+        logging.info('Waiting for ldap config-changes to complete...')
+        states = {
+            'keystone-ldap': {
+                'workload-status': 'idle',
+                'workload-status-messages': 'Unit is ready'
+            }
+        }
+
+        zaza.model.wait_for_application_states(states=states)
+
+        # NOTE(jamespage): Test fixture should have johndoe and janedoe
+        #                  accounts
+        johndoe = self._find_keystone_v3_user('john doe', 'userdomain')
+        assert johndoe is not None
+        janedoe = self._find_keystone_v3_user('jane doe', 'userdomain')
+        assert janedoe is not None

From 0c9e00610b38adfb4c40b0951e928b1ab89a7032 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Tue, 21 Jan 2020 14:43:43 -0600
Subject: [PATCH 02/18] wait for keystone to settle also

---
 zaza/openstack/charm_tests/keystone/tests.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 1d3a459..7388237 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -17,7 +17,7 @@ import collections
 import json
 import logging
 import pprint
-
+import tenacity
 import keystoneauth1
 
 import zaza.model
@@ -408,6 +408,10 @@ class LdapTests(BaseKeystoneTest):
                 'domain-name': 'userdomain',
             }
 
+    # NOTE: intermittent authentication fails. Wrap in a retry loop.
+    @tenacity.retry(wait=tenacity.wait_exponential(multiplier=1,
+                                                   min=5, max=10),
+                    reraise=True)
     def _find_keystone_v3_user(self, username, domain):
         """Find a user within a specified keystone v3 domain."""
         client = self.admin_keystone_client
@@ -438,9 +442,12 @@ class LdapTests(BaseKeystoneTest):
             'keystone-ldap': {
                 'workload-status': 'idle',
                 'workload-status-messages': 'Unit is ready'
+            },
+            'keystone': {
+                'workload-status': 'idle',
+                'workload-status-messages': 'Unit is ready'
             }
         }
-
         zaza.model.wait_for_application_states(states=states)
 
         # NOTE(jamespage): Test fixture should have johndoe and janedoe

From b4f1201da256b71bae015fb7410fd6baa8389362 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Wed, 22 Jan 2020 09:30:48 -0600
Subject: [PATCH 03/18] Integrate suggestions from ajkavanagh

---
 zaza/openstack/charm_tests/keystone/tests.py | 101 ++++++++-----------
 zaza/openstack/charm_tests/test_utils.py     |  34 +++++--
 2 files changed, 67 insertions(+), 68 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 7388237..bf9d652 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -17,7 +17,7 @@ import collections
 import json
 import logging
 import pprint
-import tenacity
+import unittest
 import keystoneauth1
 
 import zaza.model
@@ -145,11 +145,11 @@ class CharmOperationTest(BaseKeystoneTest):
                 if unit_repo != lead_repo:
                     raise zaza_exceptions.KeystoneKeyRepositoryError(
                         'expect: "{}" actual({}): "{}"'
-                        ''.format(pprint.pformat(lead_repo), unit.entity_id,
-                                  pprint.pformat(unit_repo)))
+                        .format(pprint.pformat(lead_repo), unit.entity_id,
+                                pprint.pformat(unit_repo)))
                 logging.info('"{}" == "{}"'
-                             ''.format(pprint.pformat(unit_repo),
-                                       pprint.pformat(lead_repo)))
+                             .format(pprint.pformat(unit_repo),
+                                     pprint.pformat(lead_repo)))
 
 
 class AuthenticationAuthorizationTest(BaseKeystoneTest):
@@ -258,8 +258,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest):
         logging.info('OK')
 
     def test_end_user_access_and_token(self):
-        """
-        Verify regular end-user access resources and validate token data.
+        """Verify regular end-user access resources and validate token data.
 
         In effect this also validates user creation, presence of standard
         roles (`_member_`, `Member`), effect of policy and configuration
@@ -280,7 +279,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest):
                 if len(token) != 32:
                     raise zaza_exceptions.KeystoneWrongTokenProvider(
                         'We expected a UUID token and got this: "{}"'
-                        ''.format(token))
+                        .format(token))
             else:
                 if len(token) < 180:
                     raise zaza_exceptions.KeystoneWrongTokenProvider(
@@ -382,36 +381,19 @@ class LdapTests(BaseKeystoneTest):
         """Run class setup for running Keystone ldap-tests."""
         super(LdapTests, cls).setUpClass()
 
-    def _get_ldap_config(self):
+    @staticmethod
+    def _get_ldap_config():
         ldap_ips = zaza.model.get_app_ips("ldap-server")
-        ldap_server_ip = ldap_ips[0]
-
-        keystone_ldap_config = {
-            'ldap-server': "ldap://{}".format(ldap_server_ip),
-            'ldap-user': 'cn=admin,dc=test,dc=com',
-            'ldap-password': 'crapper',
-            'ldap-suffix': 'dc=test,dc=com',
-            'domain-name': 'userdomain',
-        }
-        if all(keystone_ldap_config.values()):
-            self.ldap_configured = True
-            return keystone_ldap_config
-        else:
-            # NOTE(jamespage): Use mock values to check deployment only
-            #                  as no test fixture has been supplied
-            self.ldap_configured = False
-            return {
-                'ldap-server': 'myserver',
-                'ldap-user': 'myuser',
-                'ldap-password': 'mypassword',
-                'ldap-suffix': 'mysuffix',
+        if ldap_ips:
+            return True, {
+                'ldap-server': "ldap://{}".format(ldap_ips[0]),
+                'ldap-user': 'cn=admin,dc=test,dc=com',
+                'ldap-password': 'crapper',
+                'ldap-suffix': 'dc=test,dc=com',
                 'domain-name': 'userdomain',
             }
+        return False, {}
 
-    # NOTE: intermittent authentication fails. Wrap in a retry loop.
-    @tenacity.retry(wait=tenacity.wait_exponential(multiplier=1,
-                                                   min=5, max=10),
-                    reraise=True)
     def _find_keystone_v3_user(self, username, domain):
         """Find a user within a specified keystone v3 domain."""
         client = self.admin_keystone_client
@@ -423,36 +405,35 @@ class LdapTests(BaseKeystoneTest):
             usernames.append(user.name)
             if username.lower() == user.name.lower():
                 return user
-        logging.debug("User {} was not in these users: {}. Returning None."
-                      "".format(username, usernames))
+        logging.debug(
+            "User {} was not in these users: {}. Returning None."
+            .format(username, usernames)
+        )
         return None
 
     def test_100_keystone_ldap_users(self):
         """Validate basic functionality of keystone API with ldap."""
-        zaza.model.set_application_config('keystone-ldap',
-                                          self._get_ldap_config())
+        application_name = 'keystone-ldap'
+        can_config, config = self._get_ldap_config()
+        if not can_config:
+            raise unittest.SkipTest("Skipping API tests as no LDAP test fixture")
 
-        if not self.ldap_configured:
-            msg = 'Skipping API tests as no LDAP test fixture'
-            logging.info(msg)
-            return
-
-        logging.info('Waiting for ldap config-changes to complete...')
-        states = {
-            'keystone-ldap': {
-                'workload-status': 'idle',
-                'workload-status-messages': 'Unit is ready'
-            },
-            'keystone': {
-                'workload-status': 'idle',
-                'workload-status-messages': 'Unit is ready'
+        with self.config_change(
+                self.config_current(application_name),
+                config,
+                application_name=application_name):
+            logging.info('Waiting for users to become available in keystone...')
+            states = {
+                'keystone': {
+                    'workload-status': 'idle',
+                    'workload-status-messages': 'Unit is ready'
+                }
             }
-        }
-        zaza.model.wait_for_application_states(states=states)
+            zaza.model.wait_for_application_states(states=states)
 
-        # NOTE(jamespage): Test fixture should have johndoe and janedoe
-        #                  accounts
-        johndoe = self._find_keystone_v3_user('john doe', 'userdomain')
-        assert johndoe is not None
-        janedoe = self._find_keystone_v3_user('jane doe', 'userdomain')
-        assert janedoe is not None
+            # NOTE(jamespage): Test fixture should have johndoe and janedoe
+            #                  accounts
+            johndoe = self._find_keystone_v3_user('john doe', 'userdomain')
+            self.assertIsNotNone(johndoe, "user 'john doe' was unknown")
+            janedoe = self._find_keystone_v3_user('jane doe', 'userdomain')
+            self.assertIsNotNone(janedoe, "user 'jane doe' was unknown")
diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index 82df994..237ad12 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -133,6 +133,29 @@ class OpenStackBaseTest(unittest.TestCase):
             model_name=cls.model_name)
         logging.debug('Leader unit is {}'.format(cls.lead_unit))
 
+    def config_current(self, application_name=None, keys=None):
+        """Get Current Config of an application normalized into key-values
+
+        :param application_name: String application name for use when called
+                                 by a charm under test other than the object's
+                                 application.
+        :type application_name: str
+        :param keys: iterable of strs to index into the current config.  If
+                     None, return all keys from the config
+        :type keys: Union[iterable[str], None]
+        """
+        if not application_name:
+            application_name = self.application_name
+        _app_config = model.get_application_config(application_name)
+        # convert the more elaborate config structure from libjuju to key-value
+        keys = keys or _app_config.keys()
+        # note that conversion to string for all values is due to
+        # attempting to set any config with other types lead to Traceback
+        return {
+            str(k): str(_app_config.get(k, {}).get('value', ''))
+            for k in keys
+        }
+
     @contextlib.contextmanager
     def config_change(self, default_config, alternate_config,
                       application_name=None):
@@ -158,17 +181,12 @@ class OpenStackBaseTest(unittest.TestCase):
         """
         if not application_name:
             application_name = self.application_name
+
         # we need to compare config values to what is already applied before
         # attempting to set them.  otherwise the model will behave differently
         # than we would expect while waiting for completion of the change
-        _app_config = model.get_application_config(application_name)
-        app_config = {}
-        # convert the more elaborate config structure from libjuju to something
-        # we can compare to what the caller supplies to this function
-        for k in alternate_config.keys():
-            # note that conversion to string for all values is due to
-            # attempting to set any config with other types lead to Traceback
-            app_config[k] = str(_app_config.get(k, {}).get('value', ''))
+        app_config = self.config_current(application_name, keys=alternate_config.keys())
+
         if all(item in app_config.items()
                 for item in alternate_config.items()):
             logging.debug('alternate_config equals what is already applied '

From 59b06a956b251de960a963c5a0654b0b6be1d25d Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Wed, 22 Jan 2020 09:32:17 -0600
Subject: [PATCH 04/18] resolve another suggestion from ajkavanagh

---
 zaza/openstack/charm_tests/keystone/tests.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index bf9d652..c692255 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -284,7 +284,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest):
                 if len(token) < 180:
                     raise zaza_exceptions.KeystoneWrongTokenProvider(
                         'We expected a Fernet token and got this: "{}"'
-                        ''.format(token))
+                        .format(token))
             logging.info('token: "{}"'.format(pprint.pformat(token)))
 
             if (openstack_utils.get_os_release() <

From b75aaa79c58e631a5850c0be5b8c961156ce2f1b Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Wed, 22 Jan 2020 09:33:58 -0600
Subject: [PATCH 05/18] Resolve pep8 violations

---
 zaza/openstack/charm_tests/keystone/tests.py | 8 ++++++--
 zaza/openstack/charm_tests/test_utils.py     | 6 ++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index c692255..7f6e845 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -416,13 +416,17 @@ class LdapTests(BaseKeystoneTest):
         application_name = 'keystone-ldap'
         can_config, config = self._get_ldap_config()
         if not can_config:
-            raise unittest.SkipTest("Skipping API tests as no LDAP test fixture")
+            raise unittest.SkipTest(
+                "Skipping API tests as no LDAP test fixture"
+            )
 
         with self.config_change(
                 self.config_current(application_name),
                 config,
                 application_name=application_name):
-            logging.info('Waiting for users to become available in keystone...')
+            logging.info(
+                'Waiting for users to become available in keystone...'
+            )
             states = {
                 'keystone': {
                     'workload-status': 'idle',
diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index 237ad12..3b26414 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -134,7 +134,7 @@ class OpenStackBaseTest(unittest.TestCase):
         logging.debug('Leader unit is {}'.format(cls.lead_unit))
 
     def config_current(self, application_name=None, keys=None):
-        """Get Current Config of an application normalized into key-values
+        """Get Current Config of an application normalized into key-values.
 
         :param application_name: String application name for use when called
                                  by a charm under test other than the object's
@@ -185,7 +185,9 @@ class OpenStackBaseTest(unittest.TestCase):
         # we need to compare config values to what is already applied before
         # attempting to set them.  otherwise the model will behave differently
         # than we would expect while waiting for completion of the change
-        app_config = self.config_current(application_name, keys=alternate_config.keys())
+        app_config = self.config_current(
+            application_name, keys=alternate_config.keys()
+        )
 
         if all(item in app_config.items()
                 for item in alternate_config.items()):

From e2940340185acf3433a0d32b4d0db22e08e793da Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Wed, 22 Jan 2020 13:14:12 -0600
Subject: [PATCH 06/18] improve docstrings for config_current

---
 zaza/openstack/charm_tests/test_utils.py | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index 3b26414..6c44b0c 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -11,7 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-"""Module containg base class for implementing charm tests."""
+"""Module containing base class for implementing charm tests."""
 import contextlib
 import logging
 import subprocess
@@ -70,7 +70,7 @@ def audit_assertions(action,
     :param expected_passes: List of test names that are expected to pass
     :type expected_passes: List[str]
     :param expected_failures: List of test names that are expected to fail
-    :type expexted_failures: List[str]
+    :type expected_failures: List[str]
     :raises: AssertionError if the assertion fails.
     """
     if expected_failures is None:
@@ -115,7 +115,7 @@ class OpenStackBaseTest(unittest.TestCase):
 
     @classmethod
     def setUpClass(cls, application_name=None, model_alias=None):
-        """Run setup for test class to create common resourcea."""
+        """Run setup for test class to create common resources."""
         cls.model_aliases = model.get_juju_model_aliases()
         if model_alias:
             cls.model_name = cls.model_aliases[model_alias]
@@ -136,13 +136,15 @@ class OpenStackBaseTest(unittest.TestCase):
     def config_current(self, application_name=None, keys=None):
         """Get Current Config of an application normalized into key-values.
 
-        :param application_name: String application name for use when called
+        @param application_name: String application name for use when called
                                  by a charm under test other than the object's
                                  application.
-        :type application_name: str
-        :param keys: iterable of strs to index into the current config.  If
+        @type application_name:  Optional[str]
+        @param keys: iterable of strs to index into the current config.  If
                      None, return all keys from the config
-        :type keys: Union[iterable[str], None]
+        @type keys:  Optional[Iterable[str]]
+        @return: Dictionary of requested config from application
+        @rtype: Dict[str, str]
         """
         if not application_name:
             application_name = self.application_name

From c76b7d423a4f3efff6e26e06db0dec179880ac3d Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Wed, 22 Jan 2020 13:20:42 -0600
Subject: [PATCH 07/18] improved docstrings for ldap tests

---
 zaza/openstack/charm_tests/keystone/tests.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 7f6e845..3b13f8c 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -395,7 +395,12 @@ class LdapTests(BaseKeystoneTest):
         return False, {}
 
     def _find_keystone_v3_user(self, username, domain):
-        """Find a user within a specified keystone v3 domain."""
+        """Find a user within a specified keystone v3 domain.
+        @param str username: Username to search for in keystone
+        @param str domain: username selected from which domain
+        @return: return username if found
+        @rtype: Optional[str]
+        """
         client = self.admin_keystone_client
         domain_users = client.users.list(
             domain=client.domains.find(name=domain).id
@@ -417,7 +422,7 @@ class LdapTests(BaseKeystoneTest):
         can_config, config = self._get_ldap_config()
         if not can_config:
             raise unittest.SkipTest(
-                "Skipping API tests as no LDAP test fixture"
+                "Skipping API tests because there's no ldap-server"
             )
 
         with self.config_change(

From 873b64cde3ab14def097dbd958890ea7fe5f3962 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Wed, 22 Jan 2020 13:25:17 -0600
Subject: [PATCH 08/18] improve docstring

---
 zaza/openstack/charm_tests/keystone/tests.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 3b13f8c..21a4740 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -396,6 +396,7 @@ class LdapTests(BaseKeystoneTest):
 
     def _find_keystone_v3_user(self, username, domain):
         """Find a user within a specified keystone v3 domain.
+
         @param str username: Username to search for in keystone
         @param str domain: username selected from which domain
         @return: return username if found

From f065050d170bc26138096d1a8783bc51b0ff17fa Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Thu, 23 Jan 2020 10:52:50 -0600
Subject: [PATCH 09/18]  use ':' style argument definitions rather than '@'

---
 zaza/openstack/charm_tests/keystone/tests.py | 14 ++++++++++----
 zaza/openstack/charm_tests/test_utils.py     | 12 ++++++------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 21a4740..c402a31 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -383,6 +383,12 @@ class LdapTests(BaseKeystoneTest):
 
     @staticmethod
     def _get_ldap_config():
+        """Generate ldap config for current model.
+
+        :return: tuple of whether ldap-server is running and if so, config
+            for the keystone-ldap application.
+        :rtype: Tuple[bool, Dict[str,str]]
+        """
         ldap_ips = zaza.model.get_app_ips("ldap-server")
         if ldap_ips:
             return True, {
@@ -397,10 +403,10 @@ class LdapTests(BaseKeystoneTest):
     def _find_keystone_v3_user(self, username, domain):
         """Find a user within a specified keystone v3 domain.
 
-        @param str username: Username to search for in keystone
-        @param str domain: username selected from which domain
-        @return: return username if found
-        @rtype: Optional[str]
+        :param str username: Username to search for in keystone
+        :param str domain: username selected from which domain
+        :return: return username if found
+        :rtype: Optional[str]
         """
         client = self.admin_keystone_client
         domain_users = client.users.list(
diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index 6c44b0c..48835f5 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -136,15 +136,15 @@ class OpenStackBaseTest(unittest.TestCase):
     def config_current(self, application_name=None, keys=None):
         """Get Current Config of an application normalized into key-values.
 
-        @param application_name: String application name for use when called
+        :param application_name: String application name for use when called
                                  by a charm under test other than the object's
                                  application.
-        @type application_name:  Optional[str]
-        @param keys: iterable of strs to index into the current config.  If
+        :type application_name:  Optional[str]
+        :param keys: iterable of strs to index into the current config.  If
                      None, return all keys from the config
-        @type keys:  Optional[Iterable[str]]
-        @return: Dictionary of requested config from application
-        @rtype: Dict[str, str]
+        :type keys:  Optional[Iterable[str]]
+        :return: Dictionary of requested config from application
+        :rtype: Dict[str, str]
         """
         if not application_name:
             application_name = self.application_name

From 70630568444c6691f8dba355b4dd2e87ec347ee8 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Thu, 23 Jan 2020 10:52:50 -0600
Subject: [PATCH 10/18]  use ':' style argument definitions rather than '@'

---
 zaza/openstack/charm_tests/keystone/tests.py | 41 ++++++++++----------
 zaza/openstack/charm_tests/test_utils.py     | 12 +++---
 2 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 21a4740..9959b2b 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -17,7 +17,6 @@ import collections
 import json
 import logging
 import pprint
-import unittest
 import keystoneauth1
 
 import zaza.model
@@ -381,26 +380,30 @@ class LdapTests(BaseKeystoneTest):
         """Run class setup for running Keystone ldap-tests."""
         super(LdapTests, cls).setUpClass()
 
-    @staticmethod
-    def _get_ldap_config():
+    def _get_ldap_config(self):
+        """Generate ldap config for current model.
+
+        :return: tuple of whether ldap-server is running and if so, config
+            for the keystone-ldap application.
+        :rtype: Tuple[bool, Dict[str,str]]
+        """
         ldap_ips = zaza.model.get_app_ips("ldap-server")
-        if ldap_ips:
-            return True, {
-                'ldap-server': "ldap://{}".format(ldap_ips[0]),
-                'ldap-user': 'cn=admin,dc=test,dc=com',
-                'ldap-password': 'crapper',
-                'ldap-suffix': 'dc=test,dc=com',
-                'domain-name': 'userdomain',
-            }
-        return False, {}
+        self.assertTrue(ldap_ips, "Should be at least one ldap server")
+        return {
+            'ldap-server': "ldap://{}".format(ldap_ips[0]),
+            'ldap-user': 'cn=admin,dc=test,dc=com',
+            'ldap-password': 'crapper',
+            'ldap-suffix': 'dc=test,dc=com',
+            'domain-name': 'userdomain',
+        }
 
     def _find_keystone_v3_user(self, username, domain):
         """Find a user within a specified keystone v3 domain.
 
-        @param str username: Username to search for in keystone
-        @param str domain: username selected from which domain
-        @return: return username if found
-        @rtype: Optional[str]
+        :param str username: Username to search for in keystone
+        :param str domain: username selected from which domain
+        :return: return username if found
+        :rtype: Optional[str]
         """
         client = self.admin_keystone_client
         domain_users = client.users.list(
@@ -420,11 +423,7 @@ class LdapTests(BaseKeystoneTest):
     def test_100_keystone_ldap_users(self):
         """Validate basic functionality of keystone API with ldap."""
         application_name = 'keystone-ldap'
-        can_config, config = self._get_ldap_config()
-        if not can_config:
-            raise unittest.SkipTest(
-                "Skipping API tests because there's no ldap-server"
-            )
+        config = self._get_ldap_config()
 
         with self.config_change(
                 self.config_current(application_name),
diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index 6c44b0c..48835f5 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -136,15 +136,15 @@ class OpenStackBaseTest(unittest.TestCase):
     def config_current(self, application_name=None, keys=None):
         """Get Current Config of an application normalized into key-values.
 
-        @param application_name: String application name for use when called
+        :param application_name: String application name for use when called
                                  by a charm under test other than the object's
                                  application.
-        @type application_name:  Optional[str]
-        @param keys: iterable of strs to index into the current config.  If
+        :type application_name:  Optional[str]
+        :param keys: iterable of strs to index into the current config.  If
                      None, return all keys from the config
-        @type keys:  Optional[Iterable[str]]
-        @return: Dictionary of requested config from application
-        @rtype: Dict[str, str]
+        :type keys:  Optional[Iterable[str]]
+        :return: Dictionary of requested config from application
+        :rtype: Dict[str, str]
         """
         if not application_name:
             application_name = self.application_name

From 0614ff5ad60db931fef4a0ee0b95565ff1d457b9 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Fri, 24 Jan 2020 12:59:58 -0600
Subject: [PATCH 11/18] integrate comments from @thedac to improve waiting for
 stable, and some documentation issues

---
 zaza/openstack/charm_tests/keystone/tests.py | 13 +++++--------
 zaza/openstack/charm_tests/test_utils.py     |  4 ++--
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 9959b2b..fd81736 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -23,7 +23,7 @@ import zaza.model
 import zaza.openstack.utilities.exceptions as zaza_exceptions
 import zaza.openstack.utilities.juju as juju_utils
 import zaza.openstack.utilities.openstack as openstack_utils
-
+import zaza.charm_lifecycle.utils as lifecycle_utils
 import zaza.openstack.charm_tests.test_utils as test_utils
 from zaza.openstack.charm_tests.keystone import (
     BaseKeystoneTest,
@@ -432,13 +432,10 @@ class LdapTests(BaseKeystoneTest):
             logging.info(
                 'Waiting for users to become available in keystone...'
             )
-            states = {
-                'keystone': {
-                    'workload-status': 'idle',
-                    'workload-status-messages': 'Unit is ready'
-                }
-            }
-            zaza.model.wait_for_application_states(states=states)
+            test_config = lifecycle_utils.get_charm_config(fatal=False)
+            zaza.model.wait_for_application_states(
+                states=test_config.get("target_deploy_status", {})
+            )
 
             # NOTE(jamespage): Test fixture should have johndoe and janedoe
             #                  accounts
diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index 48835f5..c787055 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -151,8 +151,8 @@ class OpenStackBaseTest(unittest.TestCase):
         _app_config = model.get_application_config(application_name)
         # convert the more elaborate config structure from libjuju to key-value
         keys = keys or _app_config.keys()
-        # note that conversion to string for all values is due to
-        # attempting to set any config with other types lead to Traceback
+        # note the conversion to str for all values is due to
+        # attempting to set any config with other type leads to Traceback
         return {
             str(k): str(_app_config.get(k, {}).get('value', ''))
             for k in keys

From b080f5ff59deae035f747375d00a54bde3ee60a2 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Mon, 27 Jan 2020 11:46:23 -0600
Subject: [PATCH 12/18] config_current doesn't need to flatten values to , but
 applying config to zaza.model does required all values are strings

---
 zaza/openstack/charm_tests/keystone/tests.py |  2 +-
 zaza/openstack/charm_tests/test_utils.py     | 31 ++++++++++++++------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index fd81736..9069775 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -426,7 +426,7 @@ class LdapTests(BaseKeystoneTest):
         config = self._get_ldap_config()
 
         with self.config_change(
-                self.config_current(application_name),
+                self.config_current(application_name, config.keys()),
                 config,
                 application_name=application_name):
             logging.info(
diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index c787055..b81e95a 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -16,7 +16,6 @@ import contextlib
 import logging
 import subprocess
 import unittest
-import zaza.model
 
 import zaza.model as model
 import zaza.charm_lifecycle.utils as lifecycle_utils
@@ -28,7 +27,7 @@ def skipIfNotHA(service_name):
     """Run decorator to skip tests if application not in HA configuration."""
     def _skipIfNotHA_inner_1(f):
         def _skipIfNotHA_inner_2(*args, **kwargs):
-            ips = zaza.model.get_app_ips(
+            ips = model.get_app_ips(
                 service_name)
             if len(ips) > 1:
                 return f(*args, **kwargs)
@@ -144,20 +143,34 @@ class OpenStackBaseTest(unittest.TestCase):
                      None, return all keys from the config
         :type keys:  Optional[Iterable[str]]
         :return: Dictionary of requested config from application
-        :rtype: Dict[str, str]
+        :rtype: Dict[str, Any]
         """
         if not application_name:
             application_name = self.application_name
+
         _app_config = model.get_application_config(application_name)
-        # convert the more elaborate config structure from libjuju to key-value
+
         keys = keys or _app_config.keys()
-        # note the conversion to str for all values is due to
-        # attempting to set any config with other type leads to Traceback
         return {
-            str(k): str(_app_config.get(k, {}).get('value', ''))
+            str(k): _app_config.get(k, {}).get('value', '')
             for k in keys
         }
 
+    @staticmethod
+    def _stringed_value_config(config):
+        """
+        Workaround:
+
+        libjuju refuses to accept data with types other than strings
+        through the zuzu.model.set_application_config
+
+        :param config: Config dictionary with any typed values
+        :type  config: Dict[str,Any]
+        :return:       Config Dictionary with string-ly typed values
+        :rtype:        Dict[str,str]
+        """
+        return {k: str(v) for k, v in config.items()}
+
     @contextlib.contextmanager
     def config_change(self, default_config, alternate_config,
                       application_name=None):
@@ -207,7 +220,7 @@ class OpenStackBaseTest(unittest.TestCase):
                           .format(alternate_config))
             model.set_application_config(
                 application_name,
-                alternate_config,
+                self._stringed_value_config(alternate_config),
                 model_name=self.model_name)
 
             logging.debug(
@@ -227,7 +240,7 @@ class OpenStackBaseTest(unittest.TestCase):
         logging.debug('Restoring charm setting to {}'.format(default_config))
         model.set_application_config(
             application_name,
-            default_config,
+            self._stringed_value_config(default_config),
             model_name=self.model_name)
 
         logging.debug(

From 056b2552fd999b04d1c246db670b0d82643ddaf9 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Mon, 27 Jan 2020 11:48:58 -0600
Subject: [PATCH 13/18] resolve pep8 issues

---
 zaza/openstack/charm_tests/test_utils.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index b81e95a..1f68c68 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -158,9 +158,9 @@ class OpenStackBaseTest(unittest.TestCase):
 
     @staticmethod
     def _stringed_value_config(config):
-        """
-        Workaround:
+        """Stringify values in a dict.
 
+        Workaround:
         libjuju refuses to accept data with types other than strings
         through the zuzu.model.set_application_config
 

From abfa96042fc845db57d9c087dede737e7766c48a Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Mon, 27 Jan 2020 12:30:08 -0600
Subject: [PATCH 14/18] also don't cast config keys to str

---
 zaza/openstack/charm_tests/test_utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index 1f68c68..eef3b77 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -152,7 +152,7 @@ class OpenStackBaseTest(unittest.TestCase):
 
         keys = keys or _app_config.keys()
         return {
-            str(k): _app_config.get(k, {}).get('value', '')
+            k: _app_config.get(k, {}).get('value', '')
             for k in keys
         }
 

From b7890be342b3204300127a5d5c9fcf00b836bb97 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Mon, 27 Jan 2020 13:15:59 -0600
Subject: [PATCH 15/18] _stringified config value of  --> ''

---
 zaza/openstack/charm_tests/test_utils.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py
index eef3b77..5903119 100644
--- a/zaza/openstack/charm_tests/test_utils.py
+++ b/zaza/openstack/charm_tests/test_utils.py
@@ -152,7 +152,7 @@ class OpenStackBaseTest(unittest.TestCase):
 
         keys = keys or _app_config.keys()
         return {
-            k: _app_config.get(k, {}).get('value', '')
+            k: _app_config.get(k, {}).get('value')
             for k in keys
         }
 
@@ -169,7 +169,12 @@ class OpenStackBaseTest(unittest.TestCase):
         :return:       Config Dictionary with string-ly typed values
         :rtype:        Dict[str,str]
         """
-        return {k: str(v) for k, v in config.items()}
+        # if v is None, stringify to ''
+        # otherwise use a strict cast with str(...)
+        return {
+            k: '' if v is None else str(v)
+            for k, v in config.items()
+        }
 
     @contextlib.contextmanager
     def config_change(self, default_config, alternate_config,

From 416c85b08ff8e0ec690a0df2b9ae466bf5508d56 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Tue, 28 Jan 2020 14:03:06 -0600
Subject: [PATCH 16/18] attempt to force keystone_v3 for ldap tests

---
 zaza/openstack/charm_tests/keystone/tests.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 9069775..12a8313 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -405,7 +405,10 @@ class LdapTests(BaseKeystoneTest):
         :return: return username if found
         :rtype: Optional[str]
         """
-        client = self.admin_keystone_client
+        client = openstack_utils.get_keystone_session_client(
+            self.admin_keystone_session,
+            client_api_version=3)
+
         domain_users = client.users.list(
             domain=client.domains.find(name=domain).id
         )

From 51f7f2cfa76e6ad18c0cf289751b8fcffd91f618 Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Wed, 29 Jan 2020 09:17:28 -0600
Subject: [PATCH 17/18] attempt to force ldap tests to use keystone v3 api

---
 .../charm_tests/keystone/__init__.py          | 10 ++++
 zaza/openstack/charm_tests/keystone/tests.py  | 55 ++++++++++---------
 2 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/__init__.py b/zaza/openstack/charm_tests/keystone/__init__.py
index e244e16..432eac2 100644
--- a/zaza/openstack/charm_tests/keystone/__init__.py
+++ b/zaza/openstack/charm_tests/keystone/__init__.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 """Collection of code for setting up and testing keystone."""
+import contextlib
 import zaza
 import zaza.openstack.charm_tests.test_utils as test_utils
 import zaza.openstack.utilities.openstack as openstack_utils
@@ -59,3 +60,12 @@ class BaseKeystoneTest(test_utils.OpenStackBaseTest):
             openstack_utils.get_keystone_session_client(
                 cls.admin_keystone_session,
                 client_api_version=cls.default_api_version))
+
+    @contextlib.contextmanager
+    def v3_keystone_preferred(self):
+        """Set the preferred keystone api to v3 within called context."""
+        with self.config_change(
+                {'preferred-api-version': self.default_api_version},
+                {'preferred-api-version': '3'},
+                application_name="keystone"):
+            yield
diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index 12a8313..fec4c82 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -188,10 +188,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest):
                 openstack_utils.get_os_release('trusty_mitaka')):
             logging.info('skipping test < trusty_mitaka')
             return
-        with self.config_change(
-                {'preferred-api-version': self.default_api_version},
-                {'preferred-api-version': '3'},
-                application_name="keystone"):
+        with self.v3_keystone_preferred():
             for ip in self.keystone_ips:
                 try:
                     logging.info('keystone IP {}'.format(ip))
@@ -221,10 +218,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest):
                 openstack_utils.get_os_release('xenial_ocata')):
             logging.info('skipping test < xenial_ocata')
             return
-        with self.config_change(
-                {'preferred-api-version': self.default_api_version},
-                {'preferred-api-version': '3'},
-                application_name="keystone"):
+        with self.v3_keystone_preferred():
             for ip in self.keystone_ips:
                 openrc = {
                     'API_VERSION': 3,
@@ -405,18 +399,28 @@ class LdapTests(BaseKeystoneTest):
         :return: return username if found
         :rtype: Optional[str]
         """
-        client = openstack_utils.get_keystone_session_client(
-            self.admin_keystone_session,
-            client_api_version=3)
+        for ip in self.keystone_ips:
+            try:
+                logging.info('keystone IP {}'.format(ip))
+                ks_session = openstack_utils.get_keystone_session(
+                    openstack_utils.get_overcloud_auth(address=ip))
+                ks_client = openstack_utils.get_keystone_session_client(
+                    ks_session)
+
+                domain_users = ks_client.users.list(
+                    domain=ks_client.domains.find(name=domain).id
+                )
+
+                usernames = []
+                for user in domain_users:
+                    usernames.append(user.name)
+                    if username.lower() == user.name.lower():
+                        return user
+            except keystoneauth1.exceptions.http.HTTPError as e:
+                raise zaza_exceptions.KeystoneAuthorizationStrict(
+                    'Retrieve domain list as admin FAILED. ({})'.format(e)
+                )
 
-        domain_users = client.users.list(
-            domain=client.domains.find(name=domain).id
-        )
-        usernames = []
-        for user in domain_users:
-            usernames.append(user.name)
-            if username.lower() == user.name.lower():
-                return user
         logging.debug(
             "User {} was not in these users: {}. Returning None."
             .format(username, usernames)
@@ -440,9 +444,10 @@ class LdapTests(BaseKeystoneTest):
                 states=test_config.get("target_deploy_status", {})
             )
 
-            # NOTE(jamespage): Test fixture should have johndoe and janedoe
-            #                  accounts
-            johndoe = self._find_keystone_v3_user('john doe', 'userdomain')
-            self.assertIsNotNone(johndoe, "user 'john doe' was unknown")
-            janedoe = self._find_keystone_v3_user('jane doe', 'userdomain')
-            self.assertIsNotNone(janedoe, "user 'jane doe' was unknown")
+            with self.v3_keystone_preferred():
+                # NOTE(jamespage): Test fixture should have johndoe and janedoe
+                #                  accounts
+                johndoe = self._find_keystone_v3_user('john doe', 'userdomain')
+                self.assertIsNotNone(johndoe, "user 'john doe' was unknown")
+                janedoe = self._find_keystone_v3_user('jane doe', 'userdomain')
+                self.assertIsNotNone(janedoe, "user 'jane doe' was unknown")

From 7f82aa3a80ab41bfc82697388959946e1510e03e Mon Sep 17 00:00:00 2001
From: Adam Dyess <adyess@gmail.com>
Date: Wed, 29 Jan 2020 10:30:27 -0600
Subject: [PATCH 18/18] simplify looking for user from keystone

---
 zaza/openstack/charm_tests/keystone/tests.py | 31 +++++++-------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py
index fec4c82..eda5441 100644
--- a/zaza/openstack/charm_tests/keystone/tests.py
+++ b/zaza/openstack/charm_tests/keystone/tests.py
@@ -400,30 +400,21 @@ class LdapTests(BaseKeystoneTest):
         :rtype: Optional[str]
         """
         for ip in self.keystone_ips:
-            try:
-                logging.info('keystone IP {}'.format(ip))
-                ks_session = openstack_utils.get_keystone_session(
-                    openstack_utils.get_overcloud_auth(address=ip))
-                ks_client = openstack_utils.get_keystone_session_client(
-                    ks_session)
+            logging.info('Keystone IP {}'.format(ip))
+            session = openstack_utils.get_keystone_session(
+                openstack_utils.get_overcloud_auth(address=ip))
+            client = openstack_utils.get_keystone_session_client(session)
 
-                domain_users = ks_client.users.list(
-                    domain=ks_client.domains.find(name=domain).id
-                )
+            domain_users = client.users.list(
+                domain=client.domains.find(name=domain).id
+            )
 
-                usernames = []
-                for user in domain_users:
-                    usernames.append(user.name)
-                    if username.lower() == user.name.lower():
-                        return user
-            except keystoneauth1.exceptions.http.HTTPError as e:
-                raise zaza_exceptions.KeystoneAuthorizationStrict(
-                    'Retrieve domain list as admin FAILED. ({})'.format(e)
-                )
+            usernames = [u.name.lower() for u in domain_users]
+            if username.lower() in usernames:
+                return username
 
         logging.debug(
-            "User {} was not in these users: {}. Returning None."
-            .format(username, usernames)
+            "User {} was not found. Returning None.".format(username)
         )
         return None