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

Easier l4 health checks - without singleton #3

Open
wants to merge 5 commits into
base: awesome-firewall-healthchecks
Choose a base branch
from

Conversation

cezarygerard
Copy link
Owner

created on top of
kubernetes#1705

this is just to show if removing singleton pattern from healthchecks gives any benefits

- create a common singleton like struct fot l4 health checks
- new struct holds mutex for common resources
- delete shared healtcheck firewall rules safely
address review comments, improve tests, added mutex guard preventing re-initialization of L4 healhtchecks, renamed a few places, added comments

It is important that all controllers in a single test case use the same fakeHC, I have fixed this in
TestHealthCheckFirewallDeletionWithILB
TestHealthCheckFirewallDeletionWithNetLB
address review comments, improve tests, added mutex guard preventing re-initialization of L4 healhtchecks, renamed a few places, added comments

It is important that all controllers in a single test case use the same fakeHC, I have fixed this in
TestHealthCheckFirewallDeletionWithILB
TestHealthCheckFirewallDeletionWithNetLB
@cezarygerard cezarygerard force-pushed the awesome-firewall-healthchecks branch from 570dc6e to a10c879 Compare May 9, 2022 23:01
@cezarygerard cezarygerard force-pushed the awesome-firewall-healthchecks branch from eccf309 to abf0c05 Compare May 17, 2022 17:58
@cezarygerard cezarygerard force-pushed the awesome-firewall-healthchecks branch 4 times, most recently from 91c10b1 to f8353dc Compare May 31, 2022 09:54
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.

1 participant