From 4c2984373355be382b10498560a7d6aefed0252d Mon Sep 17 00:00:00 2001
From: Shay Arbov <sarbov@shayarbov.strato>
Date: Thu, 22 Jun 2017 13:19:51 +0300
Subject: [PATCH] Refactor health check methods to use requests

---
 docker_test_tools/utils.py | 28 ++++++++++++++++++----------
 requirements.txt           |  1 +
 test-requirements.txt      |  1 -
 tests/ut/test_utils.py     | 14 +++++++-------
 4 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/docker_test_tools/utils.py b/docker_test_tools/utils.py
index b6a3253..13b74c6 100644
--- a/docker_test_tools/utils.py
+++ b/docker_test_tools/utils.py
@@ -1,8 +1,7 @@
 import logging
-import subprocess
-
+import httplib
 import waiting
-
+import requests
 
 log = logging.getLogger(__name__)
 
@@ -28,29 +27,38 @@ def run_health_checks(checks, interval=1, timeout=60):
         return False
 
 
-def is_curl_responsive(address):
-    """Return True if the address is responsive using curl.
+def is_responsive(address, expected_status=httplib.OK):
+    """Return True if the address is responsive.
 
     :param string address: url address 'hostname:port'.
+    :param int expected_status: expected response status code.
     :return bool: True is the address is responsive, False otherwise.
     """
-    return subprocess.call(['curl', '-s', address], stdout=subprocess.PIPE, stderr=subprocess.PIPE) == 0
+    try:
+        return requests.get(address, timeout=5).status_code == expected_status
+    except:
+        return False
 
 
-def get_curl_health_check(service_name, url):
+def get_health_check(service_name, url, expected_status=httplib.OK):
     """Return a function used to determine if the given service is responsive.
 
     :param string service_name: service name.
     :param string url: service url 'hostname:port'.
+    :param int expected_status: expected response status code.
 
     :return function: function used to determine if the given service is responsive.
     """
     log.debug('Defining a CURL based health check for service: %s at: %s', service_name, url)
 
-    def curl_health_check():
+    def url_health_check():
         """Return True if the service is responsive."""
-        is_ready = is_curl_responsive(url)
+        is_ready = is_responsive(url, expected_status)
         log.debug('Service %s ready: %s', service_name, is_ready)
         return is_ready
 
-    return curl_health_check
+    return url_health_check
+
+
+# For backward compatability
+get_curl_health_check = get_health_check
\ No newline at end of file
diff --git a/requirements.txt b/requirements.txt
index 429ba41..b370733 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,5 @@
 pbr==1.8
 waiting==1.3.0
+requests==2.13.0
 docker-compose==1.13.0
 requests-unixsocket==0.1.5
diff --git a/test-requirements.txt b/test-requirements.txt
index fe3557b..ef1280f 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -3,6 +3,5 @@ nose2==0.6.5
 flake8==2.6.2
 pylint==1.7.1
 pytest==3.0.6
-requests==2.13.0
 cov-core==1.15.0
 docker-compose==1.11.2
\ No newline at end of file
diff --git a/tests/ut/test_utils.py b/tests/ut/test_utils.py
index b086688..c6cd631 100644
--- a/tests/ut/test_utils.py
+++ b/tests/ut/test_utils.py
@@ -1,21 +1,21 @@
 import mock
 import unittest
-import subprocess
 
 from waiting import TimeoutExpired
 from docker_test_tools import utils
 
 
 class TestUtils(unittest.TestCase):
-    @mock.patch('subprocess.call')
-    def test_run_health_checks(self, call_mock):
+    @mock.patch('requests.get')
+    def test_run_health_checks(self, get_mock):
         """Validate the run_health_checks function."""
-        call_mock.return_value = 0
+        get_mock.return_value = mock.MagicMock(status_code=200)
         utils.run_health_checks([utils.get_curl_health_check('service1', 'first_url'),
-                                 utils.get_curl_health_check('service2', 'second_url')])
+                                 utils.get_curl_health_check('service2', 'second_url')],
+                                timeout=5)
 
-        call_mock.assert_any_call(['curl', '-s', 'first_url'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        call_mock.assert_any_call(['curl', '-s', 'second_url'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        get_mock.assert_any_call('first_url', timeout=5)
+        get_mock.assert_any_call('second_url', timeout=5)
 
         with mock.patch("waiting.wait", return_value=True):
             self.assertTrue(utils.run_health_checks([]))