Skip to content

Commit 8b76d14

Browse files
author
Matthew Fisher
committed
ref(controller): move limits to Config model
A Release is a Build and a Config, not a Build, Config and a Limit. Limits make sense as configuration for the Build, so this moves it over as a Config field.
1 parent df7fb05 commit 8b76d14

4 files changed

Lines changed: 30 additions & 31 deletions

File tree

controller/api/models.py

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,8 @@ def __str__(self):
134134
def create(self, *args, **kwargs):
135135
config = Config.objects.create(owner=self.owner, app=self, values={})
136136
build = Build.objects.create(owner=self.owner, app=self, image=settings.DEFAULT_BUILD)
137-
limit = Limit.objects.create(owner=self.owner, app=self, memory={}, cpu={})
138137
Release.objects.create(version=1, owner=self.owner, app=self,
139-
config=config, build=build, limit=limit)
138+
config=config, build=build)
140139

141140
def delete(self, *args, **kwargs):
142141
for c in self.container_set.all():
@@ -303,8 +302,10 @@ def _command_announceable(self):
303302
@transition(field=state, source=INITIALIZED, target=CREATED)
304303
def create(self):
305304
image = self.release.image
306-
kwargs = {'memory': self.release.limit.memory,
307-
'cpu': self.release.limit.cpu}
305+
kwargs = {}
306+
if self.release.config.limit is not None:
307+
kwargs = {'memory': self.release.config.limit.memory,
308+
'cpu': self.release.config.limit.cpu}
308309
self._scheduler.create(name=self._job_id,
309310
image=image,
310311
command=self._command,
@@ -332,8 +333,10 @@ def deploy(self, release):
332333
new_job_id = self._job_id
333334
image = self.release.image
334335
c_type = self.type
335-
kwargs = {'memory': self.release.limit.memory,
336-
'cpu': self.release.limit.cpu}
336+
kwargs = {}
337+
if self.release.config.limit is not None:
338+
kwargs = {'memory': self.release.config.limit.memory,
339+
'cpu': self.release.config.limit.cpu}
337340
self._scheduler.create(name=new_job_id,
338341
image=image,
339342
command=self._command.format(**locals()),
@@ -424,6 +427,7 @@ class Config(UuidAuditedModel):
424427
owner = models.ForeignKey(settings.AUTH_USER_MODEL)
425428
app = models.ForeignKey('App')
426429
values = JSONField(default='{}', blank=True)
430+
limit = models.ForeignKey('Limit', null=True)
427431

428432
class Meta:
429433
get_latest_by = 'created'
@@ -470,7 +474,6 @@ class Release(UuidAuditedModel):
470474

471475
config = models.ForeignKey('Config')
472476
build = models.ForeignKey('Build')
473-
limit = models.ForeignKey('Limit', null=True)
474477
# NOTE: image contains combined build + config, ready to run
475478
image = models.CharField(max_length=256, default=settings.DEFAULT_BUILD)
476479

@@ -482,8 +485,7 @@ class Meta:
482485
def __str__(self):
483486
return "{0}-v{1}".format(self.app.id, self.version)
484487

485-
def new(self, user, config=None, build=None, limit=None,
486-
summary=None, source_version='latest'):
488+
def new(self, user, config=None, build=None, summary=None, source_version='latest'):
487489
"""
488490
Create a new application release using the provided Build and Config
489491
on behalf of a user.
@@ -494,8 +496,6 @@ def new(self, user, config=None, build=None, limit=None,
494496
config = self.config
495497
if not build:
496498
build = self.build
497-
if not limit:
498-
limit = self.limit
499499
# always create a release off the latest image
500500
source_image = '{}:{}'.format(build.image, source_version)
501501
# construct fully-qualified target image
@@ -505,8 +505,8 @@ def new(self, user, config=None, build=None, limit=None,
505505
target_image = '{}'.format(self.app.id)
506506
# create new release and auto-increment version
507507
release = Release.objects.create(
508-
owner=user, app=self.app, config=config, build=build, limit=limit,
509-
version=new_version, image=target_image, summary=summary)
508+
owner=user, app=self.app, config=config,
509+
build=build, version=new_version, image=target_image, summary=summary)
510510
# IOW, this image did not come from the builder
511511
if not build.sha:
512512
# we assume that the image is not present on our registry,
@@ -547,7 +547,7 @@ def save(self, *args, **kwargs): # noqa
547547
# compare this build to the previous build
548548
old_build = prev_release.build if prev_release else None
549549
old_config = prev_release.config if prev_release else None
550-
old_limit = prev_release.limit if prev_release else None
550+
old_limit = prev_release.config.limit if prev_release else None
551551
# if the build changed, log it and who pushed it
552552
if self.version == 1:
553553
self.summary += "{} created initial release".format(self.app.owner)
@@ -574,7 +574,7 @@ def save(self, *args, **kwargs): # noqa
574574
self.summary += ' and '
575575
self.summary += "{} {}".format(self.config.owner, changes)
576576
# if the limit changes, log the dict diff
577-
elif self.limit != old_limit:
577+
elif self.config.limit != old_limit:
578578
changes = []
579579
old_mem = old_limit.memory if old_limit else {}
580580
diff = dict_diff(self.limit.memory, old_mem)

controller/api/south_migrations/0012_auto__add_limit__add_unique_limit_app_uuid__add_field_release_limit__a.py renamed to controller/api/south_migrations/0012_auto__add_limit__add_unique_limit_app_uuid__add_field_config_limit.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,21 @@ def forwards(self, orm):
2323
# Adding unique constraint on 'Limit', fields ['app', 'uuid']
2424
db.create_unique(u'api_limit', ['app_id', 'uuid'])
2525

26-
# Adding field 'Release.limit'
27-
db.add_column(u'api_release', 'limit',
26+
# Adding field 'Config.limit'
27+
db.add_column(u'api_config', 'limit',
2828
self.gf('django.db.models.fields.related.ForeignKey')(to=orm['api.Limit'], null=True),
2929
keep_default=False)
3030

31+
3132
def backwards(self, orm):
3233
# Removing unique constraint on 'Limit', fields ['app', 'uuid']
3334
db.delete_unique(u'api_limit', ['app_id', 'uuid'])
3435

3536
# Deleting model 'Limit'
3637
db.delete_table(u'api_limit')
3738

38-
# Deleting field 'Release.limit'
39-
db.delete_column(u'api_release', 'limit_id')
39+
# Deleting field 'Config.limit'
40+
db.delete_column(u'api_config', 'limit_id')
4041

4142

4243
models = {
@@ -79,6 +80,7 @@ def backwards(self, orm):
7980
'Meta': {'ordering': "[u'-created']", 'unique_together': "((u'app', u'uuid'),)", 'object_name': 'Config'},
8081
'app': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['api.App']"}),
8182
'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
83+
'limit': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['api.Limit']", 'null': 'True'}),
8284
'owner': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['auth.User']"}),
8385
'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
8486
'uuid': ('api.fields.UuidField', [], {'unique': 'True', 'max_length': '32', 'primary_key': 'True'}),
@@ -145,7 +147,6 @@ def backwards(self, orm):
145147
'config': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['api.Config']"}),
146148
'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
147149
'image': ('django.db.models.fields.CharField', [], {'default': "u'deis/helloworld'", 'max_length': '256'}),
148-
'limit': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['api.Limit']", 'null': 'True'}),
149150
'owner': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['auth.User']"}),
150151
'summary': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
151152
'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),

controller/api/tests/test_limit.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ def test_limit_memory(self):
5555
response = self.client.get(url, content_type='application/json')
5656
self.assertEqual(response.status_code, 200)
5757
self.assertIn('memory', response.data)
58-
self.assertEqual(json.loads(response.data['memory']), {})
58+
self.assertEqual(json.loads(response.data['memory']), json.dumps({}))
5959
# set an initial limit
6060
mem = {'web': '1G'}
6161
body = {'memory': json.dumps(mem)}
@@ -119,7 +119,7 @@ def test_limit_cpu(self):
119119
response = self.client.get(url, content_type='application/json')
120120
self.assertEqual(response.status_code, 200)
121121
self.assertIn('cpu', response.data)
122-
self.assertEqual(json.loads(response.data['memory']), {})
122+
self.assertEqual(json.loads(response.data['memory']), json.dumps({}))
123123
# set an initial limit
124124
body = {'cpu': json.dumps({'web': '1024'})}
125125
response = self.client.post(url, json.dumps(body), content_type='application/json')

controller/api/views.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -404,19 +404,21 @@ class AppLimitViewSet(BaseAppViewSet):
404404

405405
model = models.Limit
406406
serializer_class = serializers.LimitSerializer
407+
permission_classes = (permissions.IsAuthenticated, IsAppUser)
407408

408409
def get_object(self, *args, **kwargs):
409410
"""Return the Limit associated with the App's latest Release."""
410411
app = get_object_or_404(models.App, id=self.kwargs['id'])
411-
user = self.request.user
412-
if user == app.owner or user in get_users_with_perms(app):
413-
return app.release_set.latest().limit
414-
raise PermissionDenied()
412+
return app.release_set.latest().config.limit
415413

416414
def post_save(self, limit, created=False):
417415
if created:
418416
release = limit.app.release_set.latest()
419-
self.release = release.new(self.request.user, limit=limit)
417+
config = models.Config.objects.create(owner=release.config.owner,
418+
app=release.config.app,
419+
values=release.config.values,
420+
limit=limit)
421+
self.release = release.new(self.request.user, config=config)
420422
limit.app.deploy(self.release)
421423

422424
def get_success_headers(self, data):
@@ -426,9 +428,6 @@ def get_success_headers(self, data):
426428

427429
def create(self, request, *args, **kwargs):
428430
request.DATA['app'] = self.kwargs['id']
429-
serializer = self.get_serializer(data=request.DATA, files=request.FILES)
430-
if not serializer.is_valid():
431-
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
432431

433432
# special logic to merge values and remove based on null values
434433
obj = self.get_object()
@@ -475,7 +474,6 @@ def rollback(self, request, *args, **kwargs):
475474
request.user,
476475
build=prev.build,
477476
config=prev.config,
478-
limit=prev.limit,
479477
summary=summary,
480478
source_version='v{}'.format(version))
481479
app.deploy(new_release)

0 commit comments

Comments
 (0)