Skip to content

Fix requeue logic in the common controller #317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
k8s-ci-robot merged 1 commit into from
Jul 8, 2020
Merged

Fix requeue logic in the common controller #317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions pkg/common-controller/framework_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,6 @@ func (r *snapshotReactor) getChangeCount() int {
// waitForIdle waits until all tests, controllers and other goroutines do their
// job and no new actions are registered for 10 milliseconds.
func (r *snapshotReactor) waitForIdle() {
r.ctrl.runningOperations.WaitForCompletion()
// Check every 10ms if the controller does something and stop if it's
// idle.
oldChanges := -1
Expand All @@ -609,9 +608,6 @@ func (r *snapshotReactor) waitTest(test controllerTest) error {
Steps: 10,
}
err := wait.ExponentialBackoff(backoff, func() (done bool, err error) {
// Finish all operations that are in progress
r.ctrl.runningOperations.WaitForCompletion()

// Return 'true' if the reactor reached the expected state
err1 := r.checkSnapshots(test.expectedSnapshots)
err2 := r.checkContents(test.expectedContents)
Expand Down Expand Up @@ -757,8 +753,6 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte
ctrl.snapshotListerSynced = alwaysReady
ctrl.classListerSynced = alwaysReady
ctrl.pvcListerSynced = alwaysReady
ctrl.createSnapshotContentInterval = time.Millisecond * 5
ctrl.createSnapshotContentRetryCount = 3

return ctrl, nil
}
Expand Down
70 changes: 21 additions & 49 deletions pkg/common-controller/snapshot_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,18 +423,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)
time.Sleep(ctrl.createSnapshotContentInterval)
}

if err != nil {
klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot))
if _, err = ctrl.updateSnapshotStatus(snapshot, newContent); err != nil {
// update snapshot status failed
klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
return err
}
Expand Down Expand Up @@ -474,17 +466,8 @@ 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)
time.Sleep(ctrl.createSnapshotContentInterval)
}

if err != nil {
klog.V(5).Infof("syncUnreadySnapshot [%s]: trying to update snapshot status", utils.SnapshotKey(snapshot))
Comment thread
xing-yang marked this conversation as resolved.
if _, err = ctrl.updateSnapshotStatus(snapshot, content); err != nil {
// update snapshot status failed
ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
return err
Expand Down Expand Up @@ -656,24 +639,18 @@ 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(context.TODO(), snapshotContent, metav1.CreateOptions{}); 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
klog.V(5).Infof("volume snapshot content %#v", snapshotContent)
// 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(context.TODO(), snapshotContent, metav1.CreateOptions{}); 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 {
Expand Down Expand Up @@ -982,19 +959,14 @@ func (ctrl *csiSnapshotCommonController) bindandUpdateVolumeSnapshot(snapshotCon
snapshotCopy := snapshotObj.DeepCopy()
// update snapshot status
var updateSnapshot *crdv1.VolumeSnapshot
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)
time.Sleep(ctrl.createSnapshotContentInterval)
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 {
// update snapshot status failed
klog.V(4).Infof("failed to update snapshot %s status: %v", utils.SnapshotKey(snapshot), err)
ctrl.updateSnapshotErrorStatusWithEvent(snapshotCopy, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
return nil, err
}
Expand Down
Loading