-
Notifications
You must be signed in to change notification settings - Fork 433
Add IP rules, routes and interface configs to support bundle #7543
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
base: main
Are you sure you want to change the base?
Add IP rules, routes and interface configs to support bundle #7543
Conversation
- IP rules (ip rule show) - IP routing tables (ip route show table all) - Interface kernel parameters (rp_filter, arp_ignore, arp_announce) Signed-off-by: Anis KHALFALLAH <[email protected]>
Signed-off-by: Anis KHALFALLAH <[email protected]>
Signed-off-by: Anis KHALFALLAH <[email protected]>
pkg/support/dump_others.go
Outdated
| "antrea.io/antrea/pkg/agent/util/iptables" | ||
| "antrea.io/antrea/pkg/agent/util/sysctl" | ||
| "antrea.io/antrea/pkg/util/logdir" | ||
| "k8s.io/klog/v2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid import grouping
it should be: 1) std library imports, 2) third-party imports, 3) antrea imports
do you get an error when you run make golangci locally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I didn't run make golangci, I ran make build-ubuntu instead, when I run make golangci I do see the import grouping error
| exe := new(testExec) | ||
|
|
||
| dumper := NewAgentDumper(fs, exe, nil, nil, nil, "7s", true, true) | ||
| err := dumper.(*agentDumper).dumpIPToolInfo(baseDir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not your fault, but this cast should not actually be needed
best practice in Go is too return the concrete type in the "constructor", not the interface type that it implements. So NewAgentDumper should be returning a value of type *agentDumper and not a value of type AgentDumper. Could you update NewAgentDumper accordingly and remove the case here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into changing NewAgentDumper to return *agentDumper, but it breaks other test files that mock this function (support_bundle_controller_test.go and rest_test.go).
Should I update those other test files too, or keep the casts ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should update the other tests and remove this cast everywhere - thanks!
| }).AnyTimes() | ||
|
|
||
| dumper := NewAgentDumper(fs, nil, nil, q, nil, "7s", true, true) | ||
| err := dumper.(*agentDumper).dumpInterfaceConfigs(baseDir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
|
|
||
| func TestDumpIPToolInfo(t *testing.T) { | ||
| fs := afero.NewMemMapFs() | ||
| exe := new(testExec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably improve the test to check that the correct commands are being invoked and that the output is handled correctly (as in https://github.com/antrea-io/antrea/pull/7547/files). However, we can leave that as a future improvement given that it is not the only unit test with this issue.
Signed-off-by: Anis KHALFALLAH <[email protected]>
28af851 to
3b310db
Compare
Summary :
Added support to dump network configuration details for interfaces :
Fixes : #7516
Component: pkg/support/dump_others.go