Repository: metallb/metallb
Pull request #2693 (2025-02-14 15:10:19)

Single local pref check: isolate v4 and v6 aggregation length
<!-- Thanks for sending a pull request!
1. If this is your first time, please read the [contributing guide](https://metallb.universe.tf/community/#contributing)
2. For non-trivial pull requests, please [file an
   issue](https://github.com/metallb/metallb/issues/new) first, and get
   agreement that the change is a good idea, and a general guideline
   for how it should be implemented, before sending code. Large PRs
   that weren't first discussed and agreed upon in an issue won't be
   accepted.
3. If the PR fixes a particular bug, please include the words "Fixed
   #<issue number>" in the PR text, so that the bug auto-closes when
   the PR is merged.
-->

**Is this a BUG FIX or a FEATURE ?**:

> Uncomment only one, leave it on its own line:
>
/kind bug
> /kind cleanup
> /kind feature
> /kind design
> /kind flake
> /kind failing
> /kind documentation
> /kind regression

**What this PR does / why we need it**:
The current check allows different local preferences for the same pool only if both the v4 aggregation length and the v6 aggregation length are different, even if the pool does not contain any v6 (or v4) addresses.

Here we narrow the check, checking both lengths only if the pool contains both families, and otherwise checking each family alone.


**Special notes for your reviewer**:

**Release note**:
<!--  Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. Follow the instructions for writing a release note from k8s: https://git.k8s.io/community/contributors/guide/release-notes.md
3. If no release note is required, just write "NONE".
-->

```release-note
Allow different local preferences if referring to different aggregation lengths. The previous check was too restrictive and was checking that both aggregation length were empty (even with v4 only addresses).
```

Fixes https://github.com/metallb/metallb/issues/2345



Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index ee6e51cf..107277e8 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -14,6 +14,7 @@ on:
       - 'LICENSE'
       - 'netlify.toml'
       - 'README.md'
+      - 'ADOPTERS.md'
     branches:
       - "main"
       - "v**"
diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml
index e195c14f..af55217c 100644
--- a/.github/workflows/publish.yaml
+++ b/.github/workflows/publish.yaml
@@ -130,11 +130,18 @@ jobs:
           git config user.name "$GITHUB_ACTOR"
           git config user.email "$GITHUB_ACTOR@users.noreply.github.com"
 
+      - name: Log into Quay
+        uses: docker/login-action@v3.3.0
+        with:
+          registry: quay.io
+          username: ${{ secrets.QUAY_USER }}
+          password: ${{ secrets.QUAY_PASSWORD }}
+
       - name: Chart releaser
         if: startsWith(github.ref, 'refs/tags/v') # we craft releases only for tags
         run: |
           # Download chart releaser
-          curl -sSLo cr.tar.gz "https://github.com/helm/chart-releaser/releases/download/v1.4.0/chart-releaser_1.4.0_linux_amd64.tar.gz"
+          curl -sSLo cr.tar.gz "https://github.com/helm/chart-releaser/releases/download/v1.7.0/chart-releaser_1.7.0_linux_amd64.tar.gz"
           tar -xzf cr.tar.gz
           rm -f cr.tar.gz
           repo=$(basename "$GITHUB_REPOSITORY")
@@ -144,6 +151,11 @@ jobs:
           exists=$(curl -s -H "Accept: application/vnd.github.v3+json" https://github.com/$GITHUB_REPOSITORY/releases/tag/$repo-chart-$tag -w %{http_code} -o /dev/null)
           helm repo add frrk8s https://metallb.github.io/frr-k8s
           if [[ $exists != "200" ]]; then
+            echo "Updated chart version"
+            sed -i -E 's/0.0.0/'"$tag"'/g' charts/metallb/charts/crds/Chart.yaml
+            sed -i -E 's/0.0.0/'"$tag"'/g' charts/metallb/Chart.yaml
+            helm dependency update charts/$repo
+
             echo "Creating release..."
             # package chart
             ./cr package charts/$repo
@@ -160,6 +172,8 @@ jobs:
                 --index-path index.yaml \
                 --release-name-template "{{ .Name }}-chart-{{ .Version }}" \
                 --push
+            # upload chart to quay
+            helm push .cr-release-packages/*.tgz oci://quay.io/metallb/chart
           else
             echo "Release already exists"
           fi
diff --git a/.golangci.exclude b/.golangci.exclude
deleted file mode 100644
index 306a6f62..00000000
--- a/.golangci.exclude
+++ /dev/null
@@ -1,1 +0,0 @@
-(github.com/go-kit/log.Logger).Log
diff --git a/.golangci.yml b/.golangci.yml
index 6d67cedc..fabfe013 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -7,7 +7,7 @@ linters:
   - gofmt
   - ginkgolinter
   - dogsled
-  - exportloopref
+  - copyloopvar
   - gocritic
   - misspell
   - nolintlint
@@ -20,7 +20,8 @@ linters:
   - wastedassign
 linters-settings:
   errcheck:
-    exclude: .golangci.exclude
+    exclude-functions:
+      - (github.com/go-kit/log.Logger).Log
   godot:
     scope: toplevel
     capital: true
diff --git a/ADOPTERS.md b/ADOPTERS.md
new file mode 100644
index 00000000..3de413e4
--- /dev/null
+++ b/ADOPTERS.md
@@ -0,0 +1,24 @@
+
+> **Note**
+> Do you want to add yourself to this list? Simply fork the repository and open a PR with the required change.
+> We have a short description of the adopter types at the bottom of this page. Each adopter type is in alphabetical order.
+
+# MetalLB Adopters
+
+| Type | Name | Since | Website | Use-Case |
+|:-|:-|:-|:-|:-|
+| End-user | 1&1 Mail & Media | 2018 | [link](https://www.mail-and-media.com/) | MetalLB is the main way to get network traffic into the clusters driving most of the internal services for WEB.DE, GMX and mail.com |
+| End-user | Deutsche Telekom AG | 2021 | [link](https://www.telekom.com) | MetalLB is used as the LoadBalancer in our own horizontal Telco Cloud platform [Das SCHIFF / T-CaaS](https://github.com/telekom/das-schiff) among several other use cases |
+| Vendor | Red Hat, Inc. | 2022 | [link](https://www.redhat.com) | MetalLB is distributed together with Red Hat's Openshift containers platform, as the supported implementation of LoadBalancer services on Bare Metal|
+| Vendor | SUSE | 2023 | [link](https://www.suse.com) | MetalLB is a core part of the SUSE Edge solution serving as a LoadBalancer on Bare Metal, as well as load balancing the Kubernetes API in highly available topologies |
+| Vendor | Nokia EDA | 2024 | [link](https://docs.eda.dev) | MetalLB implements the LoadBalancer controller role in the Nokia Event Driven Automation (EDA) product|
+| End-user | Kiratech S.p.A. | 2022 | [link](https://www.kiratech.it/) | MetalLB is the LoadBalancer reference technology in our training courses laboratories, which are [public and published under MIT license](https://github.com/kiratech/labs) |
+
+
+### Adopter Types
+
+**End-user**: The organization runs MetalLB in production in some way.
+
+**Integration**: The organization has a product that integrates with MetalLB, but does not contain MetalLB.
+
+**Vendor**: The organization packages MetalLB in their product and sells it as part of their product.
diff --git a/charts/metallb/templates/speaker.yaml b/charts/metallb/templates/speaker.yaml
index 9d9297a8..8c3dbf28 100644
--- a/charts/metallb/templates/speaker.yaml
+++ b/charts/metallb/templates/speaker.yaml
@@ -65,8 +65,8 @@ data:
     # 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 -p 0"
+    zebra_options="  -A 127.0.0.1 -s 90000000 --limit-fds 100000"
+    bgpd_options="   -A 127.0.0.1 -p 0 --limit-fds 100000"
     ospfd_options="  -A 127.0.0.1"
     ospf6d_options=" -A ::1"
     ripd_options="   -A 127.0.0.1"
@@ -79,8 +79,8 @@ data:
     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"
+    staticd_options="-A 127.0.0.1 --limit-fds 100000"
+    bfdd_options="   -A 127.0.0.1 --limit-fds 100000"
     fabricd_options="-A 127.0.0.1"
     vrrpd_options="  -A 127.0.0.1"
 
diff --git a/config/frr/frr-cm.yaml b/config/frr/frr-cm.yaml
index 5cf57aa9..3797a7bb 100644
--- a/config/frr/frr-cm.yaml
+++ b/config/frr/frr-cm.yaml
@@ -45,8 +45,8 @@ data:
     # 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 -p 0"
+    zebra_options="  -A 127.0.0.1 -s 90000000 --limit-fds 100000"
+    bgpd_options="   -A 127.0.0.1 -p 0 --limit-fds 100000"
     ospfd_options="  -A 127.0.0.1"
     ospf6d_options=" -A ::1"
     ripd_options="   -A 127.0.0.1"
@@ -59,8 +59,8 @@ data:
     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"
+    staticd_options="-A 127.0.0.1 --limit-fds 100000"
+    bfdd_options="   -A 127.0.0.1 --limit-fds 100000"
     fabricd_options="-A 127.0.0.1"
     vrrpd_options="  -A 127.0.0.1"
 
diff --git a/config/manifests/metallb-frr-prometheus.yaml b/config/manifests/metallb-frr-prometheus.yaml
index f294d0cd..b794ab71 100644
--- a/config/manifests/metallb-frr-prometheus.yaml
+++ b/config/manifests/metallb-frr-prometheus.yaml
@@ -1871,8 +1871,8 @@ data:
     # 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 -p 0"
+    zebra_options="  -A 127.0.0.1 -s 90000000 --limit-fds 100000"
+    bgpd_options="   -A 127.0.0.1 -p 0 --limit-fds 100000"
     ospfd_options="  -A 127.0.0.1"
     ospf6d_options=" -A ::1"
     ripd_options="   -A 127.0.0.1"
@@ -1885,8 +1885,8 @@ data:
     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"
+    staticd_options="-A 127.0.0.1 --limit-fds 100000"
+    bfdd_options="   -A 127.0.0.1 --limit-fds 100000"
     fabricd_options="-A 127.0.0.1"
     vrrpd_options="  -A 127.0.0.1"
 
diff --git a/config/manifests/metallb-frr.yaml b/config/manifests/metallb-frr.yaml
index 7d1f88c3..9e343368 100644
--- a/config/manifests/metallb-frr.yaml
+++ b/config/manifests/metallb-frr.yaml
@@ -1816,8 +1816,8 @@ data:
     # 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 -p 0"
+    zebra_options="  -A 127.0.0.1 -s 90000000 --limit-fds 100000"
+    bgpd_options="   -A 127.0.0.1 -p 0 --limit-fds 100000"
     ospfd_options="  -A 127.0.0.1"
     ospf6d_options=" -A ::1"
     ripd_options="   -A 127.0.0.1"
@@ -1830,8 +1830,8 @@ data:
     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"
+    staticd_options="-A 127.0.0.1 --limit-fds 100000"
+    bfdd_options="   -A 127.0.0.1 --limit-fds 100000"
     fabricd_options="-A 127.0.0.1"
     vrrpd_options="  -A 127.0.0.1"
 
diff --git a/controller/main.go b/controller/main.go
index bfcb9588..5597f8fb 100644
--- a/controller/main.go
+++ b/controller/main.go
@@ -79,11 +79,19 @@ func (c *controller) SetBalancer(l log.Logger, name string, svcRo *v1.Service, _
 	syncStateRes := controllers.SyncStateSuccess
 
 	prevIPs := c.ips.IPs(name)
+	prevAllocKey := c.ips.AllocationKey(name)
 
 	if c.convergeBalancer(l, name, svc) != nil {
 		syncStateRes = controllers.SyncStateErrorNoRetry
 	}
 
+	newAllocKey := c.ips.AllocationKey(name)
+
+	if prevAllocKey != newAllocKey {
+		level.Debug(l).Log("event", "allocation key changed", "msg", "allocation changed for shared service, reprocessing")
+		syncStateRes = controllers.SyncStateReprocessAll
+	}
+
 	if reflect.DeepEqual(svcRo, svc) {
 		level.Debug(l).Log("event", "noChange", "msg", "service converged, no change")
 		return syncStateRes
diff --git a/controller/service.go b/controller/service.go
index cb7a7c7a..f26bad08 100644
--- a/controller/service.go
+++ b/controller/service.go
@@ -88,6 +88,12 @@ func (c *controller) convergeBalancer(l log.Logger, key string, svc *v1.Service)
 	if svc.Spec.IPFamilyPolicy != nil {
 		familyPolicy = *(svc.Spec.IPFamilyPolicy)
 	}
+	if familyPolicy == v1.IPFamilyPolicyRequireDualStack && len(svc.Spec.ClusterIPs) < 2 {
+		level.Error(l).Log("event", "clearAssignment", "reason", "requires dual stack but clusterips are not dual stack", "clusterips", svc.Spec.ClusterIPs)
+		c.client.Errorf(svc, "not enough clusterips", "Service requires dual stack but not enough clusterips %v", svc.Spec.ClusterIPs)
+		c.clearServiceState(key, svc)
+		return ErrConverge
+	}
 
 	if len(lbIPs) == 0 {
 		c.clearServiceState(key, svc)
diff --git a/design/gracefulrestart-bgp.md b/design/gracefulrestart-bgp.md
index c14f90a9..6fd51bd1 100644
--- a/design/gracefulrestart-bgp.md
+++ b/design/gracefulrestart-bgp.md
@@ -106,30 +106,19 @@ $ cat frr.log
 {{% /notice %}}
 
 
-### GR and Cordoned Node
-
-GR is orthogonal to any Kubernetes node admin [operation](https://kubernetes.io/docs/tasks/administer-cluster/safely-drain-node/)
-and the expected behavior does not change. When admin drains/cordons a node from the cluster, the node will be
-labeled as Unschedulable. When that happens all routes towards that node will
-be removed as normal procedure\*. The BGP peering will remain because Daemonsets
-pods continue to run on Unschedulable nodes (`--ignore-daemonsets`). The BGP
-peer will not have anymore routes to the node, so UDP packets will be
-redirected to other nodes, existing TCP connections will break\*\*, but no new
-traffic will be blackholed. After all that, the node might be hard rebooted or removed,
-then the Daemonset pod will be removed, BGP will be properly terminated
-(TCP connection close), GR will start on the peer but no routes exists to be staled.
+### GR when Drain and Reboot Node
 
-\* If service is of traffic policy "Local", then the routes will be removed
-faster because any pod that is endpoint to the service will be removed from the node.
+When GR is enabled and a cluster admin drains a node, the BGP peering towards that
+node will remain because Daemonsets pods continue to run on Unschedulable nodes
+(`--ignore-daemonsets`). If there is service of traffic policy "Cluster", then that
+node might continue to forward traffic. If a node reboot/shutdown follows, then
+traffic will be blackholed until the GR timer (2min) will be expired in the peer
+and remove the route towards the stopped node.
+[info](https://github.com/metallb/metallb/issues/2677)
 
-\*\* TCP connection could remain established if the service is of traffic
-policy "Cluster" and if Kubernetes primary CNI plugin implements consistent-hashing
-[[Cilium]](https://docs.cilium.io/en/stable/network/kubernetes/kubeproxy-free/#maglev-consistent-hashing).
+\* If service is of traffic policy "Local", then the routes will be removed
+because any pod that is endpoint to the service will be removed from the node.
 
-**Note:** when BGP peers are removed or added, rebalancing might take place due to
-the load balancing hashing mechanism of the external peer, e.g. client A
-that had a connection to node X can switch to node Y just because node Z was
-removed from the cluster.
 
 
 ### GR and BFD
diff --git a/internal/allocator/allocation.go b/internal/allocator/allocation.go
index 19ccf702..58ad87c2 100644
--- a/internal/allocator/allocation.go
+++ b/internal/allocator/allocation.go
@@ -58,6 +58,14 @@ func (a *Allocation) selectIPsForFamilyAndPolicy(
 	ipv4 := a.getIPForFamily(ipfamily.IPv4)
 	ipv6 := a.getIPForFamily(ipfamily.IPv6)
 
+	// if we don't have dual cluster ips, we should align the lb ip to the clusterip
+	if serviceIPFamily == ipfamily.IPv4 {
+		return []net.IP{ipv4}, nil
+	}
+	if serviceIPFamily == ipfamily.IPv6 {
+		return []net.IP{ipv6}, nil
+	}
+
 	switch serviceIPFamilyPolicy {
 	case v1.IPFamilyPolicySingleStack:
 		if ip := a.getIPForFamily(serviceIPFamily); ip != nil {
diff --git a/internal/allocator/allocator.go b/internal/allocator/allocator.go
index 971ff70c..5358cd48 100644
--- a/internal/allocator/allocator.go
+++ b/internal/allocator/allocator.go
@@ -560,6 +560,13 @@ func (a *Allocator) IPs(svc string) []net.IP {
 	return nil
 }
 
+func (a *Allocator) AllocationKey(svc string) string {
+	if alloc := a.allocated[svc]; alloc != nil {
+		return alloc.key.backend + alloc.key.sharing
+	}
+	return ""
+}
+
 // PoolForIP returns the pool structure associated with an IP.
 func (a *Allocator) PoolForIP(ips []net.IP) *config.Pool {
 	return poolFor(a.pools.ByName, ips)
diff --git a/internal/bgp/frrk8s/frrk8s.go b/internal/bgp/frrk8s/frrk8s.go
index 9cec413d..f5a3a23b 100644
--- a/internal/bgp/frrk8s/frrk8s.go
+++ b/internal/bgp/frrk8s/frrk8s.go
@@ -325,7 +325,6 @@ func (sm *sessionManager) updateConfig() error {
 	}
 
 	for _, bfd := range sm.bfdProfiles {
-		bfd := bfd
 		toAdd := frrv1beta1.BFDProfile{
 			Name:             bfd.Name,
 			ReceiveInterval:  bfd.ReceiveInterval,
diff --git a/internal/config/config.go b/internal/config/config.go
index c6d28be1..78be4036 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -444,7 +444,6 @@ func peerFromCR(p metallbv1beta2.BGPPeer, passwordSecrets map[string]corev1.Secr
 
 	var nodeSels []labels.Selector
 	for _, s := range p.Spec.NodeSelectors {
-		s := s // so we can use &s
 		labelSelector, err := metav1.LabelSelectorAsSelector(&s)
 		if err != nil {
 			return nil, errors.Join(err, fmt.Errorf("failed to convert peer %s node selector", p.Name))
@@ -565,6 +564,12 @@ func addressPoolServiceAllocationsFromCR(p metallbv1beta1.IPAddressPool, namespa
 		}
 		poolNamespaces.Insert(poolNs)
 	}
+	serviceAllocations := &ServiceAllocation{Priority: p.Spec.AllocateTo.Priority, Namespaces: poolNamespaces}
+	if len(poolNamespaces) == 0 && len(p.Spec.AllocateTo.NamespaceSelectors) == 0 && len(p.Spec.AllocateTo.ServiceSelectors) == 0 {
+		// If no specific namespaces or service selectors are set, match everything
+		serviceAllocations.ServiceSelectors = []labels.Selector{labels.Everything()}
+		return serviceAllocations, nil
+	}
 	err := validateLabelSelectorDuplicate(p.Spec.AllocateTo.NamespaceSelectors, "namespaceSelectors")
 	if err != nil {
 		return nil, err
@@ -573,7 +578,6 @@ func addressPoolServiceAllocationsFromCR(p metallbv1beta1.IPAddressPool, namespa
 	if err != nil {
 		return nil, err
 	}
-	serviceAllocations := &ServiceAllocation{Priority: p.Spec.AllocateTo.Priority, Namespaces: poolNamespaces}
 	for i := range p.Spec.AllocateTo.NamespaceSelectors {
 		l, err := metav1.LabelSelectorAsSelector(&p.Spec.AllocateTo.NamespaceSelectors[i])
 		if err != nil {
@@ -894,7 +898,7 @@ func validateBGPAdvPerPool(adv *BGPAdvertisement, pool *Pool) error {
 	// Verify that BGP ADVs set a unique local preference value per BGP update.
 	for _, bgpAdv := range pool.BGPAdvertisements {
 		if adv.LocalPref != bgpAdv.LocalPref {
-			if !advertisementsAreCompatible(adv, bgpAdv) {
+			if !advertisementsAreCompatible(adv, bgpAdv, pool) {
 				return fmt.Errorf("invalid local preference %d: local preferernce %d was "+
 					"already set for the same type of BGP update. Check existing BGP advertisements "+
 					"with common pools and aggregation lengths", adv.LocalPref, bgpAdv.LocalPref)
@@ -905,8 +909,8 @@ func validateBGPAdvPerPool(adv *BGPAdvertisement, pool *Pool) error {
 	return nil
 }
 
-func advertisementsAreCompatible(newAdv, adv *BGPAdvertisement) bool {
-	if adv.AggregationLength != newAdv.AggregationLength && adv.AggregationLengthV6 != newAdv.AggregationLengthV6 {
+func advertisementsAreCompatible(newAdv, adv *BGPAdvertisement, pool *Pool) bool {
+	if isAggrLengthDifferent(newAdv, adv, pool) {
 		return true
 	}
 
@@ -934,6 +938,37 @@ func advertisementsAreCompatible(newAdv, adv *BGPAdvertisement) bool {
 	return true
 }
 
+func isAggrLengthDifferent(newAdv, adv *BGPAdvertisement, pool *Pool) bool {
+	var hasV4, hasV6 bool
+	for _, cidrs := range pool.cidrsPerAddresses {
+		family := ipfamily.ForCIDR(cidrs[0])
+		if family == ipfamily.IPv4 {
+			hasV4 = true
+		}
+		if family == ipfamily.IPv6 {
+			hasV6 = true
+		}
+		if hasV4 && hasV6 {
+			break
+		}
+	}
+
+	if !hasV6 && !hasV4 { // compatible?!
+		return true
+	}
+	if adv.AggregationLength != newAdv.AggregationLength && !hasV6 {
+		return true
+	}
+	if adv.AggregationLengthV6 != newAdv.AggregationLengthV6 && !hasV4 {
+		return true
+	}
+	// has both, both must be different
+	if adv.AggregationLength != newAdv.AggregationLength && adv.AggregationLengthV6 != newAdv.AggregationLengthV6 {
+		return true
+	}
+	return false
+}
+
 func ParseCIDR(cidr string) ([]*net.IPNet, error) {
 	if !strings.Contains(cidr, "-") {
 		_, n, err := net.ParseCIDR(cidr)
@@ -1041,7 +1076,6 @@ func containsAdvertisement(advs []*L2Advertisement, toCheck *L2Advertisement) bo
 func selectedNodes(nodes []corev1.Node, selectors []metav1.LabelSelector) (map[string]bool, error) {
 	labelSelectors := []labels.Selector{}
 	for _, selector := range selectors {
-		selector := selector // so we can use &selector
 		l, err := metav1.LabelSelectorAsSelector(&selector)
 		if err != nil {
 			return nil, errors.Join(err, fmt.Errorf("invalid label selector %v", selector))
@@ -1069,7 +1103,6 @@ OUTER:
 func selectedPools(pools []metallbv1beta1.IPAddressPool, selectors []metav1.LabelSelector) ([]string, error) {
 	labelSelectors := []labels.Selector{}
 	for _, selector := range selectors {
-		selector := selector // so we can use &selector
 		l, err := metav1.LabelSelectorAsSelector(&selector)
 		if err != nil {
 			return nil, errors.Join(err, fmt.Errorf("invalid label selector %v", selector))
diff --git a/internal/config/validation.go b/internal/config/validation.go
index 66ece4b7..d848580a 100644
--- a/internal/config/validation.go
+++ b/internal/config/validation.go
@@ -3,9 +3,8 @@
 package config
 
 import (
-	"fmt"
-
 	"errors"
+	"fmt"
 
 	metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
 	metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"
@@ -146,17 +145,17 @@ func DiscardNativeOnly(c ClusterResources) error {
 	if len(c.Peers) > 1 {
 		peerAddr := make(map[string]bool)
 		routerID := c.Peers[0].Spec.RouterID
-		peer0 := peerAddressKey(c.Peers[0].Spec)
+		peer0 := peerIdentifier(c.Peers[0].Spec)
 		peerAddr[peer0] = true
 		for _, p := range c.Peers[1:] {
 			if p.Spec.RouterID != routerID {
 				return fmt.Errorf("peer %s has RouterID different from %s, in FRR mode all RouterID must be equal", p.Spec.RouterID, c.Peers[0].Spec.RouterID)
 			}
-			peerKey := peerAddressKey(p.Spec)
-			if _, ok := peerAddr[peerKey]; ok {
+			peerID := peerIdentifier(p.Spec)
+			if _, ok := peerAddr[peerID]; ok {
 				return fmt.Errorf("peer %s already exists, FRR mode doesn't support duplicate BGPPeers", p.Spec.Address)
 			}
-			peerAddr[peerKey] = true
+			peerAddr[peerID] = true
 		}
 	}
 	for _, p := range c.Peers {
@@ -216,8 +215,12 @@ func hasBFDEcho(peer *Peer, bfdProfiles map[string]*BFDProfile) bool {
 	return false
 }
 
-func peerAddressKey(peer metallbv1beta2.BGPPeerSpec) string {
-	return fmt.Sprintf("%s-%s", peer.Address, peer.VRFName)
+func peerIdentifier(peer metallbv1beta2.BGPPeerSpec) string {
+	id := peer.Address
+	if peer.Address == "" {
+		id = peer.Interface
+	}
+	return fmt.Sprintf("%s-%s", id, peer.VRFName)
 }
 
 type poolSelector struct {
diff --git a/internal/k8s/controllers/layer2_status_controller.go b/internal/k8s/controllers/layer2_status_controller.go
index 1ef426f2..1a53b7d3 100644
--- a/internal/k8s/controllers/layer2_status_controller.go
+++ b/internal/k8s/controllers/layer2_status_controller.go
@@ -14,7 +14,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	utilErrors "k8s.io/apimachinery/pkg/util/errors"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -81,17 +81,31 @@ func (r *Layer2StatusReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	var errs []error
 	if len(ipAdvS) == 0 {
 		for key, item := range serviceL2statuses.Items {
-			if item.Status.Node != r.NodeName {
+			if item.Labels[LabelAnnounceNode] != r.NodeName {
 				continue
 			}
 			if err := r.Client.Delete(ctx, &serviceL2statuses.Items[key]); err != nil && !errors.IsNotFound(err) {
 				errs = append(errs, err)
 			}
 		}
-		if len(errs) > 0 {
-			return ctrl.Result{}, utilErrors.NewAggregate(errs)
+		return ctrl.Result{}, utilerrors.NewAggregate(errs)
+	}
+
+	deleteRedundantErrs := []error{}
+	if len(serviceL2statuses.Items) > 1 {
+		// We shouldn't get here, just in case the controller created redundant resources
+		for i := range serviceL2statuses.Items[1:] {
+			if serviceL2statuses.Items[i+1].Labels[LabelAnnounceNode] != r.NodeName {
+				continue
+			}
+			if err := r.Client.Delete(ctx, &serviceL2statuses.Items[i+1]); err != nil && !errors.IsNotFound(err) {
+				deleteRedundantErrs = append(deleteRedundantErrs, err)
+			}
 		}
-		return ctrl.Result{}, nil
+	}
+
+	if len(deleteRedundantErrs) > 0 {
+		return ctrl.Result{}, utilerrors.NewAggregate(deleteRedundantErrs)
 	}
 
 	// creating a brand new cr
@@ -131,8 +145,10 @@ func (r *Layer2StatusReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 	if result == controllerutil.OperationResultCreated {
 		// According to https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/controller/controllerutil#CreateOrPatch
-		// If the object is created for the first time, we have to requeue it to ensure that the status is updated.
-		return ctrl.Result{Requeue: true}, nil
+		// If the object is created, we have to patch it again to ensure the status is created.
+		// This will happen when we reconcile the creation event.
+		level.Debug(r.Logger).Log("controller", "Layer2StatusReconciler", "created state", dumpResource(state))
+		return ctrl.Result{}, nil
 	}
 
 	level.Debug(r.Logger).Log("controller", "Layer2StatusReconciler", "updated state", dumpResource(state))
diff --git a/internal/k8s/controllers/node_controller.go b/internal/k8s/controllers/node_controller.go
index c290a5af..dc8a9790 100644
--- a/internal/k8s/controllers/node_controller.go
+++ b/internal/k8s/controllers/node_controller.go
@@ -97,32 +97,11 @@ func NodeReconcilerPredicate() predicate.Predicate {
 		},
 	}
 
-	nodeSpecSchedulableChanged := predicate.Funcs{
-		UpdateFunc: func(e event.UpdateEvent) bool {
-			oldNode, ok := e.ObjectOld.(*corev1.Node)
-			if !ok {
-				return false
-			}
-
-			newNode, ok := e.ObjectNew.(*corev1.Node)
-			if !ok {
-				return false
-			}
-
-			if oldNode.Spec.Unschedulable != newNode.Spec.Unschedulable {
-				return true
-			}
-
-			return false
-		},
-	}
-
 	return predicate.And(
 		allowDeletions,
 		allowCreations,
 		predicate.Or(
 			nodeConditionNetworkAvailabilityStatusChanged,
-			nodeSpecSchedulableChanged,
 			predicate.LabelChangedPredicate{},
 		),
 	)
diff --git a/internal/k8s/controllers/service_controller_reload.go b/internal/k8s/controllers/service_controller_reload.go
index 6c614ebf..460d5f14 100644
--- a/internal/k8s/controllers/service_controller_reload.go
+++ b/internal/k8s/controllers/service_controller_reload.go
@@ -77,7 +77,6 @@ func (r *ServiceReconciler) reprocessAll(ctx context.Context, req ctrl.Request)
 
 	retry := false
 	for _, service := range sortedServices {
-		service := service // so we can use &service
 		if filterByLoadBalancerClass(&service, r.LoadBalancerClass) {
 			level.Debug(r.Logger).Log("controller", "ServiceReconciler", "filtered service", req.NamespacedName)
 			continue
diff --git a/internal/k8s/nodes/nodes.go b/internal/k8s/nodes/nodes.go
index be7fba2c..6c7e8ad3 100644
--- a/internal/k8s/nodes/nodes.go
+++ b/internal/k8s/nodes/nodes.go
@@ -3,33 +3,12 @@
 package nodes
 
 import (
-	"errors"
 	"net"
 
 	"go.universe.tf/metallb/internal/ipfamily"
 	corev1 "k8s.io/api/core/v1"
 )
 
-func IsNodeAvailable(n *corev1.Node) error {
-	if IsNodeUnschedulable(n) {
-		return errors.New("nodeUnschedulable")
-	}
-
-	if IsNetworkUnavailable(n) {
-		return errors.New("nodeNetworkUnavailable")
-	}
-
-	return nil
-}
-
-func IsNodeUnschedulable(n *corev1.Node) bool {
-	if n == nil {
-		return false
-	}
-
-	return n.Spec.Unschedulable
-}
-
 // IsNetworkUnavailable returns true if the given node NodeNetworkUnavailable condition status is true.
 func IsNetworkUnavailable(n *corev1.Node) bool {
 	return conditionStatus(n, corev1.NodeNetworkUnavailable) == corev1.ConditionTrue
diff --git a/speaker/bgp_controller.go b/speaker/bgp_controller.go
index 655e1b88..04c90615 100644
--- a/speaker/bgp_controller.go
+++ b/speaker/bgp_controller.go
@@ -174,9 +174,9 @@ func (c *bgpController) ShouldAnnounce(l log.Logger, name string, _ []net.IP, po
 		return "notOwner"
 	}
 
-	if err := k8snodes.IsNodeAvailable(nodes[c.myNode]); err != nil {
-		level.Warn(l).Log("event", "skipping should announce bgp", "service", name, "reason", err)
-		return err.Error()
+	if k8snodes.IsNetworkUnavailable(nodes[c.myNode]) {
+		level.Warn(l).Log("event", "skipping should announce bgp", "service", name, "reason", "speaker's node has NodeNetworkUnavailable condition")
+		return "nodeNetworkUnavailable"
 	}
 
 	if !c.ignoreExcludeLB && k8snodes.IsNodeExcludedFromBalancers(nodes[c.myNode]) {
diff --git a/speaker/layer2_controller.go b/speaker/layer2_controller.go
index f433e65e..3a8f0a8e 100644
--- a/speaker/layer2_controller.go
+++ b/speaker/layer2_controller.go
@@ -226,7 +226,7 @@ func (c *layer2Controller) speakersForPool(l log.Logger, name string, pool *conf
 	}
 	res := map[string]bool{}
 	for s := range eligibleNodes {
-		if err := k8snodes.IsNodeAvailable(nodes[s]); err != nil {
+		if k8snodes.IsNetworkUnavailable(nodes[s]) {
 			level.Debug(l).Log("event", "skipping should announce l2", "service", name, "reason", "speaker's node has NodeNetworkUnavailable condition")
 			continue
 		}
diff --git a/speaker/main.go b/speaker/main.go
index 708e3b13..68b8b66e 100644
--- a/speaker/main.go
+++ b/speaker/main.go
@@ -605,9 +605,6 @@ func isNodeAvailableChanged(oldNodes map[string]*v1.Node, newNode *v1.Node) bool
 		return false
 	}
 
-	if k8snodes.IsNodeUnschedulable(oldNode) != k8snodes.IsNodeUnschedulable(newNode) {
-		return true
-	}
 	if k8snodes.IsNetworkUnavailable(oldNode) != k8snodes.IsNetworkUnavailable(newNode) {
 		return true
 	}
diff --git a/tasks.py b/tasks.py
index a3702734..a92d696c 100644
--- a/tasks.py
+++ b/tasks.py
@@ -1130,7 +1130,7 @@ def lint(ctx, env="container"):
     convenient to install the golangci-lint binaries on the host. This can be
     achieved by running `inv lint --env host`.
     """
-    version = "1.63.4"
+    version = "1.64.7"
     golangci_cmd = "golangci-lint run --timeout 10m0s ./..."
 
     if env == "container":
diff --git a/troubleshooting/collect.sh b/troubleshooting/collect.sh
index 7c494e57..ce05ae40 100755
--- a/troubleshooting/collect.sh
+++ b/troubleshooting/collect.sh
@@ -9,7 +9,8 @@ METALLB_NS="${METALLB_NS:-metallb-system}"
 OUTPUT="./report"
 
 function get_metallb_crs() {
-    declare -a METALLB_CRDS=("bgppeers" "bfdprofiles" "bgpAdvertisements" "ipaddresspools" "l2advertisements" "communities")
+    declare -a METALLB_CRDS=("bgppeers" "bfdprofiles" "bgpAdvertisements" "ipaddresspools" 
+    "l2advertisements" "communities" "servicel2statuses" "servicebgpstatuses" "frrconfigurations" "frrnodestates")
     mkdir -p ${OUTPUT}/crds/
 
     for CRD in "${METALLB_CRDS[@]}"; do
diff --git a/website/content/apis/_index.md b/website/content/apis/_index.md
index 91ae9eb6..4228ec6c 100644
--- a/website/content/apis/_index.md
+++ b/website/content/apis/_index.md
@@ -19,6 +19,8 @@ description: MetalLB API reference documentation
 - [Community](#community)
 - [IPAddressPool](#ipaddresspool)
 - [L2Advertisement](#l2advertisement)
+- [ServiceBGPStatus](#servicebgpstatus)
+- [ServiceL2Status](#servicel2status)
 
 
 
@@ -61,6 +63,8 @@ _Appears in:_
 | `minimumTtl` _integer_ | For multi hop sessions only: configure the minimum<br />expected TTL for an incoming BFD control packet. |
 
 
+
+
 #### BGPAdvertisement
 
 
@@ -102,6 +106,8 @@ _Appears in:_
 | `peers` _string array_ | Peers limits the bgppeer to advertise the ips of the selected pools to.<br />When empty, the loadbalancer IP is announced to all the BGPPeers configured. |
 
 
+
+
 #### Community
 
 
@@ -150,6 +156,8 @@ _Appears in:_
 | `communities` _[CommunityAlias](#communityalias) array_ |  |
 
 
+
+
 #### IPAddressPool
 
 
@@ -167,6 +175,7 @@ to LoadBalancer services.
 | `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.<br />Servers should convert recognized schemas to the latest internal value, and<br />may reject unrecognized values.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources |
 | `metadata` _[ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#objectmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. |
 | `spec` _[IPAddressPoolSpec](#ipaddresspoolspec)_ |  |
+| `status` _[IPAddressPoolStatus](#ipaddresspoolstatus)_ |  |
 
 
 #### IPAddressPoolSpec
@@ -186,6 +195,35 @@ _Appears in:_
 | `serviceAllocation` _[ServiceAllocation](#serviceallocation)_ | AllocateTo makes ip pool allocation to specific namespace and/or service.<br />The controller will use the pool with lowest value of priority in case of<br />multiple matches. A pool with no priority set will be used only if the<br />pools with priority can't be used. If multiple matching IPAddressPools are<br />available it will check for the availability of IPs sorting the matching<br />IPAddressPools by priority, starting from the highest to the lowest. If<br />multiple IPAddressPools have the same priority, choice will be random. |
 
 
+#### IPAddressPoolStatus
+
+
+
+IPAddressPoolStatus defines the observed state of IPAddressPool.
+
+_Appears in:_
+- [IPAddressPool](#ipaddresspool)
+
+| Field | Description |
+| --- | --- |
+| `assignedIPv4` _integer_ | AssignedIPv4 is the number of assigned IPv4 addresses. |
+| `assignedIPv6` _integer_ | AssignedIPv6 is the number of assigned IPv6 addresses. |
+| `availableIPv4` _integer_ | AvailableIPv4 is the number of available IPv4 addresses. |
+| `availableIPv6` _integer_ | AvailableIPv6 is the number of available IPv6 addresses. |
+
+
+#### InterfaceInfo
+
+
+
+InterfaceInfo defines interface info of layer2 announcement.
+
+_Appears in:_
+- [MetalLBServiceL2Status](#metallbservicel2status)
+
+| Field | Description |
+| --- | --- |
+| `name` _string_ | Name the name of network interface card |
 
 
 #### L2Advertisement
@@ -224,6 +262,42 @@ _Appears in:_
 | `interfaces` _string array_ | A list of interfaces to announce from. The LB IP will be announced only from these interfaces.<br />If the field is not set, we advertise from all the interfaces on the host. |
 
 
+
+
+#### MetalLBServiceBGPStatus
+
+
+
+MetalLBServiceBGPStatus defines the observed state of ServiceBGPStatus.
+
+_Appears in:_
+- [ServiceBGPStatus](#servicebgpstatus)
+
+| Field | Description |
+| --- | --- |
+| `node` _string_ | Node indicates the node announcing the service. |
+| `serviceName` _string_ | ServiceName indicates the service this status represents. |
+| `serviceNamespace` _string_ | ServiceNamespace indicates the namespace of the service. |
+| `peers` _string array_ | Peers indicate the BGP peers for which the service is configured to be advertised to.<br />The service being actually advertised to a given peer depends on the session state and is not indicated here. |
+
+
+#### MetalLBServiceL2Status
+
+
+
+MetalLBServiceL2Status defines the observed state of ServiceL2Status.
+
+_Appears in:_
+- [ServiceL2Status](#servicel2status)
+
+| Field | Description |
+| --- | --- |
+| `node` _string_ | Node indicates the node that receives the directed traffic |
+| `serviceName` _string_ | ServiceName indicates the service this status represents |
+| `serviceNamespace` _string_ | ServiceNamespace indicates the namespace of the service |
+| `interfaces` _[InterfaceInfo](#interfaceinfo) array_ | Interfaces indicates the interfaces that receive the directed traffic |
+
+
 #### ServiceAllocation
 
 
@@ -241,9 +315,65 @@ _Appears in:_
 | `serviceSelectors` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#labelselector-v1-meta) array_ | ServiceSelectors list of label selector to select service(s) for which ip pool<br />can be used for ip allocation. |
 
 
+#### ServiceBGPStatus
+
+
+
+ServiceBGPStatus exposes the BGP peers a service is configured to be advertised to, per relevant node.
+
+
+
+| Field | Description |
+| --- | --- |
+| `apiVersion` _string_ | `metallb.io/v1beta1`
+| `kind` _string_ | `ServiceBGPStatus`
+| `kind` _string_ | Kind is a string value representing the REST resource this object represents.<br />Servers may infer this from the endpoint the client submits requests to.<br />Cannot be updated.<br />In CamelCase.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds |
+| `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.<br />Servers should convert recognized schemas to the latest internal value, and<br />may reject unrecognized values.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources |
+| `metadata` _[ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#objectmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. |
+| `spec` _[ServiceBGPStatusSpec](#servicebgpstatusspec)_ |  |
+| `status` _[MetalLBServiceBGPStatus](#metallbservicebgpstatus)_ |  |
+
+
+#### ServiceBGPStatusSpec
+
+
+
+ServiceBGPStatusSpec defines the desired state of ServiceBGPStatus.
+
+_Appears in:_
+- [ServiceBGPStatus](#servicebgpstatus)
+
+
+
+#### ServiceL2Status
+
+
+
+ServiceL2Status reveals the actual traffic status of loadbalancer services in layer2 mode.
+
+
+
+| Field | Description |
+| --- | --- |
+| `apiVersion` _string_ | `metallb.io/v1beta1`
+| `kind` _string_ | `ServiceL2Status`
+| `kind` _string_ | Kind is a string value representing the REST resource this object represents.<br />Servers may infer this from the endpoint the client submits requests to.<br />Cannot be updated.<br />In CamelCase.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds |
+| `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.<br />Servers should convert recognized schemas to the latest internal value, and<br />may reject unrecognized values.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources |
+| `metadata` _[ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#objectmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. |
+| `spec` _[ServiceL2StatusSpec](#servicel2statusspec)_ |  |
+| `status` _[MetalLBServiceL2Status](#metallbservicel2status)_ |  |
+
+
+#### ServiceL2StatusSpec
 
 
 
+ServiceL2StatusSpec defines the desired state of ServiceL2Status.
+
+_Appears in:_
+- [ServiceL2Status](#servicel2status)
+
+
 
 
 ## metallb.io/v1beta2
@@ -305,6 +435,8 @@ _Appears in:_
 | `disableMP` _boolean_ | To set if we want to disable MP BGP that will separate IPv4 and IPv6 route exchanges into distinct BGP sessions. |
 
 
+
+
 #### DynamicASNMode
 
 _Underlying type:_ _string_
diff --git a/website/content/faq/_index.md b/website/content/faq/_index.md
index f54ae19b..b598abb2 100644
--- a/website/content/faq/_index.md
+++ b/website/content/faq/_index.md
@@ -71,6 +71,29 @@ metadata:
   namespace: metallb-system
 ```
 
+## How can I understand which node advertises a given Service?
+In addition to logs, Kubernetes Events and Prometheus metrics, it is possible to understand the advertisement status via the ServiceL2Status and ServiceBGPStatus resources that the speakers manage, for example:
+```
+$ kubectl get servicel2statuses -n metallb-system
+NAME       ALLOCATED NODE       SERVICE NAME   SERVICE NAMESPACE
+l2-r8jwb   kind-worker2         service1       ns1
+l2-svkqj   kind-worker          service2       ns2
+
+$ kubectl get servicebgpstatuses -n metallb-system
+NAME        NODE                 SERVICE NAME   SERVICE NAMESPACE
+bgp-82jzt   kind-worker2         service4       ns4
+bgp-b8fmt   kind-worker2         service3       ns3
+bgp-bt56x   kind-worker          service4       ns4
+bgp-c64s2   kind-worker          service3       ns3
+```
+Each resource is labeled with "metallb.io/node", "metallb.io/service-name", "metallb.io/service-namespace", with the matching node / service name / service namespace respectively. This makes it easier to list the statuses related to a given combination, for example:
+```
+$ kubectl get servicebgpstatuses -n metallb-system -l metallb.io/service-name="service3",metallb.io/service-namespace="ns3",metallb.io/node=kind-worker
+NAME        NODE          SERVICE NAME   SERVICE NAMESPACE
+bgp-c64s2   kind-worker   service3       ns3
+```
+As the API docs mention, the ServiceBGPStatus resource only represents the intention of the node to advertise the Service to a set of peers, where the actual advertisements depend on the status of the corresponding BGP sessions. The status of a BGP session can be understood via metrics / logs, or in the case of [the FRR-k8s mode](https://metallb.io/concepts/bgp/index.html#frr-k8s-mode) via its BGPSessionState resource.
+
 ## Does MetalLB work on OpenStack?
 
 Yes but by default, OpenStack has anti-spoofing protection enabled which prevents the VMs from using any IP that wasn't configured for them in the OpenStack control plane, such as LoadBalancer IPs from MetalLB. See [openstack port set --allowed-address](https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/port.html).
diff --git a/website/generatecrddoc/crdgen.yaml b/website/generatecrddoc/crdgen.yaml
index a7e51a71..9afe6415 100644
--- a/website/generatecrddoc/crdgen.yaml
+++ b/website/generatecrddoc/crdgen.yaml
@@ -2,14 +2,17 @@ processor:
   # RE2 regular expressions describing types that should be excluded from the generated documentation.
   ignoreTypes:
     - "List$"
-    - "Status$"
     - "MatchExpression"
     - "NodeSelector"
     - "LegacyBgpAdvertisement"
     - "v1beta1.*BGPPeer"
   # RE2 regular expressions describing type fields that should be excluded from the generated documentation.
   ignoreFields:
-    - "status$"
+    - "BFDProfile.status"
+    - "BGPAdvertisement.status"
+    - "L2Advertisement.status"
+    - "BGPPeer.status"
+    - "Community.status"
   ignoreGroupVersions:
     - "v1alpha1"
 


Test patch:
diff --git a/controller/controller_test.go b/controller/controller_test.go
index c04b922e..0cd9a7ad 100644
--- a/controller/controller_test.go
+++ b/controller/controller_test.go
@@ -846,6 +846,51 @@ func TestControllerMutation(t *testing.T) {
 				Status: statusAssigned([]string{"1000::"}),
 			},
 		},
+		{
+			desc: "request IPs from dualstack pool with PreferDualStack policy but single stack clusterip",
+			in: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						AnnotationAddressPool: "pool5",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Type:           "LoadBalancer",
+					ClusterIPs:     []string{"1.2.3.4"},
+					IPFamilyPolicy: &IPFamilyPolicyPreferDualStack,
+				},
+			},
+			want: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						AnnotationAddressPool: "pool5",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Type:           "LoadBalancer",
+					ClusterIPs:     []string{"1.2.3.4"},
+					IPFamilyPolicy: &IPFamilyPolicyPreferDualStack,
+				},
+				Status: statusAssigned([]string{"1.2.3.0"}),
+			},
+		},
+		{
+			desc: "request IPs from dualstack pool with RequireDualStack policy but single stack clusterip",
+			in: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						AnnotationAddressPool: "pool5",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Type:           "LoadBalancer",
+					ClusterIPs:     []string{"1.2.3.4"},
+					IPFamilyPolicy: &IPFamilyPolicyRequireDualStack,
+				},
+			},
+			wantErr: true,
+		},
+
 		{
 			desc: "request specific loadbalancer IP with dual-stack",
 			in: &v1.Service{
diff --git a/e2etest/bgptests/bgp.go b/e2etest/bgptests/bgp.go
index 02da5339..e89cf3e0 100644
--- a/e2etest/bgptests/bgp.go
+++ b/e2etest/bgptests/bgp.go
@@ -389,7 +389,6 @@ var _ = ginkgo.Describe("BGP", func() {
 			func(svc *corev1.Service) {
 				svc.Spec.LoadBalancerIP = serviceIP
 				svc.Annotations = map[string]string{"metallb.io/allow-shared-ip": "foo"}
-				svc.Spec.Ports[0].Port = int32(testservice.TestServicePort)
 			})
 		defer testservice.Delete(cs, svc)
 		svc1, _ := testservice.CreateWithBackendPort(cs, testNamespace, "second-service",
@@ -397,7 +396,6 @@ var _ = ginkgo.Describe("BGP", func() {
 			func(svc *corev1.Service) {
 				svc.Spec.LoadBalancerIP = serviceIP
 				svc.Annotations = map[string]string{"metallb.io/allow-shared-ip": "foo"}
-				svc.Spec.Ports[0].Port = int32(testservice.TestServicePort + 1)
 			})
 		defer testservice.Delete(cs, svc1)
 
diff --git a/e2etest/bgptests/cordon_node.go b/e2etest/bgptests/cordon_node.go
deleted file mode 100644
index 8bfed763..00000000
--- a/e2etest/bgptests/cordon_node.go
+++ /dev/null
@@ -1,98 +0,0 @@
-// SPDX-License-Identifier:Apache-2.0
-
-package bgptests
-
-import (
-	"context"
-	"fmt"
-
-	"github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-	"go.universe.tf/e2etest/pkg/k8s"
-	"go.universe.tf/e2etest/pkg/k8sclient"
-
-	frrconfig "go.universe.tf/e2etest/pkg/frr/config"
-	"go.universe.tf/e2etest/pkg/ipfamily"
-	testservice "go.universe.tf/e2etest/pkg/service"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	clientset "k8s.io/client-go/kubernetes"
-)
-
-var _ = ginkgo.Describe("BGP Cordon Node", func() {
-	var (
-		cs            clientset.Interface
-		testNamespace string
-	)
-
-	ginkgo.BeforeEach(func() {
-		err := ConfigUpdater.Clean()
-		Expect(err).NotTo(HaveOccurred(), "cleaning k8s api CRs failed")
-
-		for _, c := range FRRContainers {
-			err := c.UpdateBGPConfigFile(frrconfig.Empty)
-			Expect(err).NotTo(HaveOccurred(),
-				fmt.Sprintf("cleaning frr config at %s failed", c.Name))
-		}
-
-		cs = k8sclient.New()
-		testNamespace, err = k8s.CreateTestNamespace(cs, "bgp")
-		Expect(err).NotTo(HaveOccurred())
-	})
-
-	ginkgo.AfterEach(func() {
-		if ginkgo.CurrentSpecReport().Failed() {
-			dumpBGPInfo(ReportPath, ginkgo.CurrentSpecReport().LeafNodeText, cs, testNamespace)
-			k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
-		}
-		err := k8s.DeleteNamespace(cs, testNamespace)
-		Expect(err).NotTo(HaveOccurred())
-	})
-
-	ginkgo.Describe("When create an IPV4 service and then cordon node", func() {
-		var (
-			svc      *corev1.Service
-			allNodes *corev1.NodeList
-		)
-
-		ginkgo.BeforeEach(func() {
-			_, svc = setupBGPService(cs, testNamespace, ipfamily.IPv4, []string{v4PoolAddresses}, FRRContainers, func(svc *corev1.Service) {
-				testservice.TrafficPolicyCluster(svc)
-			})
-			testservice.ValidateDesiredLB(svc)
-
-			var err error
-			allNodes, err = cs.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{})
-			Expect(err).NotTo(HaveOccurred())
-
-			for _, c := range FRRContainers {
-				validateService(svc, allNodes.Items, c)
-			}
-
-			err = k8s.CordonNode(cs, &allNodes.Items[0])
-			Expect(err).NotTo(HaveOccurred(), "node was not cordoned")
-		})
-
-		ginkgo.AfterEach(func() {
-
-			err := k8s.UnCordonNode(cs, &allNodes.Items[0])
-			Expect(err).NotTo(HaveOccurred())
-
-			for _, c := range FRRContainers {
-				validateService(svc, allNodes.Items, c)
-			}
-
-			testservice.Delete(cs, svc)
-		})
-
-		ginkgo.It("service should be announced only from expected nodes", func() {
-			checkServiceOnlyOnNodes(svc, allNodes.Items[1:], ipfamily.IPv4)
-			checkServiceNotOnNodes(svc, allNodes.Items[:1], ipfamily.IPv4)
-		})
-
-	})
-})
-
-// when create service, when uncordon node, cordoned node should eventually not be next NextHops
-// when uncordon node, when create service, cordoned node should not be ever as NextHops
-// when create service, when cordon uncordon, service should come back working
diff --git a/e2etest/go.mod b/e2etest/go.mod
index d4079d3c..93bcfb22 100644
--- a/e2etest/go.mod
+++ b/e2etest/go.mod
@@ -88,7 +88,7 @@ require (
 	github.com/xlab/treeprint v1.2.0 // indirect
 	go.starlark.net v0.0.0-20230525235612-a134d8f9ddca // indirect
 	go.uber.org/multierr v1.11.0 // indirect
-	golang.org/x/net v0.35.0 // indirect
+	golang.org/x/net v0.36.0 // indirect
 	golang.org/x/oauth2 v0.26.0 // indirect
 	golang.org/x/term v0.29.0 // indirect
 	golang.org/x/text v0.22.0 // indirect
diff --git a/e2etest/go.sum b/e2etest/go.sum
index 66eb5c66..9dbd4891 100644
--- a/e2etest/go.sum
+++ b/e2etest/go.sum
@@ -214,8 +214,8 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
-golang.org/x/net v0.35.0 h1:T5GQRQb2y08kTAByq9L4/bz8cipCdA8FbRTXewonqY8=
-golang.org/x/net v0.35.0/go.mod h1:EglIi67kWsHKlRzzVMUD93VMSWGFOMSZgxFjparz1Qk=
+golang.org/x/net v0.36.0 h1:vWF2fRbw4qslQsQzgFqZff+BItCvGFQqKzKIzx1rmoA=
+golang.org/x/net v0.36.0/go.mod h1:bFmbeoIPfrw4sMHNhb4J9f6+tPziuGjq7Jk/38fxi1I=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.26.0 h1:afQXWNNaeC4nvZ0Ed9XvCCzXM6UHJG7iCg0W4fPqSBE=
 golang.org/x/oauth2 v0.26.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
diff --git a/e2etest/go.work.sum b/e2etest/go.work.sum
index f85097d6..7d7e70d0 100644
--- a/e2etest/go.work.sum
+++ b/e2etest/go.work.sum
@@ -849,6 +849,7 @@ github.com/onsi/ginkgo/v2 v2.15.0/go.mod h1:HlxMHtYF57y6Dpf+mc5529KKmSq9h2FpCF+/
 github.com/onsi/ginkgo/v2 v2.17.2/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/PRJ1eCc=
 github.com/onsi/ginkgo/v2 v2.19.0/go.mod h1:rlwLi9PilAFJ8jCg9UE1QP6VBpd6/xj3SRC0d6TU0To=
 github.com/onsi/ginkgo/v2 v2.20.1/go.mod h1:lG9ey2Z29hR41WMVthyJBGUBcBhGOtoPF2VFMvBXFCI=
+github.com/onsi/ginkgo/v2 v2.22.1/go.mod h1:S6aTpoRsSq2cZOd+pssHAlKW/Q/jZt6cPrPlnj4a1xM=
 github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro=
 github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg=
 github.com/onsi/gomega v1.27.10/go.mod h1:RsS8tutOdbdgzbPtzzATp12yT7kM5I5aElG3evPbQ0M=
@@ -1268,6 +1269,7 @@ golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE=
 golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE=
 golang.org/x/net v0.28.0/go.mod h1:yqtgsTWOOnlGLG9GFRrK3++bGOUEkNBoHZc8MEDWPNg=
 golang.org/x/net v0.30.0/go.mod h1:2wGyMJ5iFasEhkwi13ChkO/t1ECNC4X4eBKkVFyYFlU=
+golang.org/x/net v0.32.0/go.mod h1:CwU0IoeOlnQQWJ6ioyFrfRuomB8GKF6KbYXZVyeXNfs=
 golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -1294,6 +1296,7 @@ golang.org/x/sync v0.2.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
 golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20181026203630-95b1ffbd15a5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20181107165924-66b7b1311ac8/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
diff --git a/e2etest/l2tests/assignment.go b/e2etest/l2tests/assignment.go
index d74bc876..1cc97c3d 100644
--- a/e2etest/l2tests/assignment.go
+++ b/e2etest/l2tests/assignment.go
@@ -20,6 +20,7 @@ import (
 	metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
 
 	jigservice "go.universe.tf/e2etest/pkg/jigservice"
+	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -29,7 +30,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-const secondNamespace = "test-namespace"
+const (
+	secondNamespace         = "test-namespace"
+	allowSharedIPAnnotation = "metallb.io" + "/" + "allow-shared-ip"
+)
 
 var (
 	firstNsLabels = map[string]string{
@@ -223,6 +227,106 @@ var _ = ginkgo.Describe("IP Assignment", func() {
 				restartAndAssert()
 			}
 		})
+
+		ginkgo.It("should reconsider services when sharing the ip and conditions change", func() {
+			ip, err := config.GetIPFromRangeByIndex(IPV4ServiceRange, 0)
+			Expect(err).NotTo(HaveOccurred())
+
+			resources := config.Resources{
+				Pools: []metallbv1beta1.IPAddressPool{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "singleip-pool",
+						},
+						Spec: metallbv1beta1.IPAddressPoolSpec{
+							Addresses: []string{
+								fmt.Sprintf("%s/32", ip),
+							},
+						},
+					},
+				},
+			}
+			err = ConfigUpdater.Update(resources)
+			Expect(err).NotTo(HaveOccurred())
+
+			checkHasLBIP := func(svc *corev1.Service, ip string) {
+				Eventually(func() string {
+					s, err := cs.CoreV1().Services(svc.Namespace).Get(context.Background(), svc.Name, metav1.GetOptions{})
+					Expect(err).NotTo(HaveOccurred())
+					if len(s.Status.LoadBalancer.Ingress) == 0 {
+						return ""
+					}
+					return s.Status.LoadBalancer.Ingress[0].IP
+				}, time.Minute, 1*time.Second).Should(Equal(ip))
+			}
+
+			checkLBIPGetsRemoved := func(svc *corev1.Service) {
+				Eventually(func() int {
+					s, err := cs.CoreV1().Services(svc.Namespace).Get(context.Background(), svc.Name, metav1.GetOptions{})
+					Expect(err).NotTo(HaveOccurred())
+					return len(s.Status.LoadBalancer.Ingress)
+				}, time.Minute, 1*time.Second).Should(BeZero())
+			}
+
+			svc, jig := service.CreateWithBackendPort(cs, testNamespace, "changesharing", 80, func(svc *corev1.Service) {
+				service.TrafficPolicyCluster(svc)
+				svc.Annotations = map[string]string{allowSharedIPAnnotation: "share"}
+			})
+
+			ginkgo.By("Creating another service")
+			svc1, jig1 := service.CreateWithBackendPort(cs, testNamespace, "changesharing1", 81, func(svc *corev1.Service) {
+				service.TrafficPolicyCluster(svc)
+				svc.Annotations = map[string]string{allowSharedIPAnnotation: "share"}
+				svc.Spec.Selector = jig.Labels // assigning the same selector to the two services
+			})
+
+			Expect(err).NotTo(HaveOccurred())
+			defer func() {
+				service.Delete(cs, svc)
+				service.Delete(cs, svc1)
+			}()
+
+			checkHasLBIP(svc, ip)
+			checkHasLBIP(svc1, ip)
+
+			ginkgo.By("changing etp policy of the second service to local")
+			jig1.UpdateService(context.Background(), func(svc *corev1.Service) {
+				svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyLocal
+			})
+
+			ginkgo.By("checking the second service loses its ip")
+			checkLBIPGetsRemoved(svc1)
+
+			ginkgo.By("checking the first service keeps its ip")
+			Consistently(func() string {
+				toCheck, err := cs.CoreV1().Services(svc.Namespace).Get(context.Background(), svc.Name, metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred())
+				return toCheck.Status.LoadBalancer.Ingress[0].IP
+			}, 2*time.Minute, 1*time.Second).Should(Equal(ip))
+
+			ginkgo.By("changing etp policy of the first service to local")
+			jig.UpdateService(context.Background(), func(svc *corev1.Service) {
+				svc.Spec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyLocal
+			})
+
+			ginkgo.By("checking both services now got their ips")
+			checkHasLBIP(svc, ip)
+			checkHasLBIP(svc1, ip)
+
+			ginkgo.By("changing the selector of the first service")
+			jig.UpdateService(context.Background(), func(svc *corev1.Service) {
+				svc.Spec.Selector = map[string]string{"foo": "bar"}
+			})
+			ginkgo.By("checking the first service loses its ip")
+			checkLBIPGetsRemoved(svc)
+
+			ginkgo.By("aligning the selector of the second service")
+			jig1.UpdateService(context.Background(), func(svc *corev1.Service) {
+				svc.Spec.Selector = map[string]string{"foo": "bar"}
+			})
+			checkHasLBIP(svc, ip)
+			checkHasLBIP(svc1, ip)
+		})
 	})
 
 	ginkgo.Context("IPV4 removing pools", func() {
@@ -382,6 +486,52 @@ var _ = ginkgo.Describe("IP Assignment", func() {
 			Expect(err).NotTo(HaveOccurred())
 		})
 
+		ginkgo.It("with only priority", func() {
+			namespacePoolWithLowerPriority := metallbv1beta1.IPAddressPool{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-priority-pool-1"},
+				Spec: metallbv1beta1.IPAddressPoolSpec{
+					Addresses: []string{
+						"192.168.5.0/32",
+					},
+					AllocateTo: &metallbv1beta1.ServiceAllocation{Priority: 20},
+				},
+			}
+			namespacePoolWithHigherPriority := metallbv1beta1.IPAddressPool{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-priority-pool-2"},
+				Spec: metallbv1beta1.IPAddressPoolSpec{
+					Addresses: []string{
+						"192.168.10.0/32",
+					},
+					AllocateTo: &metallbv1beta1.ServiceAllocation{Priority: 10},
+				},
+			}
+			resources := config.Resources{
+				Pools: []metallbv1beta1.IPAddressPool{namespacePoolWithLowerPriority, namespacePoolWithHigherPriority},
+			}
+
+			err := ConfigUpdater.Update(resources)
+			Expect(err).NotTo(HaveOccurred())
+
+			svc1, _ := service.CreateWithBackend(cs, testNamespace, "svc-test-priority-pool-1")
+			svc2, _ := service.CreateWithBackend(cs, testNamespace, "svc-test-priority-pool-2")
+			defer func() {
+				service.Delete(cs, svc1)
+				service.Delete(cs, svc2)
+			}()
+
+			// The createWithBackend method always wait for service to acquire an ingress IP, so
+			// just validate service ingress ip address are assigned from appropriate ip
+			// address pool.
+			ginkgo.By("validate LoadBalancer IP is allocated from 1st higher priority address pool")
+			err = config.ValidateIPInRange([]metallbv1beta1.IPAddressPool{namespacePoolWithHigherPriority}, jigservice.GetIngressPoint(
+				&svc1.Status.LoadBalancer.Ingress[0]))
+			Expect(err).NotTo(HaveOccurred())
+			ginkgo.By("validate LoadBalancer IP is allocated from 2nd higher priority address pool")
+			err = config.ValidateIPInRange([]metallbv1beta1.IPAddressPool{namespacePoolWithLowerPriority}, jigservice.GetIngressPoint(
+				&svc2.Status.LoadBalancer.Ingress[0]))
+			Expect(err).NotTo(HaveOccurred())
+		})
+
 		ginkgo.It("with namespace and namespace labels", func() {
 			namespacePoolWithLowerPriority := metallbv1beta1.IPAddressPool{
 				ObjectMeta: metav1.ObjectMeta{Name: "test-ns-label-pool-1"},
diff --git a/e2etest/l2tests/cordon_node.go b/e2etest/l2tests/cordon_node.go
deleted file mode 100644
index 2ba36acd..00000000
--- a/e2etest/l2tests/cordon_node.go
+++ /dev/null
@@ -1,132 +0,0 @@
-// SPDX-License-Identifier:Apache-2.0
-
-package l2tests
-
-import (
-	"context"
-	"fmt"
-	"time"
-
-	"github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-	"go.universe.tf/e2etest/pkg/config"
-	"go.universe.tf/e2etest/pkg/k8s"
-	"go.universe.tf/e2etest/pkg/k8sclient"
-	"go.universe.tf/e2etest/pkg/service"
-	metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
-
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	clientset "k8s.io/client-go/kubernetes"
-)
-
-var _ = ginkgo.Describe("L2 Cordon Node", func() {
-	emptyL2Advertisement := metallbv1beta1.L2Advertisement{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "empty",
-		},
-	}
-
-	var (
-		cs            clientset.Interface
-		testNamespace string
-	)
-
-	ginkgo.BeforeEach(func() {
-		err := ConfigUpdater.Clean()
-		Expect(err).NotTo(HaveOccurred(), "cleaning k8s api CRs failed")
-		cs = k8sclient.New()
-		testNamespace, err = k8s.CreateTestNamespace(cs, "l2test-cordon-node")
-		Expect(err).NotTo(HaveOccurred())
-
-		resources := config.Resources{
-			Pools: []metallbv1beta1.IPAddressPool{
-				{
-					ObjectMeta: metav1.ObjectMeta{
-						Name: "l2-test",
-					},
-					Spec: metallbv1beta1.IPAddressPoolSpec{
-						Addresses: []string{
-							IPV4ServiceRange,
-							IPV6ServiceRange},
-					},
-				},
-			},
-			L2Advs: []metallbv1beta1.L2Advertisement{emptyL2Advertisement},
-		}
-
-		err = ConfigUpdater.Update(resources)
-		Expect(err).NotTo(HaveOccurred())
-
-	})
-
-	ginkgo.AfterEach(func() {
-		if ginkgo.CurrentSpecReport().Failed() {
-			k8s.DumpInfo(Reporter, ginkgo.CurrentSpecReport().LeafNodeText)
-		}
-		err := k8s.DeleteNamespace(cs, testNamespace)
-		Expect(err).NotTo(HaveOccurred())
-	})
-
-	ginkgo.Describe("When create an IPV4 service and then cordon node", func() {
-		var (
-			svc       *corev1.Service
-			allNodes  *corev1.NodeList
-			nodeToCordon *corev1.Node
-		)
-
-		ginkgo.BeforeEach(func() {
-			svc, _ = service.CreateWithBackend(cs, testNamespace, "external-local-lb", service.TrafficPolicyCluster)
-
-			var err error
-			allNodes, err = cs.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{})
-			Expect(err).NotTo(HaveOccurred())
-
-			Eventually(func() error {
-				var err error
-				nodeToCordon, err = nodeForService(svc, allNodes.Items)
-				if err != nil {
-					return err
-				}
-				return nil
-			}, 30*time.Second, 1*time.Second).ShouldNot(HaveOccurred())
-
-			ginkgo.By(fmt.Sprintf("cordon node %s", nodeToCordon.Name))
-			err = k8s.CordonNode(cs, nodeToCordon)
-			Expect(err).NotTo(HaveOccurred(), "k8s api call to cordon node failed")
-			Eventually(func() bool {
-				ret, err := k8s.IsNodeCordoned(cs, nodeToCordon)
-				Expect(err).NotTo(HaveOccurred())
-				return ret
-			}, 30*time.Second, time.Second).Should(BeTrue(), "node.spec.Unschedulable not true")
-
-		})
-
-		ginkgo.AfterEach(func() {
-			ginkgo.By(fmt.Sprintf("uncordon node %s", nodeToCordon.Name))
-			err := k8s.UnCordonNode(cs, nodeToCordon)
-			Expect(err).NotTo(HaveOccurred())
-
-			Eventually(func() string {
-				node, err := nodeForService(svc, allNodes.Items)
-				if err != nil {
-					return ""
-				}
-				return node.Name
-			}, time.Minute, time.Second).Should(Equal(nodeToCordon.Name))
-
-			err = cs.CoreV1().Services(svc.Namespace).Delete(context.TODO(), svc.Name, metav1.DeleteOptions{})
-			Expect(err).NotTo(HaveOccurred())
-		})
-
-		ginkgo.It("service should be announced only from expected nodes", func() {
-			Eventually(func() string {
-				node, err := nodeForService(svc, allNodes.Items)
-				if err != nil {
-					return ""
-				}
-				return node.Name
-			}, time.Minute, time.Second).ShouldNot(Equal(nodeToCordon.Name))
-		})
-	})
-})
diff --git a/e2etest/pkg/k8s/nodes.go b/e2etest/pkg/k8s/nodes.go
index 31d1a1e5..eec384be 100644
--- a/e2etest/pkg/k8s/nodes.go
+++ b/e2etest/pkg/k8s/nodes.go
@@ -17,10 +17,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/kubernetes"
 	clientset "k8s.io/client-go/kubernetes"
-	"k8s.io/kubectl/pkg/drain"
 )
 
 func NodeIPsForFamily(nodes []v1.Node, family ipfamily.Family, vrfName string) ([]string, error) {
@@ -154,52 +151,3 @@ func conditionStatus(n *corev1.Node, ct corev1.NodeConditionType) corev1.Conditi
 
 	return corev1.ConditionUnknown
 }
-
-func CordonNode(cs kubernetes.Interface, node *corev1.Node) error {
-
-	helper := &drain.Helper{
-		Client:              cs,
-		Ctx:                 context.TODO(),
-		Force:               true,
-		GracePeriodSeconds:  -1,
-		IgnoreAllDaemonSets: true,
-	}
-	if err := drain.RunCordonOrUncordon(helper, node, true); err != nil {
-		return fmt.Errorf("error cordoning node: %v", err)
-	}
-
-	return wait.PollUntilContextTimeout(context.Background(),
-		time.Second, 30*time.Second, false, func(context.Context) (bool, error) {
-			return IsNodeCordoned(cs, node)
-		})
-}
-
-func UnCordonNode(cs kubernetes.Interface, node *corev1.Node) error {
-
-	helper := &drain.Helper{
-		Client:              cs,
-		Ctx:                 context.TODO(),
-		Force:               true,
-		GracePeriodSeconds:  -1,
-		IgnoreAllDaemonSets: true,
-	}
-	if err := drain.RunCordonOrUncordon(helper, node, false); err != nil {
-		return fmt.Errorf("error cordoning node: %v", err)
-	}
-	return wait.PollUntilContextTimeout(context.Background(),
-		time.Second, 30*time.Second, false, func(context.Context) (bool, error) {
-			ret, err := IsNodeCordoned(cs, node)
-			if err != nil {
-				return false, err
-			}
-			return !ret, nil
-		})
-}
-
-func IsNodeCordoned(cs kubernetes.Interface, node *corev1.Node) (bool, error) {
-	o, err := cs.CoreV1().Nodes().Get(context.Background(), node.Name, metav1.GetOptions{})
-	if err != nil {
-		return false, err
-	}
-	return o.Spec.Unschedulable, nil
-}
diff --git a/e2etest/pkg/service/create.go b/e2etest/pkg/service/create.go
index 219f878d..192bf483 100644
--- a/e2etest/pkg/service/create.go
+++ b/e2etest/pkg/service/create.go
@@ -50,6 +50,7 @@ func Delete(cs clientset.Interface, svc *corev1.Service) {
 }
 
 func tweakServicePort(svc *corev1.Service, port int) {
+	svc.Spec.Ports[0].Port = int32(port)
 	svc.Spec.Ports[0].TargetPort = intstr.FromInt(port)
 }
 
diff --git a/e2etest/pkg/status/l2status.go b/e2etest/pkg/status/l2status.go
index 81ca9ce5..7a666d37 100644
--- a/e2etest/pkg/status/l2status.go
+++ b/e2etest/pkg/status/l2status.go
@@ -34,5 +34,11 @@ func L2ForService(cs client.Client, svc *v1.Service) (*v1beta1.ServiceL2Status,
 	if len(statuses) > 1 {
 		return nil, fmt.Errorf("got more than 1 serviceL2Status object: %d", len(statuses))
 	}
-	return &(statuses[0]), nil
+
+	s := statusList.Items[0]
+	if s.Status.Node == "" {
+		return nil, fmt.Errorf(".Status.Node not populated for service %s/%s, resource: %v", svc.Name, svc.Namespace, s)
+	}
+
+	return &s, nil
 }
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 39866140..f6b427d2 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -635,6 +635,44 @@ func TestParse(t *testing.T) {
 			},
 		},
 
+		{
+			desc: "ip address pool with only priority",
+			crs: ClusterResources{
+				Pools: []v1beta1.IPAddressPool{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "pool1",
+						},
+						Spec: v1beta1.IPAddressPoolSpec{
+							Addresses: []string{
+								"10.20.0.0/16",
+								"10.50.0.0/24",
+							},
+							AvoidBuggyIPs: true,
+							AutoAssign:    ptr.To(false),
+							AllocateTo:    &v1beta1.ServiceAllocation{Priority: 1},
+						},
+					},
+				},
+			},
+			want: &Config{
+				Pools: &Pools{
+					ByName: map[string]*Pool{
+						"pool1": {
+							Name:               "pool1",
+							CIDR:               []*net.IPNet{ipnet("10.20.0.0/16"), ipnet("10.50.0.0/24")},
+							AvoidBuggyIPs:      true,
+							AutoAssign:         false,
+							ServiceAllocations: &ServiceAllocation{Priority: 1, ServiceSelectors: []labels.Selector{labels.Everything()}},
+						},
+					},
+					ByServiceSelector: []string{"pool1"},
+				},
+				BFDProfiles: map[string]*BFDProfile{},
+				Peers:       map[string]*Peer{},
+			},
+		},
+
 		{
 			desc: "peer-only",
 			crs: ClusterResources{
@@ -1355,7 +1393,37 @@ func TestParse(t *testing.T) {
 					},
 				},
 			},
+			want: &Config{
+				Pools: &Pools{ByName: map[string]*Pool{
+					"pool1": {
+						Name:       "pool1",
+						AutoAssign: true,
+						CIDR: []*net.IPNet{
+							ipnet("10.20.30.40/24"),
+						},
+						BGPAdvertisements: []*BGPAdvertisement{
+							{
+								AggregationLength:   24,
+								AggregationLengthV6: 128,
+								LocalPref:           100,
+								Communities:         map[community.BGPCommunity]bool{},
+								Nodes:               map[string]bool{"node1": true, "node2": true},
+							},
+							{
+								AggregationLength:   32,
+								AggregationLengthV6: 128,
+								LocalPref:           200,
+								Communities:         map[community.BGPCommunity]bool{},
+								Nodes:               map[string]bool{"node1": true, "node2": true},
+							},
+						},
+					},
+				}},
+				BFDProfiles: map[string]*BFDProfile{},
+				Peers:       map[string]*Peer{},
+			},
 		},
+
 		{
 			desc: "different local pref - different aggregation lengths",
 			crs: ClusterResources{
diff --git a/internal/config/validation_test.go b/internal/config/validation_test.go
index 5a2a0a1b..c54be3b0 100644
--- a/internal/config/validation_test.go
+++ b/internal/config/validation_test.go
@@ -561,6 +561,46 @@ func TestValidateFRR(t *testing.T) {
 			},
 			mustFail: true,
 		},
+		{
+			desc: "two peers with interface set different",
+			config: ClusterResources{
+				Peers: []v1beta2.BGPPeer{
+					{
+						Spec: v1beta2.BGPPeerSpec{
+							Interface: "eth0",
+							MyASN:     123,
+						},
+					},
+					{
+						Spec: v1beta2.BGPPeerSpec{
+							Interface: "eth1",
+							MyASN:     123,
+						},
+					},
+				},
+			},
+			mustFail: false,
+		},
+		{
+			desc: "two peers with interface set same",
+			config: ClusterResources{
+				Peers: []v1beta2.BGPPeer{
+					{
+						Spec: v1beta2.BGPPeerSpec{
+							Interface: "eth0",
+							MyASN:     123,
+						},
+					},
+					{
+						Spec: v1beta2.BGPPeerSpec{
+							Interface: "eth0",
+							MyASN:     123,
+						},
+					},
+				},
+			},
+			mustFail: true,
+		},
 	}
 
 	for _, test := range tests {
diff --git a/internal/k8s/controllers/node_controller_test.go b/internal/k8s/controllers/node_controller_test.go
index b10b4988..f1e97f4b 100644
--- a/internal/k8s/controllers/node_controller_test.go
+++ b/internal/k8s/controllers/node_controller_test.go
@@ -183,20 +183,7 @@ func TestNodeReconcilerPredicate(t *testing.T) {
 					Spec: corev1.NodeSpec{Unschedulable: true},
 				},
 			},
-			expected: true,
-		},
-		"spec schedulable change and label change": {
-			event: event.UpdateEvent{
-				ObjectOld: &corev1.Node{
-					ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"x": "y"}},
-					Spec:       corev1.NodeSpec{Unschedulable: false},
-				},
-				ObjectNew: &corev1.Node{
-					ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"x": "z"}},
-					Spec:       corev1.NodeSpec{Unschedulable: true},
-				},
-			},
-			expected: true,
+			expected: false,
 		},
 		"condition NodeNetworkUnavailable status change": {
 			event: event.UpdateEvent{
diff --git a/speaker/bgp_controller_test.go b/speaker/bgp_controller_test.go
index 0c131d98..4146ae74 100644
--- a/speaker/bgp_controller_test.go
+++ b/speaker/bgp_controller_test.go
@@ -1358,59 +1358,6 @@ func TestNodeSelectors(t *testing.T) {
 				"2.3.4.5": nil,
 			},
 		},
-
-		{
-			desc: "Change node availability - Unschedulable becomes true",
-			node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "pandora",
-					Labels: map[string]string{
-						"host": "frontend",
-					},
-				},
-				Spec: v1.NodeSpec{Unschedulable: true},
-			},
-			wantAds: map[string][]*bgp.Advertisement{
-				"1.2.3.4": nil,
-				"2.3.4.5": nil,
-			},
-			wantReturnState: controllers.SyncStateReprocessAll,
-		},
-
-		{
-			desc: "Change node availability - Unschedulable remains true",
-			node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "pandora",
-					Labels: map[string]string{
-						"host": "frontend",
-					},
-				},
-				Spec: v1.NodeSpec{Unschedulable: true},
-			},
-			wantAds: map[string][]*bgp.Advertisement{
-				"1.2.3.4": nil,
-				"2.3.4.5": nil,
-			},
-		},
-
-		{
-			desc: "Change node availability - Unschedulable becomes false",
-			node: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "pandora",
-					Labels: map[string]string{
-						"host": "frontend",
-					},
-				},
-				Spec: v1.NodeSpec{Unschedulable: false},
-			},
-			wantAds: map[string][]*bgp.Advertisement{
-				"1.2.3.4": nil,
-				"2.3.4.5": nil,
-			},
-			wantReturnState: controllers.SyncStateReprocessAll,
-		},
 	}
 
 	l := log.NewNopLogger()