Make AllocateLoadBalancerNodePorts Configurable
Would it be possible to make AllocateLoadBalancerNodePorts configurable? While disabling it is beneficial for large topologies, enabling it can simplify deployment for small to medium-sized topologies by removing the need to allocate external IP addresses for nodes (since NodePorts use the range 30000-32767).
Currently, AllocateLoadBalancerNodePorts is hardcoded to false (kne/topo/node/node.go), as shown below:
// CreateService creates services for the node based on the underlying proto.
func (n *Impl) CreateService(ctx context.Context) error {
// ...
s := &corev1.Service{
// ...
Spec: corev1.ServiceSpec{
Ports: servicePorts,
Selector: map[string]string{
"app": n.Name(),
},
Type: "LoadBalancer",
// Do not allocate a NodePort for this LoadBalancer. MetalLB
// or the equivalent load balancer should handle exposing this service.
// Large topologies may try to allocate more NodePorts than are
// supported in default clusters.
// https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation
AllocateLoadBalancerNodePorts: pointer.Bool(false),
},
Could this line be made optional/configurable, with the default remaining true (as per Kubernetes defaults)?
A more ambitious change would be to switch the service type to NodePort in this case, eliminating the dependency on MetalLB. However, I understand that this would be a more involved update.
The general problem with this suggestion is half the node implementations are outsourced to controllers who themselves create the services. There is little to no control over how they achieve that, unfortunately. I've taken to using a mutating webhook to consistently override some aspects of the service (for example, setting the ExternalTrafficPolicy to Local) -- perhaps a similar approach could be used to override the entire Service.Spec.
Right, some controllers do create a service (arista-ceoslab-operator), but others don't (srl-controller).
Arista doesn't set AllocateLoadBalancerNodePorts, so we get the default behavior by default, which I would love KNE to match unless explicitly configured otherwise.