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.

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

Prevent dataloss due to the concurrent RPC calls (occurrence is very low) (backport #4970) #4972

Merged
merged 4 commits into from
Nov 21, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Nov 21, 2024

This PR includes series for commits for the following actions

  • Introduce in memory lock for NodePublish and UnPublish (we should not be dependent on the CO and make assumption that CO makes the calls serial for the same volID and the targetPath)
  • Use os.Remove instead of os.Removeall to remove the empty directory, os.Removeall ends up removing everything in the path

Thanks to @martin-helmich @hensur and @Lucaber for the detailed analysis

fixes: #4966


This is an automatic backport of pull request #4970 done by Mergify.

We should not be dependent on the CO to ensure
that it will serialize the request instead of
that we need to have own internal locks to ensure
that we dont do concurrent operations for same
request.

Signed-off-by: Madhu Rajanna <[email protected]>
(cherry picked from commit b6bd8ca)
We should not be dependent on the CO to ensure
that it will serialize the request instead of
that we need to have own internal locks to ensure
that we dont do concurrent operations for same
request.

Signed-off-by: Madhu Rajanna <[email protected]>
(cherry picked from commit 7cfeae5)
using os.RemoveAll will remove everything
in the director after the Umount we should
be using os.Remove only to remove the empty
directory

Signed-off-by: Madhu Rajanna <[email protected]>
(cherry picked from commit cd09266)
using os.RemoveAll will remove everything
in the director after the Umount we should
be using os.Remove only to remove the empty
directory

Signed-off-by: Madhu Rajanna <[email protected]>
(cherry picked from commit 00d252e)
@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Nov 21, 2024
@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.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

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

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Nov 21, 2024
@Madhu-1 Madhu-1 added the ready-to-merge This PR is ready to be merged and it doesn't need second review (backports only) label Nov 21, 2024
@mergify mergify bot merged commit 590955e into release-v3.12 Nov 21, 2024
41 of 42 checks passed
@mergify mergify bot deleted the mergify/bp/release-v3.12/pr-4970 branch November 21, 2024 12:25
@martin-helmich
Copy link

@Madhu-1 Thanks for merging this so quickly, and thanks for the backport to 3.12. 🫶 Is there an ETA for a new 3.12 release? This would allow us to let go of our own fork that we've been maintaining with this fix. 😅

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Nov 25, 2024

@martin-helmich am back from PTO, I will do it tomorrow EOD.

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Nov 25, 2024

https://github.com/ceph/ceph-csi/releases/tag/v3.12.3 is the release, the images will get pushed in next 2 hours

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Nov 25, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR is ready to be merged and it doesn't need second review (backports only)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants