Merge pull request #1646 from edreed/edreed-new-condition-waiter-does…

…nt-fail [V2] chore: remove unneeded error return from NewConditionWaiter
kubernetes-sigs · Dec 8, 2022 · 5ebe16a · 5ebe16a
2 parents 60ffe54 + be0734b
commit 5ebe16a
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 59 deletions.
diff --git a/pkg/controller/azvolume.go b/pkg/controller/azvolume.go
@@ -280,14 +280,10 @@ func (r *ReconcileAzVolume) triggerDelete(ctx context.Context, azVolume *azdiskv
 			for _, attachment := range attachments {
 				// wait async and report error to go channel
 				go func(ctx context.Context, attachment azdiskv1beta2.AzVolumeAttachment) {
-					waiter, derr := r.conditionWatcher.NewConditionWaiter(deleteCtx, watcher.AzVolumeAttachmentType, attachment.Name, verifyObjectFailedOrDeleted)
+					waiter := r.conditionWatcher.NewConditionWaiter(deleteCtx, watcher.AzVolumeAttachmentType, attachment.Name, verifyObjectFailedOrDeleted)
 					defer waiter.Close()
-					if derr != nil {
-						errorMessageCh <- fmt.Sprintf("%s: %v", attachment.Name, derr)
-						return
-					}
 
-					_, derr = waiter.Wait(ctx)
+					_, derr := waiter.Wait(ctx)
 					if derr != nil {
 						errorMessageCh <- fmt.Sprintf("%s: %v", attachment.Name, derr)
 					} else {

diff --git a/pkg/controller/replica.go b/pkg/controller/replica.go
@@ -122,7 +122,7 @@ func (r *ReconcileReplica) Reconcile(ctx context.Context, request reconcile.Requ
 
 func (r *ReconcileReplica) handleReplicaDelete(ctx context.Context, azVolumeAttachment *azdiskv1beta2.AzVolumeAttachment) {
 	// wait for replica AzVolumeAttachment deletion
-	waiter, _ := r.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, azVolumeAttachment.Name, verifyObjectDeleted)
+	waiter := r.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, azVolumeAttachment.Name, verifyObjectDeleted)
 	defer waiter.Close()
 	_, _ = waiter.Wait(ctx)
 
@@ -208,12 +208,7 @@ func (r *ReconcileReplica) triggerCreateFailedReplicas(ctx context.Context, volu
 					}
 				}()
 
-				var waiter *watcher.ConditionWaiter
-				waiter, err = r.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, azVolumeAttachmentList.Items[index].Name, verifyObjectDeleted)
-				if err != nil {
-					errs[index] = err
-					return
-				}
+				waiter := r.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, azVolumeAttachmentList.Items[index].Name, verifyObjectDeleted)
 				defer waiter.Close()
 				if _, err = waiter.Wait(ctx); err != nil {
 					errs[index] = err

diff --git a/pkg/provisioner/crdprovisioner.go b/pkg/provisioner/crdprovisioner.go
@@ -311,11 +311,7 @@ func (c *CrdProvisioner) CreateVolume(
 		w.Logger().V(5).Info("Successfully created AzVolume CRI")
 	}
 
-	var waiter *watcher.ConditionWaiter
-	waiter, err = c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForCreateVolumeFunc)
-	if err != nil {
-		return nil, err
-	}
+	waiter := c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForCreateVolumeFunc)
 	defer waiter.Close()
 
 	var obj runtime.Object
@@ -395,19 +391,14 @@ func (c *CrdProvisioner) DeleteVolume(ctx context.Context, volumeID string, secr
 		return err
 	}
 
-	var waiter *watcher.ConditionWaiter
 	// if deletion failed requeue deletion
 	updateFunc := func(obj client.Object) error {
 		updateInstance := obj.(*azdiskv1beta2.AzVolume)
 		switch updateInstance.Status.State {
 		case azdiskv1beta2.VolumeCreating:
 			// if volume is still being created, wait for creation
-			waiter, err = c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForCreateVolumeFunc)
-			if err != nil {
-				return err
-			}
-			var obj interface{}
-			obj, err = waiter.Wait(ctx)
+			waiter := c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForCreateVolumeFunc)
+			obj, err := waiter.Wait(ctx)
 			// close cannot be called on defer because this will interfere wait for delete
 			waiter.Close()
 			if err != nil {
@@ -417,12 +408,8 @@ func (c *CrdProvisioner) DeleteVolume(ctx context.Context, volumeID string, secr
 		case azdiskv1beta2.VolumeUpdating:
 			// if volume is still being updated, wait for update
 			if azVolumeInstance.Spec.CapacityRange != nil {
-				waiter, err = c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForExpandVolumeFunc(azVolumeInstance.Spec.CapacityRange.RequiredBytes))
-				if err != nil {
-					return err
-				}
-				var obj interface{}
-				obj, err = waiter.Wait(ctx)
+				waiter := c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForExpandVolumeFunc(azVolumeInstance.Spec.CapacityRange.RequiredBytes))
+				obj, err := waiter.Wait(ctx)
 				// close cannot be called on defer because this will interfere wait for delete
 				waiter.Close()
 				if err != nil {
@@ -452,10 +439,7 @@ func (c *CrdProvisioner) DeleteVolume(ctx context.Context, volumeID string, secr
 	}
 	azVolumeInstance = updateObj.(*azdiskv1beta2.AzVolume)
 
-	waiter, err = c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForDeleteVolumeFunc)
-	if err != nil {
-		return err
-	}
+	waiter := c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForDeleteVolumeFunc)
 	defer waiter.Close()
 
 	// only make delete request if object's deletion timestamp is not set
@@ -670,11 +654,7 @@ func (c *CrdProvisioner) PublishVolume(
 		// if azVolumeAttachment was preempitvely created without attach trigger, then add attach trigger now
 		if isPreemptiveCreate {
 			// make sure CRI is created
-			var waiter *watcher.ConditionWaiter
-			waiter, err = c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, attachmentName, waitForCRICreateFunc)
-			if err != nil {
-				return publishContext, err
-			}
+			waiter := c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, attachmentName, waitForCRICreateFunc)
 			_, _ = waiter.Wait(ctx)
 			waiter.Close()
 			updateMode = azureutils.UpdateCRI
@@ -805,11 +785,7 @@ func (c *CrdProvisioner) waitForLunOrAttach(ctx context.Context, volumeID, nodeI
 		}
 	}
 
-	var waiter *watcher.ConditionWaiter
-	waiter, err = c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, attachmentName, waitFunc)
-	if err != nil {
-		return nil, err
-	}
+	waiter := c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, attachmentName, waitFunc)
 	defer waiter.Close()
 
 	obj, err := waiter.Wait(ctx)
@@ -961,12 +937,7 @@ func (c *CrdProvisioner) WaitForDetach(ctx context.Context, volumeID, nodeID str
 
 	lister := c.azCachedReader.azInformer.Disk().V1beta2().AzVolumeAttachments().Lister().AzVolumeAttachments(c.namespace)
 
-	var waiter *watcher.ConditionWaiter
-	waiter, err = c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, attachmentName, waitForDetachVolumeFunc)
-
-	if err != nil {
-		return err
-	}
+	waiter := c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeAttachmentType, attachmentName, waitForDetachVolumeFunc)
 	defer waiter.Close()
 
 	if _, err := lister.Get(attachmentName); apiErrors.IsNotFound(err) {
@@ -999,12 +970,7 @@ func (c *CrdProvisioner) ExpandVolume(
 	ctx, w := workflow.New(ctx, workflow.WithDetails(workflow.GetObjectDetails(azVolume)...))
 	defer func() { w.Finish(err) }()
 
-	var waiter *watcher.ConditionWaiter
-	waiter, err = c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForExpandVolumeFunc(capacityRange.RequiredBytes))
-	if err != nil {
-		return nil, err
-	}
-
+	waiter := c.conditionWatcher.NewConditionWaiter(ctx, watcher.AzVolumeType, azVolumeName, waitForExpandVolumeFunc(capacityRange.RequiredBytes))
 	defer waiter.Close()
 
 	updateFunc := func(obj client.Object) error {

diff --git a/pkg/watcher/conditionwatcher.go b/pkg/watcher/conditionwatcher.go
@@ -83,7 +83,7 @@ func NewConditionWatcher(informerFactory azdiskinformers.SharedInformerFactory,
 	return c
 }
 
-func (c *ConditionWatcher) NewConditionWaiter(ctx context.Context, objType ObjectType, objName string, conditionFunc func(obj interface{}, expectDelete bool) (bool, error)) (*ConditionWaiter, error) {
+func (c *ConditionWatcher) NewConditionWaiter(ctx context.Context, objType ObjectType, objName string, conditionFunc func(obj interface{}, expectDelete bool) (bool, error)) *ConditionWaiter {
 	klog.V(5).Infof("Adding a condition function for %s (%s)", objType, objName)
 	entry := waitEntry{
 		conditionFunc: conditionFunc,
@@ -104,7 +104,7 @@ func (c *ConditionWatcher) NewConditionWaiter(ctx context.Context, objType Objec
 		objName: objName,
 		entry:   &entry,
 		watcher: c,
-	}, nil
+	}
 }
 
 func (c *ConditionWatcher) onCreate(obj interface{}) {