cephfs: handle cephfs clone limit error #4276
Conversation
|
@Rakshith-R @yati1998 @nixpanic PTAL. |
|
Can we add e2e test for this? |
The ceph PR is still not merged under main or any release branch. I don't think we can test it with an e2e at the moment. |
| @@ -134,6 +135,9 @@ func (cs *ControllerServer) createBackingVolumeFromSnapshotSource( | |||
| }) | |||
| if err != nil { | |||
| log.ErrorLog(ctx, "failed to create clone from snapshot %s: %v", sID.FsSnapshotName, err) | |||
| if errors.Is(err, syscall.EAGAIN) { | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
karthik-us
force-pushed
the
clone-error
branch
from
0f1f750
to
441638f
Compare
mergify
bot
dismissed
riya-singhal31’s stale review
Pull request has been modified.
|
@Mergifyio rebase |
✅ Branch has been successfully rebased |
nixpanic
force-pushed
the
clone-error
branch
from
441638f
to
6412398
Compare
|
@Mergifyio queue |
🛑 The pull request has been removed from the queue
|
|
@Mergifyio rebase |
This is to pre-emptively add check for EAGAIN error returned from ceph as part of ceph/ceph#52670 if all the clone threads are busy and return csi compatible error. Fixes: ceph#3996 Signed-off-by: karthik-us <ksubrahm@redhat.com>
✅ Branch has been successfully rebased |
Rakshith-R
force-pushed
the
clone-error
branch
from
6412398
to
2ae31d7
Compare
|
@Mergifyio queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at f666529 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/k8s-e2e-external-storage/1.27 |
|
/test ci/centos/k8s-e2e-external-storage/1.28 |
|
/test ci/centos/upgrade-tests-rbd |
|
/test ci/centos/mini-e2e-helm/k8s-1.27 |
|
/test ci/centos/k8s-e2e-external-storage/1.26 |
|
/test ci/centos/mini-e2e-helm/k8s-1.28 |
|
/test ci/centos/mini-e2e/k8s-1.27 |
|
/test ci/centos/mini-e2e/k8s-1.28 |
|
/test ci/centos/mini-e2e-helm/k8s-1.26 |
|
/test ci/centos/mini-e2e/k8s-1.26 |
This is to pre-emptively add check for EAGAIN error returned from ceph as part of ceph/ceph#52670 if all the clone threads are busy and return csi compatible error.
Fixes: #3996