fix(limits): always set default

Author: duanhongyi

rootfs/api/models/app.py

@@ -738,19 +738,27 @@ def _scale_pods(self, scale_types, release, app_settings):
             self.log(err, logging.ERROR)
             raise ServiceUnavailable(err) from e
 
-    def _set_default_config(self, config=None, procfile_types=None):
-        procfile_types = self.procfile_types if procfile_types is None else procfile_types
+    def _set_default_limit(self, config, procfile_type):
+        if procfile_type not in config.limits:
+            plan = LimitPlan.get_default()
+            config.limits[procfile_type] = plan.id
+            config.save(update_fields=['limits'])
+        return config
+
+    def _set_default_config(self):
         plan = LimitPlan.get_default()
         limits = {PROCFILE_TYPE_WEB: plan.id, PROCFILE_TYPE_RUN: plan.id}
         try:
-            config = self.config_set.latest() if config is None else config
-            for procfile_type in procfile_types:
-                limits[procfile_type] = config.limits.get(procfile_type, plan.id)
-            if limits != config.limits:
-                config.limits = limits
-                config.save(update_fields=['limits'])
+            config = self.config_set.latest()
+            limits[PROCFILE_TYPE_WEB] = config.limits.get(PROCFILE_TYPE_WEB, plan.id)
+            limits[PROCFILE_TYPE_RUN] = config.limits.get(PROCFILE_TYPE_RUN, plan.id)
         except Config.DoesNotExist:
             config = Config.objects.create(owner=self.owner, app=self, limits=limits)
+        for procfile_type in self.procfile_types:
+            limits[procfile_type] = config.limits.get(procfile_type, plan.id)
+        if limits != config.limits:
+            config.limits = limits
+            config.save(update_fields=['limits'])
         return config
 
     def _create_default_ingress(self, target_port):
@@ -993,9 +1001,8 @@ def _gather_app_settings(self, release, app_settings, procfile_type, replicas, v
         """
 
         envs = self._build_env_vars(release)
-        config = release.config
         # Obtain a limit plan that must exist, if raise error here, it must be a bug
-        self._set_default_config(config, procfile_types=[procfile_type])
+        config = self._set_default_limit(release.config, procfile_type)
         limit_plan = LimitPlan.objects.get(id=config.limits.get(procfile_type))
 
         # see if the app config has deploy batch preference, otherwise use global

rootfs/api/tests/test_config.py

@@ -473,7 +473,6 @@ def test_unset_limits_error(self, mock_requests):
         # dockerfile + procfile worflow
         app = App.objects.get(id=app_id)
         user = User.objects.get(username='autotest')
-        build = Build.objects.create(owner=user, app=app, image="qwerty")
         build = Build.objects.create(
             owner=user,
             app=app,
@@ -545,3 +544,45 @@ def test_measure_config(self, *args, **kwargs):
         out = self.call_command()
         self.assertIn(out, "done\n")
         self.assertEqual(response.status_code, 201)
+
+    def test_set_config_limits_run(self, *args, **kwargs):
+        # create
+        app_id = self.create_app()
+        # dockerfile + procfile worflow
+        app = App.objects.get(id=app_id)
+        user = User.objects.get(username='autotest')
+        build = Build.objects.create(
+            owner=user,
+            app=app,
+            image="qwerty",
+            procfile={
+                'web': 'node server.js',
+                'worker': 'node worker.js'
+            },
+            dockerfile='foo',
+            sha='somereallylongsha'
+        )
+        # create an initial release
+        release = Release.objects.create(
+            version=3,
+            owner=user,
+            app=app,
+            config=app.config_set.latest(),
+            build=build
+        )
+        # deploy
+        app.pipeline(release)
+        body = {
+            'values': json.dumps({'PORT': 5000}),
+            'limits': {
+                  PROCFILE_TYPE_RUN: 'std1.large.c2m4',
+                  PROCFILE_TYPE_WEB: 'std1.large.c2m4',
+            },
+        }
+        url = f"/v2/apps/{app_id}/config"
+        response = self.client.post(url, body)
+        url = "/v2/apps/{app_id}/config".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.data)
+        expect = {'run': 'std1.large.c2m4', 'web': 'std1.large.c2m4', 'worker': 'std1.large.c1m1'}
+        self.assertEqual(expect, response.json()["limits"], response.data)

rootfs/api/views.py

@@ -284,8 +284,8 @@ def list(self, request, *args, **kwargs):
 
     def scale(self, request, **kwargs):
         app = self.get_object()
-        release = app.release_set.filter(failed=False).latest()
-        if release.build is not None and release.state == "created":
+        latest_release = app.release_set.filter(failed=False).latest()
+        if latest_release.build is not None and latest_release.state == "created":
             raise DryccException('There is an executing pipeline, please wait')
         scale_app.delay(self.get_object(), request.user, request.data)
         return Response(status=status.HTTP_204_NO_CONTENT)
@@ -385,22 +385,20 @@ class ConfigViewSet(ReleasableViewSet):
     serializer_class = serializers.ConfigSerializer
 
     def post_save(self, config):
-        release = config.app.release_set.filter(failed=False).latest()
-        if release.build is not None and release.state == "created":
+        latest_release = config.app.release_set.filter(failed=False).latest()
+        if latest_release.build is not None and latest_release.state == "created":
             raise DryccException('There is an executing pipeline, please wait')
         # It's possible to set config values before a build
         latest_version = config.app.release_set.latest().version
         try:
-            release = release.new(
-                self.request.user, config=config, build=release.build, canary=release.canary)
+            release = latest_release.new(
+                self.request.user, config=config, build=latest_release.build,
+                canary=latest_release.canary)
             if release.build is not None:
                 config.app.deploy(release)
             release.state = "succeed"
             release.save()
         except Exception as e:
-            if ('release' not in locals() and
-                    config.app.release_set.latest().version == latest_version+1):
-                release = config.app.release_set.latest()
             if 'release' in locals():
                 release.state = "crashed"
                 release.failed = True
@@ -461,8 +459,8 @@ def _clean_canaries(self):
 
     def release(self, request, *args, **kwargs):
         self._clean_canaries()
-        release = self.get_app().release_set.filter(failed=False).latest()
-        data = {'version': release.rollback(request.user, release.version).version}
+        latest_release = self.get_app().release_set.filter(failed=False).latest()
+        data = {'version': latest_release.rollback(request.user, latest_release.version).version}
         return Response(data, status=status.HTTP_201_CREATED)
 
     def rollback(self, request, *args, **kwargs):
@@ -596,8 +594,8 @@ def rollback(self, request, **kwargs):
         Create a new release as a copy of the state of the compiled slug and config vars of a
         previous release.
         """
-        release = self.get_app().release_set.filter(failed=False).latest()
-        new_release = release.rollback(request.user, request.data.get('version', None))
+        latest_release = self.get_app().release_set.filter(failed=False).latest()
+        new_release = latest_release.rollback(request.user, request.data.get('version', None))
         response = {'version': new_release.version}
         return Response(response, status=status.HTTP_201_CREATED)
 

rootfs/requirements.txt

@@ -11,7 +11,7 @@ docker==7.0.0
 gunicorn==21.2.0
 uvicorn[standard]==0.29.0
 asgiref==3.8.0
-idna==3.6
+idna==3.7
 jsonschema==4.21.1
 morph==0.1.5
 packaging==24.0