Skip to content

IP Assignement: match clusterip family in case of single stack clusterip - #2726

Merged
fedepaol merged 1 commit into
metallb:mainfrom
fedepaol:dualstackfollowclusterips
Mar 20, 2025
Merged

fedepaol merged 1 commit into
metallb:mainfrom
fedepaol:dualstackfollowclusterips

Conversation

@fedepaol

Copy link
Copy Markdown
Member

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 #2724 and #2723

Special notes for your reviewer:

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
fedepaol requested a review from oribon as a code owner March 18, 2025 18:48
@fedepaol

Copy link
Copy Markdown
Member Author

cc @mquhuy

Comment on lines +887 to +888
ClusterIPs: []string{"1.2.3.4"},
IPFamilyPolicy: &IPFamilyPolicyRequireDualStack,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iiuc this can't really happen: https://kubernetes.io/docs/concepts/services-networking/dual-stack/#services:

RequireDualStack: Allocates Service .spec.clusterIPs from both IPv4 and IPv6 address ranges when dual-stack is enabled. If dual-stack is not enabled or supported, the Service API object creation fails.

to me it seems we shouldn't allocate anything if we reach this case for some reason?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I considered this to be a corner case when I wrote the test. I can remove the test as we would never get here (because the service won't get cluisterips in first instance and we skip), or even make a stronger reject when we process the service, or leave this here as a safe net.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's not too big of a change to have an explicit reject I think it'd be better

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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.

Also, refusing upfront a service requiring dual stack but coming with single (or
zero) clusterips.

Signed-off-by: Federico Paolinelli <[email protected]>
@fedepaol
fedepaol force-pushed the dualstackfollowclusterips branch from 2c5e861 to 336a7b9 Compare March 19, 2025 11:21

@oribon oribon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@oribon
oribon enabled auto-merge March 19, 2025 12:17
@oribon
oribon added this pull request to the merge queue Mar 19, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Mar 19, 2025
@fedepaol
fedepaol merged commit 5dc6242 into metallb:main Mar 20, 2025
@kvaps

kvaps commented May 5, 2025

Copy link
Copy Markdown
Contributor

Hello, is there any ETA about releasing this fix?

lllamnyp added a commit to cozystack/cozystack that referenced this pull request May 15, 2025
Since it's taking a while for metallb/metallb#2726 to get released, the
binaries with the fix are recompiled in-tree. Workaround for #909.

Signed-off-by: Timofei Larkin <[email protected]>
lllamnyp added a commit to cozystack/cozystack that referenced this pull request May 15, 2025
Since it's taking a while for metallb/metallb#2726 to get released, the
binaries with the fix are recompiled in-tree. Workaround for #909.

Signed-off-by: Timofei Larkin <[email protected]>
lllamnyp added a commit to cozystack/cozystack that referenced this pull request May 16, 2025
Since it's taking a while for metallb/metallb#2726 to get released, the
binaries with the fix are recompiled in-tree. Workaround for #909.

Signed-off-by: Timofei Larkin <[email protected]>
lllamnyp added a commit to cozystack/cozystack that referenced this pull request May 16, 2025
Since it's taking a while for metallb/metallb#2726 to get released, the
binaries with the fix are recompiled in-tree. Workaround for #909.
lllamnyp added a commit to cozystack/cozystack that referenced this pull request May 16, 2025
Since it's taking a while for metallb/metallb#2726 to get released, the
binaries with the fix are recompiled in-tree. Workaround for #909.

(cherry picked from commit 73fdc5d)
Signed-off-by: Timofei Larkin <[email protected]>
kevin880202 pushed a commit to kevin880202/cozystack that referenced this pull request May 17, 2025
Since it's taking a while for metallb/metallb#2726 to get released, the
binaries with the fix are recompiled in-tree. Workaround for cozystack#909.

Signed-off-by: Timofei Larkin <[email protected]>
Signed-off-by: kevin880202 <[email protected]>
lllamnyp added a commit to cozystack/cozystack that referenced this pull request May 20, 2025
Since it's taking a while for metallb/metallb#2726 to get released, the
binaries with the fix are recompiled in-tree. Workaround for #909.

(cherry picked from commit 73fdc5d)
Signed-off-by: Timofei Larkin <[email protected]>
nbykov0 pushed a commit to cozystack/cozystack that referenced this pull request May 20, 2025
Since it's taking a while for metallb/metallb#2726 to get released, the
binaries with the fix are recompiled in-tree. Workaround for #909.

Signed-off-by: Timofei Larkin <[email protected]>
booxter added a commit to booxter/ovn-kubernetes that referenced this pull request Apr 21, 2026
This upgrade brings:

1. **Fixes for dual-stack issues** that affected v0.14.9:
   - ovn-kubernetes#2723: LoadBalancer IPs changing unexpectedly
   - ovn-kubernetes#2724: Dual-stack IPs assigned on single-stack clusters

Both fixed in v0.15.0 via PR:
metallb/metallb#2726

2. **FRR upgrade from 9.1.0 to 10.4.1**: This addresses the pthread
   crashes we were seeing with FRR 9.1.0 on Alpine/musl libc.

Assisted-by: Claude Sonnet 4.5 <[email protected]>
Signed-off-by: Ihar Hrachyshka <[email protected]>
tpantelis pushed a commit to tpantelis/ovn-kubernetes that referenced this pull request Apr 28, 2026
This upgrade brings:

1. **Fixes for dual-stack issues** that affected v0.14.9:
   - ovn-kubernetes#2723: LoadBalancer IPs changing unexpectedly
   - ovn-kubernetes#2724: Dual-stack IPs assigned on single-stack clusters

Both fixed in v0.15.0 via PR:
metallb/metallb#2726

2. **FRR upgrade from 9.1.0 to 10.4.1**: This addresses the pthread
   crashes we were seeing with FRR 9.1.0 on Alpine/musl libc.

Assisted-by: Claude Sonnet 4.5 <[email protected]>
Signed-off-by: Ihar Hrachyshka <[email protected]>
smulje pushed a commit to smulje/Upstream-ovn-kubernetes that referenced this pull request Jul 15, 2026
This upgrade brings:

1. **Fixes for dual-stack issues** that affected v0.14.9:
   - ovn-kubernetes#2723: LoadBalancer IPs changing unexpectedly
   - ovn-kubernetes#2724: Dual-stack IPs assigned on single-stack clusters

Both fixed in v0.15.0 via PR:
metallb/metallb#2726

2. **FRR upgrade from 9.1.0 to 10.4.1**: This addresses the pthread
   crashes we were seeing with FRR 9.1.0 on Alpine/musl libc.

Assisted-by: Claude Sonnet 4.5 <[email protected]>
Signed-off-by: Ihar Hrachyshka <[email protected]>
smulje pushed a commit to smulje/Upstream-ovn-kubernetes that referenced this pull request Aug 20, 2026
This upgrade brings:

1. **Fixes for dual-stack issues** that affected v0.14.9:
   - ovn-kubernetes#2723: LoadBalancer IPs changing unexpectedly
   - ovn-kubernetes#2724: Dual-stack IPs assigned on single-stack clusters

Both fixed in v0.15.0 via PR:
metallb/metallb#2726

2. **FRR upgrade from 9.1.0 to 10.4.1**: This addresses the pthread
   crashes we were seeing with FRR 9.1.0 on Alpine/musl libc.

Assisted-by: Claude Sonnet 4.5 <[email protected]>
Signed-off-by: Ihar Hrachyshka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

If a Pool is configured dualstack, a service gets assigned dual stack IPs even if the cluster is not dual stack

3 participants