Skip to content

feat: Auto BGP router-id allocation for IPv6#36451

Merged
squeed merged 1 commit intocilium:mainfrom
yushoyamaguchi:bgp-router-id-alloc-v6
Dec 18, 2024
Merged

feat: Auto BGP router-id allocation for IPv6#36451
squeed merged 1 commit intocilium:mainfrom
yushoyamaguchi:bgp-router-id-alloc-v6

Conversation

@yushoyamaguchi
Copy link
Copy Markdown
Member

@yushoyamaguchi yushoyamaguchi commented Dec 8, 2024 •
edited by harsimran-pabla
Loading

Related issue

This PR is a partial resolution of #30333

About configuration item name

#30333 (comment) ~ #30333 (comment)

Problem which is resolved in this PR

In IPv4 and dual-stack environments, the Cilium BGP Control Plane derives the Router ID from the IPv4 address assigned to the node. However, in the IPv6 single-stack environment, there's no IPv4 address to use, so users must specify the Router ID manually for each virtual router for each Node.
$ kubectl annotate node <node-name> cilium.io/bgp-virtual-router.64512="router-id=10.0.0.1"
This is a big operational overhead because users must manage the assignment by themselves.

Contents of this PR

Make helm configuration value of BGP router-id allocation mode and implement auto router-id allocation procedure using mac-address in IPv6 standalone environments.

  • Make helm configuration value of "routerIDAllocation".
  • "routerIDAllocation" is set "default" without an explicit designation.
  • When this commit, other mode than "default" are not implemented. That is a feature work.
  • In default mode, if the node have IPv4 address, Cilium use it as router-id. This is same behavior as before.
  • If the node have only IPv6 address, Cilium calculate router-id using mac-address of cilium_host device. This procedure is implemented in this commit.
  • If there are annotation about router-id, Cilium use router-id from annotation preferentially.

Further more

Currently, when using multiple bgpInstances with same AS-Number in one node, router-id duplication occur and no error or warning are emitted. This is not resolved in IPv6 auto allocation implemented in this PR.
Therefore, I will fix this and make another PR.
ref: #30333 (comment)

release-note label

release-note/minor

bgp: Introducing mac address based router-id generation.

@yushoyamaguchi yushoyamaguchi requested review from a team as code owners December 8, 2024 17:57
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 8, 2024
@yushoyamaguchi yushoyamaguchi requested a review from Artyop December 8, 2024 17:57
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Dec 8, 2024
@yushoyamaguchi yushoyamaguchi force-pushed the bgp-router-id-alloc-v6 branch 3 times, most recently from 7256b23 to d8957a2 Compare December 9, 2024 04:06
@yushoyamaguchi
Copy link
Copy Markdown
Member Author

yushoyamaguchi commented Dec 9, 2024

cc @YutaroHayakawa

@yushoyamaguchi yushoyamaguchi force-pushed the bgp-router-id-alloc-v6 branch 3 times, most recently from 14eb12a to f138c44 Compare December 9, 2024 08:31
aanm
aanm reviewed Dec 9, 2024
View reviewed changes
@yushoyamaguchi
Copy link
Copy Markdown
Member Author

yushoyamaguchi commented Dec 9, 2024 •
edited
Loading

Could you give release-note-label?

Artyop
Artyop reviewed Dec 9, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@Artyop Artyop left a comment

Choose a reason for hiding this comment

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

small nits

@yushoyamaguchi yushoyamaguchi requested review from Artyop and aanm December 9, 2024 10:47
Artyop
Artyop reviewed Dec 9, 2024
View reviewed changes
@yushoyamaguchi yushoyamaguchi requested a review from Artyop December 9, 2024 15:04
Artyop
Artyop approved these changes Dec 9, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@Artyop Artyop left a comment

Choose a reason for hiding this comment

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

lgtm, thank you !

yushoyamaguchi
yushoyamaguchi commented Dec 12, 2024
View reviewed changes
@yushoyamaguchi
Copy link
Copy Markdown
Member Author

yushoyamaguchi commented Dec 13, 2024 •
edited
Loading

@harsimran-pabla

Thank you for teaching.
I've fixed and got below log.

$ kubectl -n kube-system logs cilium-68qxp | grep -i bgp-control | grep -i router_
Defaulted container "cilium-agent" out of: cilium-agent, config (init), mount-cgroup (init), apply-sysctl-overwrites (init), mount-bpf-fs (init), clean-cilium-state (init), install-cni-binaries (init)
time="2024-12-13T02:49:07.869304901Z" level=info msg="Successfully registered BGP instance" asn=65001 instance=instance-65001 listen_port=-1 router_id=231.166.14.48 subsys=bgp-control-plane

harsimran-pabla
harsimran-pabla approved these changes Dec 13, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@harsimran-pabla harsimran-pabla left a comment

Choose a reason for hiding this comment

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

Thanks a lot for being patient with me, change looks great 🚀

@harsimran-pabla
Copy link
Copy Markdown
Contributor

harsimran-pabla commented Dec 13, 2024

Can you please add release-note in the PR description as well.

@harsimran-pabla
Copy link
Copy Markdown
Contributor

harsimran-pabla commented Dec 13, 2024

/test

@yushoyamaguchi
Copy link
Copy Markdown
Member Author

yushoyamaguchi commented Dec 15, 2024 •
edited
Loading

@harsimran-pabla

I've write about release-note-label in PR description.
Please confirm if it is okay.

How should I fix CI errors?
I think there are many errors which isn't related with this PR...
Is it better to wait for a while?

スクリーンショット 2024-12-15 15 36 59
For example, I think this test probably isn't related with my PR. (This error is in Cilium IPsec upgrade (ci-ipsec-upgrade))

@yushoyamaguchi
feat: Auto BGP router-id allocation for IPv6
85f22f8 
- Make helm configuration value of "routerIDAllocation".
- "routerIDAllocation" is set "default" without an explicit designation.
- When this commit, other mode than "default" are not implemented. That is a feature work.
- In default mode, if the node have IPv4 address, Cilium use it as router-id. This is same behavior as before.
- If the node have only IPv6 address, Cilium calclate router-id using mac-address of cilium_host device. This procedure is implemented in this commit.
- If there are annotation about router-id, Cilium use router-id from annotation preferentially.

Signed-off-by: Yusho Yamaguchi <[email protected]>
@yushoyamaguchi
Copy link
Copy Markdown
Member Author

yushoyamaguchi commented Dec 17, 2024

I've done git rebase.
Could you execute test one more time?

@harsimran-pabla
Copy link
Copy Markdown
Contributor

harsimran-pabla commented Dec 17, 2024

/test

@yushoyamaguchi
Copy link
Copy Markdown
Member Author

yushoyamaguchi commented Dec 18, 2024 •
edited
Loading

@harsimran-pabla

All tests have passed.
Thank you.

I've confirmed that everything works well after git rebase.

image

Could you merge this PR?
I've overlooked Labels.
Should I wait until next release?

@aanm aanm removed the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Dec 18, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 18, 2024
@squeed squeed added this pull request to the merge queue Dec 18, 2024
Merged via the queue into cilium:main with commit 0c51bae Dec 18, 2024
@yushoyamaguchi yushoyamaguchi mentioned this pull request Dec 19, 2024
Merged
@yushoyamaguchi yushoyamaguchi deleted the bgp-router-id-alloc-v6 branch April 29, 2025 15:13
github-merge-queue bot pushed a commit to chezmoidotsh/arcane that referenced this pull request Jul 29, 2025
@renovate