feat(tags): add ptype tags

Author: duanhongyi

rootfs/api/models/app.py

@@ -714,7 +714,7 @@ def _deploy(self, deploys, procfile_types, prev_release,
         # Sort deploys so routable comes first
         deploys = OrderedDict(sorted(deploys.items(), key=lambda d: d[1].get('routable')))
         # Check if any proc type has a Deployment in progress
-        self._check_deployment_in_progress(deploys, release, force_deploy)
+        self._check_deployment_in_progress(deploys, force_deploy)
 
         try:
             tasks = []
@@ -969,7 +969,7 @@ def _verify_tcp_health(self, service, **kwargs):
                 else:
                     time.sleep(3)
 
-    def _check_deployment_in_progress(self, deploys, release, force_deploy=False):
+    def _check_deployment_in_progress(self, deploys, force_deploy=False):
         if force_deploy:
             return
         for scale_type, kwargs in deploys.items():
@@ -1127,7 +1127,7 @@ def _gather_app_settings(self, release, app_settings, procfile_type, replicas, v
         healthcheck = config.healthcheck.get(procfile_type, {})
         volumes, volume_mounts = self._get_volumes_and_mounts(procfile_type, volumes)
         return {
-            'tags': config.tags,
+            'tags': config.tags.get(procfile_type, {}),
             'envs': envs,
             'registry': config.registry,
             'replicas': replicas,

rootfs/api/models/config.py

@@ -1,6 +1,4 @@
-import json
 import logging
-from django.conf import settings
 from django.db import models
 from django.contrib.auth import get_user_model
 from api.utils import dict_diff
@@ -43,20 +41,20 @@ def __str__(self):
 
     def previous(self):
         """
-        Return the previous Release to this one.
+        Return the previous Config to this one.
 
-        :return: the previous :class:`Release`, or None
+        :return: the previous :class:`Config`, or None
         """
         configs = self.app.config_set
         if self.pk:
             configs = configs.exclude(pk=self.pk)
 
         try:
-            # Get the Release previous to this one
-            prev_release = configs.latest()
-        except Release.DoesNotExist:
-            prev_release = None
-        return prev_release
+            # Get the Config previous to this one
+            prev_config = configs.latest()
+        except Config.DoesNotExist:
+            prev_config = None
+        return prev_config
 
     def diff(self, config=None):
         old_config = config if config else self.previous()
@@ -76,7 +74,7 @@ def save(self, **kwargs):
                 # usually means a totally new app
                 previous_config = self.app.config_set.latest()
 
-            for attr in ['tags', 'registry', 'values', 'lifecycle_post_start',
+            for attr in ['registry', 'values', 'lifecycle_post_start',
                          'lifecycle_pre_stop', 'termination_grace_period']:
                 data = getattr(previous_config, attr, {}).copy()
                 new_data = getattr(self, attr, {}).copy()
@@ -88,7 +86,7 @@ def save(self, **kwargs):
             self._set_registry()
             self._set_tags(previous_config)
         except Config.DoesNotExist:
-            self._set_tags({'tags': {}})
+            self._set_tags(previous_config={'tags': {}})
 
         return super(Config, self).save(**kwargs)
 
@@ -117,34 +115,31 @@ def _set_registry(self):
 
     def _set_tags(self, previous_config):
         """verify the tags exist on any nodes as labels"""
-        if not self.tags:
-            if settings.DRYCC_DEFAULT_CONFIG_TAGS:
-                try:
-                    tags = json.loads(settings.DRYCC_DEFAULT_CONFIG_TAGS)
-                    self.tags = tags
-                except json.JSONDecodeError as e:
-                    logger.exception(e)
-                    return
+        data = getattr(previous_config, 'tags', {}).copy()
+        new_data = getattr(self, 'tags', {}).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, 'tags'))
+                data.pop(procfile_type)
             else:
-                return
-
-        # Get all nodes with label selectors
-        nodes = self.scheduler().node.get(labels=self.tags).json()
-        if nodes['items']:
-            return
-
-        labels = ['{}={}'.format(key, value) for key, value in self.tags.items()]
-        message = 'No nodes matched the provided labels: {}'.format(', '.join(labels))
-
-        # Find out if there are any other tags around
-        old_tags = getattr(previous_config, 'tags')
-        if old_tags:
-            old = ['{}={}'.format(key, value) for key, value in old_tags.items()]
-            new = set(labels) - set(old)
-            if new:
-                message += ' - Addition of {} is the cause'.format(', '.join(new))
-
-        raise DryccException(message)
+                if not self.scheduler().node.get(labels=values).json()['items']:
+                    labels = ['{}={}'.format(key, value) for key, value in values.items()]
+                    message = 'No nodes matched the provided labels: {}'.format(', '.join(labels))
+                    # Find out if there are any other tags around
+                    old_tags = previous_config.tags.get(procfile_type, {})
+                    if old_tags:
+                        old = ['{}={}'.format(key, value) for key, value in old_tags.items()]
+                        new = set(labels) - set(old)
+                        if new:
+                            message += ' - Addition of {} is the cause'.format(', '.join(new))
+                    raise DryccException(message)
+                data[procfile_type] = self._merge_data(
+                    'tags', data.get(procfile_type, {}), values)
+        setattr(self, 'tags', data)
 
     def _set_limits(self, previous_config):
         data = getattr(previous_config, 'limits', {}).copy()

rootfs/api/serializers/__init__.py

@@ -286,8 +286,7 @@ def validate_values(data):
     @classmethod
     def validate_typed_values(cls, data):
         for procfile_type, values in data.items():
-            if not re.match(PROCTYPE_MATCH, procfile_type):
-                raise serializers.ValidationError(PROCTYPE_MISMATCH_MSG)
+            validate_procfile_type(procfile_type)
             if values is None:  # use NoneType to unset an item
                 continue
             cls.validate_values(values)
@@ -297,8 +296,7 @@ def validate_typed_values(cls, data):
     def validate_limits(data):
         req_plan_ids = []
         for procfile_type, plan_id in data.items():
-            if not re.match(PROCTYPE_MATCH, procfile_type):
-                raise serializers.ValidationError(PROCTYPE_MISMATCH_MSG)
+            validate_procfile_type(procfile_type)
             if plan_id is not None:
                 req_plan_ids.append(plan_id)
         plan_ids = [plan.id for plan in models.limit.LimitPlan.objects.filter(
@@ -311,8 +309,7 @@ def validate_limits(data):
     @staticmethod
     def validate_termination_grace_period(data):
         for procfile_type, value in data.items():
-            if not re.match(PROCTYPE_MATCH, procfile_type):
-                raise serializers.ValidationError(PROCTYPE_MISMATCH_MSG)
+            validate_procfile_type(procfile_type)
             if value is None:  # use NoneType to unset an item
                 continue
             timeout = re.match(TERMINATION_GRACE_PERIOD_MATCH, str(value))
@@ -324,37 +321,38 @@ def validate_termination_grace_period(data):
 
     @staticmethod
     def validate_tags(data):
-        for key, value in data.items():
-            if value is None:  # use NoneType to unset an item
-                continue
-
-            # split key into a prefix and name
-            if '/' in key:
-                prefix, name = key.split('/')
-            else:
-                prefix, name = None, key
+        for procfile_type, values in data.items():
+            validate_procfile_type(procfile_type)
+            for key, value in values.items():
+                if value is None:  # use NoneType to unset an item
+                    continue
 
-            # validate optional prefix
-            if prefix:
-                if len(prefix) > 253:
-                    raise serializers.ValidationError(
-                        "Tag key prefixes must 253 characters or less.")
+                # split key into a prefix and name
+                if '/' in key:
+                    prefix, name = key.split('/')
+                else:
+                    prefix, name = None, key
 
-                for part in prefix.split('/'):
-                    if not re.match(TAGVAL_MATCH, part):
+                # validate optional prefix
+                if prefix:
+                    if len(prefix) > 253:
                         raise serializers.ValidationError(
-                            "Tag key prefixes must be DNS subdomains.")
+                            "Tag key prefixes must 253 characters or less.")
 
-            # validate required name
-            if not re.match(TAGVAL_MATCH, name):
-                raise serializers.ValidationError(
-                    "Tag keys must be alphanumeric or \"-_.\", and 1-63 characters.")
+                    for part in prefix.split('/'):
+                        if not re.match(TAGVAL_MATCH, part):
+                            raise serializers.ValidationError(
+                                "Tag key prefixes must be DNS subdomains.")
 
-            # validate value if it isn't empty
-            if value and not re.match(TAGVAL_MATCH, str(value)):
-                raise serializers.ValidationError(
-                    "Tag values must be alphanumeric or \"-_.\", and 1-63 characters.")
+                # validate required name
+                if not re.match(TAGVAL_MATCH, name):
+                    raise serializers.ValidationError(
+                        "Tag keys must be alphanumeric or \"-_.\", and 1-63 characters.")
 
+                # validate value if it isn't empty
+                if value and not re.match(TAGVAL_MATCH, str(value)):
+                    raise serializers.ValidationError(
+                        "Tag values must be alphanumeric or \"-_.\", and 1-63 characters.")
         return data
 
     @staticmethod
@@ -371,8 +369,7 @@ def validate_registry(data):
     @staticmethod
     def validate_healthcheck(data):
         for procfile_type, healthcheck in data.items():
-            if not re.match(PROCTYPE_MATCH, procfile_type):
-                raise serializers.ValidationError(PROCTYPE_MISMATCH_MSG)
+            validate_procfile_type(procfile_type)
             if healthcheck is None:
                 continue
             for key, value in healthcheck.items():
@@ -630,8 +627,7 @@ def validate_path(data):
         logger.debug(f"mount validate_path data: {data}")
         new_data = {}
         for key, value in data.items():
-            if not re.match(PROCTYPE_MATCH, key):
-                raise serializers.ValidationError(PROCTYPE_MISMATCH_MSG)
+            validate_procfile_type(key)
             if value is None:  # use NoneType to unset an item
                 new_data[key] = value
                 continue

rootfs/api/settings/production.py

@@ -361,8 +361,6 @@
 
 DRYCC_APP_POD_EXEC_TIMEOUT = int(os.environ.get('DRYCC_APP_POD_EXEC_TIMEOUT', "3600"))
 
-DRYCC_DEFAULT_CONFIG_TAGS = os.environ.get('DRYCC_DEFAULT_CONFIG_TAGS', '')
-
 KUBERNETES_DEPLOYMENTS_REVISION_HISTORY_LIMIT = os.environ.get(
     'KUBERNETES_DEPLOYMENTS_REVISION_HISTORY_LIMIT', None)
 

rootfs/api/settings/testing.py

@@ -43,8 +43,6 @@
 KUBERNETES_POD_TERMINATION_GRACE_PERIOD_SECONDS = int(
     os.environ.get('KUBERNETES_POD_TERMINATION_GRACE_PERIOD_SECONDS', 2))
 
-DRYCC_DEFAULT_CONFIG_TAGS = os.environ.get('DRYCC_DEFAULT_CONFIG_TAGS', '')
-
 DRYCC_APP_STORAGE_CLASS = os.environ.get('DRYCC_APP_STORAGE_CLASS', '')
 
 

rootfs/api/tests/test_config.py

@@ -8,7 +8,6 @@
 from io import StringIO
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
-from django.conf import settings
 from django.core.management import call_command
 
 from unittest import mock
@@ -43,8 +42,6 @@ def setUp(self):
         self.app = App.objects.all()[0]
 
     def tearDown(self):
-        # Restore default tags to empty string
-        settings.DRYCC_DEFAULT_CONFIG_TAGS = ''
         # make sure every test has a clean slate for k8s mocking
         cache.clear()
 
@@ -129,40 +126,6 @@ def test_config(self, mock_requests):
         self.assertEqual(response.status_code, 405, response.data)
         return config5
 
-    def test_default_tags(self, mock_requests):
-        settings.DRYCC_DEFAULT_CONFIG_TAGS = '{"ssd": "true"}'
-        app_id = self.create_app()
-        url = f"/v2/apps/{app_id}/config"
-        response = self.client.get(url)
-        expected = {
-            'owner': self.user.username,
-            'app': app_id,
-            'values': {},
-            'limits': {
-                  PROCFILE_TYPE_RUN: 'std1.large.c1m1',
-                  PROCFILE_TYPE_WEB: 'std1.large.c1m1'
-            },
-            'tags': {'ssd': 'true'},
-            'registry': {}
-        }
-        self.assertEqual(response.data, expected | response.data)
-
-        # make sure changes not drop tags
-        body = {'values': json.dumps({'PORT': '5001'})}
-        response = self.client.post(url, body)
-        expected = {
-            'owner': self.user.username,
-            'app': app_id,
-            'values': {'PORT': '5001'},
-            'limits': {
-                PROCFILE_TYPE_RUN: 'std1.large.c1m1',
-                PROCFILE_TYPE_WEB: 'std1.large.c1m1'
-            },
-            'tags': {'ssd': 'true'},
-            'registry': {}
-        }
-        self.assertEqual(response.data, response.data | expected)
-
     def test_response_data(self, mock_requests):
         """Test that the serialized response contains only relevant data."""
         app_id = self.create_app()

rootfs/api/tests/test_event.py

@@ -6,7 +6,6 @@
 """
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
-from django.conf import settings
 
 from api.models.app import App
 
@@ -33,8 +32,6 @@ def setUp(self):
         self.app = App.objects.all()[0]
 
     def tearDown(self):
-        # Restore default tags to empty string
-        settings.DRYCC_DEFAULT_CONFIG_TAGS = ''
         # make sure every test has a clean slate for k8s mocking
         cache.clear()
 

rootfs/api/tests/test_ptypes.py

@@ -6,7 +6,6 @@
 """
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
-from django.conf import settings
 
 from api.models.app import App
 
@@ -33,8 +32,6 @@ def setUp(self):
         self.app = App.objects.all()[0]
 
     def tearDown(self):
-        # Restore default tags to empty string
-        settings.DRYCC_DEFAULT_CONFIG_TAGS = ''
         # make sure every test has a clean slate for k8s mocking
         cache.clear()
 

rootfs/api/tests/test_resource.py

@@ -6,7 +6,6 @@
 """
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
-from django.conf import settings
 from api.tests import adapter, DryccTransactionTestCase
 from api.exceptions import DryccException
 import requests_mock
@@ -27,8 +26,6 @@ def setUp(self):
         self.app_id = self.create_app()
 
     def tearDown(self):
-        # Restore default tags to empty string
-        settings.DRYCC_DEFAULT_CONFIG_TAGS = ''
         # make sure every test has a clean slate for k8s mocking
         cache.clear()