open-cluster-management-io / addon-framework

addon apis
Apache License 2.0
23 stars 40 forks source link

:bug: Set a finalizer on ClusterManagementAddOn objects #260

Closed yiraeChristineKim closed 4 months ago

yiraeChristineKim commented 5 months ago

When GRC is disabled globally within ACM, all components, including the Policy Addon controller get deleted. This means that any finalizer cleanup the addon controller would normally do, doesn't have a chance to finish.

The proposal is for the Policy Addon controller to add finalizers to the ClusterManagementAddOn objects associated with the policy addons when one or more managed cluster has the addon installed. The finalizer is removed when the policy addons are successfully cleaned up on all managed clusters.

In addition, when the ACM installer (or something else) deletes the ClusterManagementAddOn, the Policy Addon controller should start uninstalling the associated addon. The finalizer removal would work as described above.

Ref: https://issues.redhat.com/browse/ACM-8473

Summary

Related issue(s)

Fixes #

openshift-ci[bot] commented 5 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yiraeChristineKim Once this PR has been reviewed and has the lgtm label, please assign qiujian16 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/open-cluster-management-io/addon-framework/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
zhujian7 commented 5 months ago

@qiujian16 what do you think about this change?

zhujian7 commented 5 months ago

I am not sure if this is needed, because I think all managedclusteraddon CRs have an ownerReferences to the clustermanagementaddon, when the clustermanagementaddon is deleted, all the mangedclusteraddon will be deleted.

yiraeChristineKim commented 5 months ago

I am not sure if this is needed, because I think all managedclusteraddon CRs have an ownerReferences to the clustermanagementaddon, when the clustermanagementaddon is deleted, all the mangedclusteraddon will be deleted.

You're right. We are having a conversation with the hub installer team. https://github.com/open-cluster-management-io/addon-framework/pull/260#issuecomment-2050855825. After the conversation is over, then I will open this PR

mprahl commented 5 months ago

I am not sure if this is needed, because I think all managedclusteraddon CRs have an ownerReferences to the clustermanagementaddon, when the clustermanagementaddon is deleted, all the mangedclusteraddon will be deleted.

@zhujian7 so the issue is that the hub installer deletes the ClusterManagementAddOn when uninstalling the GRC components and then deletes the governance-policy-addon-controller without waiting for the addons to be uninstalled. This is fine unless there is a predelete hook involved which requires coordination from the addon framework.

The proposal here is to have a finalizer on the ClusterManagementAddOn until all uninstalls have completed so the hub installer can watch this one object and then once it is deleted, continue with removing the governance-policy-addon-controller.