feat(config): add typed_values

Author: duanhongyi

rootfs/api/migrations/0005_config_typed_values.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.2.10 on 2024-04-26 04:55
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0004_build_dryccfile_release_state_alter_service_ports'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='config',
+            name='typed_values',
+            field=models.JSONField(blank=True, default=dict),
+        ),
+    ]

rootfs/api/models/app.py

@@ -390,12 +390,12 @@ def pod_name(size=5, chars=string.ascii_lowercase + string.digits):
         data['restart_policy'] = 'Never'
         data['active_deadline_seconds'] = timeout
         data['ttl_seconds_after_finished'] = expires
-        # create application config and build the pod manifest
-        self.set_application_config(release)
         name = self._get_job_id(PROCFILE_TYPE_RUN, release.canary) + '-' + pod_name()
         self.log("{} on {} runs '{}'".format(user.username, name, command))
         kwargs.update(data)
         try:
+            # create application config and build the pod manifest
+            self.set_application_config(release, PROCFILE_TYPE_RUN)
             self.scheduler().job.create(
                 self.id,
                 name,
@@ -506,7 +506,7 @@ def image_pull_secret(self, namespace, registry, image):
 
         return name
 
-    def set_application_config(self, release):
+    def set_application_config(self, release, procfile_type):
         """
         Creates the application config as a secret in Kubernetes and
         updates it if it already exists
@@ -520,13 +520,13 @@ def set_application_config(self, release):
 
         # secrets use dns labels for keys, map those properly here
         secrets_env = {}
-        for key, value in self._build_env_vars(release).items():
+        for key, value in self._build_env_vars(release, procfile_type).items():
             secrets_env[key.lower().replace('_', '-')] = str(value)
 
         # dictionary sorted by key
         secrets_env = OrderedDict(sorted(secrets_env.items(), key=lambda t: t[0]))
 
-        secret_name = "{}-{}-env".format(self.id, version)
+        secret_name = "{}-{}-{}-env".format(self.id, procfile_type, version)
         try:
             self.scheduler().secret.get(self.id, secret_name)
         except KubeHTTPException:
@@ -588,6 +588,7 @@ def _mount(self, user, volume, release, app_settings, structure=None):
                     self.id, self._get_job_id(scale_type, release.canary)).json()
                 spec_annotations = deployment['spec']['template']['metadata'].get(
                     'annotations', {})
+                self.set_application_config(release, scale_type)
                 # gather volume proc types to be deployed
                 tasks.append(
                     functools.partial(
@@ -603,8 +604,6 @@ def _mount(self, user, volume, release, app_settings, structure=None):
                     )
                 )
         try:
-            # create the application config in k8s (secret in this case) for all deploy objects
-            self.set_application_config(release)
             apply_tasks(tasks)
         except Exception as e:
             err = f'(changed volume mount for {volume}: {e}'
@@ -620,21 +619,18 @@ def _deploy(self, deploys, structure, prev_release,
         self._check_deployment_in_progress(deploys, release, force_deploy)
 
         try:
-            # create the application config in k8s (secret in this case) for all deploy objects
-            self.set_application_config(release)
-
-            # gather all proc types to be deployed
-            tasks = [
-                functools.partial(
+            tasks = []
+            for scale_type, kwargs in deploys.items():
+                self.set_application_config(release, scale_type)
+                tasks.append(functools.partial(
                     self.scheduler().deploy,
                     namespace=self.id,
                     name=self._get_job_id(scale_type, release.canary),
                     image=release.get_deploy_image(scale_type),
                     command=release.get_deploy_command(scale_type),
                     args=release.get_deploy_args(scale_type),
                     **kwargs
-                ) for scale_type, kwargs in deploys.items()
-            ]
+                ))
             try:
                 apply_tasks(tasks)
             except KubeException as e:
@@ -721,6 +717,8 @@ def _scale_pods(self, scale_types, release, app_settings):
                 volume for volume in volumes if scale_type in volume.path.keys()]
             data = self._gather_app_settings(
                 release, app_settings, scale_type, replicas, volumes=scale_type_volumes)
+            # create the application config in k8s (secret in this case) for all deploy objects
+            self.set_application_config(release, scale_type)
             # gather all proc types to be deployed
             tasks.append(
                 functools.partial(
@@ -734,8 +732,6 @@ def _scale_pods(self, scale_types, release, app_settings):
                 )
             )
         try:
-            # create the application config in k8s (secret in this case) for all deploy objects
-            self.set_application_config(release)
             apply_tasks(tasks)
         except Exception as e:
             err = '(scale): {}'.format(e)
@@ -912,7 +908,7 @@ def _scheduler_filter(self, **kwargs):
             labels.update({'version': version})
         return labels
 
-    def _build_env_vars(self, release):
+    def _build_env_vars(self, release, procfile_type):
         """
         Build a dict of env vars, setting default vars based on app type
         and then combining with the user set ones
@@ -935,9 +931,9 @@ def _build_env_vars(self, release):
         port = release.get_port()
         if port:
             default_env['PORT'] = port
-
         # merge envs on top of default to make envs win
         default_env.update(release.config.values)
+        default_env.update(release.config.typed_values.get(procfile_type, {}))
         return default_env
 
     def _get_private_registry_config(self, image, registry=None):
@@ -999,7 +995,7 @@ def _gather_app_settings(self, release, app_settings, procfile_type, replicas, v
         Any global setting that can also be set per app goes here
         """
 
-        envs = self._build_env_vars(release)
+        envs = self._build_env_vars(release, procfile_type)
         # Obtain a limit plan that must exist, if raise error here, it must be a bug
         config = self._set_default_limit(release.config, procfile_type)
         limit_plan = LimitPlan.objects.get(id=config.limits.get(procfile_type))

rootfs/api/models/config.py

@@ -18,12 +18,13 @@ class Config(UuidAuditedModel):
     during runtime execution of the Application.
     """
     procfile_fields = ("lifecycle_post_start", "lifecycle_pre_stop", "tags", "limits",
-                       "healthcheck", "termination_grace_period")
+                       "typed_values", "healthcheck", "termination_grace_period")
     all_diff_fields = ("values", "registry") + procfile_fields
 
     owner = models.ForeignKey(User, on_delete=models.PROTECT)
     app = models.ForeignKey('App', on_delete=models.CASCADE)
     values = models.JSONField(default=dict, blank=True)
+    typed_values = models.JSONField(default=dict, blank=True)
     lifecycle_post_start = models.JSONField(default=dict, blank=True)
     lifecycle_pre_stop = models.JSONField(default=dict, blank=True)
     tags = models.JSONField(default=dict, blank=True)
@@ -79,17 +80,9 @@ def save(self, **kwargs):
                          'lifecycle_pre_stop', 'termination_grace_period']:
                 data = getattr(previous_config, attr, {}).copy()
                 new_data = getattr(self, attr, {}).copy()
-
-                # remove config keys if a null value is provided
-                for key, value in new_data.items():
-                    if value is None:
-                        # error if unsetting non-existing key
-                        if key not in data:
-                            raise UnprocessableEntity('{} does not exist under {}'.format(key, attr))  # noqa
-                        data.pop(key)
-                    else:
-                        data[key] = value
+                self._merge_data(attr, data, new_data)
                 setattr(self, attr, data)
+            self._set_typed_values(previous_config)
             self._set_limits(previous_config)
             self._set_healthcheck(previous_config)
             self._set_registry()
@@ -99,6 +92,18 @@ def save(self, **kwargs):
 
         return super(Config, self).save(**kwargs)
 
+    def _merge_data(self, field, data, new_data):
+        # remove config keys if a null value is provided
+        for key, value in new_data.items():
+            if value is None:
+                # error if unsetting non-existing key
+                if key not in data:
+                    raise UnprocessableEntity('{} does not exist under {}'.format(key, field))  # noqa
+                data.pop(key)
+            else:
+                data[key] = value
+        return data
+
     def _set_registry(self):
         # lower case all registry options for consistency
         self.registry = {key.lower(): value for key, value in self.registry.copy().items()}
@@ -144,20 +149,13 @@ def _set_tags(self, previous_config):
     def _set_limits(self, previous_config):
         data = getattr(previous_config, 'limits', {}).copy()
         new_data = getattr(self, 'limits', {}).copy()
-        # remove config keys if a null value is provided
+        # check procfile
         for key, value in new_data.items():
             if value is None:
-                # error if unsetting non-existing key
-                if key not in data:
-                    raise UnprocessableEntity(
-                        '{} does not exist under {}'.format(key, 'limits'))
                 if key in self.app.procfile_types:
                     raise UnprocessableEntity(
                         "the %s has already been used and cannot be deleted" % key)
-                else:
-                    data.pop(key)
-            else:
-                data[key] = value
+        self._merge_data('limits', data, new_data)
         setattr(self, 'limits', data)
 
     def _set_healthcheck(self, previous_config):
@@ -168,17 +166,25 @@ def _set_healthcheck(self, previous_config):
             if value is None:
                 # error if unsetting non-existing key
                 if key not in data:
-                    raise UnprocessableEntity('{} does not exist under {}'.format(key, 'healthcheck'))  # noqa
+                    raise UnprocessableEntity(
+                        '{} does not exist under {}'.format(key, 'healthcheck'))
                 data.pop(key)
             else:
-                for probeType, probe in value.items():
-                    if probe is None:
-                        # error if unsetting non-existing key
-                        if key not in data or probeType not in data[key].keys():
-                            raise UnprocessableEntity('{} does not exist under {}'.format(key, 'healthcheck'))  # noqa
-                        data[key].pop(probeType)
-                    else:
-                        if key not in data:
-                            data[key] = {}
-                        data[key][probeType] = probe
+                data[key] = self._merge_data('healthcheck', data.get(key, {}), value)
         setattr(self, 'healthcheck', data)
+
+    def _set_typed_values(self, previous_config):
+        data = getattr(previous_config, 'typed_values', {}).copy()
+        new_data = getattr(self, 'typed_values', {}).copy()
+        # remove config keys if a null value is provided
+        for procfile_type, values in new_data.items():
+            if not values:
+                # error if unsetting non-existing key
+                if procfile_type not in data:
+                    raise UnprocessableEntity(
+                        '{} does not exist under {}'.format(procfile_type, 'typed_values'))
+                data.pop(procfile_type)
+            else:
+                data[procfile_type] = self._merge_data(
+                    'typed_values', data.get(procfile_type, {}), values)
+        setattr(self, 'typed_values', data)

rootfs/api/serializers/__init__.py

@@ -89,7 +89,9 @@ def to_representation(self, obj):
                 continue
 
             try:
-                if self.convert_to_str:
+                if isinstance(v, (dict, list)):
+                    self.to_representation(v)
+                elif self.convert_to_str:
                     obj[k] = str(v)
             except ValueError:
                 obj[k] = v
@@ -236,6 +238,7 @@ class ConfigSerializer(serializers.ModelSerializer):
     app = serializers.SlugRelatedField(slug_field='id', queryset=models.app.App.objects.all())
     owner = serializers.ReadOnlyField(source='owner.username')
     values = JSONFieldSerializer(required=False, binary=True)
+    typed_values = JSONFieldSerializer(required=False, binary=True)
     limits = JSONFieldSerializer(required=False, binary=True)
     lifecycle_post_start = JSONFieldSerializer(required=False, binary=True)
     lifecycle_pre_stop = JSONFieldSerializer(required=False, binary=True)

rootfs/api/tests/test_app.py

@@ -16,7 +16,7 @@
 from django.test.utils import override_settings
 from rest_framework.authtoken.models import Token
 
-from api.models.app import App
+from api.models.app import App, PROCFILE_TYPE_WEB
 from api.models.config import Config
 from scheduler import KubeException, KubeHTTPException
 
@@ -585,14 +585,14 @@ def test_build_env_vars(self, mock_requests):
         app = App.objects.create(owner=self.user)
         # Make sure an exception is raised when calling without a build available
         with self.assertRaises(DryccException):
-            app._build_env_vars(app.release_set.latest())
+            app._build_env_vars(app.release_set.latest(), PROCFILE_TYPE_WEB)
         data = {'image': 'autotest/example', 'stack': 'heroku-18'}
         url = "/v2/apps/{app.id}/builds".format(**locals())
         response = self.client.post(url, data)
         self.assertEqual(response.status_code, 201, response.data)
         time_created = app.release_set.latest().created
         self.assertEqual(
-            app._build_env_vars(app.release_set.latest()),
+            app._build_env_vars(app.release_set.latest(), PROCFILE_TYPE_WEB),
             {
                 'DRYCC_APP': app.id,
                 'WORKFLOW_RELEASE': 'v2',
@@ -607,7 +607,7 @@ def test_build_env_vars(self, mock_requests):
         self.assertEqual(response.status_code, 201, response.data)
         time_created = app.release_set.latest().created
         self.assertEqual(
-            app._build_env_vars(app.release_set.latest()),
+            app._build_env_vars(app.release_set.latest(), PROCFILE_TYPE_WEB),
             {
                 'DRYCC_APP': app.id,
                 'WORKFLOW_RELEASE': 'v3',

rootfs/api/tests/test_config.py

@@ -173,9 +173,10 @@ def test_response_data(self, mock_requests):
         body = {'values': json.dumps({'PORT': '5000'})}
         response = self.client.post(url, body)
         for key in response.data:
-            self.assertIn(key, ['uuid', 'owner', 'created', 'updated', 'app', 'values', 'limits',
-                                'tags', 'registry', 'healthcheck', 'lifecycle_post_start',
-                                'lifecycle_pre_stop', 'termination_grace_period'])
+            self.assertIn(key, ['uuid', 'owner', 'created', 'updated', 'app', 'values',
+                                'typed_values', 'limits', 'tags', 'registry', 'healthcheck',
+                                'lifecycle_post_start', 'lifecycle_pre_stop',
+                                'termination_grace_period'])
         expected = {
             'owner': self.user.username,
             'app': app_id,
@@ -205,8 +206,9 @@ def test_response_data_types_converted(self, mock_requests):
         self.assertEqual(response.status_code, 201, response.data)
         for key in response.data:
             self.assertIn(key, ['uuid', 'owner', 'created', 'updated', 'app', 'values', 'limits',
-                                'tags', 'registry', 'healthcheck', 'lifecycle_post_start',
-                                'lifecycle_pre_stop', 'termination_grace_period'])
+                                'typed_values', 'tags', 'registry', 'healthcheck',
+                                'lifecycle_post_start', 'lifecycle_pre_stop',
+                                'termination_grace_period'])
         expected = {
             'owner': self.user.username,
             'app': app_id,
@@ -586,3 +588,33 @@ def test_set_config_limits_run(self, *args, **kwargs):
         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)
+
+    def test_config_set_typed_values(self, mock_requests):
+        """
+        Test that config sets on the same key function properly
+        """
+        app_id = self.create_app()
+        url = f"/v2/apps/{app_id}/config"
+
+        # set an initial config value
+        body = {'typed_values': {'web': {'PORT': '5000'}}}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+        self.assertIn('PORT', response.data['typed_values']['web'])
+
+        # reset same config value
+        body = {'typed_values': {'web': {'PORT': '5001'}}}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+        self.assertIn('PORT', response.data['typed_values']['web'])
+        self.assertEqual(response.data['typed_values']['web']['PORT'], '5001')
+        # unset PORT
+        body = {'typed_values': {'web': {'PORT': None}}}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+        self.assertEqual(response.data['typed_values']['web'], {}, response.data)
+        # unset web
+        body = {'typed_values': {'web': None}}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 201, response.data)
+        self.assertEqual(response.data['typed_values'], {}, response.data)

rootfs/scheduler/resources/pod.py

@@ -216,7 +216,8 @@ def _set_container(self, namespace, container_name, data, **kwargs):
 
         if env:
             # map application configuration (env secret) to env vars
-            secret_name = "{}-{}-env".format(namespace, kwargs.get('version'))
+            secret_name = "{}-{}-{}-env".format(
+                namespace, kwargs.get('app_type'), kwargs.get('version'))
             for key in env.keys():
                 item = {
                     "name": key,