From abbe1bb5704164ef4a149488b2fd3100bc219a6f Mon Sep 17 00:00:00 2001
From: lijianguo <lijianguo1991@outlook.com>
Date: Mon, 18 Dec 2023 08:59:04 +0800
Subject: [PATCH] fix(resources): after resources instance update, could not
 delete

---
 charts/controller/templates/_helpers.tpl |  3 +--
 charts/controller/values.yaml            |  1 +
 rootfs/api/models/resource.py            | 12 +++++++-----
 rootfs/api/serializers.py                |  4 +++-
 rootfs/api/tests/test_resource.py        | 16 +++++++++++-----
 rootfs/scheduler/resources/svcat.py      | 18 ++++++++++++++++++
 6 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/charts/controller/templates/_helpers.tpl b/charts/controller/templates/_helpers.tpl
index f6cb99f32..26f304817 100644
--- a/charts/controller/templates/_helpers.tpl
+++ b/charts/controller/templates/_helpers.tpl
@@ -32,9 +32,8 @@ env:
 - name: "DRYCC_APP_POD_EXEC_TIMEOUT"
   value: "{{ .Values.appPodExecTimeout }}"
 {{- end }}
-
 - name: "TZ"
-  value: {{ .Values.time_zone | default "UTC" | quote }}
+  value: {{ .Values.timezone | default "UTC" | quote }}
 - name: "DJANGO_SETTINGS_MODULE"
   value: "api.settings.production"
 {{- if (.Values.deployHookUrls) }}
diff --git a/charts/controller/values.yaml b/charts/controller/values.yaml
index 51d7c821c..090edfabb 100644
--- a/charts/controller/values.yaml
+++ b/charts/controller/values.yaml
@@ -6,6 +6,7 @@ appImagePullPolicy: "Always"
 # A comma-separated list of URLs to send app release information to
 # See https://www.drycc.cc/managing-workflow/deploy-hooks/
 deployHookUrls: ""
+timezone: "UTC"
 # limitsCpu: "100m"
 # limitsMemory: "50Mi"
 # Possible values are:
diff --git a/rootfs/api/models/resource.py b/rootfs/api/models/resource.py
index 091a76882..a50bafdd8 100644
--- a/rootfs/api/models/resource.py
+++ b/rootfs/api/models/resource.py
@@ -92,7 +92,9 @@ def attach(self, *args, **kwargs):
     @transaction.atomic
     def delete(self, *args, **kwargs):
         if self.binding == "Ready":
-            raise DryccException("the plan is still binding")
+            raise DryccException("the resource instance is still binding")
+        if self.status == "Provisioning":
+            raise DryccException("the resource instance is provisioning")
         # Deatch ServiceInstance, updates k8s
         self.detach(*args, **kwargs)
         # Delete from DB
@@ -119,9 +121,9 @@ def log(self, message, level=logging.INFO):
 
     def bind(self, *args, **kwargs):
         if self.status != "Ready":
-            raise DryccException("the resource is not ready")
+            raise DryccException("the resource instance is not ready")
         if self.binding == "Ready":
-            raise DryccException("the resource is binding")
+            raise DryccException("the resource instance is binding")
         self.binding = "Binding"
         self.save()
         try:
@@ -141,7 +143,7 @@ def bind(self, *args, **kwargs):
 
     def unbind(self, *args, **kwargs):
         if not self.binding:
-            raise DryccException("the resource is not binding")
+            raise DryccException("the resource instance is not binding")
         try:
             # We raise an exception when a resource doesn't exist
             self._scheduler.svcat.get_binding(self.app.id, self.name)
@@ -168,7 +170,7 @@ def attach_update(self, *args, **kwargs):
                 "parameters": self.options,
                 "external_id": data["spec"]["externalID"]
             }
-            self._scheduler.svcat.put_instance(
+            self._scheduler.svcat.patch_instance(
                 self.app.id, self.name, version, **kwargs
             )
         except KubeException as e:
diff --git a/rootfs/api/serializers.py b/rootfs/api/serializers.py
index 6486ff64d..254ec4dde 100644
--- a/rootfs/api/serializers.py
+++ b/rootfs/api/serializers.py
@@ -615,7 +615,9 @@ class Meta:
 
     def update(self, instance, validated_data):
         if instance.plan.split(':')[0] != validated_data.get('plan', '').split(':')[0]:  # noqa
-            raise DryccException("the resource cann't changed")
+            raise DryccException("the resource instance cann't changed")
+        if instance.state == "Provisioning":
+            raise DryccException("this resource instance is in progress")
         instance.plan = validated_data.get('plan')
         instance.options.update(validated_data.get('options', {}))
         instance.attach_update()
diff --git a/rootfs/api/tests/test_resource.py b/rootfs/api/tests/test_resource.py
index 6cb425ef2..e011dabb1 100644
--- a/rootfs/api/tests/test_resource.py
+++ b/rootfs/api/tests/test_resource.py
@@ -9,6 +9,7 @@
 from django.conf import settings
 from rest_framework.authtoken.models import Token
 from api.tests import adapter, DryccTransactionTestCase
+from api.exceptions import DryccException
 import requests_mock
 
 User = get_user_model()
@@ -112,9 +113,11 @@ def test_resource_bind(self, mock_requests):
         # bind
         url = '/v2/apps/{app_id}/resources/mysql/binding/'.format(app_id=app_id)
         data = {"bind_action": "bind"}
-        response = self.client.patch(url, data=data)
-        # expected = response.data['path']
-        self.assertEqual(response.status_code, 400)
+        self.client.patch(url, data=data)
+        self.assertRaises(
+            DryccException,
+            msg='the resource instance is not ready'
+        )
 
     def test_resource_unbind(self, mock_requests):
         # create
@@ -124,9 +127,12 @@ def test_resource_unbind(self, mock_requests):
         # unbind
         url = '/v2/apps/{app_id}/resources/mysql/binding/'.format(app_id=app_id)
         data = {"bind_action": "unbind"}
-        response = self.client.patch(url, data=data)
+        self.client.patch(url, data=data)
         # expected = response.data['path']
-        self.assertEqual(response.status_code, 400)
+        self.assertRaises(
+            DryccException,
+            msg='the resource instance is not binding'
+        )
 
     def call_command(self, *args, **kwargs):
         from io import StringIO
diff --git a/rootfs/scheduler/resources/svcat.py b/rootfs/scheduler/resources/svcat.py
index 37f3056aa..874eb707b 100644
--- a/rootfs/scheduler/resources/svcat.py
+++ b/rootfs/scheduler/resources/svcat.py
@@ -14,6 +14,9 @@ def service_instance_manifest(self, namespace, name, version=None, **kwargs):
         data = {
             "apiVersion": self.api_version,
             "kind": "ServiceInstance",
+            "finalizers": [
+                "kubernetes-incubator/service-catalog",
+            ],
             "metadata": {
                 "name": name,
                 "namespace": namespace,
@@ -98,6 +101,21 @@ def put_instance(self, namespace, name, version, **kwargs):
                 "update serviceinstances {}".format(namespace))
         return response
 
+    def patch_instance(self, namespace, name, version, ignore_exception=False, **kwargs):
+        """
+        Patch serviceinstances
+        """
+        url = self.api('/namespaces/{}/serviceinstances/{}', namespace, name)
+        data = self.service_instance_manifest(namespace, name, version, **kwargs)
+        response = self.http_patch(
+            url,
+            json=data,
+            headers={"Content-Type": "application/merge-patch+json"}
+        )
+        if not ignore_exception and self.unhealthy(response.status_code):
+            raise KubeHTTPException(response, "patch serviceinstances {}".format(namespace))
+        return response
+
     def delete_instance(self, namespace, name):
         """
         Delete serviceinstances
