Redesign for ClusterVersion controller into OperatorConfigMap controller #64
Conversation
|
@bernerhat: This pull request references Bugzilla bug 2251308, which is valid. No validations were run on this bugNo GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request. 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. |
There was a problem hiding this comment.
- pls follow
if err := func(); err != nil {}convention if returning directly based on error
While trying to uninstall the operator clusterVersion resource, which is managed by OCP
- I don't think so, no resource is being owned by clusterversion resource
- Actually bug fix should just be moving the DeepCopy line before
owning it and removing finalizer on CSIAddonNode
- unless we really don't want to watch for a delete event on configmap I don't think we need a redesign
@Madhu-1 pls correct me if my understand of existing code and the fix is wrong.
@leelavg are we trying to cleanup this things as part of uninstall? i mentioned about |
@Madhu-1 A PR, from my point of view, should be a complete contribution which this PR is and I don't see an issue with it. @bernerhat Can you act on my suggestion above? |
bernerhat
force-pushed
the
BZ-2251308
branch
2 times, most recently
from
ec056aa to
8f672fe
Compare
|
Please take care of deleting the SCC as well |
|
all my comments can be resolved and pls remove bug id from the title and commit, bugs are mentioned only during backports. you can mention the id in description or as a comment though. |
bernerhat
force-pushed
the
BZ-2251308
branch
2 times, most recently
from
adf705a to
8e23bbf
Compare
|
/hold |
bernerhat
force-pushed
the
BZ-2251308
branch
3 times, most recently
from
bd72263 to
2c79227
Compare
|
/unhold |
|
code is now tested. |
fixed ownership of resources and remove finalizer in deletion Signed-off-by: Amit Berner <[email protected]>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bernerhat, nb-ohad 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-merge-bot
bot
merged commit c99ef2f
into
red-hat-storage:main
Fixes the issue created where resources are leftover while trying to uninstall the odf client operator.
While trying to uninstall the operator clusterVersion resource, which is managed by OCP, remains in the cluster hence is not being deleted and causing the controller owned resources to remain as well.
Therefore a need arose to change the 'watched' resource from the clusterVersion into another resource who is more appropriate and in this case the ConfigMap of the operator was chosen.