fix(helmbroker): dead loop delete bug

Author: duanhongyi

rootfs/helmbroker/broker.py

@@ -15,8 +15,8 @@
 
 from .utils import get_instance_path, get_chart_path, get_plan_path, \
     get_addon_path, get_addon_updateable, get_addon_bindable, InstanceLock, \
-    get_instance_file, load_instance_meta, load_binding_meta, \
-    dump_instance_meta, load_addons_meta
+    load_instance_meta, load_binding_meta, dump_instance_meta, \
+    load_addons_meta
 from .tasks import provision, bind, deprovision, update
 
 
@@ -60,6 +60,7 @@ def provision(self,
             },
             "last_operation": {
                 "state": OperationState.IN_PROGRESS.value,
+                "operation": "provision",
                 "description": (
                     "provision %s in progress at %s" % (
                         instance_id, time.time()))
@@ -154,16 +155,18 @@ def deprovision(self,
                     details: DeprovisionDetails,
                     async_allowed: bool,
                     **kwargs) -> DeprovisionServiceSpec:
-        instance_path = get_instance_path(instance_id)
-        if os.path.exists(instance_path):
-            if not os.path.exists(get_instance_file(instance_id)):
-                return DeprovisionServiceSpec(
-                    is_async=False, operation=OperationState.SUCCEEDED)
-        else:
+        if not os.path.exists(get_instance_path(instance_id)):
             raise ErrInstanceDoesNotExist()
-        if not async_allowed:
-            raise ErrAsyncRequired()
-        deprovision.delay(instance_id)
+        with InstanceLock(instance_id):
+            data = load_instance_meta(instance_id)
+            operation = data["last_operation"]["operation"]
+            if operation == "provision":
+                if not async_allowed:
+                    raise ErrAsyncRequired()
+                deprovision.delay(instance_id)
+            elif operation == "deprovision":
+                return DeprovisionServiceSpec(
+                    is_async=True, operation=OperationState(operation))
         return DeprovisionServiceSpec(is_async=True)
 
     def last_operation(self,

rootfs/helmbroker/cleaner.py

@@ -14,13 +14,25 @@
 
 def clean_instance():
     for instance_id in os.listdir(INSTANCES_PATH):
-        if os.path.exists(get_instance_file(instance_id)):  # noqa
+        if os.path.exists(get_instance_file(instance_id)):
             data = load_instance_meta(instance_id)
             interval = time.time() - data["last_modified_time"]
-            if interval > 3600 * 24 and data["last_operation"]["state"] != OperationState.SUCCEEDED:  # noqa
+            state = data["last_operation"]["state"]
+            operation = data["last_operation"]["operation"]
+            if interval > 3600 * 24 and (
+                operation == "deprovision"
+                    and state != OperationState.SUCCEEDED):
                 deprovision.delay(instance_id)
+            if operation == "deprovision":
+                if state == OperationState.SUCCEEDED or (
+                    interval > 3600 * 24
+                        and state != OperationState.SUCCEEDED):
+                    shutil.rmtree(
+                        os.path.join(INSTANCES_PATH, instance_id),
+                        ignore_errors=True)
         else:
-            shutil.rmtree(os.path.join(INSTANCES_PATH, instance_id), ignore_errors=True)  # noqa
+            shutil.rmtree(
+                os.path.join(INSTANCES_PATH, instance_id), ignore_errors=True)
 
 
 if __name__ == "__main__":

rootfs/helmbroker/tasks.py

@@ -1,15 +1,15 @@
 import os
-import shutil
 import time
+import shutil
 import yaml
 
 from openbrokerapi.service_broker import ProvisionDetails, OperationState, \
     UpdateDetails, BindDetails
 
 from .celery import app
 from .utils import command, get_plan_path, get_chart_path, get_cred_value, \
-    InstanceLock, get_instance_file, dump_instance_meta, dump_binding_meta, \
-    load_instance_meta
+    InstanceLock, dump_instance_meta, dump_binding_meta, load_instance_meta, \
+    get_instance_file
 
 
 @app.task(serializer='pickle')
@@ -70,7 +70,8 @@ def update(instance_id: str, details: UpdateDetails):
         data['details']['service_id'] = details.parameters
     data['last_operation'] = {
         "state": OperationState.IN_PROGRESS.value,
-        "description": "update %s in progress at %s" % (instance_id, time.time())  # noqa
+        "description": (
+            "update %s in progress at %s" % (instance_id, time.time()))
     }
     dump_instance_meta(instance_id, data)
     chart_path = get_chart_path(instance_id)
@@ -94,10 +95,12 @@ def update(instance_id: str, details: UpdateDetails):
     status, output = command("helm", *args)
     if status != 0:
         data["last_operation"]["state"] = OperationState.FAILED.value
-        data["last_operation"]["description"] = "update %s failed: %s" % (instance_id, output)  # noqa
+        data["last_operation"]["description"] = (
+            "update %s failed: %s" % (instance_id, output))
     else:
         data["last_operation"]["state"] = OperationState.SUCCEEDED.value
-        data["last_operation"]["description"] = "update %s succeeded at %s" % (instance_id, time.time())  # noqa
+        data["last_operation"]["description"] = (
+            "update %s succeeded at %s" % (instance_id, time.time()))
     dump_instance_meta(instance_id, data)
 
 
@@ -112,7 +115,8 @@ def bind(instance_id: str,
         "credentials": {},
         "last_operation": {
             "state": OperationState.IN_PROGRESS.value,
-            "description": "binding %s in progress at %s" % (binding_id, time.time())  # noqa
+            "description": (
+                "binding %s in progress at %s" % (binding_id, time.time()))
         }
     }
     dump_binding_meta(instance_id, data)
@@ -167,9 +171,14 @@ def bind(instance_id: str,
 @app.task()
 def deprovision(instance_id: str):
     with InstanceLock(instance_id):
+        shutil.copy(get_instance_file(instance_id), "%s.%s" % (
+            get_instance_file(instance_id), time.time()
+        ))
         data = load_instance_meta(instance_id)
+        data["last_operation"]["operation"] = "deprovision"
         data["last_operation"]["state"] = OperationState.IN_PROGRESS.value
-        data["last_operation"]["description"] = "deprovision %s in progress at %s" % (instance_id, time.time())  # noqa
+        data["last_operation"]["description"] = (
+            "deprovision %s in progress at %s" % (instance_id, time.time()))
         dump_instance_meta(instance_id, data)
         status, output = command(
             "helm",
@@ -182,13 +191,9 @@ def deprovision(instance_id: str):
             data["last_operation"]["state"] = OperationState.FAILED.value
             data["last_operation"]["description"] = (
                 "deprovision error:\n%s" % output)
-            shutil.copy(get_instance_file(instance_id), "%s.%s" % (
-                get_instance_file(instance_id),
-                time.time()
-            ))
         else:
-            data["last_operation"]["state"] = OperationState.SUCCEEDED.value
+            data["last_operation"]["state"] = (
+                OperationState.SUCCEEDED.value)
             data["last_operation"]["description"] = (
                 "deprovision succeeded at %s" % time.time())
-        os.remove(get_instance_file(instance_id))
         dump_instance_meta(instance_id, data)

rootfs/helmbroker/utils.py

@@ -47,6 +47,7 @@ def command(cmd, *args, output_type="text"):
             "type": "object",
             "properties": {
                 "state": {"type": "string"},
+                "operation": {"type": "string"},
                 "description": {"type": "string"}
             }
         },