Merge pull request #3068 from bacongobbler/2930-kill-processes-removed-from-procfile

fix(controller): kill processes removed from procfile

Author: Matthew Fisher

controller/api/models.py

@@ -256,6 +256,9 @@ def _start_containers(self, to_add):
     def _destroy_containers(self, to_destroy):
         """Destroys containers via the scheduler"""
         destroy_threads = []
+        if not to_destroy:
+            # do nothing if we didn't request any containers
+            return
         for c in to_destroy:
             destroy_threads.append(threading.Thread(target=c.destroy))
         [t.start() for t in destroy_threads]
@@ -589,6 +592,19 @@ def create(self, user, *args, **kwargs):
             new_release.delete()
             raise
 
+    def save(self, **kwargs):
+        try:
+            previous_build = self.app.build_set.latest()
+            to_destroy = []
+            for proctype in previous_build.procfile.keys():
+                if proctype not in self.procfile.keys():
+                    for c in self.app.container_set.filter(type=proctype):
+                        to_destroy.append(c)
+            self.app._destroy_containers(to_destroy)
+        except Build.DoesNotExist:
+            pass
+        return super(Build, self).save(**kwargs)
+
     def __str__(self):
         return "{0}-{1}".format(self.app.id, self.uuid[:7])
 

controller/api/tests/test_container.py

@@ -262,7 +262,8 @@ def test_container_release(self):
         self.assertEqual(response.data['results'][0]['release'], 'v2')
         # post a new build
         url = "/v1/apps/{app_id}/builds".format(**locals())
-        body = {'image': 'autotest/example'}
+        # a web proctype must exist on the second build or else the container will be removed
+        body = {'image': 'autotest/example', 'procfile': {'web': 'echo hi'}}
         response = self.client.post(url, json.dumps(body), content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(self.token))
         self.assertEqual(response.status_code, 201)
@@ -606,3 +607,30 @@ def test_scale_with_unauthorized_user_returns_403(self):
         response = self.client.post(url, json.dumps(body), content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(unauthorized_token))
         self.assertEqual(response.status_code, 403)
+
+    def test_modified_procfile_from_build_removes_containers(self):
+        """
+        When a new procfile is posted which removes a certain process type, deis should stop the
+        existing containers.
+        """
+        url = '/v1/apps'
+        response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
+        self.assertEqual(response.status_code, 201)
+        app_id = response.data['id']
+        # post a new build
+        build_url = "/v1/apps/{app_id}/builds".format(**locals())
+        body = {'image': 'autotest/example', 'sha': 'a'*40,
+                'procfile': json.dumps({'web': 'node server.js', 'worker': 'node worker.js'})}
+        response = self.client.post(build_url, json.dumps(body), content_type='application/json',
+                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
+        url = "/v1/apps/{app_id}/scale".format(**locals())
+        body = {'web': 4}
+        response = self.client.post(url, json.dumps(body), content_type='application/json',
+                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
+        self.assertEqual(response.status_code, 204)
+        body = {'image': 'autotest/example', 'sha': 'a'*40,
+                'procfile': json.dumps({'worker': 'node worker.js'})}
+        response = self.client.post(build_url, json.dumps(body), content_type='application/json',
+                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
+        self.assertEqual(response.status_code, 201)
+        self.assertEqual(Container.objects.filter(type='web').count(), 0)