Merge pull request #1941 from k8s-infra-cherrypick-robot/cherry-pick-…

…1938-to-release-1.28 [release-1.28] fix: remove cross region snapshot copy 5s delay
kubernetes-sigs · Aug 25, 2023 · e15074a · e15074a
2 parents 37a870e + 45a2551
commit e15074a
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 9 deletions.
diff --git a/pkg/azuredisk/azuredisk.go b/pkg/azuredisk/azuredisk.go
@@ -417,25 +417,47 @@ func (d *DriverCore) getHostUtil() hostUtil {
 	return d.hostUtil
 }
 
+// getSnapshotCopyCompletionPercent returns the completion percent of copy snapshot
+func (d *DriverCore) getSnapshotCopyCompletionPercent(ctx context.Context, subsID, resourceGroup, copySnapshotName string) (float64, error) {
+	copySnapshot, rerr := d.cloud.SnapshotsClient.Get(ctx, subsID, resourceGroup, copySnapshotName)
+	if rerr != nil {
+		return 0.0, rerr.Error()
+	}
+
+	if copySnapshot.SnapshotProperties == nil || copySnapshot.SnapshotProperties.CompletionPercent == nil {
+		return 0.0, fmt.Errorf("copy snapshot(%s) under rg(%s) has no SnapshotProperties or CompletionPercent is nil", copySnapshotName, resourceGroup)
+	}
+
+	return *copySnapshot.SnapshotProperties.CompletionPercent, nil
+}
+
 // waitForSnapshotCopy wait for copy incremental snapshot to a new region until completionPercent is 100.0
 func (d *DriverCore) waitForSnapshotCopy(ctx context.Context, subsID, resourceGroup, copySnapshotName string, intervel, timeout time.Duration) error {
-	timeAfter := time.After(timeout)
-	timeTick := time.Tick(intervel)
+	completionPercent, err := d.getSnapshotCopyCompletionPercent(ctx, subsID, resourceGroup, copySnapshotName)
+	if err != nil {
+		return err
+	}
 
+	if completionPercent >= float64(100.0) {
+		klog.V(2).Infof("copy snapshot(%s) under rg(%s) complete", copySnapshotName, resourceGroup)
+		return nil
+	}
+
+	timeTick := time.Tick(intervel)
+	timeAfter := time.After(timeout)
 	for {
 		select {
 		case <-timeTick:
-			copySnapshot, rerr := d.cloud.SnapshotsClient.Get(ctx, subsID, resourceGroup, copySnapshotName)
-			if rerr != nil {
-				return rerr.Error()
+			completionPercent, err = d.getSnapshotCopyCompletionPercent(ctx, subsID, resourceGroup, copySnapshotName)
+			if err != nil {
+				return err
 			}
 
-			completionPercent := *copySnapshot.SnapshotProperties.CompletionPercent
-			klog.V(2).Infof("copy snapshot(%s) under rg(%s) region(%s) completionPercent: %f", copySnapshotName, resourceGroup, *copySnapshot.Location, completionPercent)
 			if completionPercent >= float64(100.0) {
-				klog.V(2).Infof("copy snapshot(%s) under rg(%s) region(%s) complete", copySnapshotName, resourceGroup, *copySnapshot.Location)
+				klog.V(2).Infof("copy snapshot(%s) under rg(%s) complete", copySnapshotName, resourceGroup)
 				return nil
 			}
+			klog.V(2).Infof("copy snapshot(%s) under rg(%s) completionPercent: %f", copySnapshotName, resourceGroup, completionPercent)
 		case <-timeAfter:
 			return fmt.Errorf("timeout waiting for copy snapshot(%s) under rg(%s)", copySnapshotName, resourceGroup)
 		}

diff --git a/pkg/azuredisk/controllerserver.go b/pkg/azuredisk/controllerserver.go
@@ -967,8 +967,8 @@ func (d *Driver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequ
 
 		klog.V(2).Infof("begin to delete snapshot(%s) under rg(%s) region(%s)", snapshotName, resourceGroup, d.cloud.Location)
 		if rerr := d.cloud.SnapshotsClient.Delete(ctx, subsID, resourceGroup, snapshotName); rerr != nil {
-			azureutils.SleepIfThrottled(rerr.Error(), consts.SnapshotOpThrottlingSleepSec)
 			klog.Errorf("delete snapshot error: %v", rerr.Error())
+			azureutils.SleepIfThrottled(rerr.Error(), consts.SnapshotOpThrottlingSleepSec)
 		} else {
 			klog.V(2).Infof("delete snapshot(%s) under rg(%s) region(%s) successfully", snapshotName, resourceGroup, d.cloud.Location)
 		}