fix(controller): kill processes removed from procfile

When a process type is removed in a new build, the old containers are
not killed, and there's no way for the user to stop them. This change
will destroy the containers that no longer exist in the procfile.

Author: Matthew Fisher

controller/api/models.py

@@ -255,6 +255,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]
@@ -588,6 +591,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)
@@ -565,3 +566,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)