Skip to content

rbd: return replication status and status message in case of error #5500

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
Merged
merged 1 commit into from
Aug 15, 2025
Merged

rbd: return replication status and status message in case of error #5500

merged 1 commit into from
Aug 15, 2025

Conversation

@Nikhil-Ladha
Copy link
Contributor

@Nikhil-Ladha Nikhil-Ladha commented Aug 14, 2025 •
edited
Loading

Describe what this PR does

We should return the status and status message even if we fail to fetch the local status of the volume/group so that the error/state can be propagated to the VR/VGR in csi-addons.
For ex, if the local mirror status is down+unknown when rbd is not able to fetch the mirror status, we should report the same on the VR/VGR i.e, the state being unknown and the message being status not found.

Also, it seems more logical to return a FailedPreCondition error code if sync info not found error is hit.

@mergify mergify bot added the component/rbd Issues related to RBD label Aug 14, 2025
@Nikhil-Ladha Nikhil-Ladha requested review from a team and Madhu-1 August 14, 2025 05:40
Rakshith-R
Rakshith-R reviewed Aug 14, 2025
View reviewed changes
nixpanic
nixpanic reviewed Aug 14, 2025
View reviewed changes
@Nikhil-Ladha Nikhil-Ladha force-pushed the return-state-always branch 2 times, most recently from 1472cc4 to e34dc39 Compare August 14, 2025 08:53
@nixpanic
Copy link
Member

Looks reasonable to me, do you want to adjust the commit message and the description of this PR to clarify things a little more? Specially in the PR description, an example of what was missing, and how it will look like afterwards would be very helpful.

@Nikhil-Ladha
Copy link
Contributor Author

Looks reasonable to me, do you want to adjust the commit message and the description of this PR to clarify things a little more? Specially in the PR description, an example of what was missing, and how it will look like afterwards would be very helpful.

Sure, updated the description to add some more context.
Hopefully, it looks better now :)

@nixpanic
Copy link
Member

@Mergifyio rebase

@Nikhil-Ladha @nixpanic
rbd: return replication status and status message in case of error
4998836 
we should return the status and status message even if we fail to
fetch the local status of the volume/group so that the error/state
can be propagated to the VR/VGR in csi-addons.

Signed-off-by: Nikhil-Ladha <[email protected]>
@mergify
Copy link
Contributor

mergify bot commented Aug 15, 2025

rebase

✅ Branch has been successfully rebased

@nixpanic nixpanic force-pushed the return-state-always branch from e34dc39 to 4998836 Compare August 15, 2025 10:46
@nixpanic
Copy link
Member

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Aug 15, 2025 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 7458910

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Aug 15, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.33

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Aug 15, 2025
@mergify mergify bot merged commit 7458910 into ceph:devel Aug 15, 2025
37 of 38 checks passed
mergify bot added a commit that referenced this pull request Aug 15, 2025
@mergify
Merge of #5500
432fd84
@mergify mergify bot mentioned this pull request Aug 15, 2025
Closed
78 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nixpanic nixpanic nixpanic approved these changes

@Rakshith-R Rakshith-R Rakshith-R approved these changes

@Madhu-1 Madhu-1 Awaiting requested review from Madhu-1

Assignees

No one assigned

Labels

component/rbd Issues related to RBD

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Nikhil-Ladha @nixpanic @ceph-csi-bot @Rakshith-R