Repository: metallb/metallb
Pull request #1057 (2021-11-17 08:58:16)

Fix L2 metrics tests
Getting a lot of flakes in the CI:
```
      Expected
          <*errors.errorString | 0xc000695790>: {
              s: "metric metallb_layer2_requests_received not found",
          }
      to be nil
```
It is caused by the fact that this metric represents the amount
of ARP/NS that arrived to the speaker for the svc's ip, while the
test doesn't necessarily does one. That can happen because the speaker
sends a gratuitous message when he announces an IP.

Also added a check for the gratuitous_sent metric to cover that.

Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>


Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 6ee388b3..da4a40e0 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -7,7 +7,7 @@ commands:
     steps:
       - checkout
       - run: sudo apt-get update
-      - run: sudo apt-get install python3-pip
+      - run: sudo apt-get install python3-pip arping ndisc6
       - run: sudo pip3 install invoke semver pyyaml
   setup_kind:
     description: "Install kind"
@@ -44,7 +44,9 @@ jobs:
           if [ "<< parameters.ip-family >>" = "ipv6" ]; then SKIP="$SKIP\|IPV4"; fi
           if [ "<< parameters.ip-family >>" = "ipv6" ] && [ "<< parameters.bgp-type >>" = "native" ]; then SKIP="$SKIP\|BGP"; fi # we are skipping BGP for ipv6 for native bgp stack
           echo "Skipping $SKIP"
-          inv e2etest --skip $SKIP -e /tmp/kind_logs
+          # `-E env "PATH=$PATH"` bypasses commands not found under sudo, `sudo` needed because arping requires CAP_NET_RAW
+          sudo -E env "PATH=$PATH" inv e2etest --skip $SKIP -e /tmp/kind_logs
+      - run: sudo chmod -R o+r /tmp/kind_logs # logs are exported under root (sudo), chmod required to upload the artifacts.
       - store_artifacts:
           path: /tmp/kind_logs
   lint-1-16:
diff --git a/.github/workflows/fossa.yaml b/.github/workflows/fossa.yaml
new file mode 100644
index 00000000..3a213fd1
--- /dev/null
+++ b/.github/workflows/fossa.yaml
@@ -0,0 +1,16 @@
+name: FOSSA license check
+on: [pull_request]
+
+# https://github.com/marketplace/actions/fossa-action
+
+jobs:
+  fossa:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          fetch-depth: 0
+      - uses: fossa-contrib/fossa-action@v1
+        with:
+          # https://docs.fossa.com/docs/api-reference#push-only-api-token
+          fossa-api-key: 831247a6ab6f0d4f99879e0353b512a5
diff --git a/controller/main.go b/controller/main.go
index 95759c7f..19a0826c 100644
--- a/controller/main.go
+++ b/controller/main.go
@@ -24,6 +24,7 @@ import (
 
 	"go.universe.tf/metallb/internal/allocator"
 	"go.universe.tf/metallb/internal/config"
+	metallbcfg "go.universe.tf/metallb/internal/config"
 	"go.universe.tf/metallb/internal/k8s"
 	"go.universe.tf/metallb/internal/logging"
 	"go.universe.tf/metallb/internal/version"
@@ -165,6 +166,7 @@ func main() {
 
 		ServiceChanged: c.SetBalancer,
 		ConfigChanged:  c.SetConfig,
+		ValidateConfig: metallbcfg.DontValidate, // the controller is not aware of the mode, we defer the validation to the speaker
 		Synced:         c.MarkSynced,
 	})
 	if err != nil {
diff --git a/internal/bgp/frr/config.go b/internal/bgp/frr/config.go
index a8bc8ffe..a355f0d7 100644
--- a/internal/bgp/frr/config.go
+++ b/internal/bgp/frr/config.go
@@ -10,6 +10,10 @@ import (
 	"strconv"
 	"syscall"
 	"text/template"
+	"time"
+
+	"github.com/go-kit/kit/log"
+	"github.com/go-kit/kit/log/level"
 )
 
 var configFileName = "/etc/frr_reloader/frr.conf"
@@ -200,7 +204,7 @@ var reloadConfig = func() error {
 // generateAndReloadConfigFile takes a 'struct frrConfig' and, using a template,
 // generates and writes a valid FRR configuration file. If this completes
 // successfully it will also force FRR to reload that configuration file.
-func generateAndReloadConfigFile(config *frrConfig) error {
+func generateAndReloadConfigFile(config *frrConfig, l log.Logger) {
 	filename, found := os.LookupEnv("FRR_CONFIG_FILE")
 	if found {
 		configFileName = filename
@@ -208,17 +212,49 @@ func generateAndReloadConfigFile(config *frrConfig) error {
 
 	configString, err := templateConfig(config)
 	if err != nil {
-		return err
+		level.Error(l).Log("op", "reload", "error", err, "cause", "template", "config", config)
+		return
 	}
 	err = writeConfig(configString, configFileName)
 	if err != nil {
-		return err
+		level.Error(l).Log("op", "reload", "error", err, "cause", "writeConfig", "config", config)
+		return
 	}
 
 	err = reloadConfig()
 	if err != nil {
-		return err
+		level.Error(l).Log("op", "reload", "error", err, "cause", "reload", "config", config)
+		return
 	}
 
-	return nil
+	level.Info(l).Log("op", "reload", "success", "reloaded config")
+}
+
+// debouncer takes a function that processes an frrConfig, a channel where
+// the update requests are sent, and squashes any requests coming in a given timeframe
+// as a single request.
+func debouncer(body func(config *frrConfig),
+	reload <-chan *frrConfig,
+	reloadInterval time.Duration) {
+	go func() {
+		var config *frrConfig
+		var timeOut <-chan time.Time
+		timerSet := false
+		for {
+			select {
+			case newCfg, ok := <-reload:
+				if !ok { // the channel was closed
+					return
+				}
+				config = newCfg
+				if !timerSet {
+					timeOut = time.After(reloadInterval)
+					timerSet = true
+				}
+			case <-timeOut:
+				body(config)
+				timerSet = false
+			}
+		}
+	}()
 }
diff --git a/internal/bgp/frr/frr.go b/internal/bgp/frr/frr.go
index 0efd1ba6..c5e2abac 100644
--- a/internal/bgp/frr/frr.go
+++ b/internal/bgp/frr/frr.go
@@ -19,8 +19,9 @@ import (
 // no need to lock this data structure. TODO: confirm this.
 
 type sessionManager struct {
-	sessions    map[string]*session
-	bfdProfiles []BFDProfile
+	sessions     map[string]*session
+	bfdProfiles  []BFDProfile
+	reloadConfig chan *frrConfig
 }
 
 type session struct {
@@ -92,7 +93,8 @@ func (s *session) Set(advs ...*bgp.Advertisement) error {
 		return err
 	}
 
-	return generateAndReloadConfigFile(config)
+	s.sessionManager.reloadConfig <- config
+	return nil
 }
 
 // Close() shuts down the BGP session.
@@ -107,7 +109,8 @@ func (s *session) Close() error {
 		return err
 	}
 
-	return generateAndReloadConfigFile(frrConfig)
+	s.sessionManager.reloadConfig <- frrConfig
+	return nil
 }
 
 // NewSession() creates a BGP session using the given session parameters.
@@ -139,13 +142,8 @@ func (sm *sessionManager) NewSession(l log.Logger, addr string, srcAddr net.IP,
 		return nil, err
 	}
 
-	err = generateAndReloadConfigFile(frrConfig)
-	if err != nil {
-		_ = sm.deleteSession(s)
-		return nil, err
-	}
-
-	return s, err
+	sm.reloadConfig <- frrConfig
+	return s, nil
 }
 
 func (sm *sessionManager) addSession(s *session) error {
@@ -183,7 +181,8 @@ func (sm *sessionManager) SyncBFDProfiles(profiles map[string]*config.BFDProfile
 		return err
 	}
 
-	return generateAndReloadConfigFile(frrConfig)
+	sm.reloadConfig <- frrConfig
+	return nil
 }
 
 func (sm *sessionManager) createConfig() (*frrConfig, error) {
@@ -271,11 +270,20 @@ func (sm *sessionManager) createConfig() (*frrConfig, error) {
 	return config, nil
 }
 
-func NewSessionManager() *sessionManager {
-	return &sessionManager{
-		sessions:    map[string]*session{},
-		bfdProfiles: []BFDProfile{},
+var debounceTimeout = 500 * time.Millisecond
+
+func NewSessionManager(l log.Logger) *sessionManager {
+	res := &sessionManager{
+		sessions:     map[string]*session{},
+		bfdProfiles:  []BFDProfile{},
+		reloadConfig: make(chan *frrConfig),
 	}
+	reload := func(config *frrConfig) {
+		generateAndReloadConfigFile(config, l)
+	}
+	debouncer(reload, res.reloadConfig, debounceTimeout)
+
+	return res
 }
 
 func configBFDProfileToFRR(p *config.BFDProfile) *BFDProfile {
diff --git a/internal/bgp/native/native.go b/internal/bgp/native/native.go
index e89c6af3..4cdd3a62 100644
--- a/internal/bgp/native/native.go
+++ b/internal/bgp/native/native.go
@@ -58,7 +58,7 @@ type session struct {
 type sessionManager struct {
 }
 
-func NewSessionManager() *sessionManager {
+func NewSessionManager(l log.Logger) *sessionManager {
 	return &sessionManager{}
 }
 
diff --git a/internal/config/config.go b/internal/config/config.go
index 84beb77a..9efba3ec 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -242,12 +242,17 @@ func parseKeepaliveTime(ht time.Duration, ka string) (time.Duration, error) {
 }
 
 // Parse loads and validates a Config from bs.
-func Parse(bs []byte) (*Config, error) {
+func Parse(bs []byte, validate Validate) (*Config, error) {
 	var raw configFile
 	if err := yaml.UnmarshalStrict(bs, &raw); err != nil {
 		return nil, fmt.Errorf("could not parse config: %s", err)
 	}
 
+	err := validate(&raw)
+	if err != nil {
+		return nil, err
+	}
+
 	cfg := &Config{
 		Pools:       map[string]*Pool{},
 		BFDProfiles: map[string]*BFDProfile{},
diff --git a/internal/config/validate.go b/internal/config/validate.go
new file mode 100644
index 00000000..d229de30
--- /dev/null
+++ b/internal/config/validate.go
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier:Apache-2.0
+
+package config
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+)
+
+type Validate func(*configFile) error
+
+// DiscardFRROnly returns an error if the current configFile contains
+// any options that are available only in the FRR implementation.
+func DiscardFRROnly(c *configFile) error {
+	for _, p := range c.Peers {
+		if p.BFDProfile != "" {
+			return fmt.Errorf("peer %s has bfd-profile set on native bgp mode", p.Addr)
+		}
+		if p.KeepaliveTime != "" {
+			return fmt.Errorf("peer %s has keepalive-time set on native bgp mode", p.Addr)
+		}
+	}
+	if len(c.BFDProfiles) > 0 {
+		return errors.New("bfd profiles section set")
+	}
+	for _, p := range c.Pools {
+		for _, a := range p.BGPAdvertisements {
+			if a.AggregationLengthV6 != nil {
+				return fmt.Errorf("pool %s has aggregation-lenght-v6 set on native bgp mode", p.Name)
+			}
+		}
+		if p.Protocol == BGP {
+			for _, cidr := range p.Addresses {
+				nets, err := ParseCIDR(cidr)
+				if err != nil {
+					return fmt.Errorf("invalid CIDR %q in pool %q: %s", cidr, p.Name, err)
+				}
+				for _, n := range nets {
+					if n.IP.To4() == nil {
+						return fmt.Errorf("pool %q has ipv6 CIDR %s, native bgp mode does not support ipv6", p.Name, n)
+					}
+				}
+			}
+		}
+	}
+	return nil
+}
+
+// DontValidate is a Validate function that always returns
+// success.
+func DontValidate(c *configFile) error {
+	return nil
+}
diff --git a/internal/k8s/k8s.go b/internal/k8s/k8s.go
index 6b19fb1e..242912a4 100644
--- a/internal/k8s/k8s.go
+++ b/internal/k8s/k8s.go
@@ -54,6 +54,7 @@ type Client struct {
 
 	serviceChanged func(log.Logger, string, *v1.Service, EpsOrSlices) SyncState
 	configChanged  func(log.Logger, *config.Config) SyncState
+	validateConfig config.Validate
 	nodeChanged    func(log.Logger, *v1.Node) SyncState
 	synced         func(log.Logger)
 }
@@ -91,6 +92,7 @@ type Config struct {
 
 	ServiceChanged func(log.Logger, string, *v1.Service, EpsOrSlices) SyncState
 	ConfigChanged  func(log.Logger, *config.Config) SyncState
+	ValidateConfig config.Validate
 	NodeChanged    func(log.Logger, *v1.Node) SyncState
 	Synced         func(log.Logger)
 }
@@ -137,10 +139,11 @@ func New(cfg *Config) (*Client, error) {
 	queue := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter())
 
 	c := &Client{
-		logger: cfg.Logger,
-		client: clientset,
-		events: recorder,
-		queue:  queue,
+		logger:         cfg.Logger,
+		client:         clientset,
+		events:         recorder,
+		queue:          queue,
+		validateConfig: cfg.ValidateConfig,
 	}
 
 	if cfg.ServiceChanged != nil {
@@ -535,7 +538,7 @@ func (c *Client) sync(key interface{}) SyncState {
 		// config is not going to parse any better until the k8s
 		// object changes to fix the issue.
 		cm := cmi.(*v1.ConfigMap)
-		cfg, err := config.Parse([]byte(cm.Data["config"]))
+		cfg, err := config.Parse([]byte(cm.Data["config"]), c.validateConfig)
 		if err != nil {
 			level.Error(l).Log("event", "configStale", "error", err, "msg", "config (re)load failed, config marked stale")
 			configStale.Set(1)
diff --git a/manifests/metallb-frr.yaml b/manifests/metallb-frr.yaml
index 5fa357a2..ab980fac 100644
--- a/manifests/metallb-frr.yaml
+++ b/manifests/metallb-frr.yaml
@@ -404,10 +404,13 @@ data:
   vtysh.conf: |+
     service integrated-vtysh-config
   frr.conf: |+
+    ! This file gets overriden the first time the speaker renders a config.
+    ! So anything configured here is only temporary.
     frr version 7.5.1
     frr defaults traditional
     hostname Router
     line vty
+    log file /etc/frr/frr.log informational
 
 ---
 apiVersion: apps/v1
@@ -484,6 +487,20 @@ spec:
               mountPath: /var/run/frr
             - name: frr-conf
               mountPath: /etc/frr
+          # The command is FRR's default entrypoint & waiting for the log file to appear and tailing it.
+          # If the log file isn't created in 60 seconds the tail fails and the container is restarted.
+          # This workaround is needed to have the frr logs as part of kubectl logs -c frr < speaker_pod_name >.
+          command:
+            - /bin/sh
+            - -c
+            - |
+              /sbin/tini -- /usr/lib/frr/docker-start &
+              attempts=0
+              until [[ -f /etc/frr/frr.log || $attempts -eq 60 ]]; do
+                sleep 1
+                attempts=$(( $attempts + 1 ))
+              done
+              tail -f /etc/frr/frr.log
         - name: reloader
           image: frrouting/frr:v7.5.1
           command: ["/etc/frr_reloader/frr-reloader.sh"]
diff --git a/speaker/bgp_controller.go b/speaker/bgp_controller.go
index 8b4fa23e..114528cc 100644
--- a/speaker/bgp_controller.go
+++ b/speaker/bgp_controller.go
@@ -64,10 +64,6 @@ newPeers:
 			if ep == nil {
 				continue
 			}
-			if ep.cfg.KeepaliveTime != 0 && c.bgpType == bgpNative {
-				level.Error(l).Log("op", "setConfig", "peer", ep.cfg.Addr, "configuration ignored", "keep alive time configuration is not supported")
-				continue
-			}
 			if reflect.DeepEqual(p, ep.cfg) {
 				newPeers = append(newPeers, ep)
 				c.peers[i] = nil
@@ -317,12 +313,12 @@ func (c *bgpController) SetNode(l log.Logger, node *v1.Node) error {
 }
 
 // Create a new 'bgp.SessionManager' of type 'bgpType'.
-var newBGP = func(bgpType bgpImplementation) bgp.SessionManager {
+var newBGP = func(bgpType bgpImplementation, l log.Logger) bgp.SessionManager {
 	switch bgpType {
 	case bgpNative:
-		return bgpnative.NewSessionManager()
+		return bgpnative.NewSessionManager(l)
 	case bgpFrr:
-		return bgpfrr.NewSessionManager()
+		return bgpfrr.NewSessionManager(l)
 	default:
 		panic(fmt.Sprintf("unsupported BGP implementation type: %s", bgpType))
 	}
diff --git a/speaker/main.go b/speaker/main.go
index 0610cd6c..d5fd4acf 100644
--- a/speaker/main.go
+++ b/speaker/main.go
@@ -26,6 +26,7 @@ import (
 
 	"go.universe.tf/metallb/internal/bgp"
 	"go.universe.tf/metallb/internal/config"
+	metallbcfg "go.universe.tf/metallb/internal/config"
 	"go.universe.tf/metallb/internal/k8s"
 	"go.universe.tf/metallb/internal/layer2"
 	"go.universe.tf/metallb/internal/logging"
@@ -133,6 +134,11 @@ func main() {
 		os.Exit(1)
 	}
 
+	validateConfig := metallbcfg.DontValidate
+	if bgpType == "native" {
+		validateConfig = metallbcfg.DiscardFRROnly
+	}
+
 	client, err := k8s.New(&k8s.Config{
 		ProcessName:     "metallb-speaker",
 		ConfigMapName:   *config,
@@ -148,6 +154,7 @@ func main() {
 
 		ServiceChanged: ctrl.SetBalancer,
 		ConfigChanged:  ctrl.SetConfig,
+		ValidateConfig: validateConfig,
 		NodeChanged:    ctrl.SetNode,
 	})
 	if err != nil {
@@ -195,7 +202,7 @@ func newController(cfg controllerConfig) (*controller, error) {
 			myNode:         cfg.MyNode,
 			svcAds:         make(map[string][]*bgp.Advertisement),
 			bgpType:        cfg.bgpType,
-			sessionManager: newBGP(cfg.bgpType),
+			sessionManager: newBGP(cfg.bgpType, cfg.Logger),
 		},
 	}
 
@@ -355,14 +362,6 @@ func (c *controller) SetConfig(l log.Logger, cfg *config.Config) k8s.SyncState {
 		return k8s.SyncStateErrorNoRetry
 	}
 
-	if c.bgpType == bgpNative {
-		err := validateFRROnlyConfiguration(cfg)
-		if err != nil {
-			level.Error(l).Log("op", "setConfig", "error", err)
-			return k8s.SyncStateError
-		}
-	}
-
 	for svc, ip := range c.svcIP {
 		if pool := poolFor(cfg.Pools, ip); pool == "" {
 			level.Error(l).Log("op", "setConfig", "service", svc, "ip", ip, "error", "service has no configuration under new config", "msg", "new configuration rejected")
diff --git a/speaker/validate.go b/speaker/validate.go
deleted file mode 100644
index 4fe52514..00000000
--- a/speaker/validate.go
+++ /dev/null
@@ -1,22 +0,0 @@
-// SPDX-License-Identifier:Apache-2.0
-
-package main
-
-import (
-	"fmt"
-
-	"github.com/pkg/errors"
-	"go.universe.tf/metallb/internal/config"
-)
-
-func validateFRROnlyConfiguration(cfg *config.Config) error {
-	for _, p := range cfg.Peers {
-		if p.BFDProfile != "" {
-			return fmt.Errorf("peer %s has bfd profile set", p.Addr)
-		}
-	}
-	if len(cfg.BFDProfiles) > 0 {
-		return errors.New("bfd profiles section set")
-	}
-	return nil
-}


Test patch:
diff --git a/e2etest/bgp_test.go b/e2etest/bgp_test.go
index dd704fd2..18b22c68 100644
--- a/e2etest/bgp_test.go
+++ b/e2etest/bgp_test.go
@@ -47,11 +47,13 @@ import (
 )
 
 const (
-	frrIBGP      = "frr-iBGP"
-	frrEBGP      = "frr-eBGP"
-	IBGPAsn      = 64512
-	EBGPAsn      = 64513
-	baseRouterID = "10.10.10.%d"
+	frrIBGP         = "frr-iBGP"
+	frrEBGP         = "frr-eBGP"
+	IBGPAsn         = 64512
+	EBGPAsn         = 64513
+	baseRouterID    = "10.10.10.%d"
+	v4PoolAddresses = "192.168.10.0/24"
+	v6PoolAddresses = "fc00:f853:0ccd:e799::/124"
 )
 
 var (
@@ -122,7 +124,7 @@ var _ = ginkgo.Describe("BGP", func() {
 		}
 	})
 
-	table.DescribeTable("A service of protocol load balancer should work with", func(ipFamily string, setProtocoltest string) {
+	table.DescribeTable("A service of protocol load balancer should work with", func(ipFamily, setProtocoltest, poolAddress string) {
 		var allNodes *corev1.NodeList
 		configData := configFile{
 			Pools: []addressPool{
@@ -130,8 +132,7 @@ var _ = ginkgo.Describe("BGP", func() {
 					Name:     "bgp-test",
 					Protocol: BGP,
 					Addresses: []string{
-						"192.168.10.0/24",
-						"fc00:f853:0ccd:e799::/124",
+						poolAddress,
 					},
 				},
 			},
@@ -190,12 +191,12 @@ var _ = ginkgo.Describe("BGP", func() {
 			}
 		}
 	},
-		table.Entry("IPV4 - ExternalTrafficPolicyCluster", "ipv4", "ExternalTrafficPolicyCluster"),
-		table.Entry("IPV4 - ExternalTrafficPolicyLocal", "ipv4", "ExternalTrafficPolicyLocal"),
-		table.Entry("IPV4 - FRR running in the speaker POD", "ipv4", "CheckSpeakerFRRPodRunning"),
-		table.Entry("IPV6 - ExternalTrafficPolicyCluster", "ipv6", "ExternalTrafficPolicyCluster"),
-		table.Entry("IPV6 - ExternalTrafficPolicyLocal", "ipv6", "ExternalTrafficPolicyLocal"),
-		table.Entry("IPV6 - FRR running in the speaker POD", "ipv6", "CheckSpeakerFRRPodRunning"))
+		table.Entry("IPV4 - ExternalTrafficPolicyCluster", "ipv4", "ExternalTrafficPolicyCluster", v4PoolAddresses),
+		table.Entry("IPV4 - ExternalTrafficPolicyLocal", "ipv4", "ExternalTrafficPolicyLocal", v4PoolAddresses),
+		table.Entry("IPV4 - FRR running in the speaker POD", "ipv4", "CheckSpeakerFRRPodRunning", v4PoolAddresses),
+		table.Entry("IPV6 - ExternalTrafficPolicyCluster", "ipv6", "ExternalTrafficPolicyCluster", v6PoolAddresses),
+		table.Entry("IPV6 - ExternalTrafficPolicyLocal", "ipv6", "ExternalTrafficPolicyLocal", v6PoolAddresses),
+		table.Entry("IPV6 - FRR running in the speaker POD", "ipv6", "CheckSpeakerFRRPodRunning", v6PoolAddresses))
 
 	ginkgo.Context("metrics", func() {
 		var controllerPod *corev1.Pod
@@ -220,7 +221,7 @@ var _ = ginkgo.Describe("BGP", func() {
 			}
 		})
 
-		table.DescribeTable("should be exposed by the controller", func(ipFamily string) {
+		table.DescribeTable("should be exposed by the controller", func(ipFamily, poolAddress string, addressTotal int) {
 			poolName := "bgp-test"
 
 			var peerAddrs []string
@@ -237,12 +238,9 @@ var _ = ginkgo.Describe("BGP", func() {
 			configData := configFile{
 				Pools: []addressPool{
 					{
-						Name:     poolName,
-						Protocol: BGP,
-						Addresses: []string{
-							"192.168.10.0/24",
-							"fc00:f853:0ccd:e799::/124",
-						},
+						Name:      poolName,
+						Protocol:  BGP,
+						Addresses: []string{poolAddress},
 					},
 				},
 				Peers: peersForContainers(frrContainers, ipFamily),
@@ -268,7 +266,7 @@ var _ = ginkgo.Describe("BGP", func() {
 				if err != nil {
 					return err
 				}
-				err = validateGaugeValue(272, "metallb_allocator_addresses_total", map[string]string{"pool": poolName}, controllerMetrics)
+				err = validateGaugeValue(addressTotal, "metallb_allocator_addresses_total", map[string]string{"pool": poolName}, controllerMetrics)
 				if err != nil {
 					return err
 				}
@@ -350,8 +348,8 @@ var _ = ginkgo.Describe("BGP", func() {
 				}, 2*time.Minute, 1*time.Second).Should(BeNil())
 			}
 		},
-			table.Entry("IPV4 - Checking service", "ipv4"),
-			table.Entry("IPV6 - Checking service", "ipv6"))
+			table.Entry("IPV4 - Checking service", "ipv4", v4PoolAddresses, 256),
+			table.Entry("IPV6 - Checking service", "ipv6", v6PoolAddresses, 16))
 	})
 
 	ginkgo.Context("validate different AddressPools for type=Loadbalancer", func() {
@@ -402,13 +400,21 @@ var _ = ginkgo.Describe("BGP", func() {
 				validateService(cs, svc, allNodes.Items, c, ipFamily)
 			}
 		},
+			table.Entry("IPV4 - test AddressPool defined by address range", []addressPool{
+				{
+					Name:     "bgp-test",
+					Protocol: BGP,
+					Addresses: []string{
+						"192.168.10.0-192.168.10.18",
+					},
+				}}, "ipv4",
+			),
 			table.Entry("IPV4 - test AddressPool defined by network prefix", []addressPool{
 				{
 					Name:     "bgp-test",
 					Protocol: BGP,
 					Addresses: []string{
 						"192.168.10.0/24",
-						"fc00:f853:0ccd:e799::/124",
 					},
 				}}, "ipv4",
 			),
@@ -417,24 +423,31 @@ var _ = ginkgo.Describe("BGP", func() {
 					Name:     "bgp-test",
 					Protocol: BGP,
 					Addresses: []string{
-						"192.168.10.0-192.168.10.18",
 						"fc00:f853:0ccd:e799::0-fc00:f853:0ccd:e799::18",
 					},
 				}}, "ipv6",
 			),
+			table.Entry("IPV6 - test AddressPool defined by network prefix", []addressPool{
+				{
+					Name:     "bgp-test",
+					Protocol: BGP,
+					Addresses: []string{
+						"fc00:f853:0ccd:e799::/124",
+					},
+				}}, "ipv6",
+			),
 		)
 	})
 
 	ginkgo.Context("BFD", func() {
-		table.DescribeTable("should work with the given bfd profile", func(bfd bfdProfile, ipFamily string) {
+		table.DescribeTable("should work with the given bfd profile", func(bfd bfdProfile, ipFamily, poolAddress string) {
 			configData := configFile{
 				Pools: []addressPool{
 					{
 						Name:     "bfd-test",
 						Protocol: BGP,
 						Addresses: []string{
-							"192.168.10.0/24",
-							"fc00:f853:0ccd:e799::/124",
+							poolAddress,
 						},
 					},
 				},
@@ -495,7 +508,7 @@ var _ = ginkgo.Describe("BGP", func() {
 			table.Entry("IPV4 - default",
 				bfdProfile{
 					Name: "bar",
-				}, "ipv4"),
+				}, "ipv4", v4PoolAddresses),
 			table.Entry("IPV4 - full params",
 				bfdProfile{
 					Name:             "full1",
@@ -505,7 +518,7 @@ var _ = ginkgo.Describe("BGP", func() {
 					EchoMode:         boolPtr(false),
 					PassiveMode:      boolPtr(false),
 					MinimumTTL:       uint32Ptr(254),
-				}, "ipv4"),
+				}, "ipv4", v4PoolAddresses),
 			table.Entry("IPV4 - echo mode enabled",
 				bfdProfile{
 					Name:             "echo",
@@ -515,11 +528,11 @@ var _ = ginkgo.Describe("BGP", func() {
 					EchoMode:         boolPtr(true),
 					PassiveMode:      boolPtr(false),
 					MinimumTTL:       uint32Ptr(254),
-				}, "ipv4"),
+				}, "ipv4", v4PoolAddresses),
 			table.Entry("IPV6 - default",
 				bfdProfile{
 					Name: "bar",
-				}, "ipv6"),
+				}, "ipv6", v6PoolAddresses),
 			table.Entry("IPV6 - full params",
 				bfdProfile{
 					Name:             "full1",
@@ -529,7 +542,7 @@ var _ = ginkgo.Describe("BGP", func() {
 					EchoMode:         boolPtr(false),
 					PassiveMode:      boolPtr(false),
 					MinimumTTL:       uint32Ptr(254),
-				}, "ipv6"),
+				}, "ipv6", v6PoolAddresses),
 			table.Entry("IPV6 - echo mode enabled",
 				bfdProfile{
 					Name:             "echo",
@@ -539,7 +552,7 @@ var _ = ginkgo.Describe("BGP", func() {
 					EchoMode:         boolPtr(true),
 					PassiveMode:      boolPtr(false),
 					MinimumTTL:       uint32Ptr(254),
-				}, "ipv6"),
+				}, "ipv6", v6PoolAddresses),
 		)
 	})
 })
diff --git a/e2etest/layer2_test.go b/e2etest/layer2_test.go
index 7544dba6..475ac96b 100644
--- a/e2etest/layer2_test.go
+++ b/e2etest/layer2_test.go
@@ -287,7 +287,6 @@ var _ = ginkgo.Describe("L2", func() {
 		table.Entry("IPV6", &ipv6ServiceRange))
 
 	ginkgo.Context("metrics", func() {
-		// TODO: validate gratuitous_sent metric
 		var controllerPod *corev1.Pod
 		var speakerPods map[string]*corev1.Pod
 
@@ -372,6 +371,9 @@ var _ = ginkgo.Describe("L2", func() {
 			ingressIP := e2eservice.GetIngressPoint(
 				&svc.Status.LoadBalancer.Ingress[0])
 
+			err = mac.RequestAddressResolution(ingressIP, executor.Host)
+			framework.ExpectNoError(err)
+
 			ginkgo.By("checking connectivity to its external VIP")
 			hostport := net.JoinHostPort(ingressIP, port)
 			address := fmt.Sprintf("http://%s/", hostport)
@@ -408,6 +410,11 @@ var _ = ginkgo.Describe("L2", func() {
 					return err
 				}
 
+				err = validateCounterValue(1, "metallb_layer2_gratuitous_sent", map[string]string{"ip": ingressIP}, speakerMetrics)
+				if err != nil {
+					return err
+				}
+
 				return nil
 			}, 2*time.Minute, 1*time.Second).Should(gomega.BeNil())
 
diff --git a/e2etest/pkg/mac/mac.go b/e2etest/pkg/mac/mac.go
index cf31a796..de9de52d 100644
--- a/e2etest/pkg/mac/mac.go
+++ b/e2etest/pkg/mac/mac.go
@@ -3,17 +3,28 @@
 package mac
 
 import (
+	"encoding/json"
 	"fmt"
 	"net"
 	"regexp"
 	"strings"
 
+	"github.com/pkg/errors"
 	"go.universe.tf/metallb/e2etest/pkg/executor"
 	"go.universe.tf/metallb/e2etest/pkg/routes"
 
 	corev1 "k8s.io/api/core/v1"
 )
 
+type ipIface struct {
+	Ifname   string `json:"ifname"`
+	Address  string `json:"address"`
+	AddrInfo []struct {
+		Local     string `json:"local"`
+		Prefixlen int    `json:"prefixlen"`
+	} `json:"addr_info"`
+}
+
 // ForIP returns the MAC address of a given IP.
 func ForIP(ip string, exec executor.Executor) (net.HardwareAddr, error) {
 	macRe := regexp.MustCompile("([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})")
@@ -105,3 +116,61 @@ func UpdateNodeCache(nodes []corev1.Node, exec executor.Executor) error {
 
 	return nil
 }
+
+// RequestAddressResolution does an ARP/NS request for the given IP.
+func RequestAddressResolution(ip string, exc executor.Executor) error {
+	netIP := net.ParseIP(ip)
+	if netIP == nil {
+		return fmt.Errorf("failed to convert %s to net.IP", ip)
+	}
+
+	iface, err := ifaceForIPNetwork(netIP, exc)
+	if err != nil {
+		return err
+	}
+
+	var cmd string
+	var args []string
+	if netIP.To4() != nil {
+		cmd = "arping"
+		args = []string{"-c", "1", "-I", iface, netIP.String()}
+	} else {
+		cmd = "ndisc6"
+		args = []string{netIP.String(), iface}
+	}
+
+	out, err := exc.Exec(cmd, args...)
+	if err != nil {
+		return errors.Wrapf(err, out)
+	}
+
+	return nil
+}
+
+// ifaceForIPNetwork returns the interface name that is in the same network as the IP.
+func ifaceForIPNetwork(ip net.IP, exec executor.Executor) (string, error) {
+	ifaces := []ipIface{}
+	res, err := exec.Exec("ip", "--json", "address", "show")
+	if err != nil {
+		return "", errors.Wrapf(err, "Failed to list interfaces")
+	}
+
+	err = json.Unmarshal([]byte(res), &ifaces)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to parse interfaces")
+	}
+
+	for _, intf := range ifaces {
+		for _, addr := range intf.AddrInfo {
+			_, ipnet, err := net.ParseCIDR(fmt.Sprintf("%s/%d", addr.Local, addr.Prefixlen))
+			if err != nil {
+				return "", errors.Wrap(err, fmt.Sprintf("failed to parse CIDR from interface %s %s", intf.Ifname, fmt.Sprintf("%s/%d", addr.Local, addr.Prefixlen)))
+			}
+			if ipnet.Contains(ip) {
+				return intf.Ifname, nil
+			}
+		}
+	}
+
+	return "", fmt.Errorf("no interface found in %s network", ip)
+}
diff --git a/internal/bgp/frr/debounce_test.go b/internal/bgp/frr/debounce_test.go
new file mode 100644
index 00000000..0dacb71f
--- /dev/null
+++ b/internal/bgp/frr/debounce_test.go
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier:Apache-2.0
+
+package frr
+
+import (
+	"testing"
+	"time"
+)
+
+const timer = 10 * time.Millisecond
+
+func TestDebounce(t *testing.T) {
+	result := make(chan *frrConfig, 10) // buffered to accomodate spurious rewrites
+	dummyUpdate := func(config *frrConfig) {
+		result <- config
+	}
+
+	reload := make(chan *frrConfig)
+	defer close(reload)
+	debouncer(dummyUpdate, reload, timer)
+	reload <- &frrConfig{Hostname: "1"}
+	reload <- &frrConfig{Hostname: "2"}
+	reload <- &frrConfig{Hostname: "3"}
+	if len(result) != 0 {
+		t.Fatal("received update before time")
+	}
+	time.Sleep(3 * timer)
+	if len(result) != 1 {
+		t.Fatal("received extra updates", len(result))
+	}
+	updated := <-result
+	if updated.Hostname != "3" {
+		t.Fatal("Config was not updated")
+	}
+	reload <- &frrConfig{Hostname: "3"}
+	reload <- &frrConfig{Hostname: "4"}
+	reload <- &frrConfig{Hostname: "5"}
+	time.Sleep(3 * timer)
+	if len(result) != 1 {
+		t.Fatal("received extra updates", len(result))
+	}
+	updated = <-result
+	if updated.Hostname != "5" {
+		t.Fatal("Config was not updated")
+	}
+}
diff --git a/internal/bgp/frr/docker_test.go b/internal/bgp/frr/docker_test.go
index 2fe193c3..b7c09b13 100644
--- a/internal/bgp/frr/docker_test.go
+++ b/internal/bgp/frr/docker_test.go
@@ -11,6 +11,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/ory/dockertest/v3"
 	"github.com/pkg/errors"
@@ -62,6 +63,10 @@ func TestMain(m *testing.M) {
 		log.Fatalf("failed to move vtysh.conf inside the container - res %d %s %s", resCode, err, buf.String())
 	}
 
+	// override reloadConfig so it doesn't try to reload it.
+	debounceTimeout = time.Millisecond
+	reloadConfig = func() error { return nil }
+
 	retCode := m.Run()
 	// You can't defer this because os.Exit doesn't care for defer
 	if err := pool.Purge(containerHandle); err != nil {
diff --git a/internal/bgp/frr/frr_bfd_test.go b/internal/bgp/frr/frr_bfd_test.go
index 2fd1a673..62cd2720 100644
--- a/internal/bgp/frr/frr_bfd_test.go
+++ b/internal/bgp/frr/frr_bfd_test.go
@@ -36,7 +36,10 @@ func TestBFDProfileNoSessions(t *testing.T) {
 			MinimumTTL:       uint32Ptr(60),
 		},
 	}
-	sessionManager := NewSessionManager()
+	l := log.NewNopLogger()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
+
 	err := sessionManager.SyncBFDProfiles(pp)
 	if err != nil {
 		t.Fatalf("Failed to sync bfd profiles: %s", err)
@@ -65,7 +68,10 @@ func TestBFDProfileCornerCases(t *testing.T) {
 		},
 	}
 
-	sessionManager := NewSessionManager()
+	l := log.NewNopLogger()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
+
 	err := sessionManager.SyncBFDProfiles(pp)
 	if err != nil {
 		t.Fatalf("Failed to sync bfd profiles: %s", err)
@@ -104,13 +110,15 @@ func TestBFDWithSession(t *testing.T) {
 		},
 	}
 
-	sessionManager := NewSessionManager()
+	l := log.NewNopLogger()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
+
 	err := sessionManager.SyncBFDProfiles(pp)
 	if err != nil {
 		t.Fatalf("Failed to sync bfd profiles %s", err)
 	}
 
-	l := log.NewNopLogger()
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, 2*time.Second, "password", "hostname", "foo")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -143,7 +151,10 @@ func TestBFDProfileAllDefault(t *testing.T) {
 		},
 	}
 
-	sessionManager := NewSessionManager()
+	l := log.NewNopLogger()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
+
 	err := sessionManager.SyncBFDProfiles(pp)
 	if err != nil {
 		t.Fatalf("Failed to sync bfd profiles %s", err)
diff --git a/internal/bgp/frr/frr_test.go b/internal/bgp/frr/frr_test.go
index 56eac1a8..c4869c22 100644
--- a/internal/bgp/frr/frr_test.go
+++ b/internal/bgp/frr/frr_test.go
@@ -49,20 +49,17 @@ func testGenerateFileNames(t *testing.T) (string, string) {
 
 func testSetup(t *testing.T) {
 	configFile, _ := testGenerateFileNames(t)
-
+	os.Setenv("FRR_CONFIG_FILE", configFile)
 	_, err := os.Create(configFile)
 	if err != nil {
 		t.Fatalf("cannot create config file %s: %s", configFile, err)
 	}
 
 	osHostname = testOsHostname
-
-	os.Setenv("FRR_CONFIG_FILE", configFile)
-	// override reloadConfig so it doesn't try to reload it.
-	reloadConfig = func() error { return nil }
 }
 
 func testCheckConfigFile(t *testing.T) {
+	time.Sleep(10 * time.Millisecond) // give time to the debouncer to write the file
 	configFile, goldenFile := testGenerateFileNames(t)
 
 	if *update {
@@ -81,7 +78,8 @@ func TestSingleSession(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -95,7 +93,9 @@ func TestSingleSessionClose(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
+
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -108,7 +108,8 @@ func TestTwoSessions(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -127,7 +128,8 @@ func TestTwoSessionsDuplicate(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -146,7 +148,8 @@ func TestTwoSessionsDuplicateRouter(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session1, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -165,7 +168,8 @@ func TestSingleAdvertisement(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -194,7 +198,8 @@ func TestSingleAdvertisementNoRouterID(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, nil, 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -223,7 +228,8 @@ func TestSingleAdvertisementInvalidPrefix(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -249,7 +255,8 @@ func TestSingleAdvertisementInvalidNoPort(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err == nil {
 		session.Close()
@@ -264,7 +271,8 @@ func TestSingleAdvertisementInvalidNextHop(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -292,7 +300,8 @@ func TestSingleAdvertisementStop(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -326,7 +335,8 @@ func TestSingleAdvertisementChange(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
@@ -370,7 +380,8 @@ func TestTwoAdvertisements(t *testing.T) {
 	testSetup(t)
 
 	l := log.NewNopLogger()
-	sessionManager := NewSessionManager()
+	sessionManager := NewSessionManager(l)
+	defer close(sessionManager.reloadConfig)
 	session, err := sessionManager.NewSession(l, "10.2.2.254:179", net.ParseIP("10.1.1.254"), 100, net.ParseIP("10.1.1.254"), 200, time.Second, time.Second, "password", "hostname", "")
 	if err != nil {
 		t.Fatalf("Could not create session: %s", err)
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 8a54a014..757aca25 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -792,7 +792,7 @@ bfd-profiles:
 
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			got, err := Parse([]byte(test.raw))
+			got, err := Parse([]byte(test.raw), DontValidate)
 			if err != nil && test.want != nil {
 				t.Errorf("%q: parse failed: %s", test.desc, err)
 				return
diff --git a/internal/config/validate_test.go b/internal/config/validate_test.go
new file mode 100644
index 00000000..f1738fa7
--- /dev/null
+++ b/internal/config/validate_test.go
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier:Apache-2.0
+
+package config
+
+import (
+	"testing"
+)
+
+func TestValidate(t *testing.T) {
+	tests := []struct {
+		desc     string
+		config   *configFile
+		mustFail bool
+	}{
+		{
+			desc: "peer with bfd profile",
+			config: &configFile{
+				Peers: []peer{
+					{
+						Addr:       "1.2.3.4",
+						BFDProfile: "foo",
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "bfd profile set",
+			config: &configFile{
+				Peers: []peer{
+					{
+						Addr: "1.2.3.4",
+					},
+				},
+				BFDProfiles: []bfdProfile{
+					{
+						Name: "default",
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "v6 address",
+			config: &configFile{
+				Pools: []addressPool{
+					{
+						Protocol:  BGP,
+						Addresses: []string{"2001:db8::/64"},
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "keepalive time",
+			config: &configFile{
+				Peers: []peer{
+					{
+						KeepaliveTime: "1s",
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "aggregation v6",
+			config: &configFile{
+				Pools: []addressPool{
+					{
+						Name: "foo",
+						BGPAdvertisements: []bgpAdvertisement{
+							{
+								AggregationLengthV6: intPtr(47),
+							},
+						},
+					},
+				},
+			},
+			mustFail: true,
+		},
+		{
+			desc: "should pass",
+			config: &configFile{
+				Peers: []peer{
+					{
+						Addr: "1.2.3.4",
+					},
+				},
+			},
+			mustFail: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			err := DiscardFRROnly(test.config)
+			if test.mustFail && err == nil {
+				t.Fatalf("Expected error for %s", test.desc)
+			}
+			if !test.mustFail && err != nil {
+				t.Fatalf("Not expected error %s for %s", err, test.desc)
+			}
+		})
+	}
+}
+
+func intPtr(i int) *int {
+	return &i
+}
diff --git a/speaker/bgp_controller_test.go b/speaker/bgp_controller_test.go
index 69b07cd8..0b8e46ab 100644
--- a/speaker/bgp_controller_test.go
+++ b/speaker/bgp_controller_test.go
@@ -95,7 +95,7 @@ type fakeBGP struct {
 	sessionManager fakeBGPSessionManager
 }
 
-func (f *fakeBGP) NewSessionManager(_ bgpImplementation) bgp.SessionManager {
+func (f *fakeBGP) NewSessionManager(_ bgpImplementation, _ log.Logger) bgp.SessionManager {
 	f.sessionManager.t = f.t
 	f.sessionManager.gotAds = make(map[string][]*bgp.Advertisement)
 
diff --git a/speaker/validate_test.go b/speaker/validate_test.go
deleted file mode 100644
index 4a407085..00000000
--- a/speaker/validate_test.go
+++ /dev/null
@@ -1,74 +0,0 @@
-// SPDX-License-Identifier:Apache-2.0
-
-package main
-
-import (
-	"net"
-	"testing"
-
-	"go.universe.tf/metallb/internal/config"
-	"k8s.io/apimachinery/pkg/labels"
-)
-
-func TestValidate(t *testing.T) {
-	tests := []struct {
-		desc     string
-		config   *config.Config
-		mustFail bool
-	}{
-		{
-			desc: "peer with bfd profile",
-			config: &config.Config{
-				Peers: []*config.Peer{
-					{
-						Addr:          net.ParseIP("1.2.3.4"),
-						NodeSelectors: []labels.Selector{labels.Everything()},
-						BFDProfile:    "foo",
-					},
-				},
-			},
-			mustFail: true,
-		},
-		{
-			desc: "bfd profile set",
-			config: &config.Config{
-				Peers: []*config.Peer{
-					{
-						Addr:          net.ParseIP("1.2.3.4"),
-						NodeSelectors: []labels.Selector{labels.Everything()},
-					},
-				},
-				BFDProfiles: map[string]*config.BFDProfile{
-					"default": {
-						Name: "default",
-					},
-				},
-			},
-			mustFail: true,
-		},
-		{
-			desc: "should pass",
-			config: &config.Config{
-				Peers: []*config.Peer{
-					{
-						Addr:          net.ParseIP("1.2.3.4"),
-						NodeSelectors: []labels.Selector{labels.Everything()},
-					},
-				},
-			},
-			mustFail: false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.desc, func(t *testing.T) {
-			err := validateFRROnlyConfiguration(test.config)
-			if test.mustFail && err == nil {
-				t.Fatalf("Expected error for %s", test.desc)
-			}
-			if !test.mustFail && err != nil {
-				t.Fatalf("Not expected error %s for %s", err, test.desc)
-			}
-		})
-	}
-}