-
Notifications
You must be signed in to change notification settings - Fork 71
🐛 Refactor ClusterExtensionRevision controller to rely on kubernetes for resource cleanup #2451
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
🐛 Refactor ClusterExtensionRevision controller to rely on kubernetes for resource cleanup #2451
Conversation
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
rashmigottipati
left a comment
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.
/lgtm
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2451 +/- ##
==========================================
- Coverage 73.01% 69.42% -3.59%
==========================================
Files 101 101
Lines 7730 7716 -14
==========================================
- Hits 5644 5357 -287
- Misses 1635 1925 +290
+ Partials 451 434 -17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
/approve |
camilamacedo86
left a comment
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.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, rashmigottipati, tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/override codecov/project |
|
@camilamacedo86: Overrode contexts on behalf of camilamacedo86: codecov/project DetailsIn 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-sigs/prow repository. |
0c36df6
into
operator-framework:main
Description
The ClusterExtensionRevision controller uses the Boxcutter library to teardown the revision and delete the revision objects from the cluster. This process uses the RevisionEngine's client. The recent move to use the scoped client for revision resource management puts a dependency between the teardown process and the service account and its rbac. When doing a "GitOps style" deletion (i.e. uninstalling a ClusterExtension by deleting the extension, service account, and rbac objects in unison), the service account backing the teardown process is not guaranteed to be there and this results in bundle resources persisting on the cluster.
This PR updates the ClusterExtensionRevision controller to rely on Kubernetes for the deletion and clean up of the revision resources. Since the controller already sets up the appropriate ownerrefs, its sufficient to just remove the call to the RevisionEngine Teardown method.
Reviewer Checklist