-
Notifications
You must be signed in to change notification settings - Fork 559
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
rbd: fix encrypted PVC with metadata KMS cannot be deleted #5149
rbd: fix encrypted PVC with metadata KMS cannot be deleted #5149
Conversation
ec64661
to
7e59b8f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for debugging and fixing the issue !
Just a small nit.
7e59b8f
to
5839c6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
/test ci/centos/mini-e2e/k8s-1.32 |
@iPraveenParihar please provide details in a comment about the cause of failure as well, it helps to identify unknown bugs |
@Mergifyio rebase |
✅ Branch has been successfully rebased |
5839c6f
to
a8c53a1
Compare
@Mergifyio queue |
🛑 The pull request has been removed from the queue
|
/test ci/centos/k8s-e2e-external-storage/1.32 |
/test ci/centos/mini-e2e-helm/k8s-1.32 |
/test ci/centos/k8s-e2e-external-storage/1.30 |
/test ci/centos/mini-e2e/k8s-1.32 |
/test ci/centos/mini-e2e-helm/k8s-1.30 |
/test ci/centos/mini-e2e/k8s-1.30 |
/test ci/centos/k8s-e2e-external-storage/1.31 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/upgrade-tests-rbd |
/test ci/centos/mini-e2e-helm/k8s-1.31 |
/test ci/centos/mini-e2e/k8s-1.31 |
This pull request has been removed from the queue for the following reason: 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: |
/retest ci/centos/mini-e2e/k8s-1.32 |
The logs of the failure do not immediate show what the issue is, I think:
|
@Mergifyio requeue |
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically |
i think there is a omap leak. |
I don't think this issue related to this pr.
|
|
/retest ci/centos/mini-e2e/k8s-1.32 |
Signed-off-by: Zerotens <[email protected]>
a8c53a1
to
733ba3c
Compare
/test ci/centos/k8s-e2e-external-storage/1.30 |
/test ci/centos/mini-e2e-helm/k8s-1.30 |
/test ci/centos/mini-e2e/k8s-1.30 |
/test ci/centos/k8s-e2e-external-storage/1.31 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/k8s-e2e-external-storage/1.32 |
/test ci/centos/mini-e2e-helm/k8s-1.31 |
/test ci/centos/upgrade-tests-rbd |
/test ci/centos/mini-e2e-helm/k8s-1.32 |
/test ci/centos/mini-e2e/k8s-1.31 |
/test ci/centos/mini-e2e/k8s-1.32 |
Describe what this PR does
Creating an encrypted Persistent Volume Claim with reclaim policy Retain, the Persistent Volume cannot be deleted after the Namespace with the corresponding secret of the encrypted volume was deleted.
This fix moves the logic from the kms class initialization to the method where the encryption key is needed.
As the Delete Volume CSI request does not need or call the
FetchDEK
method, the volume get's deleted successfully.Is there anything that requires special attention
Do you have any questions?
Is the change backward compatible?
Are there concerns around backward compatibility?
Related issues
Fixes: #5148
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!)