Skip to content
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

Cleanup receiver group resources #167

Merged
merged 34 commits into from
Dec 6, 2021
Merged

Conversation

siliconbrain
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
API breaks? no
Deprecations? no
Related tickets #146
License Apache 2.0

What's in this PR?

Use the NativeReconciler from operator-tools to handle the removal of no longer needed resources created for receiver groups.
Also, refactored some of the operator's code to make it simpler.

Why?

See #146.

getName already checks the optional suffix
This is transparent because:
* getName does not use receiverGroup from receiverInstance, and
* receiverExt is embedded in receiverInstance
instead of calling r.GetObjectMeta(r.getName(...)) all over the place, extract it into a method
the hashring config map does not depend on a specific receiver group
commonService was only called on receiverInstance's with receiverGroup set to nil, so there was no dependency on the receiverGroup
Copy link
Contributor

@pepov pepov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Maybe some envtest test cases could make this even better, but LGTM anyways!

@tarokkk tarokkk merged commit 7b87b4f into master Dec 6, 2021
@tarokkk tarokkk deleted the cleanup-receivergroup-resources branch December 6, 2021 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants