Repository: metallb/metallb
Pull request #2725 (2025-03-17 16:34:19)

FRR mode: put a limit to the number of open file descriptors
<!-- 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**:

Following frr's logs advice:

FD Limit set: 1073741816 is stupidly large. Is this what you intended? Consider using --limit-fds also limiting size to 100000

This should have a good effect on virtual memory size occupation, putting a limit instead of defaulting to the ulimit value.


Fixes https://github.com/metallb/metallb/issues/2584
**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
Limit the frr processes' file descriptors limit to 100000 instead of defaulting to host's ulimit value.
```



Patch:
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/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/config/config.go b/internal/config/config.go
index c6d28be1..17f782bf 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -565,6 +565,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 +579,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 {
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/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/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


Test patch:
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.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 f63d02a1..1cc97c3d 100644
--- a/e2etest/l2tests/assignment.go
+++ b/e2etest/l2tests/assignment.go
@@ -486,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/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..b8cbca99 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{
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()