cephfs: report detailed error message on clone failure

go-ceph provides a new GetFailure() method to retrieve details errors
when cloning failed. This is now included in the `cephFSCloneState`
struct, which was a simple string before.

While modifying the `cephFSCloneState` struct, the constants have been
removed, as go-ceph provides them as well.

Fixes: #3140
Signed-off-by: Niels de Vos <ndevos@redhat.com>

internal/cephfs/core/clone.go

@@ -19,43 +19,42 @@ package core
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	cerrors "github.com/ceph/ceph-csi/internal/cephfs/errors"
 	"github.com/ceph/ceph-csi/internal/util/log"
+
+	"github.com/ceph/go-ceph/cephfs/admin"
 )
 
 // cephFSCloneState describes the status of the clone.
-type cephFSCloneState string
+type cephFSCloneState struct {
+	state    admin.CloneState
+	errno    string
+	errorMsg string
+}
 
 const (
-	// CephFSCloneError indicates that fetching the clone state returned an error.
-	CephFSCloneError = cephFSCloneState("")
-	// CephFSCloneFailed indicates that clone is in failed state.
-	CephFSCloneFailed = cephFSCloneState("failed")
-	// CephFSClonePending indicates that clone is in pending state.
-	CephFSClonePending = cephFSCloneState("pending")
-	// CephFSCloneInprogress indicates that clone is in in-progress state.
-	CephFSCloneInprogress = cephFSCloneState("in-progress")
-	// CephFSCloneComplete indicates that clone is in complete state.
-	CephFSCloneComplete = cephFSCloneState("complete")
-
 	// SnapshotIsProtected string indicates that the snapshot is currently protected.
 	SnapshotIsProtected = "yes"
 )
 
-// toError checks the state of the clone if it's not cephFSCloneComplete.
+// CephFSCloneError indicates that fetching the clone state returned an error.
-func (cs cephFSCloneState) toError() error {
+var CephFSCloneError = cephFSCloneState{}
-	switch cs {
+
-	case CephFSCloneComplete:
+// ToError checks the state of the clone if it's not cephFSCloneComplete.
+func (cs cephFSCloneState) ToError() error {
+	switch cs.state {
+	case admin.CloneComplete:
 		return nil
-	case CephFSCloneError:
+	case CephFSCloneError.state:
-		return cerrors.ErrInvalidClone
+		return fmt.Errorf("%w: %s (%s)", cerrors.ErrInvalidClone, cs.errorMsg, cs.errno)
-	case CephFSCloneInprogress:
+	case admin.CloneInProgress:
 		return cerrors.ErrCloneInProgress
-	case CephFSClonePending:
+	case admin.ClonePending:
 		return cerrors.ErrClonePending
-	case CephFSCloneFailed:
+	case admin.CloneFailed:
-		return cerrors.ErrCloneFailed
+		return fmt.Errorf("%w: %s (%s)", cerrors.ErrCloneFailed, cs.errorMsg, cs.errno)
 	}
 
 	return nil
@@ -125,10 +124,11 @@ func (s *subVolumeClient) CreateCloneFromSubvolume(
 		return cloneErr
 	}
 
-	if cloneState != CephFSCloneComplete {
+	err = cloneState.ToError()
-		log.ErrorLog(ctx, "clone %s did not complete: %v", s.VolID, cloneState.toError())
+	if err != nil {
+		log.ErrorLog(ctx, "clone %s did not complete: %v", s.VolID, err)
 
-		return cloneState.toError()
+		return err
 	}
 
 	err = s.ExpandVolume(ctx, s.Size)
@@ -220,8 +220,9 @@ func (s *subVolumeClient) CreateCloneFromSnapshot(
 		return err
 	}
 
-	if cloneState != CephFSCloneComplete {
+	err = cloneState.ToError()
-		return cloneState.toError()
+	if err != nil {
+		return err
 	}
 
 	err = s.ExpandVolume(ctx, s.Size)
@@ -255,5 +256,18 @@ func (s *subVolumeClient) GetCloneState(ctx context.Context) (cephFSCloneState,
 		return CephFSCloneError, err
 	}
 
-	return cephFSCloneState(cs.State), nil
+	errno := ""
+	errStr := ""
+	if failure := cs.GetFailure(); failure != nil {
+		errno = failure.Errno
+		errStr = failure.ErrStr
+	}
+
+	state := cephFSCloneState{
+		state:    cs.State,
+		errno:    errno,
+		errorMsg: errStr,
+	}
+
+	return state, nil
 }

internal/cephfs/core/clone_test.go

@@ -17,23 +17,25 @@ limitations under the License.
 package core
 
 import (
+	"errors"
 	"testing"
 
 	cerrors "github.com/ceph/ceph-csi/internal/cephfs/errors"
 
+	fsa "github.com/ceph/go-ceph/cephfs/admin"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestCloneStateToError(t *testing.T) {
 	t.Parallel()
 	errorState := make(map[cephFSCloneState]error)
-	errorState[CephFSCloneComplete] = nil
+	errorState[cephFSCloneState{fsa.CloneComplete, "", ""}] = nil
 	errorState[CephFSCloneError] = cerrors.ErrInvalidClone
-	errorState[CephFSCloneInprogress] = cerrors.ErrCloneInProgress
+	errorState[cephFSCloneState{fsa.CloneInProgress, "", ""}] = cerrors.ErrCloneInProgress
-	errorState[CephFSClonePending] = cerrors.ErrClonePending
+	errorState[cephFSCloneState{fsa.ClonePending, "", ""}] = cerrors.ErrClonePending
-	errorState[CephFSCloneFailed] = cerrors.ErrCloneFailed
+	errorState[cephFSCloneState{fsa.CloneFailed, "", ""}] = cerrors.ErrCloneFailed
 
 	for state, err := range errorState {
-		assert.Equal(t, state.toError(), err)
+		assert.True(t, errors.Is(state.ToError(), err))
 	}
 }

internal/cephfs/store/fsjournal.go

@@ -119,15 +119,17 @@ func CheckVolExists(ctx context.Context,
 
 			return nil, err
 		}
-		if cloneState == core.CephFSCloneInprogress {
+		err = cloneState.ToError()
-			return nil, cerrors.ErrCloneInProgress
+		if errors.Is(err, cerrors.ErrCloneInProgress) {
+			return nil, err
 		}
-		if cloneState == core.CephFSClonePending {
+		if errors.Is(err, cerrors.ErrClonePending) {
-			return nil, cerrors.ErrClonePending
+			return nil, err
 		}
-		if cloneState == core.CephFSCloneFailed {
+		if errors.Is(err, cerrors.ErrCloneFailed) {
 			log.ErrorLog(ctx,
-				"clone failed, deleting subvolume clone. vol=%s, subvol=%s subvolgroup=%s",
+				"clone failed (%v), deleting subvolume clone. vol=%s, subvol=%s subvolgroup=%s",
+				err,
 				volOptions.FsName,
 				vid.FsSubvolName,
 				volOptions.SubvolumeGroup)
@@ -149,8 +151,8 @@ func CheckVolExists(ctx context.Context,
 
 			return nil, err
 		}
-		if cloneState != core.CephFSCloneComplete {
+		if err != nil {
-			return nil, fmt.Errorf("clone is not in complete state for %s", vid.FsSubvolName)
+			return nil, fmt.Errorf("clone is not in complete state for %s: %w", vid.FsSubvolName, err)
 		}
 	}