fix(cvr delete, finalizer): remove finalizers to ensure successful deletion #955
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AmitKumarDas
added
component: CStorVolMgmt
pr/upgrade-changes-pending
…letion This commit removes the finalizers from a cstor volume replica object. This ensures removal of cstor volume replica object from kubernetes. In addition, it will try to delete the underlying volume if cvr controller gets the delete event. With the removal of finalizer there is no more retry mechanism to actually delete the underlying volume if there were issues encountered during earlier deletes. Why was finalizer put in first place? CstorVolumeReplica aliased CVR has finalizers that prohibits deletion unless the volume gets deleted. This is a check to ensure proper cleanup even if delete events are missed & / or underlying volume was not deleted in earlier attempts. Observations: There were observations that point to the fact that when the namespace (that contains these volumes) as well when the operator (that contains the CRD schema definitions) are deleted, CVR finalizers pose a significant problem to the overall deletion & later re-creation procedure. In fact the PODs that are responsible for actual volume deletion & confirming the delete status is no more available when its namespace or the openebs operator get deleted. Future Work: A better design needs to be implemented that ensures volume cleanup & hence space reclamation. fixes openebs/openebs#2374 fixes openebs/openebs#2349 Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
AmitKumarDas
force-pushed
the
cvr-finalizer-deletion
branch
from
7603668
to
3f1aed4
Compare
Codecov Report
@@ Coverage Diff @@
## master #955 +/- ##
==========================================
+ Coverage 45.03% 45.05% +0.01%
==========================================
Files 168 168
Lines 10622 10622
==========================================
+ Hits 4784 4786 +2
+ Misses 5527 5526 -1
+ Partials 311 310 -1
Continue to review full report at Codecov.
|
vishnuitta
approved these changes
Feb 28, 2019
ashishranjan738
added a commit
to ashishranjan738/maya
that referenced
this pull request
Mar 4, 2019
Signed-off-by: Ashish Ranjan <ashishranjan738@gmail.com> This PR addresses comments of PR openebs#956.
AmitKumarDas
pushed a commit
to AmitKumarDas/maya
that referenced
this pull request
Mar 12, 2019
…letion (openebs#955) This commit removes the finalizers from a cstor volume replica object. This ensures removal of cstor volume replica object from kubernetes. In addition, it will try to delete the underlying volume if cvr controller gets the delete event. With the removal of finalizer there is no more retry mechanism to actually delete the underlying volume if there were issues encountered during earlier deletes. Why was finalizer put in first place? CstorVolumeReplica aliased CVR has finalizers that prohibits deletion unless the volume gets deleted. This is a check to ensure proper cleanup even if delete events are missed & / or underlying volume was not deleted in earlier attempts. Observations: There were observations that point to the fact that when the namespace (that contains these volumes) as well when the operator (that contains the CRD schema definitions) are deleted, CVR finalizers pose a significant problem to the overall deletion & later re-creation procedure. In fact the PODs that are responsible for actual volume deletion & confirming the delete status is no more available when its namespace or the openebs operator get deleted. Future Work: A better design needs to be implemented that ensures volume cleanup & hence space reclamation. fixes openebs/openebs#2374 fixes openebs/openebs#2349 Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
vishnuitta
pushed a commit
that referenced
this pull request
Mar 12, 2019
…letion (#955) This commit removes the finalizers from a cstor volume replica object. This ensures removal of cstor volume replica object from kubernetes. In addition, it will try to delete the underlying volume if cvr controller gets the delete event. With the removal of finalizer there is no more retry mechanism to actually delete the underlying volume if there were issues encountered during earlier deletes. Why was finalizer put in first place? CstorVolumeReplica aliased CVR has finalizers that prohibits deletion unless the volume gets deleted. This is a check to ensure proper cleanup even if delete events are missed & / or underlying volume was not deleted in earlier attempts. Observations: There were observations that point to the fact that when the namespace (that contains these volumes) as well when the operator (that contains the CRD schema definitions) are deleted, CVR finalizers pose a significant problem to the overall deletion & later re-creation procedure. In fact the PODs that are responsible for actual volume deletion & confirming the delete status is no more available when its namespace or the openebs operator get deleted. Future Work: A better design needs to be implemented that ensures volume cleanup & hence space reclamation. fixes openebs/openebs#2374 fixes openebs/openebs#2349 Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
prateekpandey14
pushed a commit
to prateekpandey14/maya
that referenced
this pull request
Apr 3, 2019
…letion (openebs#955) This commit removes the finalizers from a cstor volume replica object. This ensures removal of cstor volume replica object from kubernetes. In addition, it will try to delete the underlying volume if cvr controller gets the delete event. With the removal of finalizer there is no more retry mechanism to actually delete the underlying volume if there were issues encountered during earlier deletes. Why was finalizer put in first place? CstorVolumeReplica aliased CVR has finalizers that prohibits deletion unless the volume gets deleted. This is a check to ensure proper cleanup even if delete events are missed & / or underlying volume was not deleted in earlier attempts. Observations: There were observations that point to the fact that when the namespace (that contains these volumes) as well when the operator (that contains the CRD schema definitions) are deleted, CVR finalizers pose a significant problem to the overall deletion & later re-creation procedure. In fact the PODs that are responsible for actual volume deletion & confirming the delete status is no more available when its namespace or the openebs operator get deleted. Future Work: A better design needs to be implemented that ensures volume cleanup & hence space reclamation. fixes openebs/openebs#2374 fixes openebs/openebs#2349 Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
9 tasks
vishnuitta
pushed a commit
that referenced
this pull request
Apr 4, 2019
…letion (#955) This commit removes the finalizers from a cstor volume replica object. This ensures removal of cstor volume replica object from kubernetes. In addition, it will try to delete the underlying volume if cvr controller gets the delete event. With the removal of finalizer there is no more retry mechanism to actually delete the underlying volume if there were issues encountered during earlier deletes. Why was finalizer put in first place? CstorVolumeReplica aliased CVR has finalizers that prohibits deletion unless the volume gets deleted. This is a check to ensure proper cleanup even if delete events are missed & / or underlying volume was not deleted in earlier attempts. Observations: There were observations that point to the fact that when the namespace (that contains these volumes) as well when the operator (that contains the CRD schema definitions) are deleted, CVR finalizers pose a significant problem to the overall deletion & later re-creation procedure. In fact the PODs that are responsible for actual volume deletion & confirming the delete status is no more available when its namespace or the openebs operator get deleted. Future Work: A better design needs to be implemented that ensures volume cleanup & hence space reclamation. fixes openebs/openebs#2374 fixes openebs/openebs#2349 Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
AmitKumarDas
pushed a commit
to AmitKumarDas/maya
that referenced
this pull request
Jun 7, 2019
This commit reverts a previous commit i.e. openebs#955 There were a few observations (that were expected) due to above commit. CVR used to have finalizers to execute cleanup operations due to delete event of corresponding volume. This is a standard practice in kubernetes community to handle cleanups via reconciliation process. However, we had to remove finalizers as seen in the commit 955 due to various reasons. These were: [1] Inability to delete a openebs system namespace and expect entire openebs to get uninstalled [2] Deletion of cstor volume did not delete CVRs due to the presence of finalizers. This was seen most of the times across various users and platforms. This was not easily reproducible in development environments. With the current release i.e. 0.9.0 some users have started to face issues like space not getting reclaimed even after cstor volumes get deleted successfully. On further analysis, it was found that user had tried following operation: `kubectl delete pvc --all -n some-namespace` which led to deletion of pvc, pv, cv, cvr resources from k8s cluster but corresponding events especially cvr based events were received much later at the controller code. This resulted in `cvr resource not found` and hence these resources were not cleaned up successfully leading to space issues. With this fix we expect original symptoms to recur. However, we plan to provide another fix after the automated test suite is able to discover these issues at will. This next fix will deal only with symptom '2' NOTE: Ability to un-install openebs by deleting openebs system namespace needs to be handled by a higher order logic/process. This is now considered as out of scope w.r.t cvr controller. Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
9 tasks
AmitKumarDas
pushed a commit
to AmitKumarDas/maya
that referenced
this pull request
Jun 7, 2019
This commit reverts a previous commit i.e. openebs#955 There were a few observations (that were expected) due to above commit. CVR used to have finalizers to execute cleanup operations due to delete event of corresponding volume. This is a standard practice in kubernetes community to handle cleanups via reconciliation process. However, we had to remove finalizers as seen in the commit 955 due to various reasons. These were: [1] Inability to delete a openebs system namespace and expect entire openebs to get uninstalled [2] Deletion of cstor volume did not delete CVRs due to the presence of finalizers. This was seen most of the times across various users and platforms. This was not easily reproducible in development environments. With the current release i.e. 0.9.0 some users have started to face issues like space not getting reclaimed even after cstor volumes get deleted successfully. On further analysis, it was found that user had tried following operation: `kubectl delete pvc --all -n some-namespace` which led to deletion of pvc, pv, cv, cvr resources from k8s cluster but corresponding events especially cvr based events were received much later at the controller code. This resulted in `cvr resource not found` and hence these resources were not cleaned up successfully leading to space issues. With this fix we expect original symptoms to recur. However, we plan to provide another fix after the automated test suite is able to discover these issues at will. This next fix will deal only with symptom '2' NOTE: Ability to un-install openebs by deleting openebs system namespace needs to be handled by a higher order logic/process. This is now considered as out of scope w.r.t cvr controller. Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
AmitKumarDas
pushed a commit
to AmitKumarDas/maya
that referenced
this pull request
Jun 7, 2019
This commit reverts a previous commit i.e. openebs#955 There were a few observations (that were expected) due to above commit. CVR used to have finalizers to execute cleanup operations due to delete event of corresponding volume. This is a standard practice in kubernetes community to handle cleanups via reconciliation process. However, we had to remove finalizers as seen in the commit 955 due to various reasons. These were: [1] Inability to delete a openebs system namespace and expect entire openebs to get uninstalled [2] Deletion of cstor volume did not delete CVRs due to the presence of finalizers. This was seen most of the times across various users and platforms. This was not easily reproducible in development environments. With the current release i.e. 0.9.0 some users have started to face issues like space not getting reclaimed even after cstor volumes get deleted successfully. On further analysis, it was found that user had tried following operation: `kubectl delete pvc --all -n some-namespace` which led to deletion of pvc, pv, cv, cvr resources from k8s cluster but corresponding events especially cvr based events were received much later at the controller code. This resulted in `cvr resource not found` and hence these resources were not cleaned up successfully leading to space issues. With this fix we expect original symptoms to recur. However, we plan to provide another fix after the automated test suite is able to discover these issues at will. This next fix will deal only with symptom '2' NOTE: Ability to un-install openebs by deleting openebs system namespace needs to be handled by a higher order logic/process. This is now considered as out of scope w.r.t cvr controller. Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
vishnuitta
pushed a commit
that referenced
this pull request
Jun 7, 2019
) This commit reverts a previous commit i.e. #955 There were a few observations (that were expected) due to above commit. CVR used to have finalizers to execute cleanup operations due to delete event of corresponding volume. This is a standard practice in kubernetes community to handle cleanups via reconciliation process. However, we had to remove finalizers as seen in the commit 955 due to various reasons. These were: [1] Inability to delete a openebs system namespace and expect entire openebs to get uninstalled [2] Deletion of cstor volume did not delete CVRs due to the presence of finalizers. This was seen most of the times across various users and platforms. This was not easily reproducible in development environments. With the current release i.e. 0.9.0 some users have started to face issues like space not getting reclaimed even after cstor volumes get deleted successfully. On further analysis, it was found that user had tried following operation: `kubectl delete pvc --all -n some-namespace` which led to deletion of pvc, pv, cv, cvr resources from k8s cluster but corresponding events especially cvr based events were received much later at the controller code. This resulted in `cvr resource not found` and hence these resources were not cleaned up successfully leading to space issues. With this fix we expect original symptoms to recur. However, we plan to provide another fix after the automated test suite is able to discover these issues at will. This next fix will deal only with symptom '2' NOTE: Ability to un-install openebs by deleting openebs system namespace needs to be handled by a higher order logic/process. This is now considered as out of scope w.r.t cvr controller. Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
AmitKumarDas
pushed a commit
to AmitKumarDas/maya
that referenced
this pull request
Jun 7, 2019
…enebs#1256) This commit reverts a previous commit i.e. openebs#955 There were a few observations (that were expected) due to above commit. CVR used to have finalizers to execute cleanup operations due to delete event of corresponding volume. This is a standard practice in kubernetes community to handle cleanups via reconciliation process. However, we had to remove finalizers as seen in the commit 955 due to various reasons. These were: [1] Inability to delete a openebs system namespace and expect entire openebs to get uninstalled [2] Deletion of cstor volume did not delete CVRs due to the presence of finalizers. This was seen most of the times across various users and platforms. This was not easily reproducible in development environments. With the current release i.e. 0.9.0 some users have started to face issues like space not getting reclaimed even after cstor volumes get deleted successfully. On further analysis, it was found that user had tried following operation: `kubectl delete pvc --all -n some-namespace` which led to deletion of pvc, pv, cv, cvr resources from k8s cluster but corresponding events especially cvr based events were received much later at the controller code. This resulted in `cvr resource not found` and hence these resources were not cleaned up successfully leading to space issues. With this fix we expect original symptoms to recur. However, we plan to provide another fix after the automated test suite is able to discover these issues at will. This next fix will deal only with symptom '2' NOTE: Ability to un-install openebs by deleting openebs system namespace needs to be handled by a higher order logic/process. This is now considered as out of scope w.r.t cvr controller. Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
AmitKumarDas
pushed a commit
that referenced
this pull request
Jun 7, 2019
) (#1260) This commit reverts a previous commit i.e. #955 There were a few observations (that were expected) due to above commit. CVR used to have finalizers to execute cleanup operations due to delete event of corresponding volume. This is a standard practice in kubernetes community to handle cleanups via reconciliation process. However, we had to remove finalizers as seen in the commit 955 due to various reasons. These were: [1] Inability to delete a openebs system namespace and expect entire openebs to get uninstalled [2] Deletion of cstor volume did not delete CVRs due to the presence of finalizers. This was seen most of the times across various users and platforms. This was not easily reproducible in development environments. With the current release i.e. 0.9.0 some users have started to face issues like space not getting reclaimed even after cstor volumes get deleted successfully. On further analysis, it was found that user had tried following operation: `kubectl delete pvc --all -n some-namespace` which led to deletion of pvc, pv, cv, cvr resources from k8s cluster but corresponding events especially cvr based events were received much later at the controller code. This resulted in `cvr resource not found` and hence these resources were not cleaned up successfully leading to space issues. With this fix we expect original symptoms to recur. However, we plan to provide another fix after the automated test suite is able to discover these issues at will. This next fix will deal only with symptom '2' NOTE: Ability to un-install openebs by deleting openebs system namespace needs to be handled by a higher order logic/process. This is now considered as out of scope w.r.t cvr controller. Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why this PR?
This commit removes the finalizers from a cstor volume replica object. This ensures removal of cstor volume replica object from kubernetes. In addition, it will try to delete the underlying volume if cvr controller gets the delete event. With the removal of finalizer there is no more retry mechanism to
actually delete the underlying volume if there were issues encountered during earlier deletes.
Why was finalizer put in first place?
CstorVolumeReplica aliased CVR has finalizers that prohibits deletion unless the volume gets deleted. This is a check to ensure proper cleanup even if delete events are missed & / or underlying volume was not deleted in earlier attempts.
Observations:
There were observations that point to the fact that when the namespace (that contains these volumes) as well when the operator (that contains the CRD schema definitions) are deleted, CVR finalizers pose a significant problem to the overall deletion & later re-creation procedure. In fact the PODs that are responsible for actual volume deletion & confirming the delete status is no more available when its namespace or the openebs operator get deleted.
Future Work / Special notes for your reviewer
A better design needs to be implemented that ensures volume cleanup & hence space reclamation.
fixes openebs/openebs#2374
fixes openebs/openebs#2349
Signed-off-by: AmitKumarDas amit.das@mayadata.io
Checklist:
documentationtagbreaking-changestagrequires-upgradetag