Remove createSnapshotContentRetryCount and createSnapshotContentInterval

from command line options
kubernetes-csi · Dec 18, 2019 · 60c696c · 60c696c
1 parent ed82822
commit 60c696c
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 68 deletions.
diff --git a/cmd/snapshot-controller/main.go b/cmd/snapshot-controller/main.go
@@ -46,11 +46,9 @@ const (
 
 // Command line flags
 var (
-	kubeconfig                      = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Required only when running out of cluster.")
-	createSnapshotContentRetryCount = flag.Int("create-snapshotcontent-retrycount", 5, "Number of retries when we create a snapshot content object for a snapshot.")
-	createSnapshotContentInterval   = flag.Duration("create-snapshotcontent-interval", 10*time.Second, "Interval between retries when we create a snapshot content object for a snapshot.")
-	resyncPeriod                    = flag.Duration("resync-period", 60*time.Second, "Resync interval of the controller.")
-	showVersion                     = flag.Bool("version", false, "Show version.")
+	kubeconfig   = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Required only when running out of cluster.")
+	resyncPeriod = flag.Duration("resync-period", 60*time.Second, "Resync interval of the controller.")
+	showVersion  = flag.Bool("version", false, "Show version.")
 
 	leaderElection          = flag.Bool("leader-election", false, "Enables leader election.")
 	leaderElectionNamespace = flag.String("leader-election-namespace", "", "The namespace where the leader election resource exists. Defaults to the pod namespace if not set.")
@@ -96,7 +94,7 @@ func main() {
 	// Add Snapshot types to the defualt Kubernetes so events can be logged for them
 	snapshotscheme.AddToScheme(scheme.Scheme)
 
-	klog.V(2).Infof("Start NewCSISnapshotController with kubeconfig [%s] createSnapshotContentRetryCount [%d] createSnapshotContentInterval [%d] resyncPeriod [%+v]", *kubeconfig, *createSnapshotContentRetryCount, *createSnapshotContentInterval, *resyncPeriod)
+	klog.V(2).Infof("Start NewCSISnapshotController with kubeconfig [%s] resyncPeriod [%+v]", *kubeconfig, *resyncPeriod)
 
 	ctrl := controller.NewCSISnapshotCommonController(
 		snapClient,
@@ -105,8 +103,6 @@ func main() {
 		factory.Snapshot().V1beta1().VolumeSnapshotContents(),
 		factory.Snapshot().V1beta1().VolumeSnapshotClasses(),
 		coreFactory.Core().V1().PersistentVolumeClaims(),
-		*createSnapshotContentRetryCount,
-		*createSnapshotContentInterval,
 		*resyncPeriod,
 	)
 

diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go
@@ -761,8 +761,6 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte
 		informerFactory.Snapshot().V1beta1().VolumeSnapshotContents(),
 		informerFactory.Snapshot().V1beta1().VolumeSnapshotClasses(),
 		coreFactory.Core().V1().PersistentVolumeClaims(),
-		3,
-		5*time.Millisecond,
 		60*time.Second,
 	)
 

diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go
@@ -337,16 +337,10 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
 		}
 
 		// update snapshot status
-		for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
-			klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot))
-			_, err = ctrl.updateSnapshotStatus(snapshot, newContent)
-			if err == nil {
-				break
-			}
-			klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
-		}
-
+		klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot))
+		_, err = ctrl.updateSnapshotStatus(snapshot, newContent)
 		if err != nil {
+			klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
 			// update snapshot status failed
 			ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
 			return err
@@ -399,16 +393,10 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
 				}
 
 				// Update snapshot status with BoundVolumeSnapshotContentName
-				for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
-					klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot))
-					_, err = ctrl.updateSnapshotStatus(snapshot, content)
-					if err == nil {
-						break
-					}
-					klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
-				}
-
+				klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot))
+				_, err = ctrl.updateSnapshotStatus(snapshot, content)
 				if err != nil {
+					klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
 					// update snapshot status failed
 					ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
 					return err
@@ -501,23 +489,17 @@ func (ctrl *csiSnapshotCommonController) createSnapshotContent(snapshot *crdv1.V
 
 	var updateContent *crdv1.VolumeSnapshotContent
 	klog.V(3).Infof("volume snapshot content %#v", snapshotContent)
-	// Try to create the VolumeSnapshotContent object several times
-	for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
-		klog.V(5).Infof("createSnapshotContent [%s]: trying to save volume snapshot content %s", utils.SnapshotKey(snapshot), snapshotContent.Name)
-		if updateContent, err = ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Create(snapshotContent); err == nil || apierrs.IsAlreadyExists(err) {
-			// Save succeeded.
-			if err != nil {
-				klog.V(3).Infof("volume snapshot content %q for snapshot %q already exists, reusing", snapshotContent.Name, utils.SnapshotKey(snapshot))
-				err = nil
-				updateContent = snapshotContent
-			} else {
-				klog.V(3).Infof("volume snapshot content %q for snapshot %q saved, %v", snapshotContent.Name, utils.SnapshotKey(snapshot), snapshotContent)
-			}
-			break
+	// Try to create the VolumeSnapshotContent object
+	klog.V(5).Infof("createSnapshotContent [%s]: trying to save volume snapshot content %s", utils.SnapshotKey(snapshot), snapshotContent.Name)
+	if updateContent, err = ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Create(snapshotContent); err == nil || apierrs.IsAlreadyExists(err) {
+		// Save succeeded.
+		if err != nil {
+			klog.V(3).Infof("volume snapshot content %q for snapshot %q already exists, reusing", snapshotContent.Name, utils.SnapshotKey(snapshot))
+			err = nil
+			updateContent = snapshotContent
+		} else {
+			klog.V(3).Infof("volume snapshot content %q for snapshot %q saved, %v", snapshotContent.Name, utils.SnapshotKey(snapshot), snapshotContent)
 		}
-		// Save failed, try again after a while.
-		klog.V(3).Infof("failed to save volume snapshot content %q for snapshot %q: %v", snapshotContent.Name, utils.SnapshotKey(snapshot), err)
-		time.Sleep(ctrl.createSnapshotContentInterval)
 	}
 
 	if err != nil {
@@ -867,17 +849,14 @@ func (ctrl *csiSnapshotCommonController) bindandUpdateVolumeSnapshot(snapshotCon
 	snapshotCopy := snapshotObj.DeepCopy()
 
 	// update snapshot status
-	for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
-		klog.V(5).Infof("bindandUpdateVolumeSnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshotCopy))
-		updateSnapshot, err := ctrl.updateSnapshotStatus(snapshotCopy, snapshotContent)
-		if err == nil {
-			snapshotCopy = updateSnapshot
-			break
-		}
-		klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
+	klog.V(5).Infof("bindandUpdateVolumeSnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshotCopy))
+	updateSnapshot, err := ctrl.updateSnapshotStatus(snapshotCopy, snapshotContent)
+	if err == nil {
+		snapshotCopy = updateSnapshot
 	}
 
 	if err != nil {
+		klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
 		// update snapshot status failed
 		ctrl.updateSnapshotErrorStatusWithEvent(snapshotCopy, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
 		return nil, err

diff --git a/pkg/common-controller/snapshot_controller_base.go b/pkg/common-controller/snapshot_controller_base.go
@@ -64,9 +64,7 @@ type csiSnapshotCommonController struct {
 	// Map of scheduled/running operations.
 	runningOperations goroutinemap.GoRoutineMap
 
-	createSnapshotContentRetryCount int
-	createSnapshotContentInterval   time.Duration
-	resyncPeriod                    time.Duration
+	resyncPeriod time.Duration
 }
 
 // NewCSISnapshotController returns a new *csiSnapshotCommonController
@@ -77,8 +75,6 @@ func NewCSISnapshotCommonController(
 	volumeSnapshotContentInformer storageinformers.VolumeSnapshotContentInformer,
 	volumeSnapshotClassInformer storageinformers.VolumeSnapshotClassInformer,
 	pvcInformer coreinformers.PersistentVolumeClaimInformer,
-	createSnapshotContentRetryCount int,
-	createSnapshotContentInterval time.Duration,
 	resyncPeriod time.Duration,
 ) *csiSnapshotCommonController {
 	broadcaster := record.NewBroadcaster()
@@ -88,17 +84,15 @@ func NewCSISnapshotCommonController(
 	eventRecorder = broadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: fmt.Sprintf("snapshot-controller")})
 
 	ctrl := &csiSnapshotCommonController{
-		clientset:                       clientset,
-		client:                          client,
-		eventRecorder:                   eventRecorder,
-		runningOperations:               goroutinemap.NewGoRoutineMap(true),
-		createSnapshotContentRetryCount: createSnapshotContentRetryCount,
-		createSnapshotContentInterval:   createSnapshotContentInterval,
-		resyncPeriod:                    resyncPeriod,
-		snapshotStore:                   cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc),
-		contentStore:                    cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc),
-		snapshotQueue:                   workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "snapshot-controller-snapshot"),
-		contentQueue:                    workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "snapshot-controller-content"),
+		clientset:         clientset,
+		client:            client,
+		eventRecorder:     eventRecorder,
+		runningOperations: goroutinemap.NewGoRoutineMap(true),
+		resyncPeriod:      resyncPeriod,
+		snapshotStore:     cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc),
+		contentStore:      cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc),
+		snapshotQueue:     workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "snapshot-controller-snapshot"),
+		contentQueue:      workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "snapshot-controller-content"),
 	}
 
 	ctrl.pvcLister = pvcInformer.Lister()