From 84c1fe52c7508cb5e78f9db30b0098c98f9fb021 Mon Sep 17 00:00:00 2001 From: Yati Padia Date: Mon, 5 Jul 2021 18:40:53 +0530 Subject: [PATCH] cleanup: resolve exhaustive linter This commit resolves exhaustive linter error. Updates: #2240 Signed-off-by: Yati Padia --- e2e/pod.go | 11 +++-- internal/cephfs/clone.go | 97 ++++++++++++++++++++------------------- internal/cephfs/errors.go | 6 +++ 3 files changed, 62 insertions(+), 52 deletions(-) diff --git a/e2e/pod.go b/e2e/pod.go index e96b84e33..0e5dbf422 100644 --- a/e2e/pod.go +++ b/e2e/pod.go @@ -314,12 +314,13 @@ func waitForPodInRunningState(name, ns string, c kubernetes.Interface, t int, ex return true, err } } + case v1.PodUnknown: + e2elog.Logf( + "%s app is in %s phase expected to be in Running state (%d seconds elapsed)", + name, + pod.Status.Phase, + int(time.Since(start).Seconds())) } - e2elog.Logf( - "%s app is in %s phase expected to be in Running state (%d seconds elapsed)", - name, - pod.Status.Phase, - int(time.Since(start).Seconds())) return false, nil }) } diff --git a/internal/cephfs/clone.go b/internal/cephfs/clone.go index bcfc52730..60081a027 100644 --- a/internal/cephfs/clone.go +++ b/internal/cephfs/clone.go @@ -19,7 +19,6 @@ package cephfs import ( "context" "errors" - "fmt" "github.com/ceph/ceph-csi/internal/util" ) @@ -43,6 +42,23 @@ const ( snapshotIsProtected = "yes" ) +// toError checks the state of the clone if it's not cephFSCloneComplete. +func (cs cephFSCloneState) toError() error { + switch cs { + case cephFSCloneComplete: + return nil + case cephFSCloneError: + return ErrInvalidClone + case cephFSCloneInprogress: + return ErrCloneInProgress + case cephFSClonePending: + return ErrClonePending + case cephFSCloneFailed: + return ErrCloneFailed + } + return nil +} + func createCloneFromSubvolume(ctx context.Context, volID, cloneID volumeID, volOpt, parentvolOpt *volumeOptions) error { snapshotID := cloneID err := parentvolOpt.createSnapshot(ctx, snapshotID, volID) @@ -95,42 +111,34 @@ func createCloneFromSubvolume(ctx context.Context, volID, cloneID volumeID, volO cloneState, cloneErr := volOpt.getCloneState(ctx, cloneID) if cloneErr != nil { + util.ErrorLog(ctx, "failed to get clone state: %v", cloneErr) return cloneErr } - switch cloneState { - case cephFSCloneInprogress: - util.ErrorLog(ctx, "clone is in progress for %v", cloneID) - return ErrCloneInProgress - case cephFSClonePending: - util.ErrorLog(ctx, "clone is pending for %v", cloneID) - return ErrClonePending - case cephFSCloneFailed: - util.ErrorLog(ctx, "clone failed for %v", cloneID) - cloneFailedErr := fmt.Errorf("clone %s is in %s state", cloneID, cloneState) - return cloneFailedErr - case cephFSCloneComplete: - // This is a work around to fix sizing issue for cloned images - err = volOpt.resizeVolume(ctx, cloneID, volOpt.Size) - if err != nil { - util.ErrorLog(ctx, "failed to expand volume %s: %v", cloneID, err) - return err - } - // As we completed clone, remove the intermediate snap - if err = parentvolOpt.unprotectSnapshot(ctx, snapshotID, volID); err != nil { - // In case the snap is already unprotected we get ErrSnapProtectionExist error code - // in that case we are safe and we could discard this error and we are good to go - // ahead with deletion - if !errors.Is(err, ErrSnapProtectionExist) { - util.ErrorLog(ctx, "failed to unprotect snapshot %s %v", snapshotID, err) - return err - } - } - if err = parentvolOpt.deleteSnapshot(ctx, snapshotID, volID); err != nil { - util.ErrorLog(ctx, "failed to delete snapshot %s %v", snapshotID, err) + if cloneState != cephFSCloneComplete { + util.ErrorLog(ctx, "clone %s did not complete: %v", cloneID, cloneState.toError()) + return cloneState.toError() + } + // This is a work around to fix sizing issue for cloned images + err = volOpt.resizeVolume(ctx, cloneID, volOpt.Size) + if err != nil { + util.ErrorLog(ctx, "failed to expand volume %s: %v", cloneID, err) + return err + } + // As we completed clone, remove the intermediate snap + if err = parentvolOpt.unprotectSnapshot(ctx, snapshotID, volID); err != nil { + // In case the snap is already unprotected we get ErrSnapProtectionExist error code + // in that case we are safe and we could discard this error and we are good to go + // ahead with deletion + if !errors.Is(err, ErrSnapProtectionExist) { + util.ErrorLog(ctx, "failed to unprotect snapshot %s %v", snapshotID, err) return err } } + if err = parentvolOpt.deleteSnapshot(ctx, snapshotID, volID); err != nil { + util.ErrorLog(ctx, "failed to delete snapshot %s %v", snapshotID, err) + return err + } return nil } @@ -192,25 +200,20 @@ func createCloneFromSnapshot( cloneState, err := volOptions.getCloneState(ctx, volumeID(vID.FsSubvolName)) if err != nil { + util.ErrorLog(ctx, "failed to get clone state: %v", err) return err } - switch cloneState { - case cephFSCloneInprogress: - return ErrCloneInProgress - case cephFSClonePending: - return ErrClonePending - case cephFSCloneFailed: - return fmt.Errorf("clone %s is in %s state", vID.FsSubvolName, cloneState) - case cephFSCloneComplete: - // The clonedvolume currently does not reflect the proper size due to an issue in cephfs - // however this is getting addressed in cephfs and the parentvolume size will be reflected - // in the new cloned volume too. Till then we are explicitly making the size set - err = volOptions.resizeVolume(ctx, volumeID(vID.FsSubvolName), volOptions.Size) - if err != nil { - util.ErrorLog(ctx, "failed to expand volume %s with error: %v", vID.FsSubvolName, err) - return err - } + if cloneState != cephFSCloneComplete { + return cloneState.toError() + } + // The clonedvolume currently does not reflect the proper size due to an issue in cephfs + // however this is getting addressed in cephfs and the parentvolume size will be reflected + // in the new cloned volume too. Till then we are explicitly making the size set + err = volOptions.resizeVolume(ctx, volumeID(vID.FsSubvolName), volOptions.Size) + if err != nil { + util.ErrorLog(ctx, "failed to expand volume %s with error: %v", vID.FsSubvolName, err) + return err } return nil } diff --git a/internal/cephfs/errors.go b/internal/cephfs/errors.go index 03f0f5b4b..2af5e46d4 100644 --- a/internal/cephfs/errors.go +++ b/internal/cephfs/errors.go @@ -33,6 +33,12 @@ var ( // ErrClonePending is returned when snapshot clone state is `pending` ErrClonePending = errors.New("clone from snapshot is pending") + // ErrInvalidClone is returned when the clone state is invalid + ErrInvalidClone = errors.New("invalid clone state") + + // ErrCloneFailed is returned when the clone state is failed. + ErrCloneFailed = errors.New("clone from snapshot failed") + // ErrInvalidVolID is returned when a CSI passed VolumeID is not conformant to any known volume ID // formats. ErrInvalidVolID = errors.New("invalid VolumeID")