-
Notifications
You must be signed in to change notification settings - Fork 369
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
feat: load balancer class support #1840
Conversation
2c109c3
to
9d0776f
Compare
hi @wondersd , thanks for raising a PR for this feature. |
@arkodg Yes that is correct. Assuming the user modifies it via a mutator on creation or otherwise deletes/recreates the service, the only presented option here would be to allow syncing what EG expects to reality. It could be possible to update EG to destroy the |
Signed-off-by: Don Wonders <[email protected]>
9d0776f
to
633194a
Compare
sure, once this PR is merged, can you raise a follow up issue to handle the mutate case which will probably need a |
Codecov Report
@@ Coverage Diff @@
## main #1840 +/- ##
==========================================
+ Coverage 65.16% 65.19% +0.02%
==========================================
Files 86 86
Lines 12469 12472 +3
==========================================
+ Hits 8126 8131 +5
+ Misses 3825 3823 -2
Partials 518 518
|
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 thanks !
ptal @qicz
load balancer class support Signed-off-by: Don Wonders <[email protected]>
What type of PR is this?
fix: fixes stuck controller when loadBalancerClass mutators exist and use of loadBalancerClass when mutators do not exist
What this PR does / why we need it:
updates EP CR to include ability to specify
Service.spec.loadBalancerClass
Which issue(s) this PR fixes:
Fixes #1793
Think this includes everything. My go is not well practiced so may be off. Still working on setting up for e2e test