Skip to content

Commit 050263c

Browse files
committed
fix(controller): update App.structure field correctly after scaling
While the scaling logic is correct, we were naively using the requested scaling values to update the App.structure JSON field, rather than the actual containers in existence after scaling. This was cosmetic only, but is fixed by a SQL aggregate query and python dict comprehension. Includes regression unit tests.
1 parent 211361b commit 050263c

2 files changed

Lines changed: 16 additions & 2 deletions

File tree

controller/api/models.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from django.contrib.auth import get_user_model
1919
from django.core.exceptions import ValidationError
2020
from django.db import models
21+
from django.db.models import Count
2122
from django.db.models import Max
2223
from django.db.models.signals import post_delete, post_save
2324
from django.dispatch import receiver
@@ -201,7 +202,8 @@ def scale(self, user, structure): # noqa
201202
if to_remove:
202203
self._destroy_containers(to_remove)
203204
# save new structure to the database
204-
self.structure = structure
205+
vals = self.container_set.values('type').annotate(Count('pk')).order_by()
206+
self.structure = {v['type']: v['pk__count'] for v in vals}
205207
self.save()
206208
return changed
207209

controller/api/tests/test_container.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,12 @@ def test_container_api_heroku(self):
107107
self.assertEqual(response.status_code, 201)
108108
# scale up
109109
url = "/v1/apps/{app_id}/scale".format(**locals())
110-
body = {'web': 4, 'worker': 2}
110+
# test setting one proc type at a time
111+
body = {'web': 4}
112+
response = self.client.post(url, json.dumps(body), content_type='application/json',
113+
HTTP_AUTHORIZATION='token {}'.format(self.token))
114+
self.assertEqual(response.status_code, 204)
115+
body = {'worker': 2}
111116
response = self.client.post(url, json.dumps(body), content_type='application/json',
112117
HTTP_AUTHORIZATION='token {}'.format(self.token))
113118
self.assertEqual(response.status_code, 204)
@@ -118,6 +123,9 @@ def test_container_api_heroku(self):
118123
url = "/v1/apps/{app_id}".format(**locals())
119124
response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
120125
self.assertEqual(response.status_code, 200)
126+
# ensure the structure field is up-to-date
127+
self.assertEqual(response.data['structure']['web'], 4)
128+
self.assertEqual(response.data['structure']['worker'], 2)
121129
# test listing/retrieving container info
122130
url = "/v1/apps/{app_id}/containers/web".format(**locals())
123131
response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
@@ -130,6 +138,7 @@ def test_container_api_heroku(self):
130138
self.assertEqual(response.data['num'], num)
131139
# scale down
132140
url = "/v1/apps/{app_id}/scale".format(**locals())
141+
# test setting two proc types at a time
133142
body = {'web': 2, 'worker': 1}
134143
response = self.client.post(url, json.dumps(body), content_type='application/json',
135144
HTTP_AUTHORIZATION='token {}'.format(self.token))
@@ -142,6 +151,9 @@ def test_container_api_heroku(self):
142151
url = "/v1/apps/{app_id}".format(**locals())
143152
response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
144153
self.assertEqual(response.status_code, 200)
154+
# ensure the structure field is up-to-date
155+
self.assertEqual(response.data['structure']['web'], 2)
156+
self.assertEqual(response.data['structure']['worker'], 1)
145157
# scale down to 0
146158
url = "/v1/apps/{app_id}/scale".format(**locals())
147159
body = {'web': 0, 'worker': 0}

0 commit comments

Comments
 (0)