Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect IPConflicting and gatewayReachable in ipam without coordinator… #4587

Merged

Conversation

cyclinder
Copy link
Collaborator

@cyclinder cyclinder commented Jan 26, 2025

Thanks for contributing!

Notice:

What issue(s) does this PR fix:

Fixes #

Special notes for your reviewer:

作为 #4474 的后续修复:

确保 IP 冲突检测先于网关检测,这防止当 IP 冲突检测失败且网关检测发送 ARP 探测报时意外更新了错误的 ARP 缓存表,导致通信失败。
移动 IP 冲突检测和网关检测的逻辑到 IPAM 中完成,在之前是由 Coordinator 完成。之前检测到 IP 冲突的时候,冲突的 IP 已经配置到了网卡上,可能会导致 IP 偶现不可通信的情况。
在 IPAM 中完成 IP 冲突检测和网关检测,这可能会增加 IPAM 调用的时间,在同时检测 IPv4 和 IPv6 的情况下,大概增加 1 s,具体时间视网络而定。特别对于 IPv6,开启 Duplicate Address Detection(DAD)的情况下,内核会检查本地链路地址是否冲突,这可能会花费一段时间。
As a follow-up fix to #4474:

Ensure that IP conflict detection precedes gateway detection. This prevents the ARP cache table from being incorrectly updated when IP conflict detection fails and gateway detection sends ARP probe packets, which could lead to communication failures.
Move the logic for IP conflict detection and gateway detection into IPAM, which was previously handled by the Coordinator. Previously, when an IP conflict was detected, the conflicting IP was already configured on the network interface card, potentially causing occasional IP communication failures.
Performing IP conflict detection and gateway detection within IPAM may increase the time for IPAM calls. When detecting both IPv4 and IPv6, the time may increase by approximately 1 second, depending on the network. Particularly for IPv6, when Duplicate Address Detection (DAD) is enabled, the kernel will check for conflicts with local link addresses, which may take some time.

Copy link

codecov bot commented Jan 26, 2025

Codecov Report

Attention: Patch coverage is 12.16931% with 332 lines in your changes missing coverage. Please review.

Project coverage is 76.31%. Comparing base (601c22a) to head (c619841).
Report is 1 commits behind head on release-v0.9.

Files with missing lines Patch % Lines
cmd/spiderpool/cmd/ipam_detection.go 7.50% 296 Missing ⚠️
cmd/spiderpool/cmd/command_add.go 41.86% 19 Missing and 6 partials ⚠️
cmd/spiderpool/cmd/utils.go 8.33% 11 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##           release-v0.9    #4587      +/-   ##
================================================
- Coverage         80.62%   76.31%   -4.31%     
================================================
  Files                51       52       +1     
  Lines              5578     5950     +372     
================================================
+ Hits               4497     4541      +44     
- Misses              908     1231     +323     
- Partials            173      178       +5     
Flag Coverage Δ
unittests 76.31% <12.16%> (-4.31%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/spiderpool/cmd/command_delete.go 86.58% <100.00%> (ø)
pkg/ippoolmanager/config.go 100.00% <ø> (ø)
pkg/ippoolmanager/ippool_manager.go 83.15% <100.00%> (+0.18%) ⬆️
cmd/spiderpool/cmd/utils.go 52.17% <8.33%> (-47.83%) ⬇️
cmd/spiderpool/cmd/command_add.go 69.31% <41.86%> (-8.95%) ⬇️
cmd/spiderpool/cmd/ipam_detection.go 7.50% <7.50%> (ø)

... and 1 file with indirect coverage changes

@cyclinder cyclinder force-pushed the 0.9_cp/ipam_detection branch from 4132390 to c619841 Compare January 26, 2025 07:59
@cyclinder cyclinder merged commit 5a29bf4 into spidernet-io:release-v0.9 Jan 26, 2025
53 of 54 checks passed
@cyclinder cyclinder deleted the 0.9_cp/ipam_detection branch January 26, 2025 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants