Repository: metallb/metallb
Pull request #2726 (2025-03-18 18:48:49)

IP Assignement: match clusterip family in case of single stack clusterip
<!-- 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**:

There is no point in adding a dual stack loadbalancer ip if the cluster ip is single family.

Additionally, when they do not match we feel free to clean the service and reassign:
https://github.com/metallb/metallb/blob/v0.14.9/controller/service.go#L109

which might cause an active service to change its ip.


Fixes https://github.com/metallb/metallb/issues/2724 and https://github.com/metallb/metallb/issues/2723

**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
Even if the service is prefer dual stack and dual stack pools are available, provide loadbalancer ips that match address family (single, dual) of the service's cluster ip.
```


fedepaol: cc @mquhuy 

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/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/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 {


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{