Merge pull request #689 from kmala/proc

fix(port): retry a fixed number of times to find the port

Author: kmala

rootfs/api/tests/__init__.py

@@ -7,6 +7,10 @@
 from django.test.runner import DiscoverRunner
 
 
+def mock_port(*args, **kwargs):
+    return 5000
+
+
 # Mock out router requests and add in some jitter
 # Used for application is available in router checks
 def fake_responses(request, context):

rootfs/api/tests/test_app.py

@@ -17,6 +17,7 @@
 from scheduler import KubeException
 
 from . import adapter
+from . import mock_port
 import requests_mock
 
 
@@ -30,6 +31,7 @@ def _mock_run(*args, **kwargs):
 
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
+@mock.patch('scheduler.KubeHTTPClient._get_port', mock_port)
 class AppTest(APITestCase):
     """Tests creation of applications"""
 

rootfs/api/tests/test_build.py

@@ -19,11 +19,13 @@
 from scheduler import KubeException
 
 from . import adapter
+from . import mock_port
 import requests_mock
 
 
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
+@mock.patch('scheduler.KubeHTTPClient._get_port', mock_port)
 class BuildTest(APITransactionTestCase):
 
     """Tests build notification from build system"""

rootfs/api/tests/test_config.py

@@ -15,11 +15,13 @@
 from api.models import App, Config
 
 from . import adapter
+from . import mock_port
 import requests_mock
 
 
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
+@mock.patch('scheduler.KubeHTTPClient._get_port', mock_port)
 class ConfigTest(APITransactionTestCase):
 
     """Tests setting and updating config values"""

rootfs/api/tests/test_hooks.py

@@ -11,6 +11,7 @@
 from rest_framework.authtoken.models import Token
 
 from . import adapter
+from . import mock_port
 import requests_mock
 
 RSA_PUBKEY = (
@@ -34,6 +35,7 @@
 
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
+@mock.patch('scheduler.KubeHTTPClient._get_port', mock_port)
 class HookTest(APITransactionTestCase):
 
     """Tests API hooks used to trigger actions from external components"""

rootfs/api/tests/test_pods.py

@@ -18,11 +18,13 @@
 from scheduler import KubeException
 
 from . import adapter
+from . import mock_port
 import requests_mock
 
 
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
+@mock.patch('scheduler.KubeHTTPClient._get_port', mock_port)
 class PodTest(APITransactionTestCase):
     """Tests creation of pods on nodes"""
 

rootfs/api/tests/test_release.py

@@ -17,11 +17,13 @@
 from api.models import Release
 from scheduler import KubeHTTPException
 from . import adapter
+from . import mock_port
 import requests_mock
 
 
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
+@mock.patch('scheduler.KubeHTTPClient._get_port', mock_port)
 class ReleaseTest(APITransactionTestCase):
 
     """Tests push notification from build system"""

rootfs/requirements.txt

@@ -16,3 +16,4 @@ PyYAML==3.11
 requests==2.10.0
 requests-toolbelt==0.6.0
 simpleflock==0.0.3
+retrying==1.3.3

rootfs/scheduler/__init__.py

@@ -13,6 +13,7 @@
 import requests
 from requests_toolbelt import user_agent
 from .utils import dict_merge
+from retrying import retry
 
 from deis import __version__ as deis_version
 
@@ -347,7 +348,22 @@ def __init__(self):
     def deploy(self, namespace, name, image, command, **kwargs):  # noqa
         logger.debug('deploy {}, img {}, params {}, cmd "{}"'.format(name, image, kwargs, command))
         app_type = kwargs.get('app_type')
+        build_type = kwargs.get('build_type')
         routable = kwargs.get('routable', False)
+        port = None
+
+        try:
+            if routable:
+                if build_type == "buildpack":
+                    logger.debug("Using default port 5000 for build pack app {}".format(name))
+                    port = 5000
+                else:
+                    port = self._get_port(image)
+                    if port is None:
+                        raise Exception("Expose a port or make the app non routable by changing"
+                                        " the process type")
+        except Exception as e:
+            raise KubeException('{} (scheduler::deploy): {}'.format(name, e))
 
         # Fetch old RC and create the new one for a release
         old_rc = self._get_old_rc(namespace, app_type)
@@ -425,9 +441,9 @@ def deploy(self, namespace, name, image, command, **kwargs):  # noqa
         # Make sure the application is routable and uses the correct port
         # Done after the fact to let initial deploy settle before routing
         # traffic to the application
-        self._update_application_service(namespace, name, app_type, image, routable)
+        self._update_application_service(namespace, name, app_type, port, routable)
 
-    def _update_application_service(self, namespace, name, app_type, image, routable=False):
+    def _update_application_service(self, namespace, name, app_type, port, routable=False):
         """Update application service with all the various required information"""
         try:
             # Fetch service
@@ -444,10 +460,6 @@ def _update_application_service(self, namespace, name, app_type, image, routable
 
             # Find if target port exists already, update / create as required
             if routable:
-                port = self._get_port(image)
-                if port is None:
-                    logger.debug("Failed to find port for Docker image {}, defaulting to 5000".format(image))  # noqa
-                    port = 5000
                 for pos, item in enumerate(service['spec']['ports']):
                     if item['port'] == 80 and port != item['targetPort']:
                         # port 80 is the only one we care about right now
@@ -689,18 +701,18 @@ def resolve_state(self, pod):
 
         return states.get(pod_state, pod_state)
 
+    @retry(stop_max_attempt_number=3, wait_fixed=1000)
     def _get_port(self, image):
-        try:
-            image = self.registry + '/' + image
-            repo = image.split(":")
-            # image already includes the tag, so we split it out here
-            docker_cli = Client(version="auto")
-            docker_cli.pull(repo[0]+":"+repo[1], tag=repo[2], insecure_registry=True)
-            image_info = docker_cli.inspect_image(image)
-            port = int(list(image_info['Config']['ExposedPorts'].keys())[0].split("/")[0])
-        except Exception:
-            port = None
-
+        # try thrice to find the port before raising exception as docker-py is flaky
+        imagepath = self.registry + '/' + image
+        repo = imagepath.split(":")
+        # image already includes the tag, so we split it out here
+        docker_cli = Client(version="auto")
+        docker_cli.pull(repo[0]+":"+repo[1], tag=repo[2], insecure_registry=True)
+        image_info = docker_cli.inspect_image(imagepath)
+        if 'ExposedPorts' not in image_info['Config']:
+            return None
+        port = int(list(image_info['Config']['ExposedPorts'].keys())[0].split("/")[0])
         return port
 
     def _api(self, tmpl, *args):
@@ -1199,8 +1211,11 @@ def _default_buildpack_readiness_probe(self, delay=30, timeout=5, period_seconds
     '''
     def _default_dockerapp_readiness_probe(self, image, delay=5, timeout=5, period_seconds=5,
                                            success_threshold=1, failure_threshold=1):
-        port = self._get_port(image)
-        if port is None:
+        try:
+            port = self._get_port(image)
+            if port is None:
+                return None
+        except Exception:
             return None
         readinessprobe = {
             'readinessProbe': {