diff --git a/cmd/snapshot-controller/main.go b/cmd/snapshot-controller/main.go index 88d91a7a..cbe92f0f 100644 --- 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 index 82531eae..1aaf51ee 100644 --- 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 index 70e059ce..b814770b 100644 --- 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 index 958f03fa..66c73e8c 100644 --- 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()