Sort deploys so routable application types come first

Author: helgi

rootfs/api/models/app.py

@@ -1,3 +1,4 @@
+from collections import OrderedDict
 from datetime import datetime
 import logging
 import random
@@ -341,31 +342,33 @@ def deploy(self, user, release):
             self.save()
 
         # deploy application to k8s. Also handles initial scaling
+        deploys = {}
         build_type = app_build_type(release)
         for scale_type in self.structure.keys():
-            image = release.image
-            version = "v{}".format(release.version)
-            kwargs = {
+            deploys[scale_type] = {
                 'memory': release.config.memory,
                 'cpu': release.config.cpu,
                 'tags': release.config.tags,
                 'envs': release.config.values,
                 'replicas': 0,  # Scaling up happens in a separate operation
-                'version': version,
+                'version': "v{}".format(release.version),
                 'app_type': scale_type,
                 'build_type': 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
             }
 
-            command = self._get_command(scale_type)
+        # Sort deploys so routable comes first
+        deploys = OrderedDict(sorted(deploys.items(), key=lambda d: d[1].get('routable')))
+
+        for scale_type, kwargs in deploys.items():
             try:
                 self._scheduler.deploy(
                     namespace=self.id,
                     name=self._get_job_id(scale_type),
-                    image=image,
-                    command=command,
+                    image=release.image,
+                    command=self._get_command(scale_type),
                     **kwargs
                 )
 

rootfs/api/tests/test_app.py

@@ -14,6 +14,9 @@
 
 from api.models import App
 
+from . import adapter
+import requests_mock
+
 
 def mock_none(*args, **kwargs):
     return None
@@ -38,7 +41,7 @@ def tearDown(self):
         # make sure every test has a clean slate for k8s mocking
         cache.clear()
 
-    def test_app(self):
+    def test_app(self, mock_requests):
         """
         Test that a user can create, read, update and delete an application
         """
@@ -59,7 +62,7 @@ def test_app(self):
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 204)
 
-    def test_response_data(self):
+    def test_response_data(self, mock_requests):
         """Test that the serialized response contains only relevant data."""
         body = {'id': 'test'}
         response = self.client.post('/v2/apps', body)
@@ -72,7 +75,7 @@ def test_response_data(self):
         }
         self.assertDictContainsSubset(expected, response.data)
 
-    def test_app_override_id(self):
+    def test_app_override_id(self, mock_requests):
         body = {'id': 'myid'}
         response = self.client.post('/v2/apps', body)
         self.assertEqual(response.status_code, 201)
@@ -82,7 +85,7 @@ def test_app_override_id(self):
         return response
 
     @mock.patch('requests.get')
-    def test_app_actions(self, mock_get):
+    def test_app_actions(self, mock_requests, mock_get):
         url = '/v2/apps'
         body = {'id': 'autotest'}
         response = self.client.post(url, body)
@@ -126,7 +129,7 @@ def test_app_actions(self, mock_get):
         # TODO: test run needs an initial build
 
     @mock.patch('api.models.logger')
-    def test_app_release_notes_in_logs(self, mock_logger):
+    def test_app_release_notes_in_logs(self, mock_requests, mock_logger):
         """Verifies that an app's release summary is dumped into the logs."""
         url = '/v2/apps'
         body = {'id': 'autotest'}
@@ -137,7 +140,7 @@ def test_app_release_notes_in_logs(self, mock_logger):
         exp_log_call = mock.call(logging.INFO, exp_msg)
         mock_logger.log.has_calls(exp_log_call)
 
-    def test_app_errors(self):
+    def test_app_errors(self, mock_requests):
         app_id = 'autotest-errors'
         url = '/v2/apps'
         body = {'id': 'camelCase'}
@@ -160,7 +163,7 @@ def test_app_errors(self):
             response = self.client.get(url)
             self.assertEqual(response.status_code, 404)
 
-    def test_app_reserved_names(self):
+    def test_app_reserved_names(self, mock_requests):
         """Nobody should be able to create applications with names which are reserved."""
         url = '/v2/apps'
         reserved_names = ['foo', 'bar']
@@ -173,7 +176,7 @@ def test_app_reserved_names(self):
                     '{} is a reserved name.'.format(name),
                     status_code=400)
 
-    def test_app_structure_is_valid_json(self):
+    def test_app_structure_is_valid_json(self, mock_requests):
         """Application structures should be valid JSON objects."""
         url = '/v2/apps'
         response = self.client.post(url)
@@ -190,7 +193,7 @@ def test_app_structure_is_valid_json(self):
         self.assertEqual(response.data['structure'], {"web": 1})
 
     @mock.patch('api.models.logger')
-    def test_admin_can_manage_other_apps(self, mock_logger):
+    def test_admin_can_manage_other_apps(self, mock_requests, mock_logger):
         """Administrators of Deis should be able to manage all applications.
         """
         # log in as non-admin user and create an app
@@ -218,7 +221,7 @@ def test_admin_can_manage_other_apps(self, mock_logger):
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 204)
 
-    def test_admin_can_see_other_apps(self):
+    def test_admin_can_see_other_apps(self, mock_requests):
         """If a user creates an application, the administrator should be able
         to see it.
         """
@@ -236,7 +239,7 @@ def test_admin_can_see_other_apps(self):
         response = self.client.get(url)
         self.assertEqual(response.data['count'], 1)
 
-    def test_run_without_release_should_error(self):
+    def test_run_without_release_should_error(self, mock_requests):
         """
         A user should not be able to run a one-off command unless a release
         is present.
@@ -255,7 +258,7 @@ def test_run_without_release_should_error(self):
     @mock.patch('api.models.App.run', _mock_run)
     @mock.patch('api.models.App.deploy', mock_none)
     @mock.patch('api.models.Release.publish', mock_none)
-    def test_run(self):
+    def test_run(self, mock_requests):
         """
         A user should be able to run a one off command
         """
@@ -278,7 +281,7 @@ def test_run(self):
         self.assertEqual(response.data['rc'], 0)
         self.assertEqual(response.data['output'], 'mock')
 
-    def test_unauthorized_user_cannot_see_app(self):
+    def test_unauthorized_user_cannot_see_app(self, mock_requests):
         """
         An unauthorized user should not be able to access an app's resources.
 
@@ -309,7 +312,7 @@ def test_unauthorized_user_cannot_see_app(self):
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 403)
 
-    def test_app_info_not_showing_wrong_app(self):
+    def test_app_info_not_showing_wrong_app(self, mock_requests):
         app_id = 'autotest'
         base_url = '/v2/apps'
         body = {'id': app_id}
@@ -318,7 +321,7 @@ def test_app_info_not_showing_wrong_app(self):
         response = self.client.get(url)
         self.assertEqual(response.status_code, 404)
 
-    def test_app_transfer(self):
+    def test_app_transfer(self, mock_requests):
         owner = User.objects.get(username='autotest2')
         owner_token = Token.objects.get(user=owner).key
         self.client.credentials(HTTP_AUTHORIZATION='Token ' + owner_token)
@@ -366,7 +369,7 @@ def test_app_transfer(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['owner'], self.user.username)
 
-    def test_app_exists_in_kubernetes(self):
+    def test_app_exists_in_kubernetes(self, mock_requests):
         """
         Create an app that has the same namespace as an existing kubernetes namespace
         """