fix(proctypes): update service after all the proctypes are deployed

Author: kmala

rootfs/api/models/app.py

@@ -469,6 +469,7 @@ def deploy(self, release, force_deploy=False):
             raise DeisException('No build associated with this release')
 
         app_settings = self.appsettings_set.latest()
+        service_annotations = {'maintenance': app_settings.maintenance}
 
         # use create to make sure minimum resources are created
         self.create()
@@ -518,7 +519,6 @@ def deploy(self, release, force_deploy=False):
                 'build_type': release.build.type,
                 'healthcheck': healthcheck,
                 'routable': routable,
-                'service_annotations': {'maintenance': app_settings.maintenance},
                 'deploy_batches': batches,
                 'deploy_timeout': deploy_timeout,
                 'deployment_history_limit': deployment_history,
@@ -529,18 +529,7 @@ def deploy(self, release, force_deploy=False):
         deploys = OrderedDict(sorted(deploys.items(), key=lambda d: d[1].get('routable')))
 
         # Check if any proc type has a Deployment in progress
-        for scale_type, kwargs in deploys.items():
-            if force_deploy:
-                continue
-
-            # Is there an existing deployment in progress?
-            name = self._get_job_id(scale_type)
-            in_progress, deploy_okay = self._scheduler.deployment_in_progress(
-                self.id, name, deploy_timeout, batches, replicas, tags
-            )
-            # throw a 409 if things are in progress but we do not want to let through the deploy
-            if in_progress and not deploy_okay:
-                raise AlreadyExists('Deployment for {} is already in progress'.format(name))
+        self._check_deployment_in_progress(deploys, force_deploy)
 
         try:
             # create the application config in k8s (secret in this case) for all deploy objects
@@ -565,15 +554,37 @@ def deploy(self, release, force_deploy=False):
             self.log(err, logging.ERROR)
             raise ServiceUnavailable(err) from e
 
-        # Wait until application is available in the router
-        # Only run when there is no previous build / release
-        old = release.previous()
-        if old is None or old.build is None:
-            self.verify_application_health(**kwargs)
+        app_type = 'web' if 'web' in deploys else 'cmd' if 'cmd' in deploys else None
+        # 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
+        if deploys and app_type:
+            routable = deploys[app_type].get('routable')
+            port = deploys[app_type].get('envs', {}).get('PORT', None)
+            self._update_application_service(self.id, app_type, port, routable, service_annotations)  # noqa
+
+            # Wait until application is available in the router
+            # Only run when there is no previous build / release
+            old = release.previous()
+            if old is None or old.build is None:
+                self.verify_application_health(**deploys[app_type])
 
         # cleanup old release objects from kubernetes
         release.cleanup_old()
 
+    def _check_deployment_in_progress(self, deploys, force_deploy=False):
+        if force_deploy:
+            return
+        for scale_type, kwargs in deploys.items():
+            # Is there an existing deployment in progress?
+            name = self._get_job_id(scale_type)
+            in_progress, deploy_okay = self._scheduler.deployment_in_progress(
+                self.id, name, kwargs.get("deploy_timeout"), kwargs.get("deploy_batches"),
+                kwargs.get("replicas"), kwargs.get("tags")
+            )
+            # throw a 409 if things are in progress but we do not want to let through the deploy
+            if in_progress and not deploy_okay:
+                raise AlreadyExists('Deployment for {} is already in progress'.format(name))
+
     def _default_structure(self, release):
         """Scale to default structure based on release type"""
         # if there is no SHA, assume a docker image is being promoted
@@ -869,3 +880,35 @@ def routable(self, routable):
         except KubeException as e:
             self._scheduler.update_service(self.id, self.id, data=old_service)
             raise ServiceUnavailable(str(e)) from e
+
+    def _update_application_service(self, namespace, app_type, port, routable=False, annotations={}):  # noqa
+        """Update application service with all the various required information"""
+        service = self._fetch_service_config(namespace)
+        old_service = service.copy()  # in case anything fails for rollback
+
+        try:
+            # Update service information
+            for key, value in annotations.items():
+                service['metadata']['annotations']['router.deis.io/%s' % key] = str(value)
+            if routable:
+                service['metadata']['labels']['router.deis.io/routable'] = 'true'
+            else:
+                # delete the annotation
+                service['metadata']['labels'].pop('router.deis.io/routable', None)
+
+            # Set app type if there is not one available
+            if 'type' not in service['spec']['selector']:
+                service['spec']['selector']['type'] = app_type
+
+            # Find if target port exists already, update / create as required
+            if routable:
+                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
+                        service['spec']['ports'][pos]['targetPort'] = int(port)
+
+            self._scheduler.update_service(namespace, namespace, data=service)
+        except Exception as e:
+            # Fix service to old port and app type
+            self._scheduler.update_service(namespace, namespace, data=old_service)
+            raise KubeException(str(e)) from e

rootfs/api/tests/test_app.py

@@ -498,6 +498,16 @@ def test_list_ordering(self, mock_requests):
         self.assertEqual(apps[2]['id'], 'tango')
         self.assertEqual(apps[3]['id'], 'zulu')
 
+    def test_app_service_metadata(self, mock_requests):
+        """
+        Test that application service has annotations and labels in the metadata
+        """
+        app_id = self.create_app()
+        app = App.objects.get(id=app_id)
+        svc = app._fetch_service_config(app_id)
+        self.assertIn('labels', svc['metadata'])
+        self.assertIn('annotations', svc['metadata'])
+
 
 FAKE_LOG_DATA = """
 2013-08-15 12:41:25 [33454] [INFO] Starting gunicorn 17.5

rootfs/api/tests/test_build.py

@@ -196,6 +196,22 @@ def test_build_default_containers(self, mock_requests):
         # pod name is auto generated so use regex
         self.assertRegex(container['name'], app_id + '-web-[0-9]{8,10}-[a-z0-9]{5}')
 
+        # start with a new app
+        app_id = self.create_app()
+        # post a new build with procfile and no routable type
+
+        url = "/v2/apps/{app_id}/builds".format(**locals())
+        body = {
+            'image': 'autotest/example',
+            'sha': 'a'*40,
+            'procfile': json.dumps({
+                'rake': 'node server.js',
+                'worker': 'node worker.js'
+            })
+        }
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+
     def test_build_str(self, mock_requests):
         """Test the text representation of a build."""
         app_id = self.create_app()

rootfs/scheduler/__init__.py

@@ -88,9 +88,6 @@ def deploy(self, namespace, name, image, entrypoint, command, **kwargs):  # noqa
         self.deploy_timeout = kwargs.get('deploy_timeout')
         app_type = kwargs.get('app_type')
         version = kwargs.get('version')
-        routable = kwargs.get('routable', False)
-        service_annotations = kwargs.get('service_annotations', {})
-        port = kwargs.get('envs', {}).get('PORT', None)
 
         # If an RC already exists then stop processing of the deploy
         try:
@@ -141,11 +138,6 @@ def deploy(self, namespace, name, image, entrypoint, command, **kwargs):  # noqa
                     "Additional information:\n{}".format(version, namespace, app_type, str(e))
                 ) from e
 
-        # 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, app_type, port, routable, service_annotations)  # noqa
-
     def cleanup_release(self, namespace, controller, timeout):
         """
         Cleans up resources related to an application deployment
@@ -186,38 +178,6 @@ def _get_deploy_batches(self, steps, desired):
 
         return batches
 
-    def _update_application_service(self, namespace, app_type, port, routable=False, annotations={}):  # noqa
-        """Update application service with all the various required information"""
-        service = self.get_service(namespace, namespace).json()
-        old_service = service.copy()  # in case anything fails for rollback
-
-        try:
-            # Update service information
-            for key, value in annotations.items():
-                service['metadata']['annotations']['router.deis.io/%s' % key] = str(value)
-            if routable:
-                service['metadata']['labels']['router.deis.io/routable'] = 'true'
-            else:
-                # delete the annotation
-                service['metadata']['labels'].pop('router.deis.io/routable', None)
-
-            # Set app type if there is not one available
-            if 'type' not in service['spec']['selector']:
-                service['spec']['selector']['type'] = app_type
-
-            # Find if target port exists already, update / create as required
-            if routable:
-                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
-                        service['spec']['ports'][pos]['targetPort'] = int(port)
-
-            self.update_service(namespace, namespace, data=service)
-        except Exception as e:
-            # Fix service to old port and app type
-            self.update_service(namespace, namespace, data=old_service)
-            raise KubeException(str(e)) from e
-
     def scale(self, namespace, name, image, entrypoint, command, **kwargs):
         """Scale Deployment"""
         self.deploy_timeout = kwargs.get('deploy_timeout')