Browse Source

HUE-6160 [sentry] Refactor retry logic to ensure try-once round-robin, add tests

Jenny Kim 8 years ago
parent
commit
5f6b612c50

+ 53 - 38
desktop/libs/libsentry/src/libsentry/api.py

@@ -17,8 +17,6 @@
 
 import logging
 import threading
-import random
-import time
 
 from django.utils.translation import ugettext as _
 
@@ -26,66 +24,83 @@ from desktop.lib.exceptions import StructuredThriftTransportException
 from desktop.lib.exceptions_renderable import PopupException
 
 from libsentry.client import SentryClient
-from libsentry.sentry_site import get_sentry_client, is_ha_enabled
+from libsentry.sentry_ha import get_next_available_server, create_client
+from libsentry.sentry_site import get_sentry_server, is_ha_enabled
 
 
 LOG = logging.getLogger(__name__)
 
 API_CACHE = None
 API_CACHE_LOCK = threading.Lock()
-MAX_RETRIES = 15
 
 
 def ha_error_handler(func):
+
   def decorator(*args, **kwargs):
-    retries = 0
-    seed = random.random()
-
-    while retries <= MAX_RETRIES:
-      try:
-        return func(*args, **kwargs)
-      except StructuredThriftTransportException, e:
-        if not is_ha_enabled() or retries == MAX_RETRIES:
-          raise PopupException(_('Failed to retry connecting to an available Sentry server.'), detail=e)
+    try:
+      return func(*args, **kwargs)
+    except StructuredThriftTransportException, e:
+      if not is_ha_enabled():
+        raise PopupException(_('Failed to connect to Sentry server %s, and Sentry HA is not enabled.') % args[0].client.host, detail=e)
+      else:
+        LOG.warn("Failed to connect to Sentry server %s, will attempt to find next available host." % args[0].client.host)
+        server, attempts = get_next_available_server(SentryClient, args[0].client.username, args[0].client.host)
+        if server is not None:
+          args[0].client = create_client(SentryClient, args[0].client.username, server)
+          set_api_cache(server)
+          return func(*args, **kwargs)
         else:
-          LOG.info('Could not connect to Sentry server %s, attempting to fetch next available client.' % args[0].client.host)
-          time.sleep(1)
-          args[0].client = get_cached_client(args[0].client.username, retries=retries, seed=seed)
-          LOG.info('Picked %s at attempt %s' % (args[0].client, retries))
-          retries += 1
-      except SentryException, e:
-        raise e
-      except Exception, e:
-        raise PopupException(_('Encountered unexpected error in SentryApi.'), detail=e)
+          raise PopupException(_('Failed to find an available Sentry server.'))
+    except (SentryException, PopupException), e:
+      raise e
+    except Exception, e:
+      raise PopupException(_('Encountered unexpected error in SentryApi.'), detail=e)
 
   return decorator
 
 
-def get_api(user):
-  client = get_cached_client(user.username)
-  return SentryApi(client)
+def clear_api_cache():
+  global API_CACHE
+  if API_CACHE is not None:
+    LOG.info("Force resetting the currently cached Sentry server: %s:%s" % (API_CACHE['hostname'], API_CACHE['port']))
+    API_CACHE = None
 
 
-def get_cached_client(username, retries=0, seed=None):
-  exempt_host = None
-  force_reset = retries > 0
+def set_api_cache(server):
+  global API_CACHE
+  global API_CACHE_LOCK
+  API_CACHE_LOCK.acquire()
+  try:
+    API_CACHE = server
+    LOG.info("Setting cached Sentry server: %s:%s" % (API_CACHE['hostname'], API_CACHE['port']))
+  finally:
+    API_CACHE_LOCK.release()
+  return server
+
 
+def get_cached_server(current_host=None):
   global API_CACHE
-  if force_reset and API_CACHE is not None:
-    exempt_host = API_CACHE.host
-    LOG.info("Force resetting the cached Sentry client to exempt current host: %s" % exempt_host)
-    API_CACHE = None
+  if current_host and API_CACHE is not None:
+    clear_api_cache()
 
   if API_CACHE is None:
-    API_CACHE_LOCK.acquire()
-    try:
-      API_CACHE = get_sentry_client(username, SentryClient, exempt_host=exempt_host, retries=retries, seed=seed)
-      LOG.info("Setting cached Sentry client to host: %s" % API_CACHE.host)
-    finally:
-      API_CACHE_LOCK.release()
+    server = get_sentry_server(current_host)
+    if server is not None:
+      set_api_cache(server)
+    else:
+      raise PopupException(_('Failed to find an available Sentry server.'))
+  else:
+    LOG.debug("Returning cached Sentry server: %s:%s" % (API_CACHE['hostname'], API_CACHE['port']))
+
   return API_CACHE
 
 
+def get_api(user):
+  server = get_cached_server()
+  client = create_client(SentryClient, user.username, server)
+  return SentryApi(client)
+
+
 class SentryApi(object):
 
   def __init__(self, client):

+ 55 - 42
desktop/libs/libsentry/src/libsentry/api2.py

@@ -17,8 +17,6 @@
 
 import logging
 import threading
-import time
-import random
 
 from django.utils.translation import ugettext as _
 
@@ -26,71 +24,86 @@ from desktop.lib.exceptions import StructuredThriftTransportException
 from desktop.lib.exceptions_renderable import PopupException
 
 from libsentry.client2 import SentryClient
-from libsentry.sentry_site import get_sentry_client, is_ha_enabled
+from libsentry.sentry_ha import get_next_available_server, create_client
+from libsentry.sentry_site import get_sentry_server, is_ha_enabled
 
 
 LOG = logging.getLogger(__name__)
 
 API_CACHE = None
 API_CACHE_LOCK = threading.Lock()
-MAX_RETRIES = 15
 
 
 def ha_error_handler(func):
+
   def decorator(*args, **kwargs):
-    retries = 0
-    seed = random.random()
-
-    while retries <= MAX_RETRIES:
-      try:
-        return func(*args, **kwargs)
-      except StructuredThriftTransportException, e:
-        if not is_ha_enabled() or retries == MAX_RETRIES:
-          raise PopupException(_('Failed to retry connecting to an available Sentry server.'), detail=e)
+    try:
+      return func(*args, **kwargs)
+    except StructuredThriftTransportException, e:
+      if not is_ha_enabled():
+        raise PopupException(_('Failed to connect to Sentry server %s, and Sentry HA is not enabled.') % args[0].client.host, detail=e)
+      else:
+        LOG.warn("Failed to connect to Sentry server %s, will attempt to find next available host." % args[0].client.host)
+        server, attempts = get_next_available_server(SentryClient, args[0].client.username, args[0].client.host)
+        if server is not None:
+          args[0].client = create_client(SentryClient, args[0].client.username, server)
+          set_api_cache(server)
+          return func(*args, **kwargs)
         else:
-          LOG.info('Could not connect to Sentry server %s, attempting to fetch next available client.' % args[0].client.host)
-          time.sleep(1)
-          args[0].client = get_cached_client(args[0].client.username, retries=retries, seed=seed)
-          LOG.info('Picked %s at attempt %s' % (args[0].client, retries))
-          retries += 1
-      except SentryException, e:
-        raise e
-      except Exception, e:
-        raise PopupException(_('Encountered unexpected error in SentryApi.'), detail=e)
+          raise PopupException(_('Failed to find an available Sentry server.'))
+    except (SentryException, PopupException), e:
+      raise e
+    except Exception, e:
+      raise PopupException(_('Encountered unexpected error in SentryApi.'), detail=e)
 
   return decorator
 
 
-def get_api(user, component):
-  if component == 'solr':
-    component = component.upper()
-
-  client = get_cached_client(user.username, component=component)
+def clear_api_cache():
+  global API_CACHE
+  if API_CACHE is not None:
+    LOG.info("Force resetting the currently cached Sentry server: %s:%s" % (API_CACHE['hostname'], API_CACHE['port']))
+    API_CACHE = None
 
-  return SentryApi(client)
 
+def set_api_cache(server):
+  global API_CACHE
+  global API_CACHE_LOCK
+  API_CACHE_LOCK.acquire()
+  try:
+    API_CACHE = server
+    LOG.info("Setting cached Sentry server: %s:%s" % (API_CACHE['hostname'], API_CACHE['port']))
+  finally:
+    API_CACHE_LOCK.release()
+  return server
 
-def get_cached_client(username, component=None, retries=0, seed=None):
-  exempt_host = None
-  force_reset = retries > 0
 
+def get_cached_server(current_host=None):
   global API_CACHE
-  if force_reset and API_CACHE is not None:
-    exempt_host = API_CACHE.host
-    component = API_CACHE.component
-    LOG.info("Force resetting the cached Sentry client for component %s to exempt current host: %s" % (component, exempt_host))
-    API_CACHE = None
+  if current_host and API_CACHE is not None:
+    clear_api_cache()
 
   if API_CACHE is None:
-    API_CACHE_LOCK.acquire()
-    try:
-      API_CACHE = get_sentry_client(username, SentryClient, exempt_host=exempt_host, component=component, retries=retries, seed=seed)
-      LOG.info("Setting cached Sentry client to host: %s" % API_CACHE.host)
-    finally:
-      API_CACHE_LOCK.release()
+    server = get_sentry_server(current_host)
+    if server is not None:
+      set_api_cache(server)
+    else:
+      raise PopupException(_('Failed to find an available Sentry server.'))
+  else:
+    LOG.debug("Returning cached Sentry server: %s:%s" % (API_CACHE['hostname'], API_CACHE['port']))
+
   return API_CACHE
 
 
+def get_api(user, component):
+  if component == 'solr':
+    component = component.upper()
+
+  server = get_cached_server()
+  client = create_client(SentryClient, user.username, server, component)
+  return SentryApi(client)
+
+
 class SentryApi(object):
 
   def __init__(self, client):

+ 78 - 0
desktop/libs/libsentry/src/libsentry/sentry_ha.py

@@ -0,0 +1,78 @@
+#!/usr/bin/env python
+# Licensed to Cloudera, Inc. under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  Cloudera, Inc. licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+
+import logging
+import time
+
+from django.utils.translation import ugettext as _
+
+from desktop.lib.exceptions import StructuredThriftTransportException
+from desktop.lib.exceptions_renderable import PopupException
+
+from libsentry.client2 import SentryClient
+from libsentry.sentry_site import get_sentry_server, is_ha_enabled
+
+
+LOG = logging.getLogger(__name__)
+
+
+def create_client(client_class, username, server, component=None):
+  if server is not None:
+    kwargs = {
+      'host': server['hostname'],
+      'port': server['port'],
+      'username': username
+    }
+
+    if client_class == SentryClient:
+      kwargs.update({'component': component})
+
+    return client_class(**kwargs)
+  else:
+    raise PopupException(_('Cannot create a Sentry client without server hostname and port.'))
+
+
+def get_next_available_server(client_class, username, failed_host=None, component=None, create_client_fn=create_client):
+  '''
+  Given a failed host, attempts to find the next available host and returns a Sentry server if found, as well as a list
+  of all Sentry hosts attempted.
+  '''
+  current_host = failed_host
+  has_next = True
+  attempted_hosts = []
+
+  while has_next:
+    LOG.warn('Could not connect to Sentry server %s, attempting to fetch next available client.' % current_host)
+    next_server = get_sentry_server(current_host=current_host)
+    time.sleep(1)
+    try:
+      client = create_client_fn(client_class, username, next_server, component)
+      client.list_sentry_roles_by_group(groupName='*')
+      # If above operation succeeds, return client
+      LOG.info('Successfully connected to Sentry server %s, after attempting [%s], returning client.' % (client.host, ', '.join(attempted_hosts)))
+      return next_server, attempted_hosts
+    except StructuredThriftTransportException, e:
+      # If we have come back around to the original failed client, exit
+      if client.host == failed_host:
+        has_next = False
+      else:
+        current_host = client.host
+        attempted_hosts.append(current_host)
+    except Exception, e:
+      raise PopupException(_('Encountered unexpected error while trying to find available Sentry client: %s' % e))
+
+  return None, attempted_hosts

+ 27 - 21
desktop/libs/libsentry/src/libsentry/sentry_site.py

@@ -91,25 +91,36 @@ def get_sentry_server_rpc_addresses():
 
 
 def get_sentry_server_rpc_port():
-  return int(get_conf().get(_CONF_SENTRY_SERVER_RPC_PORT, '8038'))
+  return get_conf().get(_CONF_SENTRY_SERVER_RPC_PORT, '8038')
 
 
 def is_ha_enabled():
   return get_sentry_server_rpc_addresses() is not None
 
 
-def get_sentry_client(username, client_class, exempt_host=None, component=None, retries=0, seed=None):
-  server = None
-
+def get_sentry_server(current_host=None):
+  '''
+  Returns the next Sentry server if current_host is set, or a random server if current_host is None.
+    If servers contains a single server, the server will be set to the same current_host.
+    If servers is None, attempts to fallback to libsentry configs, else raises exception.
+  @param current_host: currently set host, if any
+  @return: server dict with hostname and port key/values
+  '''
   if is_ha_enabled():
-    servers = _get_server_properties(exempt_host=exempt_host)
-    seed_function = lambda: seed if seed else random.random()
-
-    random.shuffle(servers, seed_function)
-    if servers and retries < len(servers):
-      server = servers[retries]
-    else:
-      raise PopupException(_('Tried %s Sentry servers HA, none are available.') % retries)
+    servers = get_sentry_servers()
+    hosts = [s['hostname'] for s in servers]
+
+    next_idx = random.randint(0, len(servers)-1)
+    if current_host is not None and hosts:
+      try:
+        current_idx = hosts.index(current_host)
+        LOG.debug("Current Sentry host, %s, index is: %d." % (current_host, current_idx))
+        next_idx = (current_idx + 1) % len(servers)
+      except ValueError, e:
+        LOG.warn("Current host: %s not found in list of servers: %s" % (current_host, ','.join(hosts)))
+
+    server = servers[next_idx]
+    LOG.debug("Returning Sentry host, %s, at next index: %d." % (server['hostname'], next_idx))
   else:
     if HOSTNAME.get() and PORT.get():
       LOG.info('No Sentry servers configured in %s, falling back to libsentry configured host: %s:%s' %
@@ -121,15 +132,10 @@ def get_sentry_client(username, client_class, exempt_host=None, component=None,
     else:
       raise PopupException(_('No Sentry servers are configured.'))
 
-  if component:
-    client = client_class(server['hostname'], server['port'], username, component=component)
-  else:
-    client = client_class(server['hostname'], server['port'], username)
-
-  return client
+  return server
 
 
-def _get_server_properties(exempt_host=None):
+def get_sentry_servers():
   try:
     servers = []
     sentry_servers = get_sentry_server_rpc_addresses()
@@ -141,11 +147,11 @@ def _get_server_properties(exempt_host=None):
         port = get_sentry_server_rpc_port()
       else:
         port = PORT.get()
-      if host != exempt_host:
-        servers.append({'hostname': host, 'port': int(port)})
+      servers.append({'hostname': host, 'port': int(port)})
   except Exception, e:
     raise PopupException(_('Error in retrieving Sentry server properties.'), detail=e)
 
+  LOG.debug("Sentry servers are: %s" % ', '.join(['%s:%d' % (s['hostname'], s['port']) for s in servers]))
   return servers
 
 

+ 170 - 49
desktop/libs/libsentry/src/libsentry/tests.py

@@ -26,27 +26,42 @@ from nose.tools import assert_equal, assert_true, assert_false, assert_not_equal
 from django.contrib.auth.models import User
 
 from desktop.lib.django_test_util import make_logged_in_client
+from desktop.lib.exceptions import StructuredThriftTransportException
 from desktop.lib.exceptions_renderable import PopupException
 from desktop.lib.test_utils import add_to_group, grant_access
 
 from hadoop.pseudo_hdfs4 import is_live_cluster
 
 from libsentry import sentry_site
-from libsentry.api import get_api, API_CACHE
-from libsentry.api2 import get_api as get_api2, API_CACHE as API2_CACHE
+from libsentry.api import get_api, clear_api_cache
+from libsentry.api2 import get_api as get_api2, clear_api_cache as clear_api2_cache
 from libsentry.conf import is_enabled, HOSTNAME, PORT, SENTRY_CONF_DIR
 from libsentry.client import SentryClient
+from libsentry.sentry_ha import get_next_available_server
+from libsentry.sentry_site import get_sentry_server
 
 
-class TestWithSentry(object):
+def create_mock_client_fn(client_class, username, server, component=None):
+  class MockSentryClient(object):
+    def __init__(self, host):
+      self.host = host
+
+    def list_sentry_roles_by_group(self, groupName='*'):
+      if self.host.startswith('bad'):
+        raise StructuredThriftTransportException(ex=None)
+      else:
+        return []
+
+  if server is not None:
+    return MockSentryClient(server['hostname'])
+  else:
+    raise PopupException(_('Cannot create a Sentry client without server hostname and port.'))
 
-  requires_hadoop = True
+
+class TestWithSentry(object):
 
   @classmethod
   def setup_class(cls):
-    if not is_live_cluster():
-      raise SkipTest('Sentry tests require a live sentry server')
-
     if not os.path.exists(os.path.join(SENTRY_CONF_DIR.get(), 'sentry-site.xml')):
       raise SkipTest('Could not find sentry-site.xml, skipping sentry tests')
 
@@ -55,7 +70,6 @@ class TestWithSentry(object):
     add_to_group('test')
     grant_access("test", "test", "libsentry")
 
-    cls.db = SentryClient(HOSTNAME.get(), PORT.get(), 'test')
     cls.config_path = os.path.join(SENTRY_CONF_DIR.get(), 'sentry-site.xml')
 
 
@@ -69,10 +83,9 @@ class TestWithSentry(object):
     self.resets = [
       SENTRY_CONF_DIR.set_for_testing(self.tmpdir),
     ]
-    if API_CACHE is not None:
-      self.resets.append(API_CACHE.set_for_testing(None))
-    if API2_CACHE is not None:
-      self.resets.append(API2_CACHE.set_for_testing(None))
+
+    clear_api_cache()
+    clear_api2_cache()
 
 
   def tearDown(self):
@@ -82,61 +95,117 @@ class TestWithSentry(object):
     shutil.rmtree(self.tmpdir)
 
 
-  def test_get_collections(self):
-    resp = self.db.list_sentry_roles_by_group() # Non Sentry Admin can do that
-    assert_not_equal(0, resp.status.value, resp)
-    assert_true('denied' in resp.status.message, resp)
+  def test_get_random_sentry_server(self):
+    # Test that with no current_host, a server for a random host is returned
+    xml = self._sentry_site_xml(rpc_addresses='%s,host-1,host-2' % self.rpc_addresses, rpc_port=self.rpc_port)
+    file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
+    sentry_site.reset()
 
-    resp = self.db.list_sentry_roles_by_group(groupName='*')
-    assert_equal(0, resp.status.value, resp)
+    server = get_sentry_server()
+    assert_true(server is not None)
+    assert_true(server['hostname'] in '%s,host-1,host-2' % self.rpc_addresses)
 
 
-  def test_ha_failover_good_bad_bad(self):
-    # Test with good-host,bad-host-1,bad-host-2
-    xml = self._sentry_site_xml(rpc_addresses='%s,bad-host-1:8039,bad-host-2' % self.rpc_addresses, rpc_port=self.rpc_port)
+  def test_get_single_sentry_server(self):
+    # Test that with a current host and single server, the single server is returned
+    xml = self._sentry_site_xml(rpc_addresses='host-1', rpc_port=self.rpc_port)
     file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
     sentry_site.reset()
 
-    api = get_api(self.user)
-    assert_equal('%s,bad-host-1:8039,bad-host-2' % self.rpc_addresses, ','.join(sentry_site.get_sentry_server_rpc_addresses()))
-    resp = api.list_sentry_roles_by_group(groupName='*')
-    assert_true(isinstance(resp, list))
+    server = get_sentry_server(current_host='host-1')
+    assert_true(server is not None)
+    assert_equal(server['hostname'], 'host-1')
 
-    api2 = get_api2(self.user, 'solr')
-    resp = api2.list_sentry_roles_by_group(groupName='*')
-    assert_true(isinstance(resp, list))
 
+  def test_get_next_sentry_server(self):
+    # Test that with a current host and multiple servers, the next server is returned
+    xml = self._sentry_site_xml(rpc_addresses='%s,host-1,host-2' % self.rpc_addresses, rpc_port=self.rpc_port)
+    file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
+    sentry_site.reset()
+
+    server = get_sentry_server(current_host='host-1')
+    assert_true(server is not None)
+    assert_equal(server['hostname'], 'host-2')
 
-  def test_ha_failover_bad_bad_good(self):
-    # Test with bad-host-1,bad-host-2,good-host
-    xml = self._sentry_site_xml(rpc_addresses='bad-host-1:8039,bad-host-2,%s' % self.rpc_addresses, rpc_port=self.rpc_port)
+
+  def test_get_first_sentry_server(self):
+    # Test that if the current host is the last host of multiple servers, the first server is returned
+    xml = self._sentry_site_xml(rpc_addresses='host-1,%s,host-2' % self.rpc_addresses, rpc_port=self.rpc_port)
     file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
     sentry_site.reset()
 
-    api = get_api(self.user)
-    assert_equal('bad-host-1:8039,bad-host-2,%s' % self.rpc_addresses, ','.join(sentry_site.get_sentry_server_rpc_addresses()))
-    resp = api.list_sentry_roles_by_group(groupName='*')
-    assert_true(isinstance(resp, list))
+    server = get_sentry_server(current_host='host-2')
+    assert_true(server is not None)
+    assert_equal(server['hostname'], 'host-1')
 
-    api2 = get_api2(self.user, 'solr')
-    resp = api2.list_sentry_roles_by_group(groupName='*')
-    assert_true(isinstance(resp, list))
 
+  def test_round_robin(self):
+    # Test that get_next_available_client will check each server once and only once then exit
+    xml = self._sentry_site_xml(rpc_addresses='host-1,host-2,host-3,host-4,host-5', rpc_port=self.rpc_port)
+    file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
+    sentry_site.reset()
+
+    server, attempts = get_next_available_server(SentryClient, self.user.username, failed_host='host-1')
+    assert_equal(None, server)
+    assert_equal(['host-2','host-3','host-4','host-5'], attempts)
 
-  def test_ha_failover_bad_good_bad(self):
-    # Test with bad-host-1,good-host,bad-host-2
-    xml = self._sentry_site_xml(rpc_addresses='bad-host-1:8039,%s,bad-host-2' % self.rpc_addresses, rpc_port=self.rpc_port)
+
+  def test_get_next_good_host(self):
+    # Test that get_next_available_client will return the next good/successful server
+    xml = self._sentry_site_xml(rpc_addresses='bad-host-1,good-host-1,bad-host-2,good-host-2,good-host-3', rpc_port=self.rpc_port)
     file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
     sentry_site.reset()
 
-    api = get_api(self.user)
-    assert_equal('bad-host-1:8039,%s,bad-host-2' % self.rpc_addresses, ','.join(sentry_site.get_sentry_server_rpc_addresses()))
-    resp = api.list_sentry_roles_by_group(groupName='*')
-    assert_true(isinstance(resp, list))
+    server, attempts = get_next_available_server(SentryClient, self.user.username, failed_host='bad-host-2',
+                                                 create_client_fn=create_mock_client_fn)
+    assert_equal('good-host-2', server['hostname'])
+    assert_equal([], attempts)
 
-    api2 = get_api2(self.user, 'solr')
-    resp = api2.list_sentry_roles_by_group(groupName='*')
-    assert_true(isinstance(resp, list))
+
+  def test_single_good_host(self):
+    # Test that get_next_available_client will return the single good host on first try
+    xml = self._sentry_site_xml(rpc_addresses='good-host-1', rpc_port=self.rpc_port)
+    file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
+    sentry_site.reset()
+
+    server, attempts = get_next_available_server(SentryClient, self.user.username, failed_host=None,
+                                                 create_client_fn=create_mock_client_fn)
+    assert_equal('good-host-1', server['hostname'])
+    assert_equal([], attempts)
+
+
+  def test_single_bad_host(self):
+    # Test that get_next_available_client will raise an exception on single bad host
+    xml = self._sentry_site_xml(rpc_addresses='bad-host-1', rpc_port=self.rpc_port)
+    file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
+    sentry_site.reset()
+
+    assert_raises(PopupException, get_next_available_server, SentryClient, self.user.username, failed_host=None,
+                  create_client_fn=create_mock_client_fn)
+
+
+  def test_bad_good_host(self):
+    # Test that get_next_available_client will return the good host
+    xml = self._sentry_site_xml(rpc_addresses='bad-host-1,good-host-1', rpc_port=self.rpc_port)
+    file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
+    sentry_site.reset()
+
+    server, attempts = get_next_available_server(SentryClient, self.user.username, failed_host='bad-host-1',
+                                                 create_client_fn=create_mock_client_fn)
+    assert_equal('good-host-1', server['hostname'])
+    assert_equal([], attempts)
+
+
+  def test_good_bad_host(self):
+    # Test that get_next_available_client will return the good host
+    xml = self._sentry_site_xml(rpc_addresses='good-host-1,bad-host-1', rpc_port=self.rpc_port)
+    file(os.path.join(self.tmpdir, 'sentry-site.xml'), 'w').write(xml)
+    sentry_site.reset()
+
+    server, attempts = get_next_available_server(SentryClient, self.user.username, failed_host='bad-host-1',
+                                                 create_client_fn=create_mock_client_fn)
+    assert_equal('good-host-1', server['hostname'])
+    assert_equal([], attempts)
 
 
   def test_ha_failover_all_bad(self):
@@ -182,4 +251,56 @@ class TestWithSentry(object):
       elif name.text == 'sentry.service.client.server.rpc-port':
         value = prop.find('value')
         value.text = rpc_port
-    return lxml.etree.tostring(root)
+    return lxml.etree.tostring(root)
+
+
+class TestSentryWithHadoop(object):
+
+  requires_hadoop = True
+
+  @classmethod
+  def setup_class(cls):
+    if not is_live_cluster():
+      raise SkipTest('TestSentryWithHadoop requires a live cluster.')
+
+    if not os.path.exists(os.path.join(SENTRY_CONF_DIR.get(), 'sentry-site.xml')):
+      raise SkipTest('Could not find sentry-site.xml, skipping sentry tests')
+
+    cls.client = make_logged_in_client(username='test', is_superuser=False)
+    cls.user = User.objects.get(username='test')
+    add_to_group('test')
+    grant_access("test", "test", "libsentry")
+
+    cls.config_path = os.path.join(SENTRY_CONF_DIR.get(), 'sentry-site.xml')
+
+
+  def setUp(self):
+    self.rpc_addresses = ''
+    if sentry_site.get_sentry_server_rpc_addresses() is not None:
+      self.rpc_addresses = ','.join(sentry_site.get_sentry_server_rpc_addresses())
+    self.rpc_port = sentry_site.get_sentry_server_rpc_port() or '8038'
+
+    self.tmpdir = tempfile.mkdtemp()
+    self.resets = [
+      SENTRY_CONF_DIR.set_for_testing(self.tmpdir),
+    ]
+
+    clear_api_cache()
+    clear_api2_cache()
+
+
+  def tearDown(self):
+    sentry_site.reset()
+    for reset in self.resets:
+      reset()
+    shutil.rmtree(self.tmpdir)
+
+
+  def test_get_collections(self):
+    client = SentryClient(HOSTNAME.get(), PORT.get(), 'test')
+    resp = client.list_sentry_roles_by_group() # Non Sentry Admin can do that
+    assert_not_equal(0, resp.status.value, resp)
+    assert_true('denied' in resp.status.message, resp)
+
+    resp = client.list_sentry_roles_by_group(groupName='*')
+    assert_equal(0, resp.status.value, resp)