From 66aa595e77a37c2a0d48ecd537742ca2c1d7bf1c Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Wed, 28 Oct 2020 15:20:41 +0100 Subject: [PATCH] cephfs: do not export internal CloneStatus type getCloneInfo() does not need to return a full CloneStatus struct that only has one member. Instead, it can just return the value of the single member, so the JSON type/struct does not need to be exposed. This makes the API for getCloneInfo() a little simpler, so it can be replaced by a go-ceph implementation later on. As the function does not return any of the unused attributes anymore, it is renamed to getCloneStatu() as well. Signed-off-by: Niels de Vos --- internal/cephfs/clone.go | 52 ++++++++++++++++++++---------------- internal/cephfs/fsjournal.go | 10 +++---- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/internal/cephfs/clone.go b/internal/cephfs/clone.go index 9ab9297a3..faeb89abb 100644 --- a/internal/cephfs/clone.go +++ b/internal/cephfs/clone.go @@ -24,15 +24,21 @@ import ( "github.com/ceph/ceph-csi/internal/util" ) +// cephFSCloneState describes the status of the clone. +type cephFSCloneState string + const ( + // cephFSCloneError indicates that fetching the clone state returned an error. + cephFSCloneError = cephFSCloneState("") // cephFSCloneFailed indicates that clone is in failed state. - cephFSCloneFailed = "failed" + cephFSCloneFailed = cephFSCloneState("failed") // cephFSClonePending indicates that clone is in pending state. - cephFSClonePending = "pending" + cephFSClonePending = cephFSCloneState("pending") // cephFSCloneInprogress indicates that clone is in in-progress state. - cephFSCloneInprogress = "in-progress" + cephFSCloneInprogress = cephFSCloneState("in-progress") // cephFSCloneComplete indicates that clone is in complete state. - cephFSCloneComplete = "complete" + cephFSCloneComplete = cephFSCloneState("complete") + // snapshotIsProtected string indicates that the snapshot is currently protected. snapshotIsProtected = "yes" ) @@ -86,13 +92,13 @@ func createCloneFromSubvolume(ctx context.Context, volID, cloneID volumeID, volO util.ErrorLog(ctx, "failed to clone snapshot %s %s to %s %v", volID, snapshotID, cloneID, cloneErr) return cloneErr } - var clone CloneStatus - clone, cloneErr = getCloneInfo(ctx, volOpt, cr, cloneID) + + cloneState, cloneErr := getCloneState(ctx, volOpt, cr, cloneID) if cloneErr != nil { return cloneErr } - switch clone.Status.State { + switch cloneState { case cephFSCloneInprogress: util.ErrorLog(ctx, "clone is in progress for %v", cloneID) return ErrCloneInProgress @@ -101,7 +107,7 @@ func createCloneFromSubvolume(ctx context.Context, volID, cloneID volumeID, volO return ErrClonePending case cephFSCloneFailed: util.ErrorLog(ctx, "clone failed for %v", cloneID) - cloneFailedErr := fmt.Errorf("clone %s is in %s state", cloneID, clone.Status.State) + 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 @@ -176,18 +182,19 @@ func createCloneFromSnapshot(ctx context.Context, parentVolOpt, volOptions *volu } } }() - var clone = CloneStatus{} - clone, err = getCloneInfo(ctx, volOptions, cr, volumeID(vID.FsSubvolName)) + + cloneState, err := getCloneState(ctx, volOptions, cr, volumeID(vID.FsSubvolName)) if err != nil { return err } - switch clone.Status.State { + + switch cloneState { case cephFSCloneInprogress: return ErrCloneInProgress case cephFSClonePending: return ErrClonePending case cephFSCloneFailed: - return fmt.Errorf("clone %s is in %s state", vID.FsSubvolName, clone.Status.State) + 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 @@ -201,15 +208,14 @@ func createCloneFromSnapshot(ctx context.Context, parentVolOpt, volOptions *volu return nil } -// CloneStatus represents the subvolume clone status. -type CloneStatus struct { - Status struct { - State string `json:"state"` - } `json:"status"` -} +func getCloneState(ctx context.Context, volOptions *volumeOptions, cr *util.Credentials, volID volumeID) (cephFSCloneState, error) { + type cloneStatus struct { + Status struct { + State string `json:"state"` + } `json:"status"` + } -func getCloneInfo(ctx context.Context, volOptions *volumeOptions, cr *util.Credentials, volID volumeID) (CloneStatus, error) { - clone := CloneStatus{} + cs := cloneStatus{} args := []string{ "fs", "clone", @@ -226,12 +232,12 @@ func getCloneInfo(ctx context.Context, volOptions *volumeOptions, cr *util.Crede } err := execCommandJSON( ctx, - &clone, + &cs, "ceph", args[:]...) if err != nil { util.ErrorLog(ctx, "failed to get subvolume clone info %s(%s) in fs %s", string(volID), err, volOptions.FsName) - return clone, err + return cephFSCloneError, err } - return clone, nil + return cephFSCloneState(cs.Status.State), nil } diff --git a/internal/cephfs/fsjournal.go b/internal/cephfs/fsjournal.go index a6058205e..75413a7e5 100644 --- a/internal/cephfs/fsjournal.go +++ b/internal/cephfs/fsjournal.go @@ -83,7 +83,7 @@ func checkVolExists(ctx context.Context, vid.FsSubvolName = imageData.ImageAttributes.ImageName if sID != nil || pvID != nil { - clone, cloneInfoErr := getCloneInfo(ctx, volOptions, cr, volumeID(vid.FsSubvolName)) + cloneState, cloneInfoErr := getCloneState(ctx, volOptions, cr, volumeID(vid.FsSubvolName)) if cloneInfoErr != nil { if errors.Is(cloneInfoErr, ErrVolumeNotFound) { if pvID != nil { @@ -102,13 +102,13 @@ func checkVolExists(ctx context.Context, } return nil, err } - if clone.Status.State == cephFSCloneInprogress { + if cloneState == cephFSCloneInprogress { return nil, ErrCloneInProgress } - if clone.Status.State == cephFSClonePending { + if cloneState == cephFSClonePending { return nil, ErrClonePending } - if clone.Status.State == cephFSCloneFailed { + if cloneState == cephFSCloneFailed { err = purgeVolume(ctx, volumeID(vid.FsSubvolName), cr, volOptions, true) if err != nil { util.ErrorLog(ctx, "failed to delete volume %s: %v", vid.FsSubvolName, err) @@ -128,7 +128,7 @@ func checkVolExists(ctx context.Context, volOptions.MetadataPool, vid.FsSubvolName, volOptions.RequestName) return nil, err } - if clone.Status.State != cephFSCloneComplete { + if cloneState != cephFSCloneComplete { return nil, fmt.Errorf("clone is not in complete state for %s", vid.FsSubvolName) } } else {