Skip to content

rbd: add check before calling Destroy #5446

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 2 commits into from
Jul 28, 2025
Merged

rbd: add check before calling Destroy #5446

merged 2 commits into from
Jul 28, 2025

Conversation

@Madhu-1
Copy link
Collaborator

@Madhu-1 Madhu-1 commented Jul 24, 2025 •
edited
Loading

It seems possible that the .Destroy() function is called on a nil pointer. This would cause a panic in the node-plugin.

Depending on how far GenVolFromVolID() comes, the returned rbdVolume can be connected. If an error occurs, the
rbdVolume should not be connected at all, so call the .Destroy() function in those cases too.

small part of #4564 taken to fix the exact problem for now.

Fixes: #4562

@mergify mergify bot added the component/rbd Issues related to RBD label Jul 24, 2025
@Madhu-1 Madhu-1 mentioned this pull request Jul 24, 2025
Open
@Madhu-1 Madhu-1 requested review from a team and nixpanic July 24, 2025 08:06
Rakshith-R
Rakshith-R previously approved these changes Jul 24, 2025
View reviewed changes
nixpanic
nixpanic previously approved these changes Jul 24, 2025
View reviewed changes
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jul 24, 2025

@Rakshith-R @nixpanic i had to push one more commit to refactor other part of code in this function to fix CI problem. PTAL

@Madhu-1 Madhu-1 requested review from Rakshith-R and nixpanic July 24, 2025 09:26
@mergify mergify bot dismissed stale reviews from Rakshith-R and nixpanic July 24, 2025 09:26

Pull request has been modified.

@Madhu-1 Madhu-1 force-pushed the fix-4562 branch 2 times, most recently from 01c95d7 to 238dfa1 Compare July 24, 2025 09:42
nixpanic
nixpanic previously approved these changes Jul 25, 2025
View reviewed changes
@nixpanic nixpanic requested a review from a team July 25, 2025 06:58
black-dragon74
black-dragon74 previously approved these changes Jul 25, 2025
View reviewed changes
@nixpanic
Copy link
Member

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Jul 25, 2025

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 25, 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 about the failure.

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

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

@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/k8s-e2e-external-storage/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/upgrade-tests-cephfs

@mergify
Copy link
Contributor

mergify bot commented Jul 25, 2025 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

Pull request #5446 has been dequeued by a dequeue command.

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

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jul 28, 2025

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Jul 28, 2025

refresh

✅ Pull request refreshed

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jul 28, 2025

@Mergifyio dequeue

@mergify
Copy link
Contributor

mergify bot commented Jul 28, 2025

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

Pull request #5446 has been dequeued by a dequeue command.

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 do update this pull request, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify
Copy link
Contributor

mergify bot commented Jul 28, 2025

dequeue

✅ The pull request has been removed from the queue default

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jul 28, 2025

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Jul 28, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jul 28, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 28, 2025 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at e0ecbea

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jul 28, 2025

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Jul 28, 2025

rebase

☑️ Nothing to do, the required conditions are not met

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

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jul 28, 2025

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

@Madhu-1 Madhu-1 added the ok-to-test Label to trigger E2E tests label Jul 28, 2025
@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@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-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/k8s-e2e-external-storage/1.32

@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/mini-e2e-helm/k8s-1.32

@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 Jul 28, 2025
@mergify mergify bot merged commit e0ecbea into ceph:devel Jul 28, 2025
37 checks passed
mergify bot added a commit that referenced this pull request Jul 28, 2025
@mergify
Merge of #5446
050b307
This was referenced Jul 28, 2025
Closed
Merged
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

@black-dragon74 black-dragon74 black-dragon74 approved these changes

@Rakshith-R Rakshith-R Awaiting requested review from Rakshith-R

Assignees

No one assigned

Labels

backport-to-release-v3.14 Label to backport from devel to release-v3.14 branch component/rbd Issues related to RBD

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect Error Message when Volume Mount Fails

5 participants

@Madhu-1 @nixpanic @ceph-csi-bot @black-dragon74 @Rakshith-R