cephfs: Log clone progress during a clone operation #4918
Conversation
f923e0a to
3f24712
Compare
Yes, it will be empty. |
3f24712 to
992ff84
Compare
992ff84 to
500fde7
Compare
500fde7 to
946609f
Compare
|
Testing results: PVC Events: CSI Logs: I am unable to understand what does this "MISSING" indicates in the log, can someone please help? |
In your PR, I don't see that to be the case 🤔. You could verify that once with you latest PR changes? |
The above output is using the latest changes only. |
|
/test ci/centos/mini-e2e-helm/k8s-1.31 |
946609f to
f7ff8c3
Compare
|
This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏 |
f7ff8c3 to
a91c8aa
Compare
I don't see the code for which you are suggesting the change, can you please re-highlight? |
Pull request has been modified.
internal/cephfs/store/fsjournal.go
Outdated
| log.UsefulLog(ctx, err.Error()) | ||
|
|
||
| return nil, err | ||
| } |
There was a problem hiding this comment.
| log.UsefulLog(ctx, err.Error()) | |
| return nil, err | |
| } | |
| } | |
| log.ErrorLog(ctx, err.Error()) | |
| return nil, err |
There was a problem hiding this comment.
It would be logically incorrect to term it as error here.
Since, there is no error as such with the clone, rather it is just in progress and should get complete in sometime. So, I don't think we should log it as an error log.
There was a problem hiding this comment.
It is returned as a error. I would be fine with using debuglog, we don't use useful log anywhere else.
And your recent change no longer returns err if progress is missing. This needs to be changed as well. Observe the change in curly bracket
There was a problem hiding this comment.
Ah, right.
Updated the code now, please take a look.
b9023dc to
c5da289
Compare
Pull request has been modified.
|
@Mergifyio rebase |
log cephfs clone progress report during cephfs clone operation Signed-off-by: Nikhil-Ladha <[email protected]>
✅ Branch has been successfully rebased |
c5da289 to
68b4f9c
Compare
|
@Mergifyio queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 98cf078 |
|
/test ci/centos/k8s-e2e-external-storage/1.30 |
|
/test ci/centos/k8s-e2e-external-storage/1.29 |
|
/test ci/centos/mini-e2e-helm/k8s-1.30 |
|
/test ci/centos/mini-e2e-helm/k8s-1.29 |
|
/test ci/centos/mini-e2e/k8s-1.30 |
|
/test ci/centos/mini-e2e/k8s-1.29 |
|
/test ci/centos/k8s-e2e-external-storage/1.31 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/mini-e2e-helm/k8s-1.31 |
|
/test ci/centos/upgrade-tests-rbd |
|
/test ci/centos/mini-e2e/k8s-1.31 |
Describe what this PR does
Add clone progress report in the clone status operation and report the same in the logs for user's reference.
Fixes: #4813
Checklist:
guidelines in the developer
guide.
Request
notes
updated with breaking and/or notable changes for the next major release.
Show available bot commands
These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:
/retest ci/centos/<job-name>: retest the<job-name>after unrelatedfailure (please report the failure too!)