AMBARI-6909. Usability: Incorrect Postgres dependency causing issues (aonishuk)

ambari-server/src/main/python/ambari-server.py

@@ -250,7 +250,7 @@ PG_STATUS_RUNNING = utils.get_postgre_running_status(OS_TYPE)
 PG_DEFAULT_PASSWORD = "bigdata"
 SERVICE_CMD = "/usr/bin/env service"
 PG_SERVICE_NAME = "postgresql"
-PG_HBA_DIR = utils.get_postgre_hba_dir(OS_TYPE)
+PG_HBA_DIR = utils.get_postgre_hba_dir(OS_FAMILY)
 
 PG_ST_CMD = "%s %s status" % (SERVICE_CMD, PG_SERVICE_NAME)
 if os.path.isfile("/usr/bin/postgresql-setup"):

ambari-server/src/main/python/ambari_server/utils.py

@@ -21,12 +21,16 @@ import os
 import signal
 import sys
 import time
+import glob
+import subprocess
 from ambari_commons import OSConst
 
-#PostgreSQL settings
-UBUNTU_PG_HBA_ROOT = "/etc/postgresql"
-PG_HBA_ROOT_DEFAULT = "/var/lib/pgsql/data"
+# PostgreSQL settings
 PG_STATUS_RUNNING_DEFAULT = "running"
+PG_HBA_ROOT_DEFAULT = "/var/lib/pgsql/data"
+PG_HBA_INIT_FILES = {'debian': '/etc/postgresql',
+                     'redhat': '/etc/rc.d/init.d/postgresql',
+                     'suse': '/etc/init.d/postgresql'}
 
 #Environment
 ENV_PATH_DEFAULT = ['/bin', '/usr/bin', '/sbin', '/usr/sbin']  # default search path
@@ -167,25 +171,52 @@ def get_ubuntu_pg_version():
   """
   postgre_ver = ""
 
-  if os.path.isdir(UBUNTU_PG_HBA_ROOT):  # detect actual installed versions of PG and select a more new one
+  if os.path.isdir(PG_HBA_INIT_FILES[
+    'debian']):  # detect actual installed versions of PG and select a more new one
     postgre_ver = sorted(
-    [fld for fld in os.listdir(UBUNTU_PG_HBA_ROOT) if os.path.isdir(os.path.join(UBUNTU_PG_HBA_ROOT, fld))], reverse=True)
+      [fld for fld in os.listdir(PG_HBA_INIT_FILES[OSConst.DEBIAN_FAMILY]) if
+       os.path.isdir(os.path.join(PG_HBA_INIT_FILES[OSConst.DEBIAN_FAMILY], fld))],
+      reverse=True)
     if len(postgre_ver) > 0:
       return postgre_ver[0]
   return postgre_ver
 
 
-def get_postgre_hba_dir(OS):
-  """Return postgre hba dir location depends on OS"""
-  if OS == OSConst.OS_UBUNTU:
-    return os.path.join(UBUNTU_PG_HBA_ROOT, get_ubuntu_pg_version(), "main")
+def get_postgre_hba_dir(OS_FAMILY):
+  """Return postgre hba dir location depends on OS.
+  Also depends on version of postgres creates symlink like postgresql-->postgresql-9.3
+  1) /etc/rc.d/init.d/postgresql --> /etc/rc.d/init.d/postgresql-9.3
+  2) /etc/init.d/postgresql --> /etc/init.d/postgresql-9.1
+  """
+  if OS_FAMILY == OSConst.DEBIAN_FAMILY:
+    # Like: /etc/postgresql/9.1/main/
+    return os.path.join(PG_HBA_INIT_FILES[OS_FAMILY], get_ubuntu_pg_version(),
+                        "main")
   else:
-    return PG_HBA_ROOT_DEFAULT
-
-
-def get_postgre_running_status(OS):
+    if not os.path.isfile(PG_HBA_INIT_FILES[OS_FAMILY]):
+      # Link: /etc/init.d/postgresql --> /etc/init.d/postgresql-9.1
+      os.symlink(glob.glob(PG_HBA_INIT_FILES[OS_FAMILY] + '*')[0],
+                 PG_HBA_INIT_FILES[OS_FAMILY])
+
+    # Get postgres_data location (default: /var/lib/pgsql/data)
+    cmd = "alias exit=return; source " + PG_HBA_INIT_FILES[
+      OS_FAMILY] + " status &>/dev/null; echo $PGDATA"
+    p = subprocess.Popen(cmd,
+                         stdout=subprocess.PIPE,
+                         stdin=subprocess.PIPE,
+                         stderr=subprocess.PIPE,
+                         shell=True)
+    (PG_HBA_ROOT, err) = p.communicate()
+
+    if PG_HBA_ROOT and len(PG_HBA_ROOT.strip()) > 0:
+      return PG_HBA_ROOT.strip()
+    else:
+      return PG_HBA_ROOT_DEFAULT
+
+
+def get_postgre_running_status(OS_FAMILY):
   """Return postgre running status indicator"""
-  if OS == OSConst.OS_UBUNTU:
+  if OS_FAMILY == OSConst.DEBIAN_FAMILY:
     return os.path.join(get_ubuntu_pg_version(), "main")
   else:
     return PG_STATUS_RUNNING_DEFAULT

ambari-server/src/test/python/TestAmbariServer.py

@@ -31,11 +31,15 @@ import platform
 import shutil
 from pwd import getpwnam
 from ambari_server.resourceFilesKeeper import ResourceFilesKeeper, KeeperException
+ 
+# We have to use this import HACK because the filename contains a dash
 from ambari_commons import Firewall, OSCheck, OSConst, FirewallChecks
 
 with patch("platform.linux_distribution", return_value = ('Suse','11','Final')):
-  # We have to use this import HACK because the filename contains a dash
-  ambari_server = __import__('ambari-server')
+  with patch("os.symlink"):
+    with patch("__builtin__.open"):
+      with patch("glob.glob", return_value = ['/etc/init.d/postgresql-9.3']):
+        ambari_server = __import__('ambari-server')
 
 FatalException = ambari_server.FatalException
 NonFatalException = ambari_server.NonFatalException

ambari-server/src/test/python/TestOSCheck.py

@@ -30,9 +30,11 @@ from mock.mock import patch
 from ambari_commons import OSCheck, OSConst
 import os_check_type
 
-with patch("platform.linux_distribution", return_value=('Suse', '11', 'Final')):
-  # We have to use this import HACK because the filename contains a dash
-  ambari_server = __import__('ambari-server')
+utils = __import__('ambari_server.utils').utils
+# We have to use this import HACK because the filename contains a dash
+with patch("platform.linux_distribution", return_value = ('Suse','11','Final')):
+  with patch.object(utils, "get_postgre_hba_dir"):
+    ambari_server = __import__('ambari-server')
 
 
 class TestOSCheck(TestCase):

ambari-server/src/test/python/TestUtils.py

@@ -19,7 +19,7 @@ limitations under the License.
 import StringIO
 import sys
 from unittest import TestCase
-from mock.mock import patch
+from mock.mock import patch, MagicMock
 
 
 utils = __import__('ambari_server.utils').utils
@@ -35,20 +35,50 @@ class TestUtils(TestCase):
     self.assertEqual('9.1', utils.get_ubuntu_pg_version())
 
   @patch('ambari_server.utils.get_ubuntu_pg_version')
-  def test_get_postgre_hba_dir(self, get_ubuntu_pg_version_mock):
-    utils.UBUNTU_PG_HBA_ROOT = '/tmp'
-    utils.PG_HBA_ROOT_DEFAULT = '/redhat/postgre/data'
+  @patch('os.path.isfile')
+  @patch("subprocess.Popen")
+  def test_get_postgre_hba_dir(self, popenMock, os_path_is_fine_mock,
+                               get_ubuntu_pg_version_mock):
+    p = MagicMock()
+    utils.PG_HBA_INIT_FILES['debian'] = '/tmp'
     get_ubuntu_pg_version_mock.return_value = '9.1'
-
-    self.assertEqual('/tmp/9.1/main', utils.get_postgre_hba_dir('ubuntu'))
-    self.assertEqual('/redhat/postgre/data', utils.get_postgre_hba_dir('redhat'))
+    self.assertEqual('/tmp/9.1/main', utils.get_postgre_hba_dir('debian'))
+
+    # ## Tests depends on postgres version ###
+    # 1) PGDATA=/var/lib/pgsql/data
+    os_path_is_fine_mock.return_value = True
+    utils.PG_HBA_ROOT_DEFAULT = '/def/dir'
+    p.communicate.return_value = ('/my/new/location\n', None)
+    p.returncode = 0
+    popenMock.return_value = p
+    self.assertEqual('/my/new/location', utils.get_postgre_hba_dir('redhat'))
+
+    # 2) No value set
+    os_path_is_fine_mock.return_value = True
+    utils.PG_HBA_ROOT_DEFAULT = '/def/dir'
+    p.communicate.return_value = ('\n', None)
+    p.returncode = 0
+    popenMock.return_value = p
+    self.assertEqual('/def/dir', utils.get_postgre_hba_dir('redhat'))
+
+    # 3) Value set - check diff systems
+    os_path_is_fine_mock.return_value = True
+    popenMock.reset()
+    p.communicate.return_value = (None, None)
+    utils.get_postgre_hba_dir('redhat')
+    popenMock.assert_called_with('alias exit=return; source /etc/rc.d/init.d/postgresql status &>/dev/null; echo $PGDATA', shell=True, stdin=-1, stderr=-1, stdout=-1)
+
+    popenMock.reset()
+    p.communicate.return_value = (None, None)
+    utils.get_postgre_hba_dir('suse')
+    popenMock.assert_called_with('alias exit=return; source /etc/init.d/postgresql status &>/dev/null; echo $PGDATA', shell=True, stdin=-1, stderr=-1, stdout=-1)
 
   @patch('ambari_server.utils.get_ubuntu_pg_version')
   def test_get_postgre_running_status(self, get_ubuntu_pg_version_mock):
     utils.PG_STATUS_RUNNING_DEFAULT = "red_running"
     get_ubuntu_pg_version_mock.return_value = '9.1'
 
-    self.assertEqual('9.1/main', utils.get_postgre_running_status('ubuntu'))
+    self.assertEqual('9.1/main', utils.get_postgre_running_status('debian'))
     self.assertEqual('red_running', utils.get_postgre_running_status('redhat'))
 
   @patch('os.path.isfile')