fix(deploy): add a global / per-app setting called PROCFILE_REMOVE_PROCS_ON_DEPLOY to allow turning off / on removal of processes on deploys (#1099)

helgi · web-flow · commit 0d830247b363 · 2016-10-13T10:00:53.000-07:00
By default this setting is On to keep backwards compatibility but will allow Deis operators and developers to turns this off.

To set the global version of PROCFILE_REMOVE_PROCS_ON_DEPLOY give it an empty value to be considered off

ref #1062
diff --git a/rootfs/api/exceptions.py b/rootfs/api/exceptions.py
@@ -19,6 +19,10 @@ class AlreadyExists(APIException):
     status_code = 409
 
 
+class Conflict(AlreadyExists):
+    pass
+
+
 class UnprocessableEntity(APIException):
     status_code = 422
 
diff --git a/rootfs/api/models/build.py b/rootfs/api/models/build.py
@@ -2,7 +2,8 @@
 from django.db import models
 from jsonfield import JSONField
 
-from api.models import UuidAuditedModel, DeisException
+from api.models import UuidAuditedModel
+from api.exceptions import DeisException, Conflict
 
 import logging
 logger = logging.getLogger(__name__)
@@ -78,16 +79,55 @@ def create(self, user, *args, **kwargs):
             raise DeisException(str(e)) from e
 
     def save(self, **kwargs):
-        removed = {}
         previous_release = self.app.release_set.filter(failed=False).latest()
-        if previous_release.build is not None:
+
+        if (
+            settings.DEIS_DEPLOY_REJECT_IF_PROCFILE_MISSING is True and
+            # previous release had a Procfile and the current one does not
+            (
+                previous_release.build is not None and
+                len(previous_release.build.procfile) > 0 and
+                len(self.procfile) == 0
+            )
+        ):
+            # Reject deployment
+            raise Conflict(
+                'Last deployment had a Procfile but is missing in this deploy. '
+                'For a successful deployment provide a Procfile.'
+            )
+
+        # See if processes are permitted to be removed
+        remove_procs = (
+            # If set to True then contents of Procfile does not affect the outcome
+            settings.DEIS_DEPLOY_PROCFILE_MISSING_REMOVE is True or
+            # previous release had a Procfile and the current one does as well
+            (
+                previous_release.build is not None and
+                len(previous_release.build.procfile) > 0 and
+                len(self.procfile) > 0
+            )
+        )
+
+        # spin down any proc type removed between the last procfile and the newest one
+        if remove_procs and previous_release.build is not None:
+            removed = {}
             for proc in previous_release.build.procfile:
                 if proc not in self.procfile:
                     # Scale proc type down to 0
                     removed[proc] = 0
 
             self.app.scale(self.owner, removed)
 
+        # make sure the latest build has procfile if the intent is to
+        # allow empty Procfile without removals
+        if (
+            settings.DEIS_DEPLOY_PROCFILE_MISSING_REMOVE is False and
+            previous_release.build is not None and
+            len(previous_release.build.procfile) > 0 and
+            len(self.procfile) == 0
+        ):
+            self.procfile = previous_release.build.procfile
+
         return super(Build, self).save(**kwargs)
 
     def __str__(self):
diff --git a/rootfs/api/settings/production.py b/rootfs/api/settings/production.py
@@ -1,7 +1,7 @@
 """
 Django settings for the Deis project.
 """
-
+from distutils.util import strtobool
 import os.path
 import tempfile
 
@@ -256,6 +256,29 @@
 SLUGRUNNER_IMAGE = os.environ.get('SLUGRUNNER_IMAGE_NAME', 'quay.io/deisci/slugrunner:canary')  # noqa
 IMAGE_PULL_POLICY = os.environ.get('IMAGE_PULL_POLICY', "IfNotPresent")  # noqa
 
+# True, true, yes, y and more evaluate to True
+# False, false, no, n and more evaluate to False
+# https://docs.python.org/3/distutils/apiref.html?highlight=distutils.util#distutils.util.strtobool
+# see the above for all available options
+#
+# If a user deploys one build with a Procfile but then forgets to in the next one
+# then let that go through without scaling the missing process types down
+#
+# If the user has a Procfile in both deploys then processes are scaled up / down as per usual
+#
+# By default the process types are scaled down unless this setting is turned on
+DEIS_DEPLOY_PROCFILE_MISSING_REMOVE = bool(strtobool(os.environ.get('DEIS_DEPLOY_PROCFILE_MISSING_REMOVE', 'true')))  # noqa
+
+# True, true, yes, y and more evaluate to True
+# False, false, no, n and more evaluate to False
+# https://docs.python.org/3/distutils/apiref.html?highlight=distutils.util#distutils.util.strtobool
+# see the above for all available options
+#
+# If a previous deploy had a Procfile but then the following deploy has no Procfile then it will
+# result in a 406 - Not Acceptable
+# Has priority over DEIS_DEPLOY_PROCFILE_MISSING_REMOVE
+DEIS_DEPLOY_REJECT_IF_PROCFILE_MISSING = bool(strtobool(os.environ.get('DEIS_DEPLOY_REJECT_IF_PROCFILE_MISSING', 'false')))  # noqa
+
 # Define a global default on how many pods to bring up and then
 # take down sequentially during a deploy
 # Defaults to None, the default is to deploy to as many nodes as
diff --git a/rootfs/api/tests/test_build.py b/rootfs/api/tests/test_build.py
@@ -8,6 +8,7 @@
 from django.contrib.auth.models import User
 from django.core.cache import cache
 from django.conf import settings
+from django.test.utils import override_settings
 from unittest import mock
 from rest_framework.authtoken.models import Token
 
@@ -235,6 +236,207 @@ def test_build_default_containers(self, mock_requests):
         response = self.client.post(url, body)
         self.assertEqual(response.status_code, 201, response.data)
 
+    @override_settings(DEIS_DEPLOY_PROCFILE_MISSING_REMOVE=True)
+    def test_build_forgotten_procfile(self, mock_requests):
+        """
+        Test that when a user first posts a build with a Procfile
+        and then later without it that missing process are actually
+        scaled down
+        """
+        # start with a new app
+        app_id = self.create_app()
+
+        # post a new build with procfile
+        url = "/v2/apps/{app_id}/builds".format(**locals())
+        body = {
+            'image': 'autotest/example',
+            'sha': 'a'*40,
+            'procfile': {
+                'web': 'node server.js',
+                'worker': 'node worker.js'
+            }
+        }
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+
+        # scale worker
+        url = "/v2/apps/{app_id}/scale".format(**locals())
+        body = {'worker': 1}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 204, response.data)
+
+        # verify worker
+        url = "/v2/apps/{app_id}/pods/worker".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(len(response.data['results']), 1)
+        container = response.data['results'][0]
+        self.assertEqual(container['type'], 'worker')
+        self.assertEqual(container['release'], 'v2')
+        # pod name is auto generated so use regex
+        self.assertRegex(container['name'], app_id + '-worker-[0-9]{8,10}-[a-z0-9]{5}')
+
+        # do another deploy for this time forget Procfile
+        url = "/v2/apps/{app_id}/builds".format(**locals())
+        body = {'image': 'autotest/example'}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+
+        # verify worker is not there
+        url = "/v2/apps/{app_id}/pods/worker".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(len(response.data['results']), 0)
+
+        # verify web is still there
+        url = "/v2/apps/{app_id}/pods/web".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(len(response.data['results']), 0)
+
+        # look at the app structure
+        url = "/v2/apps/{app_id}".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(response.json()['structure'], {'web': 0, 'worker': 0})
+
+    @override_settings(DEIS_DEPLOY_PROCFILE_MISSING_REMOVE=False)
+    def test_build_no_remove_process(self, mock_requests):
+        """
+        Specifically test PROCFILE_REMOVE_PROCS_ON_DEPLOY being turned off
+        and a user posting a new build without a Procfile that was previously
+        applied
+        """
+        # start with a new app
+        app_id = self.create_app()
+
+        # post a new build with procfile
+        url = "/v2/apps/{app_id}/builds".format(**locals())
+        body = {
+            'image': 'autotest/example',
+            'sha': 'a'*40,
+            'procfile': {
+                'web': 'node server.js',
+                'worker': 'node worker.js'
+            }
+        }
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+
+        # scale worker
+        url = "/v2/apps/{app_id}/scale".format(**locals())
+        body = {'worker': 1}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 204, response.data)
+
+        # verify worker
+        url = "/v2/apps/{app_id}/pods/worker".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(len(response.data['results']), 1)
+        container = response.data['results'][0]
+        self.assertEqual(container['type'], 'worker')
+        self.assertEqual(container['release'], 'v2')
+        # pod name is auto generated so use regex
+        self.assertRegex(container['name'], app_id + '-worker-[0-9]{8,10}-[a-z0-9]{5}')
+
+        # do another deploy for this time forget Procfile
+        url = "/v2/apps/{app_id}/builds".format(**locals())
+        body = {'image': 'autotest/example'}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+
+        # verify worker is still there
+        url = "/v2/apps/{app_id}/pods/worker".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(len(response.data['results']), 1)
+        container = response.data['results'][0]
+        self.assertEqual(container['type'], 'worker')
+        self.assertEqual(container['release'], 'v3')
+        # pod name is auto generated so use regex
+        self.assertRegex(container['name'], app_id + '-worker-[0-9]{8,10}-[a-z0-9]{5}')
+
+        # verify web is still there
+        url = "/v2/apps/{app_id}/pods/web".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(len(response.data['results']), 1)
+        container = response.data['results'][0]
+        self.assertEqual(container['type'], 'web')
+        self.assertEqual(container['release'], 'v3')
+        # pod name is auto generated so use regex
+        self.assertRegex(container['name'], app_id + '-web-[0-9]{8,10}-[a-z0-9]{5}')
+
+        # look at the app structure
+        url = "/v2/apps/{app_id}".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(response.json()['structure'], {'web': 1, 'worker': 1})
+
+        # scale worker to make sure no info was lost
+        url = "/v2/apps/{app_id}/scale".format(**locals())
+        body = {'worker': 2}  # bump from 1 to 2
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 204, response.data)
+
+        # verify worker info
+        url = "/v2/apps/{app_id}/pods/worker".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(len(response.data['results']), 2)
+        container = response.data['results'][0]
+        self.assertEqual(container['type'], 'worker')
+        self.assertEqual(container['release'], 'v3')
+        # pod name is auto generated so use regex
+        self.assertRegex(container['name'], app_id + '-worker-[0-9]{8,10}-[a-z0-9]{5}')
+
+    @override_settings(DEIS_DEPLOY_REJECT_IF_PROCFILE_MISSING=True)
+    def test_build_forgotten_procfile_reject(self, mock_requests):
+        """
+        Test that when a user first posts a build with a Procfile
+        and then later without it that missing process are actually
+        scaled down
+        """
+        # start with a new app
+        app_id = self.create_app()
+
+        # post a new build with procfile
+        url = "/v2/apps/{app_id}/builds".format(**locals())
+        body = {
+            'image': 'autotest/example',
+            'sha': 'a'*40,
+            'procfile': {
+                'web': 'node server.js',
+                'worker': 'node worker.js'
+            }
+        }
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+
+        # scale worker
+        url = "/v2/apps/{app_id}/scale".format(**locals())
+        body = {'worker': 1}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 204, response.data)
+
+        # verify worker
+        url = "/v2/apps/{app_id}/pods/worker".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        self.assertEqual(len(response.data['results']), 1)
+        container = response.data['results'][0]
+        self.assertEqual(container['type'], 'worker')
+        self.assertEqual(container['release'], 'v2')
+        # pod name is auto generated so use regex
+        self.assertRegex(container['name'], app_id + '-worker-[0-9]{8,10}-[a-z0-9]{5}')
+
+        # do another deploy for this time forget Procfile
+        url = "/v2/apps/{app_id}/builds".format(**locals())
+        body = {'image': 'autotest/example'}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 409, response.data)
+
     def test_build_str(self, mock_requests):
         """Test the text representation of a build."""
         app_id = self.create_app()
