fix(dryccfile): config not set

duanhongyi · duanhongyi · commit 9cfc4ec6dfd2 · 2024-12-10T15:56:12.000+08:00
diff --git a/rootfs/api/models/build.py b/rootfs/api/models/build.py
@@ -110,28 +110,43 @@ def _get_or_create_config(self):
         """
         dryccfile to config
         """
-        latest_release = Release.latest(self.app)
-        config_values, config_values_ref, config_healthcheck = [], {}, {}
-        for group, values in self.dryccfile.get('config', {}).items():
-            for value in values:
-                value['group'] = group
-                config_values.append(value)
+        config_values, config_values_ref, config_healthcheck, changed_fields = [], {}, {}, set()
+        if 'config' in self.dryccfile:
+            for group, values in self.dryccfile.get('config', {}).items():
+                for value in values:
+                    value['group'] = group
+                    config_values.append(value)
+            changed_fields.update(["values", "values_refs"])
+
+        replace_values_ptypes, replace_healthcheck_ptypes = set(), set()
         for ptype, values in self.dryccfile.get('deploy', {}).items():
-            for value in values.get('config', {}).get('env', []):
-                value['ptype'] = ptype
-                config_values.append(value)
-            for config_ref in values.get('config', {}).get('ref', []):
-                if ptype not in config_values_ref:
-                    config_values_ref[ptype] = [config_ref]
-                else:
-                    config_values_ref[ptype].append(config_ref)
+            if 'config' in values:
+                for value in values.get('config', {}).get('env', []):
+                    value['ptype'] = ptype
+                    config_values.append(value)
+                    replace_values_ptypes.add(ptype)
+                for config_ref in values.get('config', {}).get('ref', []):
+                    if ptype not in config_values_ref:
+                        config_values_ref[ptype] = [config_ref]
+                    else:
+                        config_values_ref[ptype].append(config_ref)
+                replace_values_ptypes.add(ptype)
+                changed_fields.update(["values", "values_refs"])
+
             if 'healthcheck' in values:
                 config_healthcheck[ptype] = values.get('healthcheck')
-        if not config_values:
-            return latest_release.config
+                changed_fields.add("healthcheck")
+                replace_healthcheck_ptypes.add(ptype)
+
+        old_config = self.app.release_set.filter(failed=False).latest().config
+        if not changed_fields:
+            return old_config
         config = Config(
             owner=self.owner, app=self.app, values=config_values, values_refs=config_values_ref,
             healthcheck=config_healthcheck,
         )
-        config.save(ignore_update_fields=["values", "values_refs", "healthcheck"])
+        config.merge_field("values", old_config, replace_values_ptypes)
+        config.merge_field("values_refs", old_config, replace_values_ptypes)
+        config.merge_field("healthcheck", old_config, replace_healthcheck_ptypes)
+        config.save(ignore_update_fields=changed_fields)
         return config
diff --git a/rootfs/api/models/config.py b/rootfs/api/models/config.py
@@ -134,17 +134,23 @@ def save(self, ignore_update_fields=None, *args, **kwargs):
                 previous_config = self.app.config_set.latest()
             for field in self.allof_fields:
                 if ignore_update_fields is None or field not in ignore_update_fields:
-                    getattr(
-                        self,
-                        "_update_%s" % field,
-                        partial(self._update_field, field)
-                    )(previous_config)
+                    self.merge_field(field, previous_config)
         except Config.DoesNotExist:
             self._update_tags(previous_config={'tags': {}})
         return super(Config, self).save(*args, **kwargs)
 
-    def _update_field(self, field, previous_config):
-        data = getattr(previous_config, field, {}).copy()
+    def merge_field(self, field, old_config, replace_ptypes=[]):
+        getattr(
+            self,
+            "_update_%s" % field,
+            partial(self._update_field, field)
+        )(old_config, replace_ptypes)
+
+    def _update_field(self, field, previous_config, replace_ptypes=[]):
+        data = {
+            k: v for k, v in getattr(previous_config, field, {}).copy().items()
+            if k not in replace_ptypes
+        }
         new_data = getattr(self, field, {}).copy()
         # remove config keys if a null value is provided
         for key, value in new_data.items():
@@ -158,8 +164,11 @@ def _update_field(self, field, previous_config):
                 data[key] = self._merge_data(field, data.get(key, {}), value)
         setattr(self, field, data)
 
-    def _update_values(self, previous_config):
-        data = getattr(previous_config, 'values', []).copy()
+    def _update_values(self, previous_config, replace_ptypes=[]):
+        data = [
+            item for item in getattr(previous_config, 'values', []).copy()
+            if item.get('ptype') not in replace_ptypes
+        ]
         new_data = getattr(self, 'values', []).copy()
         for new_item in new_data:
             added = True
@@ -175,12 +184,15 @@ def _update_values(self, previous_config):
                     else:  # force to string
                         new_item['value'] = str(new_item['value'])
                     break
-            if added and new_item['value'] is not None and new_item['value'] != "":
+            if added and new_item['value'] is not None:
                 data.append(new_item)
         setattr(self, 'values', data)
 
-    def _update_values_refs(self, previous_config):
-        data = getattr(previous_config, 'values_refs', {}).copy()
+    def _update_values_refs(self, previous_config, replace_ptypes=[]):
+        data = {
+            k: v for k, v in getattr(previous_config, 'values_refs', {}).copy().items()
+            if k not in replace_ptypes
+        }
         new_data = getattr(self, 'values_refs', {}).copy()
         # remove config keys if a null value is provided
         for ptype, values in new_data.items():
@@ -192,7 +204,7 @@ def _update_values_refs(self, previous_config):
                 data.pop(ptype)
             else:
                 values_refs = data.get(ptype, [])
-                values_refs.extend(new_data.get(ptype, []))
+                values_refs.extend(values)
                 data[ptype] = list(set(values_refs))
         setattr(self, 'values_refs', data)
 
@@ -208,9 +220,12 @@ def _merge_data(self, field, data, new_data):
                 data[key] = value
         return data
 
-    def _update_tags(self, previous_config):
+    def _update_tags(self, previous_config, replace_ptypes=[]):
         """verify the tags exist on any nodes as labels"""
-        data = getattr(previous_config, 'tags', {}).copy()
+        data = {
+            k: v for k, v in getattr(previous_config, 'tags', {}).copy().items()
+            if k not in replace_ptypes
+        }
         new_data = getattr(self, 'tags', {}).copy()
         # remove config keys if a null value is provided
         for ptype, values in new_data.items():
@@ -236,14 +251,17 @@ def _update_tags(self, previous_config):
                     'tags', data.get(ptype, {}), values)
         setattr(self, 'tags', data)
 
-    def _update_limits(self, previous_config):
-        data = getattr(previous_config, 'limits', {}).copy()
+    def _update_limits(self, previous_config, replace_ptypes=[]):
+        data = {
+            k: v for k, v in getattr(previous_config, 'limits', {}).copy().items()
+            if k not in replace_ptypes
+        }
         new_data = getattr(self, 'limits', {}).copy()
         # check procfile
-        for key, value in new_data.items():
+        for ptype, value in new_data.items():
             if value is None:
-                if key in self.app.ptypes:
+                if ptype in self.app.ptypes:
                     raise UnprocessableEntity(
-                        "the %s has already been used and cannot be deleted" % key)
+                        "the %s has already been used and cannot be deleted" % ptype)
         self._merge_data('limits', data, new_data)
         setattr(self, 'limits', data)
diff --git a/rootfs/api/tests/test_config.py b/rootfs/api/tests/test_config.py
@@ -4,6 +4,7 @@
 
 Run the tests with "./manage.py test api"
 """
+import copy
 from io import StringIO
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
@@ -829,7 +830,7 @@ def test_config_from_dryccfile(self, mock_requests):
                 'deploy': {
                     'web': {
                         'image': "127.0.0.1/cat/cat"
-                    }
+                    },
                 }
             },
         }
@@ -876,7 +877,7 @@ def test_config_from_dryccfile(self, mock_requests):
             release.config.envs("web"),
             {
                 'GROUP': 'g1', 'DEBUG': 'tr', 'TEST1': 'g1', 'TEST2': 'tr', 'PENV1': 'web',
-                'PENV2': 'web'
+                'PENV2': 'web', "WEBSITE": "www.drycc.cc",
             }
         )
         build_body['dryccfile']['deploy']['web']['healthcheck'] = {
@@ -901,3 +902,33 @@ def test_config_from_dryccfile(self, mock_requests):
             release.config.healthcheck['web'],
             build_body['dryccfile']['deploy']['web']['healthcheck'],
         )
+        # test use old config and healthcheck
+        new_build_body = copy.deepcopy(build_body)
+        del new_build_body['dryccfile']['config']
+        del new_build_body['dryccfile']['deploy']['web']['config']
+        del new_build_body['dryccfile']['deploy']['web']['healthcheck']
+        with mock.patch('scheduler.resources.pod.Pod.watch') as mock_kube:
+            mock_kube.return_value = ['up', 'down']
+            url = f"/v2/apps/{app_id}/build"
+            response = self.client.post(url, new_build_body)
+            self.assertEqual(response.status_code, 201, response.data)
+        release = app.release_set.latest()
+        self.assertEqual(release.failed, False)
+        self.assertEqual(release.config.healthcheck, release.previous().config.healthcheck)
+        self.assertEqual(release.config.values, release.previous().config.values)
+        self.assertEqual(release.config.values_refs, release.previous().config.values_refs)
+        # set empty
+        new_build_body = copy.deepcopy(build_body)
+        new_build_body['dryccfile']['config'] = {}
+        new_build_body['dryccfile']['deploy']['web']['config'] = {}
+        new_build_body['dryccfile']['deploy']['web']['healthcheck'] = {}
+        with mock.patch('scheduler.resources.pod.Pod.watch') as mock_kube:
+            mock_kube.return_value = ['up', 'down']
+            url = f"/v2/apps/{app_id}/build"
+            response = self.client.post(url, new_build_body)
+            self.assertEqual(response.status_code, 201, response.data)
+        release = app.release_set.latest()
+        self.assertEqual(release.failed, False)
+        self.assertEqual(release.config.healthcheck, {'web': {}})
+        self.assertEqual(release.config.envs("web"), {"WEBSITE": "www.drycc.cc"})
+        self.assertEqual(release.config.values_refs, {})
