-
Notifications
You must be signed in to change notification settings - Fork 976
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
Make Deleting PVs Idempotent #1935
Conversation
Hi @sherifabdlnaby, Thank you for opening this PR, however, it does not fix the issue you are referring to. This fix extends error handling returned by the method |
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 spotting this. @arybolovlev is right, you are checking the error returned by the MainClientSet getter, not the actual Delete operation.
Instead, you should be doing this exact same check with the error on line 328.
Fat-fingered the Approve button. I intended to request changes as per observations of @arybolovlev
@arybolovlev Thanks for spotting my silly mistake, I moved the error check at the delete operation which should work as the PR Intended. @alexsomesan PR Is Updated & Rebased on |
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.
Hi @sherifabdlnaby,
All looks good, just one more query from my end.
Could you please add a changelog file so we can then auto-generate change log for the upcoming release?
It will be a simple text file in .changelog
folder. Like this: .changelog/1935.txt
. With the following content:
```release-note:enhancement
`resource/kubernetes_persistent_volume`: add additional validation on the delete operation to make it idempotent
```
```release-note:enhancement
`resource/kubernetes_persistent_volume_v1`: add additional validation on the delete operation to make it idempotent
```
Thank you! 👍🏻
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.
Amazing! 👍🏻 Thank you and congratulations on the first-time contribution! 🎉
Your changes will be available in the upcoming release. Please keep an eye on the release notes.
Have a great day!
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
Add validation on the delete operation for PVs to make it idempotent. (extension of #1914 )
Acceptance tests
Output from acceptance testing:
Release Note
Release note for CHANGELOG:
References
Community Note