Detect IP conflicts before gateway detection to fix communication fail #4474
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cyclinder
added
release/bug
cherrypick-release-v0.8
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4474 +/- ##
=====================================
Coverage 6.79% 6.79%
=====================================
Files 59 59
Lines 7177 7177
=====================================
Hits 488 488
Misses 6654 6654
Partials 35 35
Flags with carried forward coverage won't be shown. Click here to find out more. |
cyclinder
changed the title
cyclinder
force-pushed
the
coordinator/serial_detect
branch
from
6ec9f62 to
eca35ed
Compare
weizhoublue
reviewed
Dec 31, 2024
| @@ -189,8 +189,56 @@ func CmdAdd(args *skel.CmdArgs) (err error) { | |||
|
|
|||
| logger.Sugar().Infof("Get coordinator config: %+v", c) | |||
|
|
|||
| errg := errgroup.Group{} | |||
| // we do detect gateway connection firstly | |||
| errgConflict := errgroup.Group{} | |||
There was a problem hiding this comment.
补充 code comment，关于 每一步先后循序 原因说明
arp 问题，引用 issue来详细说明
cyclinder
force-pushed
the
coordinator/serial_detect
branch
3 times, most recently
from
1c5c3db to
0a14822
Compare
weizhoublue
reviewed
Dec 31, 2024
| if err != nil { | ||
| return fmt.Errorf("failed to run NewIPChecker: %w", err) | ||
| } | ||
| ipc.DoIPConflictChecking(prevResult.IPs, c.currentInterface, &errgConflict) |
There was a problem hiding this comment.
这个超时控制 在哪体现的
比如 最多 100ms 内要给出 测试结论
There was a problem hiding this comment.
ipc, err = ipchecking.NewIPChecker(conf.DetectOptions.Retry, conf.DetectOptions.Interval, conf.DetectOptions.TimeOut, c.hostNs, c.netns, logger)
There was a problem hiding this comment.
(1) 这默认超时时间多少，在代码上 comment 下，这个变量 不容易 查源头的值
(2) 在 ipchecking.NewIPChecker 前打一行 有关超时 时间 的日志
(3) 缺省值 是 1s ？太长了
func ValidateDelectOptions(config *DetectOptions) (*DetectOptions, error) {
if config == nil {
return &DetectOptions{
Interval: "1s",
TimeOut: "3s",
Retry: 3,
}, nil
}
if config.Retry == 0 {
config.Retry = 3
}
if config.Interval == "" {
config.Interval = "1s"
}
There was a problem hiding this comment.
这些信息会在 final configuration 中打印，类似：
{"level":"DEBUG","ts":"2024-12-31T09:41:43.783Z","logger":"coordinator","caller":"cmd/command_add.go:84","msg":"final configuration","Action":"ADD","ContainerID":"17dc097c71526a6ab4ac53c07e48cc43e30d9b13bcac603ac2891eb23921dd2a","Netns":"/var/run/netns/cni-************************************","IfName":"net1","PodName":"dep-name-4ef71f347c-5496b8784b-r9s2v","PodNamespace":"ns-7dff59fa8a","conf":{"cniVersion":"0.3.1","name":"test-multus-6b86b94379","type":"coordinator","ipam":{},"dns":{},"detectGateway":false,"podDefaultCniNic":"eth0","overlayPodCIDR":["***********/18","fd00:10:243::/112"],"serviceCIDR":["**********/18","fd00:10:233::/116"],"hijackCIDR":["***********/16"],"tunePodRoutes":true,"mode":"overlay","hostRuleTable":500,"podRPFilter":0,"txQueueLen":0,"detectIPConflict":true,"detectOptions":{"retries":3,"interval":"1s","timeout":"3s"},"logOptions":{"logLevel":"debug","logFile":"/var/log/spidernet/coordinator.log","logMaxSize":0,"logMaxAge":0,"logMaxCount":0}}}
缺省值 是 1s ？太长了
多少合适？100ms？
weizhoublue
reviewed
Dec 31, 2024
| if err = errg.Wait(); err != nil { | ||
| logger.Error("failed to detect gateway and ip checking", zap.Error(err)) | ||
| if errors.Is(err, constant.ErrIPConflict) || errors.Is(err, constant.ErrGatewayUnreachable) { | ||
| if err = errgGateway.Wait(); err != nil { |
cyclinder
force-pushed
the
coordinator/serial_detect
branch
from
0a14822 to
0c49bf6
Compare
… could potentially update the arp table causing communication fail Signed-off-by: Cyclinder Kuo <[email protected]>
cyclinder
force-pushed
the
coordinator/serial_detect
branch
from
0c49bf6 to
c7c347a
Compare
weizhoublue
approved these changes
Dec 31, 2024
This was referenced Dec 31, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Detect IP conflicts before gateway detection to fix gateway detection could potentially update the arp table causing communication fail
Thanks for contributing!
Notice:
"release/none"
"release/bug"
"release/feature"
What issue(s) does this PR fix:
Fixes #4475
Special notes for your reviewer: