-
Notifications
You must be signed in to change notification settings - Fork 19
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
Clean up when deployment is being deleted #96
Clean up when deployment is being deleted #96
Conversation
ae8c60f
to
c8f8abb
Compare
I'll be writing a test for this in our integration suite, since it primarily deals with the situation when the deployment is being deleted, and the tests here do not have a deployment to check (in the tests here, it runs locally) |
Related integration test (draft): stolostron/governance-policy-framework#541 |
/hold Need to mess with cache settings so that the Deployment can actually be gotten. |
1bccd1f
to
87e0c3b
Compare
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.
Looks great! I left a couple of optional comments.
/hold
Previously, when the controller is removed, it could leave a mess behind in the form of ConfigurationPolicy objects with finalizers on them. That would also result in the CRD being stuck. Now, the controller will put a finalizer on its own deployment when at least one ConfigurationPolicy has a finalizer. Also, while the deployment is being deleted, the finalizers on ConfigurationPolicies will be removed immediately, in the same way they would be when the CRD was being deleted. Note: in this implementation, related objects might *never* be pruned when the controller is removed. Since the controller deployment is in a different namespace than the ConfigutaionPolicies it watches, the controller's cache cannot be limited to a single namespace the way it was before. Instead, this uses additional cache selectors to limit watches on ConfigurationPolicies and Deployments to the relevant namespaces. Refs: - https://issues.redhat.com/browse/ACM-2923 Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
87e0c3b
to
60d6b2b
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JustinKuli, mprahl 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 |
/unhold |
Previously, when the controller is removed, it could leave a mess behind in the form of ConfigurationPolicy objects with finalizers on them. That would also result in the CRD being stuck.
Now, the controller will put a finalizer on its own deployment when at least one ConfigurationPolicy has a finalizer. Also, while the deployment is being deleted, the finalizers on ConfigurationPolicies will be removed immediately, in the same way they would be when the CRD was being deleted.
Note: in this implementation, related objects will never be pruned when the controller is removed.
Since the controller deployment is in a different namespace than the ConfigutaionPolicies it watches, the controller's cache cannot be limited to a single namespace the way it was before. Instead, this uses additional cache selectors to limit watches on ConfigurationPolicies and Deployments to the relevant namespaces.
Refs:
Signed-off-by: Justin Kulikauskas jkulikau@redhat.com