Allow loadbalancer service external IP #275
Conversation
|
push a dummy commit so that latest pr pipeline is run using |
|
@aslafy-z can you plz review it? |
d3adb5
force-pushed
the
feature/load-balancer-ip
branch
from
4216a56 to
71d4012
Compare
There was a problem hiding this comment.
I suggested a few minor changes, one of which looks like a necessary fix. Let me know if you need help implementing any of this, @YonatanRubin. Thank you for working on this!
application/templates/service.yaml
Outdated
| {{- with .Values.service.clusterIP }} | ||
| {{- if (or (eq .Values.service.type "ClusterIP") (empty .Values.service.type)) }} | ||
| type: ClusterIP | ||
| {{- with .Values.service.clusterIP }} |
There was a problem hiding this comment.
This setting is being ignored when declaring services of type NodePort or LoadBalancer. According to the spec, however, since a cluster IP is still allocated for those types of services (unless None is specified), it is still valid to try and choose the IP address manually.
This should be present regardless, as headless services need to specify this as None.
|
@YonatanRubin can you plz take care of the comments? |
|
Hi, sorry I will not be able to handle this in the next couple of weeks, I will work on this then. If you prefer you can close the PR and I will open a new one when I can. |
|
@YonatanRubin you can fix when you have time no problem |
Define the type property of our Service spec outside the branches of these conditional statements, as it should always honor the chart value.
Proxy the chart value for the Service's clusterIP field for all service types.
Remove empty conditional branch from the Service template.
Warn the chart's user of the deprecation of the Service loadBalacerIP field since Kubernetes v1.24.
d3adb5
force-pushed
the
feature/load-balancer-ip
branch
from
71d4012 to
5488bbc
Compare
Address whitespace concerns when rendering Service labels and annotations.
|
@d3adb5 validation successful` |
d3adb5
dismissed
their stale review
I have pushed commits to this pull request.
|
@rasheedamir @aslafy-z I pushed commits addressing the issues I raised in my last review. Since it would be presumptuous of me to give myself a green light, could either of you take a quick look and merge if possible? |
No description provided.