Allow pools dedicated to k8s namespaces in the controller

[uablrek]

This PR partially implements #383. Only loadBalancer address allocation is considered, i.e only "controller" code is affected.

The configuration now allows a new "namespace" parameter for pools;

  config: |
    address-pools:
    - name: app001
      namespace: app001
      protocol: layer2
      addresses:
      - 1000::1:0/124
      - 10.0.1.0/28
    - name: default
      protocol: layer2
      addresses:
      - 1000::/124
      - 10.0.0.0/28

In the example services in the "app001" namespace will only get addresses from the "app001" (even if free addresses are available in the "default" pool).

Services in namespaces without any dedicated pool will get addresses from the "default" pool (never from the "app001" pool).

The Allocator has a new "nspools" field;

type Allocator struct {
	pools map[string]*config.Pool

	allocated       map[string]*alloc          // svc -> alloc
	sharingKeyForIP map[string]*key            // ip.String() -> assigned sharing key
	portsInUse      map[string]map[Port]string // ip.String() -> Port -> svc
	servicesOnIP    map[string]map[string]bool // ip.String() -> svc -> allocated?
	poolIPsInUse    map[string]map[string]int  // poolName -> ip.String() -> number of users
	poolServices    map[string]int             // poolName -> #services
	nspools         map[string][]string        // namespace -> poolNames
}

Pools with the "namespace" attribute set are added to this map. The lenght of the map is used as "feature-gate" in the code;

    if len(nspools) > 0 {
        // Do namespace related stuff...
    }

This ensures that existing setups not using this new feature are not affected.

The nspools maps to a slice of pool-names but this list is currently not used, only the existence of pools dedicated to a namespace is checked.

[uablrek]

@danderson I have deliberately avoided to write unit-tests until a preliminary go/no-go.

[uablrek]

Test have been performed with metallb config; metallb-config.yaml.txt, and service manifests;
app.yaml.txt.

The services gets addresses as;

> kubectl get svc -A
NAMESPACE   NAME            TYPE           CLUSTER-IP        EXTERNAL-IP   PORT(S)          AGE
app001      app001-ipv4     LoadBalancer   12.0.75.73        10.0.1.0      5001:32578/TCP   20s
app001      app001-ipv6     LoadBalancer   fd00:4000::e702   1000::1:0     5001:32767/TCP   20s
app002      app002-ipv4     LoadBalancer   12.0.139.14       10.0.2.1      5001:31804/TCP   20s
app002      app002-ipv4-2   LoadBalancer   12.0.112.238      <pending>     5001:30866/TCP   20s
app002      app002-ipv6     LoadBalancer   fd00:4000::c776   1000::2:0     5001:30301/TCP   20s
app002      app002-ipv6-2   LoadBalancer   fd00:4000::4a59   1000::2:1     5001:32448/TCP   20s
app003      app003-ipv4     LoadBalancer   12.0.176.141      10.0.3.0      5001:31293/TCP   20s
app003      app003-ipv4-2   LoadBalancer   12.0.119.74       10.0.3.1      5001:30326/TCP   20s
app003      app003-ipv6     LoadBalancer   fd00:4000::f4a2   1000::3:0     5001:30778/TCP   20s
app003      app003-ipv6-2   LoadBalancer   fd00:4000::c471   1000::3:1     5001:30567/TCP   20s
default     mconnect-ipv4   LoadBalancer   12.0.109.250      10.0.0.0      5001:32416/TCP   26s
default     mconnect-ipv6   LoadBalancer   fd00:4000::1f47   1000::        5001:31274/TCP   26s

[uablrek]

Re-start of the controller is also tested (should trig a re-read of the config) but not updates of the metallb config.

[danderson]

Looks reasonable to me. Two questions:

• Do you personally need this for your deployments? I worry about implementing more things "just because", because adding more complexity leads to more bugs. I know one other person requested this in that bug, but I'm wondering how "popular" this request is.
• Can you add "update documentation" to the implementation plan? This needs to be mentioned in the sample config, and probably a section in either "Configuration" or "Usage" on the website.

Thanks for making improvements to MetalLB!

[uablrek]

Good question. I get requests for features internally and got some more info; in this case the namespace-pools were just a step towards the goal;

Use-case

Applications are developed and deployed individually, but each with it's own namespace. But all applications must update the metallb configmap with their addresses. This PR would remove the need for annotations in all external services, but the central update is still a problem.

A solution that is considered is to have a central function that reads metallb configs from applications in some way (probably some API-server object, configmap or other) and updates the "real" metallb config map. Namespace bound pools would remove the need for some annotation convention.

I will discuss this a bit further internally...

[nealf]

Thanks to both of you for a great project and the potential enhancement! I'd like to second the benefit of a feature like this.

For our primary use case, we want to reserve our public IPv4 addresses to be used by a small number of ingress points into our cluster (e.g. nginx-ingress and api gateways). Besides accidental misconfiguration by our own developers, we run a number of third-party projects that have services of type LoadBalancer that aren't easily configurable, so we ignore them (since we haven't implemented MetalLB yet) and create our own ingress resources. Having to run all these through kustomize (or something similar), or creating and running a mutating webhook to change the service type automatically would be a pain and somewhat obtuse. Having MetalLB recognize requests from only specific namespaces for pools gives cluster and network administrators a simpler way to define who has access to those IP ranges.

Additionally, it would simplify our use-case of giving teams access to a particular pool of addresses. Instead of needing to remember to add the annotation with the correct pool name, services in a particular namespace would grab from a particular pool by default. Gives administrators a way to set some team defaults more easily.

Just thought I'd add our use cases as another data point. Thanks again!

[uablrek]

On a closer look this does not solve the entire problem for us. Closing...

[cgiraldo]

This feature is useful for our use case. Configure a pool of IPs for each tenant in a multi-tenant platform.
Is there any chance to have this feature merged?

[fedepaol]

Hi @cgiraldo , this is getting discussed in the broader design proposal here: #942
I suggest tracking that.