fix(scheduler): move _get_port to use docker functions to split image info

Fixes #734

Author: helgi

rootfs/api/models/app.py

@@ -307,21 +307,26 @@ def scale(self, user, structure):  # noqa
 
     def _scale_pods(self, scale_types):
         release = self.release_set.latest()
+        envs = release.config.values
         for scale_type, replicas in scale_types.items():
+            # only web / cmd are routable
+            # http://docs.deis.io/en/latest/using_deis/process-types/#web-vs-cmd-process-types
+            routable = True if scale_type in ['web', 'cmd'] else False
+            # fetch application port and inject into ENV Vars as needed
+            envs['PORT'] = release.get_port(routable)
+
             image = release.image
-            version = "v{}".format(release.version)
             kwargs = {
                 'memory': release.config.memory,
                 'cpu': release.config.cpu,
                 'tags': release.config.tags,
-                'envs': release.config.values,
-                'version': version,
+                'envs': envs,
+                'version': "v{}".format(release.version),
                 'replicas': replicas,
                 'app_type': scale_type,
                 'build_type': release.build.type,
                 'healthcheck': release.config.healthcheck(),
-                # http://docs.deis.io/en/latest/using_deis/process-types/#web-vs-cmd-process-types
-                'routable': True if scale_type in ['web', 'cmd'] else False
+                'routable': routable
             }
 
             command = self._get_command(scale_type)
@@ -355,20 +360,26 @@ def deploy(self, release):
 
         # deploy application to k8s. Also handles initial scaling
         deploys = {}
+        envs = release.config.values
         for scale_type, replicas in self.structure.items():
+            # only web / cmd are routable
+            # http://docs.deis.io/en/latest/using_deis/process-types/#web-vs-cmd-process-types
+            routable = True if scale_type in ['web', 'cmd'] else False
+            # fetch application port and inject into ENV Vars as needed
+            envs['PORT'] = release.get_port(routable)
+
             deploys[scale_type] = {
                 'memory': release.config.memory,
                 'cpu': release.config.cpu,
                 'tags': release.config.tags,
-                'envs': release.config.values,
+                'envs': envs,
                 # only used if there is no previous RC
                 'replicas': replicas,
                 'version': "v{}".format(release.version),
                 'app_type': scale_type,
                 'build_type': release.build.type,
                 'healthcheck': release.config.healthcheck(),
-                # http://docs.deis.io/en/latest/using_deis/process-types/#web-vs-cmd-process-types
-                'routable': True if scale_type in ['web', 'cmd'] else False,
+                'routable': routable,
                 'batches': batches
             }
 

rootfs/api/models/release.py

@@ -3,6 +3,10 @@
 from django.conf import settings
 from django.db import models
 
+import docker
+from docker import Client
+from retrying import retry
+
 from registry import publish_release, RegistryException
 from api.utils import dict_diff
 from api.models import UuidAuditedModel, log_event, DeisException
@@ -104,18 +108,51 @@ def publish(self):
         if ':' not in source_image:
             source_image = "{}:{}".format(source_image, self.build.version)
 
-        # gather custom login information for registry if needed
+        # if build is source based then it was pushed into the deis registry
+        deis_registry = bool(self.build.source_based)
+        publish_release(source_image, self.image, deis_registry, self.get_registry_auth())
+
+    @retry(stop_max_attempt_number=3, wait_fixed=1000)
+    def get_port(self, routable=False):
+        """
+        Get a port from a Docker image
+        """
+        port = None
+        # Only care about port for routable application
+        if not routable:
+            return port
+
+        if self.build.type == "buildpack":
+            msg = "Using default port 5000 for build pack image {}".format(self.image)
+            log_event(self.app, msg)
+            return 5000
+
+        # get the target repository name and tag
+        repo, tag = docker.utils.parse_repository_tag(self.image)
+
+        docker_cli = Client(version="auto")
+        docker_cli.pull(repo, tag=tag, insecure_registry=True)
+        image_info = docker_cli.inspect_image(self.image)
+        if 'ExposedPorts' not in image_info['Config']:
+            return None
+
+        port = int(list(image_info['Config']['ExposedPorts'].keys())[0].split("/")[0])
+        return port
+
+    def get_registry_auth(self):
+        """
+        Gather login information for private registry if needed
+        """
         auth = None
-        if self.config.registry.get('username', None):
+        registry = self.config.registry
+        if registry.get('username', None):
             auth = {
-                'username': self.config.registry.get('username', None),
-                'password': self.config.registry.get('password', None),
+                'username': registry.get('username', None),
+                'password': registry.get('password', None),
                 'email': self.owner.email
             }
 
-        # if build is source based then it was pushed into the deis registry
-        deis_registry = bool(self.build.source_based)
-        publish_release(source_image, self.image, deis_registry, auth)
+        return auth
 
     def previous(self):
         """

rootfs/api/tests/test_app.py

@@ -31,7 +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)
+@mock.patch('api.models.release.Release.get_port', mock_port)
 class AppTest(APITestCase):
     """Tests creation of applications"""
 

rootfs/api/tests/test_build.py

@@ -25,7 +25,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)
+@mock.patch('api.models.release.Release.get_port', mock_port)
 class BuildTest(APITransactionTestCase):
 
     """Tests build notification from build system"""

rootfs/api/tests/test_config.py

@@ -21,7 +21,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)
+@mock.patch('api.models.release.Release.get_port', mock_port)
 class ConfigTest(APITransactionTestCase):
 
     """Tests setting and updating config values"""

rootfs/api/tests/test_hooks.py

@@ -35,7 +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)
+@mock.patch('api.models.release.Release.get_port', mock_port)
 class HookTest(APITransactionTestCase):
 
     """Tests API hooks used to trigger actions from external components"""

rootfs/api/tests/test_pods.py

@@ -24,7 +24,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)
+@mock.patch('api.models.release.Release.get_port', mock_port)
 class PodTest(APITransactionTestCase):
     """Tests creation of pods on nodes"""
 

rootfs/api/tests/test_release.py

@@ -23,7 +23,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)
+@mock.patch('api.models.release.Release.get_port', mock_port)
 class ReleaseTest(APITransactionTestCase):
 
     """Tests push notification from build system"""

rootfs/scheduler/__init__.py

@@ -8,12 +8,10 @@
 import base64
 
 from django.conf import settings
-from docker import Client
 from .states import JobState
 import requests
 from requests_toolbelt import user_agent
 from .utils import dict_merge
-from retrying import retry
 
 from deis import __version__ as deis_version
 
@@ -343,22 +341,9 @@ 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))
+        envs = kwargs.get('envs', {})
+        port = envs.get('PORT', None)
 
         # Fetch old RC and create the new one for a release
         old_rc = self._get_old_rc(namespace, app_type)
@@ -686,7 +671,7 @@ def _set_container(self, namespace, data, **kwargs):  # noqa
         if kwargs.get('healthcheck', None):
             self._healthcheck(namespace, data, kwargs.get('routable'), **kwargs['healthcheck'])
         else:
-            self._default_readiness_probe(data, kwargs.get('build_type'), kwargs.get('image'))
+            self._default_readiness_probe(data, kwargs.get('build_type'), kwargs.get('port', None))
 
     def resolve_state(self, pod):
         # See "Pod Phase" at http://kubernetes.io/v1.1/docs/user-guide/pod-states.html
@@ -722,19 +707,6 @@ 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 thrice to find the port before raising exception as docker-py is flaky
-        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)
-        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):
         """Return a fully-qualified Kubernetes API URL from a string template with args."""
         url = "/api/{}".format(self.apiversion) + tmpl.format(*args)
@@ -1165,12 +1137,12 @@ def _healthcheck(self, namespace, container, routable=False, path='/', port=5000
         # Update only the application container with the health check
         container.update(healthcheck)
 
-    def _default_readiness_probe(self, container, build_type, image):
+    def _default_readiness_probe(self, container, build_type, port=None):
         # Update only the application container with the health check
         if build_type == "buildpack":
             container.update(self._default_buildpack_readiness_probe())
-        else:
-            container.update(self._default_dockerapp_readiness_probe(image))
+        elif port:
+            container.update(self._default_dockerapp_readiness_probe(port))
 
     '''
     Applies exec readiness probe to the slugrunner container.
@@ -1213,15 +1185,8 @@ def _default_buildpack_readiness_probe(self, delay=30, timeout=5, period_seconds
     Applies tcp socket readiness probe to the docker app container only if some port is exposed
     by the docker image.
     '''
-    def _default_dockerapp_readiness_probe(self, image, delay=5, timeout=5, period_seconds=5,
+    def _default_dockerapp_readiness_probe(self, port, delay=5, timeout=5, period_seconds=5,
                                            success_threshold=1, failure_threshold=1):
-        try:
-            port = self._get_port(image)
-            if port is None:
-                return None
-        except Exception:
-            return None
-
         readinessprobe = {
             'readinessProbe': {
                 # an exec probe