StorageCluster: add delete functions for remaining resources as part of uninstall procedure #782
Conversation
|
cc @iamniting @jarrpa @obnoxxx Please review. This fixes the problem of cephObjectStoreUsers and cephBlockPools not being deleted as during uninstall. |
| if errors.IsNotFound(err) { | ||
| reqLogger.Info("Uninstall: CephFilesystem not found", "CephFilesystem Name", cephFilesystem.Name) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Are we sure we want to return from here itself? Don't we want to check the further ones? It is also possible if the first one gets deleted we won't check for further ones as of now.
| if errors.IsNotFound(err) { | ||
| reqLogger.Info("Uninstall: CephFilesystem is deleted", "CephFilesystem Name", cephFilesystem.Name) | ||
| return nil | ||
| } |
| } | ||
|
|
||
| // deleteCephBlockPools deletes the CephBlockPools owned by the StorageCluster | ||
| func (r *ReconcileStorageCluster) deleteCephBlockPools(sc *ocsv1.StorageCluster, reqLogger logr.Logger) error { |
| } | ||
|
|
||
| // deleteCephObjectStoreUsers deletes the CephObjectStoreUsers owned by the StorageCluster | ||
| func (r *ReconcileStorageCluster) deleteCephObjectStoreUsers(sc *ocsv1.StorageCluster, reqLogger logr.Logger) error { |
|
|
||
| err = r.client.Delete(context.TODO(), sc) | ||
| if err != nil { | ||
| reqLogger.Error(err, fmt.Sprintf("Uninstall: Ignoring error deleting the SnapshotClass %s", existing.Name)) |
There was a problem hiding this comment.
Just keeping it same as the storageclass behavior. Considering the failure of this deletion as non fatal.
| } | ||
|
|
||
| // deleteCephObjectStores deletes the CephObjectStores owned by the StorageCluster | ||
| func (r *ReconcileStorageCluster) deleteCephObjectStores(sc *ocsv1.StorageCluster, reqLogger logr.Logger) error { |
jarrpa
left a comment
jarrpa
left a comment
There was a problem hiding this comment.
The reorder commit should be integrated into the other commits, that is make sure they are in the correct order when you first add them.
| return err | ||
| } | ||
|
|
||
| err = r.deleteCephObjectStores(sc, reqLogger) |
There was a problem hiding this comment.
Are you intentionally removing the call to this function?
There was a problem hiding this comment.
Sorry for the distraction @jarrpa . I am debugging something with this PR. It is not ready yet. I will mark it WIP.
raghavendra-talur
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
raghavendra-talur
force-pushed
the
rtalur-more-uninstall-funcs
branch
2 times, most recently
from
cd76ed8 to
119b3d3
Compare
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
raghavendra-talur
force-pushed
the
rtalur-more-uninstall-funcs
branch
from
119b3d3 to
40dbef6
Compare
raghavendra-talur
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jarrpa The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
All of these functions share a lot of code... I'd like to see a generic function that operates on |
|
/cherrypick release-4.6 |
|
@raghavendra-talur: new pull request created: #793 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
6 participants
In summary, this PR adds delete functions for the resources which were missed in the previous iteration of uninstall code.
It looks like the delete functions are counterpart to the ensure functions already found in the reconciler.go. In a future PR we should attempt to refactor the code such that the developer is prompted to add the delete function when they add the ensure function.