From 98939c02c5a78238c40aa9928ff4c80b4e70ebde Mon Sep 17 00:00:00 2001 From: Sameer Shaikh Date: Wed, 28 Jun 2023 12:55:20 +0530 Subject: [PATCH 1/5] external-snapshotter constantly retrying CreateSnapshot calls on error w/o backoff Signed-off-by: Sameer Shaikh --- pkg/sidecar-controller/snapshot_controller_base.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/sidecar-controller/snapshot_controller_base.go b/pkg/sidecar-controller/snapshot_controller_base.go index ecb7ecd8..852b54a9 100644 --- a/pkg/sidecar-controller/snapshot_controller_base.go +++ b/pkg/sidecar-controller/snapshot_controller_base.go @@ -124,12 +124,7 @@ func NewCSISnapshotSideCarController( // So we are skipping the re-queue here to avoid CreateSnapshot being called without exponential backoff. newSnapContent := newObj.(*crdv1.VolumeSnapshotContent) if newSnapContent.Status != nil && newSnapContent.Status.Error != nil { - oldSnapContent := oldObj.(*crdv1.VolumeSnapshotContent) - _, newExists := newSnapContent.ObjectMeta.Annotations[utils.AnnVolumeSnapshotBeingCreated] - _, oldExists := oldSnapContent.ObjectMeta.Annotations[utils.AnnVolumeSnapshotBeingCreated] - if !newExists && oldExists { return - } } ctrl.enqueueContentWork(newObj) }, From a8c9b7b644fad95e86fbc48a6d8000607c8aca1b Mon Sep 17 00:00:00 2001 From: Sameer Shaikh Date: Wed, 28 Jun 2023 15:59:21 +0530 Subject: [PATCH 2/5] external-snapshotter constantly retrying CreateSnapshot calls on error w/o backoff --- pkg/sidecar-controller/snapshot_controller_base.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/sidecar-controller/snapshot_controller_base.go b/pkg/sidecar-controller/snapshot_controller_base.go index 852b54a9..ebdf9366 100644 --- a/pkg/sidecar-controller/snapshot_controller_base.go +++ b/pkg/sidecar-controller/snapshot_controller_base.go @@ -41,7 +41,6 @@ import ( groupsnapshotlisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumegroupsnapshot/v1alpha1" snapshotlisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v6/pkg/snapshotter" - "github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils" ) type csiSnapshotSideCarController struct { From ffaba0f81dcefc3457fd073c04c5cc694ca05144 Mon Sep 17 00:00:00 2001 From: Sameer Shaikh Date: Wed, 28 Jun 2023 18:32:07 +0530 Subject: [PATCH 3/5] external-snapshotter constantly retrying CreateSnapshot calls on error w/o backoff --- pkg/sidecar-controller/snapshot_controller_base.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sidecar-controller/snapshot_controller_base.go b/pkg/sidecar-controller/snapshot_controller_base.go index ebdf9366..b9e6f8da 100644 --- a/pkg/sidecar-controller/snapshot_controller_base.go +++ b/pkg/sidecar-controller/snapshot_controller_base.go @@ -123,7 +123,7 @@ func NewCSISnapshotSideCarController( // So we are skipping the re-queue here to avoid CreateSnapshot being called without exponential backoff. newSnapContent := newObj.(*crdv1.VolumeSnapshotContent) if newSnapContent.Status != nil && newSnapContent.Status.Error != nil { - return + return } ctrl.enqueueContentWork(newObj) }, From d8e698f6092e70e17ee14058c4fd0396d3b5b253 Mon Sep 17 00:00:00 2001 From: Sameer Shaikh Date: Thu, 29 Jun 2023 08:03:57 +0530 Subject: [PATCH 4/5] external-snapshotter constantly retrying CreateSnapshot calls on error w/o backoff --- pkg/sidecar-controller/snapshot_controller_base.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/sidecar-controller/snapshot_controller_base.go b/pkg/sidecar-controller/snapshot_controller_base.go index b9e6f8da..bd4e695b 100644 --- a/pkg/sidecar-controller/snapshot_controller_base.go +++ b/pkg/sidecar-controller/snapshot_controller_base.go @@ -120,6 +120,8 @@ func NewCSISnapshotSideCarController( // VolumeSnapshotContent. // This will trigger a VolumeSnapshotContent update and it will cause the obj to be re-queued immediately // and CSI CreateSnapshot will be called again without exponential backoff. + // Considering the object is modified more than once during the workflow we are not relying on the annoations of oldobj and newobj. + // We will just check if newobj status has error and avoid re-queue. // So we are skipping the re-queue here to avoid CreateSnapshot being called without exponential backoff. newSnapContent := newObj.(*crdv1.VolumeSnapshotContent) if newSnapContent.Status != nil && newSnapContent.Status.Error != nil { From 7fac9cdaf87a7d859771e9147a4797d582d6b8df Mon Sep 17 00:00:00 2001 From: Sameer Shaikh Date: Thu, 29 Jun 2023 21:17:27 +0530 Subject: [PATCH 5/5] external-snapshotter constantly retrying CreateSnapshot calls on error w/o backoff --- pkg/sidecar-controller/snapshot_controller_base.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pkg/sidecar-controller/snapshot_controller_base.go b/pkg/sidecar-controller/snapshot_controller_base.go index bd4e695b..427d85de 100644 --- a/pkg/sidecar-controller/snapshot_controller_base.go +++ b/pkg/sidecar-controller/snapshot_controller_base.go @@ -115,14 +115,9 @@ func NewCSISnapshotSideCarController( cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { ctrl.enqueueContentWork(obj) }, UpdateFunc: func(oldObj, newObj interface{}) { - // If the CSI driver fails to create a snapshot and returns a failure (indicated by content.Status.Error), the - // CSI Snapshotter sidecar will remove the "AnnVolumeSnapshotBeingCreated" annotation from the - // VolumeSnapshotContent. - // This will trigger a VolumeSnapshotContent update and it will cause the obj to be re-queued immediately - // and CSI CreateSnapshot will be called again without exponential backoff. - // Considering the object is modified more than once during the workflow we are not relying on the annoations of oldobj and newobj. - // We will just check if newobj status has error and avoid re-queue. - // So we are skipping the re-queue here to avoid CreateSnapshot being called without exponential backoff. + // Considering the object is modified more than once during the workflow we are not relying on the + // "AnnVolumeSnapshotBeingCreated" annotation. Instead we will just check if newobj status has error + // and avoid the immediate re-queue. This allows the retry to happen with exponential backoff. newSnapContent := newObj.(*crdv1.VolumeSnapshotContent) if newSnapContent.Status != nil && newSnapContent.Status.Error != nil { return