-
Notifications
You must be signed in to change notification settings - Fork 283
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
Enable lb_class_only env var in tink stack kubevip #8493
Enable lb_class_only env var in tink stack kubevip #8493
Conversation
Signed-off-by: Rahul Ganesh <rahulgab@amazon.com>
Signed-off-by: Rahul Ganesh <rahulgab@amazon.com>
/cherrypick release-0.20 |
@rahulbabu95: once the present PR merges, I will cherry-pick it on top of release-0.20 in a new PR and assign it to you. In 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/test-infra repository. |
@@ -563,6 +563,10 @@ func (s *Installer) createValuesOverride(bundle releasev1alpha1.TinkerbellBundle | |||
"name": "prometheus_server", | |||
"value": ":2213", | |||
}, | |||
{ | |||
"name": "lb_class_only", |
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.
It would be useful and helpful for future us (and other EKSA contributors) to have a code comment here that explains the business logic for this. The why this is being set. You PR description alludes to what i think is the why but it doesn't quite hit it for me. This is the business logic from my perspective, feel free to disagree or add or subtract from it.
// The Tinkerbell stack need a load balancer to work properly.
// We bundle Kube-vip in, as the load balancer, when we deploy the stack.
// We don't want this load balancer to be used by any other workloads.
// It allows us greater confidence in successful lifecycle events for the Tinkerbell stack, amongst other things.
// Also, the user should be free from Tinkerbell stack constraints
// and free to deploy a load balancer of their choosing and not be coupled to ours.
// setting lb_class_only=true means that k8s services must explicitly populate
// the kube-vip loadBalancerClass with the kube-vip value for kube-vip to serve an IP.
Signed-off-by: Rahul Ganesh <rahulgab@amazon.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8493 +/- ##
==========================================
+ Coverage 73.49% 73.52% +0.03%
==========================================
Files 578 578
Lines 36504 36552 +48
==========================================
+ Hits 26829 26876 +47
+ Misses 7956 7955 -1
- Partials 1719 1721 +2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Rahul Ganesh <rahulgab@amazon.com>
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rahulbabu95 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 |
@rahulbabu95: new pull request created: #8559 In 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/test-infra repository. |
Description of changes:
Enable
lb_class_only
env var on kube-vip so that it only manages IP for services with LoadBalancerClass set tokube-vip.io/kube-vip-class
on the service. Without this env var, kube-vip is trying to manage IPs for any service requesting for a Load Balancer IP which isn't expected.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.