-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add support for allocateLoadBalancerNodePorts, ipFamilyPolicy and ipFamilies #2418
Conversation
1aecc1b
to
41f342d
Compare
Hi all. Is there any more development required for this to be accepted/merged? |
41f342d
to
b5d1dc8
Compare
Codecov Report
@@ Coverage Diff @@
## main #2418 +/- ##
==========================================
+ Coverage 53.39% 53.42% +0.02%
==========================================
Files 52 52
Lines 14706 14706
==========================================
+ Hits 7853 7857 +4
+ Misses 6591 6589 -2
+ Partials 262 260 -2
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Hi @centromere Sorry for the long wait! I'll take a look at this shortly. |
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.
@centromere thanks again for the PR, I left some comments.
{{- if hasKey .Values.controller.service "allocateLoadBalancerNodePorts" }} | ||
allocateLoadBalancerNodePorts: {{ .Values.controller.service.allocateLoadBalancerNodePorts }} | ||
{{- end }} |
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.
I think allocateLoadBalancerNodePorts
is still in beta, I would wait until it reaches GA before adding it
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.
I am using 1.22 and would like to use this feature. With the chart as-is, there is no way for me to achieve this.
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.
@centromere the good news is that it's now stable in 1.24
🎉 the not so good news is that I think we need another version check for the feature. I think since beta in 1.22
should be fine.
something like
{{- if and (semverCompare ">=1.22.0-0" .Capabilities.KubeVersion.Version) (.Values.controller.service.allocateLoadBalancerNodePorts) }}
7be97c0
to
51f7172
Compare
{{- if hasKey .Values.controller.service "allocateLoadBalancerNodePorts" }} | ||
allocateLoadBalancerNodePorts: {{ .Values.controller.service.allocateLoadBalancerNodePorts }} | ||
{{- end }} |
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.
@centromere the good news is that it's now stable in 1.24
🎉 the not so good news is that I think we need another version check for the feature. I think since beta in 1.22
should be fine.
something like
{{- if and (semverCompare ">=1.22.0-0" .Capabilities.KubeVersion.Version) (.Values.controller.service.allocateLoadBalancerNodePorts) }}
Co-authored-by: Luca Comellini <luca.com@gmail.com>
75234ce
to
f484f83
Compare
Proposed changes
This PR adds support for the
allocateLoadBalancerNodePorts
andipFamilyPolicy
fields of the Service resource.allocateLoadBalancerNodePorts
is only allowed when the Service is of typeLoadBalancer
.Checklist
Before creating a PR, run through this checklist and mark each as complete.