redpanda: convert service.loadbalancer.yaml to go
#1347
Conversation
| for name, listener := range values.Listeners.Admin.External { | ||
| if !ptr.Deref(listener.Enabled, values.External.Enabled) { | ||
| continue | ||
| } | ||
|
|
||
| fallbackPorts := append(listener.AdvertisedPorts, values.Listeners.Admin.Port) | ||
|
|
||
| ports = append(ports, corev1.ServicePort{ | ||
| Name: fmt.Sprintf("admin-%s", name), | ||
| Protocol: corev1.ProtocolTCP, | ||
| // TODO this diverges from all the other cases that use | ||
| // listener.Port. Is this a bug in the original template? | ||
| TargetPort: intstr.FromInt32(values.Listeners.Admin.Port), | ||
| Port: ptr.Deref(listener.NodePort, fallbackPorts[0]), |
There was a problem hiding this comment.
I wonder how values.Listeners.Admin.External could be deterministic with ranging and always returning the same port.
Would there be a case where running helm template in a loop without changing values input would generate different loadbalancer manifest?
There was a problem hiding this comment.
If the value is a map and the keys are of basic type with a defined order, the elements will be visited in sorted key order.
But I should be ranging over the sorted keys to make sure the go implementation is working as expected and lining up with the helm version. Will change!
There was a problem hiding this comment.
Ah, I'm actually going to punt on this change for now. The nodeport service works the same way and templates' ranges are actually deterministic.
This commit implements the `tpl` or `Tpl` function for go and adds a few tests thereof. Gotohelm's implementation of `tpl` is NOT fully featured in go nor within our test harness.
This commit converts `service.loadbalancer.yaml` to go code. It includes a small fix for a typo that seems to have gone unnoticed. In the case of using a loadbalancer service, `listeners.admin.external[*].port` was being ignored in favor of `listeners.admin.external.port`, which was divergent from all other port mappings. Fixes #1215
chrisseto
force-pushed
the
chris/p/convert-loadbalancer
branch
from
febd052
to
57dbee4
Compare
Fixes #1215