fix(controller): prevent DeployLock deadlocks and remove redundant admission locks

Author: duanhongyi

rootfs/api/admissions.py

@@ -27,30 +27,28 @@ def detect(self, request: Request) -> bool:
 
     def handle(self, request: Request) -> bool:
         app_id = request["object"]["metadata"]["namespace"]
-        app = models.app.App.objects.filter(id=app_id).first()
         ptype = request["object"]["metadata"].get("labels", {}).get("type", "")
-        if app and ptype:
-            lock = app.lock()
-            try:
-                lock.acquire()
-                status = request["object"]["status"]
-                replicas = request["object"]["spec"].get("replicas", 0)
-                if "active" in status:
-                    replicas += 1
-                elif "succeeded" in status or "failed" in status:
-                    replicas -= 1
-                replicas = 0 if replicas < 0 else replicas
-                if app.structure.get(ptype, 0) != replicas:
-                    models.app.App.objects.filter(id=app.id).update(
-                        structure=Func(
-                            F("structure"),
-                            Value([ptype]),
-                            Value(replicas, JSONField()),
-                            function="jsonb_set",
-                        )
-                    )
-            finally:
-                lock.release()
+        if not ptype:
+            return True
+        status = request["object"]["status"]
+        replicas = request["object"]["spec"].get("replicas", 0)
+        if "active" in status:
+            replicas += 1
+        elif "succeeded" in status or "failed" in status:
+            replicas -= 1
+        replicas = 0 if replicas < 0 else replicas
+        # jsonb_set on a single row is atomic at the PostgreSQL level; no lock
+        # is needed because `replicas` is computed from the request payload
+        # alone (not read-modify-write of app.structure). Filter touches 0 rows
+        # if the app no longer exists, which is safe.
+        models.app.App.objects.filter(id=app_id).update(
+            structure=Func(
+                F("structure"),
+                Value([ptype]),
+                Value(replicas, JSONField()),
+                function="jsonb_set",
+            )
+        )
         return True
 
 
@@ -68,28 +66,25 @@ def detect(self, request: Request) -> bool:
 
     def handle(self, request: Request) -> bool:
         app_id = request["object"]["metadata"]["namespace"]
-        app = models.app.App.objects.filter(id=app_id).first()
         ptype = None
         for item in request["object"]["status"]["selector"].split(","):
             key, value = item.split("=")
             if key == "type":
                 ptype = value
-        if app and ptype:
-            lock = app.lock()
-            try:
-                lock.acquire()
-                replicas = request["object"]["spec"].get("replicas", 0)
-                if app.structure.get(ptype, 0) != replicas:
-                    models.app.App.objects.filter(id=app.id).update(
-                        structure=Func(
-                            F("structure"),
-                            Value([ptype]),
-                            Value(replicas, JSONField()),
-                            function="jsonb_set",
-                        )
-                    )
-            finally:
-                lock.release()
+        if not ptype:
+            return True
+        replicas = request["object"]["spec"].get("replicas", 0)
+        # jsonb_set is an atomic single-row UPDATE in PostgreSQL; no lock is
+        # needed because `replicas` comes straight from the request payload.
+        # Filter touches 0 rows if the app no longer exists, which is safe.
+        models.app.App.objects.filter(id=app_id).update(
+            structure=Func(
+                F("structure"),
+                Value([ptype]),
+                Value(replicas, JSONField()),
+                function="jsonb_set",
+            )
+        )
         return True
 
 

rootfs/api/exceptions.py

@@ -42,6 +42,8 @@ def custom_exception_handler(exc, context):
     # No response means DRF couldn't handle it
     # Output a generic 500 in a JSON format
     if response is None:
+        import traceback
+        traceback.print_exc()
         logging.exception('Uncaught Exception', exc_info=exc)
         set_rollback()
         return Response({'detail': 'Server Error'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR)

rootfs/api/migrations/0028_workspace_remove_app_owner_remove_appsettings_owner_and_more.py

@@ -2,6 +2,7 @@
 
 import api.models.workspace
 import django.db.models.deletion
+import uuid
 from django.conf import settings
 from django.db import migrations, models
 
@@ -16,12 +17,16 @@ class Migration(migrations.Migration):
         migrations.CreateModel(
             name='Workspace',
             fields=[
-                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
-                ('name', models.SlugField(max_length=150, unique=True, validators=[api.models.workspace.validate_workspace_name], verbose_name='workspace name')),
-                ('email', models.EmailField(max_length=254, verbose_name='email address')),
                 ('created', models.DateTimeField(auto_now_add=True)),
                 ('updated', models.DateTimeField(auto_now=True)),
+                ('uuid', models.UUIDField(auto_created=True, default=uuid.uuid4, editable=False, primary_key=True, serialize=False, unique=True, verbose_name='UUID')),
+                ('id', models.SlugField(max_length=150, unique=True, validators=[api.models.workspace.validate_workspace_id], verbose_name='workspace name')),
+                ('email', models.EmailField(max_length=254, verbose_name='email address')),
+                ('uid', models.PositiveIntegerField(unique=True)),
             ],
+            options={
+                'abstract': False,
+            },
         ),
         migrations.RemoveField(
             model_name='app',
@@ -116,4 +121,4 @@ class Migration(migrations.Migration):
                 'unique_together': {('user', 'workspace')},
             },
         ),
-    ]
+    ]

rootfs/api/migrations/0031_app_uid_alter_workspace_uid.py

@@ -0,0 +1,36 @@
+# Generated by Django 5.2.14 on 2026-05-26 01:01
+
+from django.db import migrations, models
+
+
+def set_app_uids(apps, schema_editor):
+    App = apps.get_model('api', 'App')
+    
+    # 模拟 get_next_uid 的类似逻辑，或者是直接调用，但需要防范现有库里全是 null 导致的 TypeError
+    # 使用 id 排序来保证赋值的确定性
+    uid = App.objects.aggregate(models.Max('uid'))['uid__max'] or 0
+    for app in App.objects.filter(uid__isnull=True).order_by('created'):
+        uid += 1
+        app.uid = uid
+        app.save(update_fields=['uid'])
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0030_delete_blocklist_remove_app_suspended_state'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='app',
+            name='uid',
+            field=models.PositiveIntegerField(blank=True, null=True),
+        ),
+        migrations.RunPython(set_app_uids, migrations.RunPython.noop),
+        migrations.AlterField(
+            model_name='app',
+            name='uid',
+            field=models.PositiveIntegerField(unique=True),
+        ),
+    ]

rootfs/api/models/app.py

@@ -22,7 +22,7 @@
 from rest_framework.exceptions import ValidationError
 
 from api.tasks import send_app_log
-from api.utils import get_httpclient, dict_diff
+from api.utils import get_httpclient, dict_diff, get_next_uid
 from api.exceptions import AlreadyExists, DryccException, ServiceUnavailable
 from api.utils import (
     CacheLock, DeployLock, generate_app_name, apply_tasks, validate_reserved_names)
@@ -71,6 +71,7 @@ class App(UuidAuditedModel):
 
     id = models.SlugField(max_length=63, unique=True, null=True,
                           validators=[validate_app_id])
+    uid = models.PositiveIntegerField(unique=True, editable=False)
     workspace = models.ForeignKey('Workspace', on_delete=models.CASCADE)
     structure = models.JSONField(
         default=dict, blank=True, validators=[validate_app_structure])
@@ -99,6 +100,8 @@ def save(self, *args, **kwargs):
             except KubeHTTPException:
                 pass
 
+        if not self.uid:
+            self.uid = get_next_uid(App)
         application = super(App, self).save(**kwargs)
 
         # create all the required resources
@@ -1062,8 +1065,11 @@ def _check_deployment_in_progress(self, deploys, force_deploy=False):
     def _merge_structure(self, release, prev_release):
         """Scale to default structure based on release type"""
         lock = self.lock()
+        if not lock.acquire():
+            raise ServiceUnavailable(
+                f'could not acquire app lock for {self.id}'
+            )
         try:
-            lock.acquire()
             self.refresh_from_db()
             default_structure = {}
             for ptype in release.ptypes:

rootfs/api/models/appsettings.py

@@ -77,7 +77,7 @@ def _update_field(self, field, previous_settings):
             setattr(self, field, True)
         elif old != new:
             self.summary += [
-                "{} changed {} from {} to {}".format(self.app.workspace.name, field, old, new)
+                "{} changed {} from {} to {}".format(self.app.workspace.id, field, old, new)
             ]
 
     def _update_autoscale(self, previous_settings):
@@ -118,7 +118,7 @@ def _update_autoscale(self, previous_settings):
             deleted = 'deleted autoscale for process type ' + deleted if deleted else ''
             changes = ', '.join(i for i in (added, changed, deleted) if i)
             if changes:
-                self.summary += ["{} {}".format(self.app.workspace.name, changes)]
+                self.summary += ["{} {}".format(self.app.workspace.id, changes)]
 
     def _update_label(self, previous_settings):
         data = getattr(previous_settings, 'label', {}).copy()
@@ -150,7 +150,7 @@ def _update_label(self, previous_settings):
             if changes:
                 if self.summary:
                     self.summary += ' and '
-                self.summary += ["{} {}".format(self.app.workspace.name, changes)]
+                self.summary += ["{} {}".format(self.app.workspace.id, changes)]
 
     @transaction.atomic
     def save(self, ignore_update_fields=None, *args, **kwargs):
@@ -176,7 +176,7 @@ def save(self, ignore_update_fields=None, *args, **kwargs):
 
         if not self.summary and previous_settings:
             self.delete()
-            raise AlreadyExists("{} changed nothing".format(self.app.workspace.name))
+            raise AlreadyExists("{} changed nothing".format(self.app.workspace.id))
         super(AppSettings, self).save(**kwargs)
         summary = ' '.join(self.summary)
         self.log('summary of app setting changes: {}'.format(summary), logging.DEBUG)

rootfs/api/models/base.py

@@ -63,7 +63,7 @@ def scheduler(self):
         labels = annotations = {}
         if hasattr(self, 'app'):
             labels["drycc.cc/app_id"] = str(self.app.id)
-            labels["drycc.cc/workspace_id"] = str(self.app.workspace_id)
+            labels["drycc.cc/workspace_id"] = str(self.app.workspace.id)
         return get_scheduler(metadata={"labels": labels, "annotations": annotations})
 
 

rootfs/api/models/release.py

@@ -425,28 +425,28 @@ def save(self, *args, **kwargs):  # noqa
             old_config = prev_release.config if prev_release else None
             # if the build changed, log it and who pushed it
             if self.version == 1:
-                self.summary += "{} created initial release".format(self.app.workspace.name)
+                self.summary += "{} created initial release".format(self.app.workspace.id)
             elif self.build != old_build:
                 if self.build.sha:
                     self.summary += "{} deployed {}".format(
-                        self.app.workspace.name, self.build.sha[:7])
+                        self.app.workspace.id, self.build.sha[:7])
                 else:
                     self.summary += "{} deployed {}".format(
-                        self.app.workspace.name, self.build.image)
+                        self.app.workspace.id, self.build.image)
             elif self.config != old_config:
                 for field, diff in self.config.diff(old_config).items():
                     diff_list = []
                     for diff_type, values in diff.items():
                         diff_list.append(f'{diff_type} {field} {", ".join(values)}')
                     if diff_list:
                         changes = ', '.join(diff_list)
-                        self.summary += "{} {}".format(self.app.workspace.name, changes)
+                        self.summary += "{} {}".format(self.app.workspace.id, changes)
             if not self.summary:
                 if self.version == 1:
-                    self.summary = "{} created the initial release".format(self.app.workspace.name)
+                    self.summary = "{} created the initial release".format(self.app.workspace.id)
                 else:
                     # There were no changes to this release
                     raise AlreadyExists(
-                        "{} changed nothing - release stopped".format(self.app.workspace.name)
+                        "{} changed nothing - release stopped".format(self.app.workspace.id)
                     )
         super(Release, self).save(*args, **kwargs)

rootfs/api/models/tls.py

@@ -133,12 +133,12 @@ def _check_previous_tls_settings(self):
             if self.https_enforced is not None:
                 if previous_tls_settings.https_enforced == self.https_enforced:
                     raise AlreadyExists(
-                        "{} changed nothing".format(self.app.workspace.name))
+                        "{} changed nothing".format(self.app.workspace.id))
                 self.certs_auto_enabled = previous_tls_settings.certs_auto_enabled
             elif self.certs_auto_enabled is not None:
                 if previous_tls_settings.certs_auto_enabled == self.certs_auto_enabled:
                     raise AlreadyExists(
-                        "{} changed nothing".format(self.app.workspace.name))
+                        "{} changed nothing".format(self.app.workspace.id))
                 self.https_enforced = previous_tls_settings.https_enforced
             previous_tls_settings.delete()
         except TLS.DoesNotExist:

rootfs/api/models/workspace.py

@@ -10,13 +10,14 @@
 from django.template.loader import render_to_string
 
 from rest_framework.exceptions import ValidationError
-from api.utils import validate_reserved_names, get_local_host
+from api.utils import validate_reserved_names, get_local_host, get_next_uid
+from api.models.base import UuidAuditedModel
 
 User = get_user_model()
 logger = logging.getLogger(__name__)
 
 
-def validate_workspace_name(value):
+def validate_workspace_id(value):
     """
     Check that the value follows the kubernetes name constraints
     """
@@ -27,25 +28,29 @@ def validate_workspace_name(value):
     validate_reserved_names(value)
 
 
-class Workspace(models.Model):
-    name = models.SlugField(
+class Workspace(UuidAuditedModel):
+    id = models.SlugField(
         _("workspace name"),
         max_length=150,
         unique=True,
-        validators=[validate_workspace_name],
+        validators=[validate_workspace_id],
     )
+    uid = models.PositiveIntegerField(unique=True, editable=False)
     email = models.EmailField(_("email address"))
-    created = models.DateTimeField(auto_now_add=True)
-    updated = models.DateTimeField(auto_now=True)
 
     def has_member(self, user, role=None):
         kwargs = {'user': user, 'workspace': self}
         if role:
             kwargs['role'] = role
         return WorkspaceMember.objects.filter(**kwargs).exists()
 
+    def save(self, *args, **kwargs):
+        if not self.uid:
+            self.uid = get_next_uid(Workspace)
+        super().save(*args, **kwargs)
+
     def __str__(self):
-        return self.name
+        return self.id
 
 
 class WorkspaceMember(models.Model):
@@ -63,7 +68,7 @@ class WorkspaceMember(models.Model):
     workspace = models.ForeignKey(Workspace, on_delete=models.CASCADE)
 
     def __str__(self):
-        return f"{self.user.username} - {self.workspace.name} ({self.role})"
+        return f"{self.user.username} - {self.workspace.id} ({self.role})"
 
     class Meta:
         unique_together = ('user', 'workspace')
@@ -97,12 +102,12 @@ def send_email(self, request):
         if count > settings.DRYCC_INVITATION_EMAIL_LIMIT:
             raise ValidationError("Too many invitation emails, please try again later")
         domain = get_local_host(request)
-        mail_subject = f'We Invite You to Join the {self.workspace.name} Workspace.'
+        mail_subject = f'We Invite You to Join the {self.workspace.id} Workspace.'
         message = render_to_string(
             'workspace/workspace_invitation.html',
             {'domain': domain, 'invitation': self}
         )
         send_mail(mail_subject, message, None, [self.email], fail_silently=True)
 
     def __str__(self):
-        return f"Invitation for {self.email} to join {self.workspace.name}"
+        return f"Invitation for {self.email} to join {self.workspace.id}"