Merge pull request #296 from humblec/lco

Remove unwanted code conditions

pkg/common-controller/snapshot_controller.go

@@ -22,8 +22,6 @@ import (
 	"strings"
 	"time"
 
-	crdv1 "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1"
-	"github.com/kubernetes-csi/external-snapshotter/v2/pkg/utils"
 	v1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
@@ -34,6 +32,9 @@ import (
 	ref "k8s.io/client-go/tools/reference"
 	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/util/slice"
+
+	crdv1 "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1"
+	"github.com/kubernetes-csi/external-snapshotter/v2/pkg/utils"
 )
 
 // ==================================================================
@@ -334,8 +335,8 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveSnapshotFinalizersAndChec
 func (ctrl *csiSnapshotCommonController) checkandAddSnapshotFinalizers(snapshot *crdv1.VolumeSnapshot) error {
 	// get the content for this Snapshot
 	var (
-		content *crdv1.VolumeSnapshotContent = nil
-		err     error                        = nil
+		content *crdv1.VolumeSnapshotContent
+		err     error
 	)
 	if snapshot.Spec.Source.VolumeSnapshotContentName != nil {
 		content, err = ctrl.getPreprovisionedContentFromStore(snapshot)
@@ -374,7 +375,7 @@ func (ctrl *csiSnapshotCommonController) checkandAddSnapshotFinalizers(snapshot
 // If there is any problem with the binding (e.g., snapshot points to a non-existent snapshot content), update the snapshot status and emit event.
 func (ctrl *csiSnapshotCommonController) syncReadySnapshot(snapshot *crdv1.VolumeSnapshot) error {
 	if !utils.IsBoundVolumeSnapshotContentNameSet(snapshot) {
-		return fmt.Errorf("snapshot %s is not bound to a content.", utils.SnapshotKey(snapshot))
+		return fmt.Errorf("snapshot %s is not bound to a content", utils.SnapshotKey(snapshot))
 	}
 	content, err := ctrl.getContentFromStore(*snapshot.Status.BoundVolumeSnapshotContentName)
 	if err != nil {
@@ -464,30 +465,29 @@ func (ctrl *csiSnapshotCommonController) syncUnreadySnapshot(snapshot *crdv1.Vol
 		if snapshot.Spec.Source.PersistentVolumeClaimName == nil {
 			ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotPVCSourceMissing", fmt.Sprintf("PVC source for snapshot %s is missing", uniqueSnapshotName))
 			return fmt.Errorf("expected PVC source for snapshot %s but got nil", uniqueSnapshotName)
-		} else {
-			var err error
-			var content *crdv1.VolumeSnapshotContent
-			if content, err = ctrl.createSnapshotContent(snapshot); err != nil {
-				ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentCreationFailed", fmt.Sprintf("Failed to create snapshot content with error %v", err))
-				return err
-			}
+		}
+		var err error
+		var content *crdv1.VolumeSnapshotContent
+		if content, err = ctrl.createSnapshotContent(snapshot); err != nil {
+			ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotContentCreationFailed", fmt.Sprintf("Failed to create snapshot content with error %v", err))
+			return err
+		}
 
-			// 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)
+		// 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 {
-				// update snapshot status failed
-				ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
-				return err
-			}
+		if err != nil {
+			// update snapshot status failed
+			ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotStatusUpdateFailed", fmt.Sprintf("Snapshot status update failed, %v", err))
+			return err
 		}
 	}
 	return nil
@@ -1356,14 +1356,14 @@ func (ctrl *csiSnapshotCommonController) getSnapshotFromStore(snapshotName strin
 		klog.V(4).Infof("getSnapshotFromStore: snapshot %s not found", snapshotName)
 		// Fall through with snapshot = nil
 		return nil, nil
-	} else {
-		var ok bool
-		snapshot, ok = obj.(*crdv1.VolumeSnapshot)
-		if !ok {
-			return nil, fmt.Errorf("cannot convert object from snapshot cache to snapshot %q!?: %#v", snapshotName, obj)
-		}
-		klog.V(4).Infof("getSnapshotFromStore: snapshot %s found", snapshotName)
 	}
+	var ok bool
+	snapshot, ok = obj.(*crdv1.VolumeSnapshot)
+	if !ok {
+		return nil, fmt.Errorf("cannot convert object from snapshot cache to snapshot %q!?: %#v", snapshotName, obj)
+	}
+	klog.V(4).Infof("getSnapshotFromStore: snapshot %s found", snapshotName)
+
 	return snapshot, nil
 }
 

pkg/sidecar-controller/snapshot_controller.go

@@ -67,33 +67,33 @@ func (ctrl *csiSnapshotSideCarController) syncContent(content *crdv1.VolumeSnaps
 			// update content SnapshotHandle to nil upon a successful deletion. At this
 			// point, the finalizer on content should NOT be removed to avoid leaking.
 			return ctrl.deleteCSISnapshot(content)
-		} else {
-			// otherwise, either the snapshot has been deleted from the underlying
-			// storage system, or the deletion policy is Retain, remove the finalizer
-			// if there is one so that API server could delete the object if there is
-			// no other finalizer.
-			return ctrl.removeContentFinalizer(content)
-		}
-	} else {
-		if content.Spec.Source.VolumeHandle != nil && content.Status == nil {
-			klog.V(5).Infof("syncContent: Call CreateSnapshot for content %s", content.Name)
-			ctrl.createSnapshot(content)
-		} else {
-			// Skip checkandUpdateContentStatus() if ReadyToUse is
-			// already true. We don't want to keep calling CreateSnapshot
-			// or ListSnapshots CSI methods over and over again for
-			// performance reasons.
-			if content.Status != nil && content.Status.ReadyToUse != nil && *content.Status.ReadyToUse == true {
-				// Try to remove AnnVolumeSnapshotBeingCreated if it is not removed yet for some reason
-				err := ctrl.removeAnnVolumeSnapshotBeingCreated(content)
-				if err != nil {
-					return fmt.Errorf("failed to remove VolumeSnapshotBeingCreated annotation from the content %s: %q", content.Name, err)
-				}
-				return nil
-			}
-			ctrl.checkandUpdateContentStatus(content)
 		}
+		// otherwise, either the snapshot has been deleted from the underlying
+		// storage system, or the deletion policy is Retain, remove the finalizer
+		// if there is one so that API server could delete the object if there is
+		// no other finalizer.
+		return ctrl.removeContentFinalizer(content)
+
 	}
+	if content.Spec.Source.VolumeHandle != nil && content.Status == nil {
+		klog.V(5).Infof("syncContent: Call CreateSnapshot for content %s", content.Name)
+		ctrl.createSnapshot(content)
+	} else {
+		// Skip checkandUpdateContentStatus() if ReadyToUse is
+		// already true. We don't want to keep calling CreateSnapshot
+		// or ListSnapshots CSI methods over and over again for
+		// performance reasons.
+		if content.Status != nil && content.Status.ReadyToUse != nil && *content.Status.ReadyToUse == true {
+			// Try to remove AnnVolumeSnapshotBeingCreated if it is not removed yet for some reason
+			err := ctrl.removeAnnVolumeSnapshotBeingCreated(content)
+			if err != nil {
+				return fmt.Errorf("failed to remove VolumeSnapshotBeingCreated annotation from the content %s: %q", content.Name, err)
+			}
+			return nil
+		}
+		ctrl.checkandUpdateContentStatus(content)
+	}
+
 	return nil
 }
 
@@ -296,9 +296,9 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateContentStatusOperation(c
 			return nil, err
 		}
 		return updatedContent, nil
-	} else {
-		return ctrl.createSnapshotWrapper(content)
 	}
+	return ctrl.createSnapshotWrapper(content)
+
 }
 
 // This is a wrapper function for the snapshot creation process.
@@ -347,10 +347,9 @@ func (ctrl *csiSnapshotSideCarController) createSnapshotWrapper(content *crdv1.V
 	newContent, err := ctrl.updateSnapshotContentStatus(content, snapshotID, readyToUse, creationTime.UnixNano(), size)
 	if err != nil {
 		klog.Errorf("error updating status for volume snapshot content %s: %v.", content.Name, err)
-		return nil, fmt.Errorf("error updating status for volume snapshot content %s: %v.", content.Name, err)
-	} else {
-		content = newContent
+		return nil, fmt.Errorf("error updating status for volume snapshot content %s: %v", content.Name, err)
 	}
+	content = newContent
 
 	// NOTE(xyang): handle create timeout
 	// Remove annotation to indicate storage system has successfully

pkg/sidecar-controller/snapshot_delete_test.go

@@ -30,7 +30,7 @@ import (
 )
 
 var defaultSize int64 = 1000
-var emptySize int64 = 0
+var emptySize int64
 var deletePolicy = crdv1.VolumeSnapshotContentDelete
 var retainPolicy = crdv1.VolumeSnapshotContentRetain
 var timeNow = time.Now()

pkg/utils/util.go

@@ -215,10 +215,10 @@ func verifyAndGetSecretNameAndNamespaceTemplate(secret secretParamsMap, snapshot
 	} else if numName == 0 {
 		// No secrets specified
 		return "", "", nil
-	} else {
-		// THIS IS NOT A VALID CASE
-		return "", "", fmt.Errorf("unknown error with getting secret name and namespace templates")
 	}
+	// THIS IS NOT A VALID CASE
+	return "", "", fmt.Errorf("unknown error with getting secret name and namespace templates")
+
 }
 
 // getSecretReference returns a reference to the secret specified in the given nameTemplate