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

rbd: return gRPC code Aborted when the RBD-image is in-use on delete #5168

Merged
merged 1 commit into from
Feb 24, 2025
Merged

rbd: return gRPC code Aborted when the RBD-image is in-use on delete #5168

merged 1 commit into from
Feb 24, 2025

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Feb 20, 2025 •
edited
Loading

According to the error scheme documented in the CSI specification, the
Aborted error code should be initiate retries, whereas the Internal
error code does not require this behaviour.

When an RBD-image is still in-use, it can not be removed. The
DeleteVolume procedure should be retried and will succeed once the
RBD-image is not in-use anymore.

Updates: #5166

@nixpanic
rbd: return gRPC code Aborted when the RBD-image is in-use on delete
b79df92 
According to the error scheme documented in the CSI specification, the
Aborted error code should be initiate retries, whereas the Internal
error code does not require this behaviour.

When an RBD-image is still in-use, it can not be removed. The
DeleteVolume procedure should be retried and will succeed once the
RBD-image is not in-use anymore.

Fixes: ceph#5166
Signed-off-by: Niels de Vos <[email protected]>
@mergify mergify bot added the component/rbd Issues related to RBD label Feb 20, 2025
Madhu-1
Madhu-1 approved these changes Feb 20, 2025
View reviewed changes
Copy link
Collaborator

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

LGTM, @nixpanic IMO this is not a complete fix we need to do RCA for this one.

@nixpanic
Copy link
Member Author

LGTM, @nixpanic IMO this is not a complete fix we need to do RCA for this one.

Yes, it is not certain this will prevent the issue from happening, but if it happens, things should be cleaned-up now, eventually.

@nixpanic nixpanic requested a review from a team February 20, 2025 13:29
@iPraveenParihar
Copy link
Contributor

@Mergifyio rebase

@mergify Mergify
Copy link
Contributor

mergify bot commented Feb 24, 2025

rebase

☑️ Nothing to do

  • any of:
    • #commits > 1 [📌 rebase requirement]
    • #commits-behind > 0 [📌 rebase requirement]
    • -linear-history [📌 rebase requirement]
  • -closed [📌 rebase requirement]
  • -conflict [📌 rebase requirement]
  • queue-position = -1 [📌 rebase requirement]

@iPraveenParihar
Copy link
Contributor

@Mergifyio queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Feb 24, 2025 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

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

/test ci/centos/upgrade-tests-cephfs

@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.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

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

@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.30

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Feb 24, 2025
@mergify Mergify
Copy link
Contributor

mergify bot commented Feb 24, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

@iPraveenParihar
Copy link
Contributor

/retest ci/centos/mini-e2e-helm/k8s-1.30

@iPraveenParihar
Copy link
Contributor

failed CI logs

  �[38;5;9m[FAILED] failed to list rbd images: failed to list images 2025-02-24T07:58:39.775+0000 7fc75bfff640 -1 monclient: handle_auth_reply_more no authorizer?

@mergify mergify bot merged commit 43b150f into ceph:devel Feb 24, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Madhu-1 Madhu-1 Madhu-1 approved these changes

@yati1998 yati1998 yati1998 approved these changes

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.
5 participants
@nixpanic @iPraveenParihar @ceph-csi-bot @Madhu-1 @yati1998