Repository: metallb/metallb
Pull request #687 (2020-08-04 19:30:52)

dev-env: Add BGP support
Expand the dev-env setup with an option to configure MetalLB in BGP
mode.  It will run frr in a container along side the kind cluster and
each of the speakers will peer with it.

Running `inv dev-env` will stand up a cluster with MetalLB
unconfigured as before.  If you run `inv dev-env -p bgp`, the BGP
configuration will be applied.

To keep the environment completely isolated, no routing is done
outside of the frr container.  You can get a shell in the container to
test connectivity to LoadBalancer IPs.  You can also use the vtysh
console of the router to see the router's status information.  A
README.md file is included in dev-env/bgp/ to demonstrate how to use
this bgp dev env.

aojea: /lgtm
very nice

Patch:
diff --git a/dev-env/README.md b/dev-env/README.md
new file mode 100644
index 00000000..c03e0750
--- /dev/null
+++ b/dev-env/README.md
@@ -0,0 +1,16 @@
+# dev-env
+
+This directory contains supporting files for the included MetalLB development
+environment. The environment is run using:
+
+```
+inv dev-env
+```
+
+There is also support for configuring MetalLB to peer with a BGP router running
+in a conatiner. For more information, see [the BGP dev-env
+README](bgp/README.md).
+
+```
+inv dev-env --protocol bgp
+```
diff --git a/dev-env/bgp/.gitignore b/dev-env/bgp/.gitignore
new file mode 100644
index 00000000..d96f1de9
--- /dev/null
+++ b/dev-env/bgp/.gitignore
@@ -0,0 +1,2 @@
+config.yaml
+frr-volume
diff --git a/dev-env/bgp/README.md b/dev-env/bgp/README.md
new file mode 100644
index 00000000..94d99f8e
--- /dev/null
+++ b/dev-env/bgp/README.md
@@ -0,0 +1,95 @@
+# BGP test env
+
+This assumes a 3-node `kind` based test cluster as set up by running `inv
+dev-env -p bgp`, which sets up the development environment with some
+configuration and a BGP router for development and testing purposes.
+
+Along with the cluster, there will be a container named `frr` which is the BGP
+router. Note that this only works in an IPv4 configuration for now.
+
+The configuration used for MetalLB can be found in `config.yaml`. The FRR
+configuration is in the `frr/` directory.
+
+Observe that the BGP speakers have peered with our router:
+
+```
+$ docker exec frr vtysh -c "show ip bgp summary"
+
+IPv4 Unicast Summary:
+BGP router identifier 172.18.0.5, local AS number 64512 vrf-id 0
+BGP table version 1
+RIB entries 1, using 192 bytes of memory
+Peers 3, using 43 KiB of memory
+
+Neighbor        V         AS   MsgRcvd   MsgSent   TblVer  InQ OutQ  Up/Down State/PfxRcd   PfxSnt
+172.18.0.2      4      64512        22        21        0    0    0 00:09:47            1        0
+172.18.0.3      4      64512        22        21        0    0    0 00:09:47            1        0
+172.18.0.4      4      64512        22        21        0    0    0 00:09:47            1        0
+
+Total number of neighbors 3
+```
+
+Create an `nginx` Deployment and a corresponding Service LoadBalancer:
+
+```
+kubectl apply -f dev-env/testsvc.yaml
+```
+
+Observe the IP address allocated to the Service by MetalLB:
+
+```
+$ kubectl get svc nginx
+NAME    TYPE           CLUSTER-IP      EXTERNAL-IP    PORT(S)        AGE
+nginx   LoadBalancer   10.106.88.254   192.168.10.0   80:30076/TCP   53m
+```
+
+Ensure a route to this Service has been published by the MetalLB BGP speaker:
+
+```
+$ docker exec frr vtysh -c "show ip bgp detail"
+BGP table version is 1, local router ID is 172.18.0.5, vrf id 0
+Default local pref 100, local AS 64512
+Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
+               i internal, r RIB-failure, S Stale, R Removed
+Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
+Origin codes:  i - IGP, e - EGP, ? - incomplete
+
+   Network          Next Hop            Metric LocPrf Weight Path
+*>i192.168.10.0/32  172.18.0.2                      0      0 ?
+*=i                 172.18.0.3                      0      0 ?
+*=i                 172.18.0.4                      0      0 ?
+
+Displayed  1 routes and 3 total paths
+```
+
+Now validate that you can connect to the Service from the router:
+
+```
+$ docker exec -it frr bash
+bash-5.0# echo "GET /" | nc 192.168.10.0 80
+<!DOCTYPE html>
+<html>
+<head>
+<title>Welcome to nginx!</title>
+<style>
+    body {
+        width: 35em;
+        margin: 0 auto;
+        font-family: Tahoma, Verdana, Arial, sans-serif;
+    }
+</style>
+</head>
+<body>
+<h1>Welcome to nginx!</h1>
+<p>If you see this page, the nginx web server is successfully installed and
+working. Further configuration is required.</p>
+
+<p>For online documentation and support please refer to
+<a href="http://nginx.org/">nginx.org</a>.<br/>
+Commercial support is available at
+<a href="http://nginx.com/">nginx.com</a>.</p>
+
+<p><em>Thank you for using nginx.</em></p>
+</body>
+</html>
+```
diff --git a/dev-env/bgp/config.yaml.tmpl b/dev-env/bgp/config.yaml.tmpl
new file mode 100644
index 00000000..9445f3a7
--- /dev/null
+++ b/dev-env/bgp/config.yaml.tmpl
@@ -0,0 +1,18 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  namespace: metallb-system
+  name: config
+data:
+  config: |
+    peers:
+      # NOTE: PEER_ADDRESS will be substituted with the IP address
+      # assigned to the frr container.
+    - peer-address: PEER_ADDRESS
+      peer-asn: 64512
+      my-asn: 64512
+    address-pools:
+    - name: default
+      protocol: bgp
+      addresses:
+      - 192.168.10.0/24
diff --git a/dev-env/bgp/frr/bgpd.conf.tmpl b/dev-env/bgp/frr/bgpd.conf.tmpl
new file mode 100644
index 00000000..0a58b60f
--- /dev/null
+++ b/dev-env/bgp/frr/bgpd.conf.tmpl
@@ -0,0 +1,43 @@
+! -*- bgp -*-
+!
+! BGPd sample configuration file
+!
+! $Id: bgpd.conf.sample,v 1.1 2002/12/13 20:15:29 paul Exp $
+!
+hostname bgpd
+password zebra
+!enable password please-set-at-here
+!
+!
+! NOTE: These NODE_IPs will be substituted with the IP addresses
+! assigned to the kind cluster Nodes if they are different.
+!
+router bgp 64512
+ neighbor NODE0_IP remote-as 64512
+ neighbor NODE0_IP next-hop-self
+ neighbor NODE1_IP remote-as 64512
+ neighbor NODE1_IP next-hop-self
+ neighbor NODE2_IP remote-as 64512
+ neighbor NODE2_IP next-hop-self
+
+!
+!router bgp 7675
+! bgp router-id 10.0.0.1
+! network 10.0.0.0/8
+! neighbor 10.0.0.2 remote-as 7675
+! neighbor 10.0.0.2 ebgp-multihop
+!
+! address-family ipv4 unicast
+!  neighbor 10.0.0.2 route-map set-nexthop out
+!  neighbor 10.0.0.2 next-hop-self
+! exit-address-family
+!
+! access-list all permit any
+!
+!route-map set-nexthop permit 10
+! match ip address all
+! set ip next-hop 10.0.0.1
+!
+!log file /var/log/bgpd.log
+!
+log stdout debugging
diff --git a/dev-env/bgp/frr/daemons b/dev-env/bgp/frr/daemons
new file mode 100644
index 00000000..5e1f5bdc
--- /dev/null
+++ b/dev-env/bgp/frr/daemons
@@ -0,0 +1,80 @@
+# This file tells the frr package which daemons to start.
+#
+# Sample configurations for these daemons can be found in
+# /usr/share/doc/frr/examples/.
+#
+# ATTENTION:
+#
+# When activating a daemon for the first time, a config file, even if it is
+# empty, has to be present *and* be owned by the user and group "frr", else
+# the daemon will not be started by /etc/init.d/frr. The permissions should
+# be u=rw,g=r,o=.
+# When using "vtysh" such a config file is also needed. It should be owned by
+# group "frrvty" and set to ug=rw,o= though. Check /etc/pam.d/frr, too.
+#
+# The watchfrr and zebra daemons are always started.
+#
+bgpd=yes
+ospfd=no
+ospf6d=no
+ripd=no
+ripngd=no
+isisd=no
+pimd=no
+ldpd=no
+nhrpd=no
+eigrpd=no
+babeld=no
+sharpd=no
+pbrd=no
+bfdd=no
+fabricd=no
+vrrpd=no
+
+#
+# If this option is set the /etc/init.d/frr script automatically loads
+# the config via "vtysh -b" when the servers are started.
+# Check /etc/pam.d/frr if you intend to use "vtysh"!
+#
+vtysh_enable=yes
+zebra_options="  -A 127.0.0.1 -s 90000000"
+bgpd_options="   -A 127.0.0.1"
+ospfd_options="  -A 127.0.0.1"
+ospf6d_options=" -A ::1"
+ripd_options="   -A 127.0.0.1"
+ripngd_options=" -A ::1"
+isisd_options="  -A 127.0.0.1"
+pimd_options="   -A 127.0.0.1"
+ldpd_options="   -A 127.0.0.1"
+nhrpd_options="  -A 127.0.0.1"
+eigrpd_options=" -A 127.0.0.1"
+babeld_options=" -A 127.0.0.1"
+sharpd_options=" -A 127.0.0.1"
+pbrd_options="   -A 127.0.0.1"
+staticd_options="-A 127.0.0.1"
+bfdd_options="   -A 127.0.0.1"
+fabricd_options="-A 127.0.0.1"
+vrrpd_options="  -A 127.0.0.1"
+
+# configuration profile
+#
+#frr_profile="traditional"
+#frr_profile="datacenter"
+
+#
+# This is the maximum number of FD's that will be available.
+# Upon startup this is read by the control files and ulimit
+# is called. Uncomment and use a reasonable value for your
+# setup if you are expecting a large number of peers in
+# say BGP.
+#MAX_FDS=1024
+
+# The list of daemons to watch is automatically generated by the init script.
+#watchfrr_options=""
+
+# for debugging purposes, you can specify a "wrap" command to start instead
+# of starting the daemon directly, e.g. to use valgrind on ospfd:
+#   ospfd_wrap="/usr/bin/valgrind"
+# or you can use "all_wrap" for all daemons, e.g. to use perf record:
+#   all_wrap="/usr/bin/perf record --call-graph -"
+# the normal daemon command is added to this at the end.
diff --git a/dev-env/bgp/frr/zebra.conf b/dev-env/bgp/frr/zebra.conf
new file mode 100644
index 00000000..49c6a01e
--- /dev/null
+++ b/dev-env/bgp/frr/zebra.conf
@@ -0,0 +1,24 @@
+! -*- zebra -*-
+!
+! zebra sample configuration file
+!
+hostname Router
+password zebra
+enable password zebra
+!
+! Interface's description.
+!
+!interface lo
+! description test of desc.
+!
+!interface sit0
+! multicast
+
+!
+! Static default route sample.
+!
+!ip route 0.0.0.0/0 203.181.89.241
+!
+
+!log file zebra.log
+
diff --git a/internal/config/config.go b/internal/config/config.go
index f41c4266..866f8229 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"reflect"
 	"strconv"
 	"strings"
 	"time"
@@ -200,6 +201,14 @@ func Parse(bs []byte) (*Config, error) {
 		if err != nil {
 			return nil, fmt.Errorf("parsing peer #%d: %s", i+1, err)
 		}
+		for _, ep := range cfg.Peers {
+			// TODO: Be smarter regarding conflicting peers. For example, two
+			// peers could have a different hold time but they'd still result
+			// in two BGP sessions between the speaker and the remote host.
+			if reflect.DeepEqual(peer, ep) {
+				return nil, fmt.Errorf("peer #%d already exists", i+1)
+			}
+		}
 		cfg.Peers = append(cfg.Peers, peer)
 	}
 
diff --git a/tasks.py b/tasks.py
index 002255af..7a03dcef 100644
--- a/tasks.py
+++ b/tasks.py
@@ -71,7 +71,7 @@ def build(ctx, binaries, architectures, tag="dev", docker_user="metallb"):
     binaries = _check_binaries(binaries)
     architectures = _check_architectures(architectures)
     _make_build_dirs()
-    
+
     commit = run("git describe --dirty --always", hide=True).stdout.strip()
     branch = run("git rev-parse --abbrev-ref HEAD", hide=True).stdout.strip()
 
@@ -136,7 +136,7 @@ def push_multiarch(ctx, binaries, tag="dev", docker_user="metallb"):
     binaries = _check_binaries(binaries)
     architectures = _check_architectures(["all"])
     push(ctx, binaries=binaries, architectures=architectures, tag=tag, docker_user=docker_user)
-    
+
     platforms = ",".join("linux/{}".format(arch) for arch in architectures)
     for bin in binaries:
         run("manifest-tool push from-args "
@@ -152,8 +152,10 @@ def push_multiarch(ctx, binaries, tag="dev", docker_user="metallb"):
 @task(help={
     "architecture": "CPU architecture of the local machine. Default 'amd64'.",
     "name": "name of the kind cluster to use.",
+    "protocol": "Pre-configure MetalLB with the specified protocol. "
+                "Unconfigured by default. Supported: 'bgp'",
 })
-def dev_env(ctx, architecture="amd64", name="kind", cni=None):
+def dev_env(ctx, architecture="amd64", name="kind", cni=None, protocol=None):
     """Build and run MetalLB in a local Kind cluster.
 
     If the cluster specified by --name (default "kind") doesn't exist,
@@ -231,6 +233,95 @@ stringData:
         tmp.flush()
         run("kubectl apply -f {}".format(tmp.name), echo=True)
 
+    if protocol == "bgp":
+        print("Configuring MetalLB with a BGP test environment")
+        bgp_dev_env()
+    else:
+        print("Leaving MetalLB unconfigured")
+
+
+# Configure MetalLB in the dev-env for BGP testing. Start an frr based BGP
+# router in a container and configure MetalLB to peer with it.
+# See dev-env/bgp/README.md for some more information.
+def bgp_dev_env():
+    dev_env_dir = os.getcwd() + "/dev-env/bgp"
+    frr_volume_dir = dev_env_dir + "/frr-volume"
+
+    # TODO -- The IP address handling will need updates to add support for IPv6
+
+    # We need the IPs for each Node in the cluster to place them in the BGP
+    # router configuration file (bgpd.conf). Each Node will peer with this
+    # router.
+    node_ips = run("kubectl get nodes -o jsonpath='{.items[*].status.addresses"
+            "[?(@.type==\"InternalIP\")].address}{\"\\n\"}'", echo=True)
+    node_ips = node_ips.stdout.strip().split()
+    if len(node_ips) != 3:
+        raise Exit(message='Expected 3 nodes, got %d' % len(node_ips))
+
+    # Create a new directory that will be used as the config volume for frr.
+    try:
+        # sudo because past docker runs will have changed ownership of this dir
+        run('sudo rm -rf "%s"' % frr_volume_dir)
+        os.mkdir(frr_volume_dir)
+    except FileExistsError:
+        pass
+    except Exception as e:
+        raise Exit(message='Failed to create frr-volume directory: %s'
+                   % str(e))
+
+    # These two config files are static, so we just copy them straight in.
+    shutil.copyfile("%s/frr/zebra.conf" % dev_env_dir,
+                    "%s/zebra.conf" % frr_volume_dir)
+    shutil.copyfile("%s/frr/daemons" % dev_env_dir,
+                    "%s/daemons" % frr_volume_dir)
+
+    # bgpd.conf is created from a template so that we can include the current
+    # Node IPs.
+    with open("%s/frr/bgpd.conf.tmpl" % dev_env_dir, 'r') as f:
+        bgpd_config = "! THIS FILE IS AUTOGENERATED\n" + f.read()
+    for n in range(0, len(node_ips)):
+        bgpd_config = bgpd_config.replace("NODE%d_IP" % n, node_ips[n])
+    with open("%s/bgpd.conf" % frr_volume_dir, 'w') as f:
+        f.write(bgpd_config)
+
+    # Run a BGP router in a container for all of the speakers to peer with.
+    run('for frr in $(docker ps -a -f name=frr --format {{.Names}}) ; do '
+        '    docker rm -f $frr ; '
+        'done', echo=True)
+    run("docker run -d --privileged --network kind --rm --name frr --volume %s:/etc/frr "
+            "frrouting/frr:latest" % frr_volume_dir, echo=True)
+
+    peer_address = run('docker inspect -f "{{ '
+            'range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}" frr', echo=True)
+    with open("%s/config.yaml.tmpl" % dev_env_dir, 'r') as f:
+        mlb_config = "# THIS FILE IS AUTOGENERATED\n" + f.read()
+    mlb_config = mlb_config.replace("PEER_ADDRESS", peer_address.stdout.strip())
+    with open("%s/config.yaml" % dev_env_dir, 'w') as f:
+        f.write(mlb_config)
+    # Apply the MetalLB ConfigMap
+    run("kubectl apply -f %s/config.yaml" % dev_env_dir)
+
+
+@task
+def dev_env_cleanup(ctx):
+    """Remove traces of the dev env"""
+    try:
+        run("kind delete cluster")
+    except Exception:
+        # This will fail if there's no cluster.
+        pass
+
+    run('for frr in $(docker ps -a -f name=frr --format {{.Names}}) ; do '
+        '    docker rm -f $frr ; '
+        'done', echo=True)
+
+    dev_env_dir = os.getcwd() + "/dev-env/bgp"
+    frr_volume_dir = dev_env_dir + "/frr-volume"
+
+    # sudo because past docker runs will have changed ownership of this dir
+    run('sudo rm -rf "%s"' % frr_volume_dir)
+    run('rm -f "%s"/config.yaml' % dev_env_dir)
+
 @task
 def test_cni_manifests(ctx):
     """Update CNI manifests for e2e tests."""
@@ -266,7 +357,7 @@ def release(ctx, version, skip_release_notes=False):
     status = run("git status --porcelain", hide=True).stdout.strip()
     if status != "":
         raise Exit(message="git checkout not clean, cannot release")
-    
+
     version = semver.parse_version_info(version)
     is_patch_release = version.patch != 0
 
diff --git a/website/content/configuration/_index.md b/website/content/configuration/_index.md
index a44b3825..25909bb1 100644
--- a/website/content/configuration/_index.md
+++ b/website/content/configuration/_index.md
@@ -265,6 +265,11 @@ Addresses can still be specifically allocated from the "expensive"
 pool with the methods described in
 the [usage](/usage/#requesting-specific-ips) section.
 
+{{% notice note %}}
+To specify a single IP address in a pool, use `/32` in the CIDR notation
+(e.g. `42.176.25.64/32`).
+{{% /notice %}}
+
 ### Handling buggy networks
 
 Some old consumer network equipment mistakenly blocks IP addresses


Test patch:
diff --git a/dev-env/testsvc.yaml b/dev-env/testsvc.yaml
new file mode 100644
index 00000000..9da7ea7e
--- /dev/null
+++ b/dev-env/testsvc.yaml
@@ -0,0 +1,34 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx
+spec:
+  selector:
+    matchLabels:
+      app: nginx
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1
+        ports:
+        - name: http
+          containerPort: 80
+
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: nginx
+spec:
+  ports:
+  - name: http
+    port: 80
+    protocol: TCP
+    targetPort: 80
+  selector:
+    app: nginx
+  type: LoadBalancer
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 2ff39125..0a50d10b 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -345,6 +345,19 @@ peers:
 `,
 		},
 
+		{
+			desc: "duplicate peers",
+			raw: `
+peers:
+- my-asn: 42
+  peer-asn: 42
+  peer-address: 1.2.3.4
+- my-asn: 42
+  peer-asn: 42
+  peer-address: 1.2.3.4
+`,
+		},
+
 		{
 			desc: "no pool name",
 			raw: `