Add an option to randomize LDAP urls list

Since LDAP is now readonly, the current behavior might be
unexpected. By randomizing the list, we assure a more gradual
failure scenario if the first server on the list (as specified
by the user) fails.

Change-Id: I23f31bd85443784013a6aa158d80c7aeeb343993
Closes-Bug: #1953622
Resolves: rhbz#2024602
(cherry picked from commit 36d57d2a83)

doc/source/admin/integrate-with-ldap.inc

@@ -68,14 +68,32 @@ Define the destination LDAP server in the ``/etc/keystone/keystone.conf`` file:
    suffix = dc=example,dc=org
 
 
-Multiple LDAP servers can be supplied to ``url`` to provide high-availability
-support for a single LDAP backend. To specify multiple LDAP servers, simply
-change the ``url`` option in the ``[ldap]`` section to be a list, separated by
-commas:
+Although it's not recommended (see note below), multiple LDAP servers can be
+supplied to ``url`` to provide high-availability support for a single LDAP
+backend. By default, these will be tried in order of apperance, but an
+additional option, ``randomize_urls`` can be set to true, to randomize the
+list in each process (when it starts). To specify multiple LDAP servers,
+simply change the ``url`` option in the ``[ldap]`` section to be a list,
+separated by commas:
 
 .. code-block:: ini
 
    url = "ldap://localhost,ldap://backup.localhost"
+   randomize_urls = true
+
+.. NOTE::
+
+    Failover mechanisms in the LDAP backend can cause delays when switching
+    over to the next working LDAP server. Randomizing the order in which the
+    servers are tried only makes the failure behavior not dependent on which
+    of the ordered servers fail. Individual processes can still be delayed or
+    time out, so this doesn't fix the issue at hand, but only makes the
+    failure mode more gradual. This behavior cannot be easily fixed inside the
+    service, because keystone would have to monitor the status of each LDAP
+    server, which is in fact a task for a load balancer. Because of this, it
+    is recommended to use a load balancer in front of the LDAP servers,
+    which can monitor the state of the cluster and instantly redirect
+    connections to the working LDAP server.
 
 **Additional LDAP integration settings**
 

keystone/conf/ldap.py

@@ -24,6 +24,18 @@ as a comma separated string. The first URL to successfully bind is used for the
 connection.
 """))
 
+randomize_urls = cfg.BoolOpt(
+    'randomize_urls',
+    default=False,
+    help=utils.fmt("""
+Randomize the order of URLs in each keystone process. This makes the failure
+behavior more gradual, since if the first server is down, a process/thread
+will wait for the specified timeout before attempting a connection to a
+server further down the list. This defaults to False, for backward
+compatibility.
+"""))
+
+
 user = cfg.StrOpt(
     'user',
     help=utils.fmt("""
@@ -479,6 +491,7 @@ use_auth_pool` is also enabled.
 GROUP_NAME = __name__.split('.')[-1]
 ALL_OPTS = [
     url,
+    randomize_urls,
     user,
     password,
     suffix,

keystone/identity/backends/ldap/common.py

@@ -15,6 +15,7 @@
 import abc
 import codecs
 import os.path
+import random
 import re
 import sys
 import uuid
@@ -1166,6 +1167,11 @@ class BaseLdap(object):
     tree_dn = None
 
     def __init__(self, conf):
+        if conf.ldap.randomize_urls:
+            urls = re.split(r'[\s,]+', conf.ldap.url)
+            random.shuffle(urls)
+            self.LDAP_URL = ','.join(urls)
+        else:
             self.LDAP_URL = conf.ldap.url
         self.LDAP_USER = conf.ldap.user
         self.LDAP_PASSWORD = conf.ldap.password

keystone/tests/unit/identity/backends/test_ldap_common.py

@@ -245,6 +245,33 @@ class MultiURLTests(unit.TestCase):
         ldap_connection = base_ldap.get_connection()
         self.assertEqual(urls, ldap_connection.conn.conn_pool.uri)
 
+    @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'simple_bind_s')
+    def test_multiple_urls_with_comma_randomized(self, mock_ldap_bind):
+        urls = ('ldap://localhost1,ldap://localhost2,'
+                'ldap://localhost3,ldap://localhost4,'
+                'ldap://localhost5,ldap://localhost6,'
+                'ldap://localhost7,ldap://localhost8,'
+                'ldap://localhost9,ldap://localhost0')
+        self.config_fixture.config(group='ldap', url=urls,
+                                   randomize_urls=True)
+        base_ldap = common_ldap.BaseLdap(CONF)
+        ldap_connection = base_ldap.get_connection()
+
+        # Sanity check
+        self.assertEqual(len(urls.split(',')), 10)
+
+        # Check that the list is split into the same number of URIs
+        self.assertEqual(len(urls.split(',')),
+                         len(ldap_connection.conn.conn_pool.uri.split(',')))
+
+        # Check that the list is randomized
+        self.assertNotEqual(urls.split(','),
+                            ldap_connection.conn.conn_pool.uri.split(','))
+
+        # Check that the list contains the same URIs
+        self.assertEqual(set(urls.split(',')),
+                         set(ldap_connection.conn.conn_pool.uri.split(',')))
+
 
 class LDAPConnectionTimeoutTest(unit.TestCase):
     """Test for Network Connection timeout on LDAP URL connection."""

releasenotes/notes/randomize_urls-c0c19f48b2bfa299.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    A new option 'randomize_urls' can be used to randomize the order in which
+    keystone connects to the LDAP servers in [ldap] 'url' list.
+    It is false by default.