Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cephfs: handle cephfs clone limit error #4276

Merged
merged 1 commit into from Nov 24, 2023
Merged

cephfs: handle cephfs clone limit error #4276

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions internal/cephfs/controllerserver.go
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"syscall"

"github.com/ceph/ceph-csi/internal/cephfs/core"
cerrors "github.com/ceph/ceph-csi/internal/cephfs/errors"
Expand Down Expand Up @@ -134,6 +135,11 @@ func (cs *ControllerServer) createBackingVolumeFromSnapshotSource(
})
if err != nil {
log.ErrorLog(ctx, "failed to create clone from snapshot %s: %v", sID.FsSnapshotName, err)
// TODO: Add error handle for EAGAIN in go-ceph and replace the
// syscall.EAGAIN check with the go-ceph compatible error.
if errors.Is(err, syscall.EAGAIN) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking for the syscall error is not very clean, and is potentially not correct. It would be much better to check for a valid go-ceph error.

Can you explain how you verified that checking against syscall.EAGAIN is correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently go-ceph doesn't have a compatible error code for EAGAIN to check for. Since we get EAGAIN from ceph with the proposed change and that will be formated by go-ceph and sent back to csi, checking it against syscall.EAGAIN should work for the time being.

I will add a TODO note for implementing EAGAIN error handle in go-ceph and use that once available.

return status.Error(codes.ResourceExhausted, err.Error())
}

return err
}
Expand All @@ -156,6 +162,11 @@ func (cs *ControllerServer) createBackingVolumeFromVolumeSource(

if err := volClient.CreateCloneFromSubvolume(ctx, &parentVolOpt.SubVolume); err != nil {
log.ErrorLog(ctx, "failed to create clone from subvolume %s: %v", fsutil.VolumeID(pvID.FsSubvolName), err)
// TODO: Add error handle for EAGAIN in go-ceph and replace the
// syscall.EAGAIN check with the go-ceph compatible error.
if errors.Is(err, syscall.EAGAIN) {
return status.Error(codes.ResourceExhausted, err.Error())
}

return err
}
Expand Down