From eef83be10ce046fbd317f677946c1076a9c091c9 Mon Sep 17 00:00:00 2001
From: Qiu Jian <qiujian@yunionyun.com>
Date: Sun, 6 Jan 2019 20:54:49 +0800
Subject: [PATCH 1/2] =?UTF-8?q?=E4=BF=AE=E6=AD=A3=EF=BC=9A1.=20server=20de?=
 =?UTF-8?q?tach=20disk=20=E7=BC=BA=E5=B0=91parentTaskId=202.=20aliyun=20?=
 =?UTF-8?q?=E5=85=B3=E6=9C=BA=E5=90=8E=E7=AB=8B=E5=8D=B3=20detachDisk=20?=
 =?UTF-8?q?=E4=BC=9A=E5=87=BA=E7=8E=B0InvalidOperation.Conflict=E7=9A=84?=
 =?UTF-8?q?=E9=94=99=E8=AF=AF?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 pkg/cloudprovider/retry.go             | 28 ++++++++++++++++++++++++++
 pkg/compute/models/guest_actions.go    |  2 +-
 pkg/compute/tasks/guest_delete_task.go |  5 +++--
 pkg/util/aliyun/aliyun.go              |  2 +-
 pkg/util/aliyun/instance.go            |  9 ++++++++-
 5 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 pkg/cloudprovider/retry.go

diff --git a/pkg/cloudprovider/retry.go b/pkg/cloudprovider/retry.go
new file mode 100644
index 0000000000..006db70f49
--- /dev/null
+++ b/pkg/cloudprovider/retry.go
@@ -0,0 +1,28 @@
+package cloudprovider
+
+import (
+	"strings"
+	"time"
+)
+
+func IsError(err error, errs []string) bool {
+	for i := range errs {
+		if strings.Index(err.Error(), errs[i]) >= 0 {
+			return true
+		}
+	}
+	return false
+}
+
+func RetryOnError(tryFunc func() error, errs []string, maxTries int) error {
+	tried := 0
+	for tried < maxTries {
+		err := tryFunc()
+		if err != nil && !IsError(err, errs) {
+			return err
+		}
+		tried += 1
+		time.Sleep(10 * time.Duration(tried) * time.Second)
+	}
+	return ErrTimeout
+}
diff --git a/pkg/compute/models/guest_actions.go b/pkg/compute/models/guest_actions.go
index df0b76d18b..e5d4f5844c 100644
--- a/pkg/compute/models/guest_actions.go
+++ b/pkg/compute/models/guest_actions.go
@@ -1213,7 +1213,7 @@ func (self *SGuest) StartGuestDetachdiskTask(ctx context.Context, userCred mccli
 		})
 	}
 	disk.SetStatus(userCred, DISK_DETACHING, "")
-	return self.GetDriver().StartGuestDetachdiskTask(ctx, userCred, self, taskData, "")
+	return self.GetDriver().StartGuestDetachdiskTask(ctx, userCred, self, taskData, parentTaskId)
 }
 
 func (self *SGuest) AllowPerformDetachIsolatedDevice(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data jsonutils.JSONObject) bool {
diff --git a/pkg/compute/tasks/guest_delete_task.go b/pkg/compute/tasks/guest_delete_task.go
index 8a7746daca..5521851d8d 100644
--- a/pkg/compute/tasks/guest_delete_task.go
+++ b/pkg/compute/tasks/guest_delete_task.go
@@ -153,8 +153,9 @@ func (self *GuestDeleteTask) OnSyncConfigComplete(ctx context.Context, obj db.IS
 }
 
 func (self *GuestDeleteTask) OnSyncConfigCompleteFailed(ctx context.Context, obj db.IStandaloneModel, err jsonutils.JSONObject) {
-	guest := obj.(*models.SGuest)
-	self.OnFailed(ctx, guest, err)
+	// guest := obj.(*models.SGuest)
+	// self.OnFailed(ctx, guest, err)
+	self.OnSyncConfigComplete(ctx, obj, err) // ignore sync config failed error
 }
 
 func (self *GuestDeleteTask) OnGuestDeleteFailed(ctx context.Context, obj db.IStandaloneModel, err jsonutils.JSONObject) {
diff --git a/pkg/util/aliyun/aliyun.go b/pkg/util/aliyun/aliyun.go
index 45eb2e88ab..27d1aac4d3 100644
--- a/pkg/util/aliyun/aliyun.go
+++ b/pkg/util/aliyun/aliyun.go
@@ -82,7 +82,7 @@ func _jsonRequest(client *sdk.Client, domain string, version string, apiName str
 
 	resp, err := processCommonRequest(client, req)
 	if err != nil {
-		log.Errorf("request error %s", err)
+		log.Errorf("request error %s with params %s", err, params)
 		return nil, err
 	}
 	body, err := jsonutils.Parse(resp.GetHttpContentBytes())
diff --git a/pkg/util/aliyun/instance.go b/pkg/util/aliyun/instance.go
index 2c7f101f1b..44e815329c 100644
--- a/pkg/util/aliyun/instance.go
+++ b/pkg/util/aliyun/instance.go
@@ -512,7 +512,14 @@ func (self *SInstance) AttachDisk(ctx context.Context, diskId string) error {
 }
 
 func (self *SInstance) DetachDisk(ctx context.Context, diskId string) error {
-	return self.host.zone.region.DetachDisk(self.InstanceId, diskId)
+	return cloudprovider.RetryOnError(
+		func() error {
+			return self.host.zone.region.DetachDisk(self.InstanceId, diskId)
+		},
+		[]string{
+			`"Code":"InvalidOperation.Conflict"`,
+		},
+		4)
 }
 
 func (self *SRegion) GetInstance(instanceId string) (*SInstance, error) {

From 766039194898267d32322aca50213cc06daa82d6 Mon Sep 17 00:00:00 2001
From: Qiu Jian <qiujian@yunionyun.com>
Date: Mon, 7 Jan 2019 11:50:42 +0800
Subject: [PATCH 2/2] return nil immediately if error is nil

---
 pkg/cloudprovider/retry.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pkg/cloudprovider/retry.go b/pkg/cloudprovider/retry.go
index 006db70f49..7fb70fd6da 100644
--- a/pkg/cloudprovider/retry.go
+++ b/pkg/cloudprovider/retry.go
@@ -18,6 +18,9 @@ func RetryOnError(tryFunc func() error, errs []string, maxTries int) error {
 	tried := 0
 	for tried < maxTries {
 		err := tryFunc()
+		if err == nil {
+			return nil
+		}
 		if err != nil && !IsError(err, errs) {
 			return err
 		}