Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 53 additions & 6 deletions pkg/support/dump_others.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@
package support

import (
"bytes"
"fmt"
"net"
"path"
"path/filepath"
"strings"

"antrea.io/antrea/pkg/agent/util/iptables"
"antrea.io/antrea/pkg/agent/util/sysctl"
"antrea.io/antrea/pkg/util/logdir"
"k8s.io/klog/v2"
)

func (d *agentDumper) DumpLog(basedir string) error {
Expand All @@ -44,6 +48,9 @@ func (d *agentDumper) DumpHostNetworkInfo(basedir string) error {
if err := d.dumpIPToolInfo(basedir); err != nil {
return err
}
if err := d.dumpInterfaceConfigs(basedir); err != nil {
return err
}
return nil
}

Expand All @@ -60,21 +67,61 @@ func (d *agentDumper) dumpIPTables(basedir string) error {
}

func (d *agentDumper) dumpIPToolInfo(basedir string) error {
dump := func(name string) error {
output, err := d.executor.Command("ip", name).CombinedOutput()
dump := func(args ...string) error {
output, err := d.executor.Command("ip", args...).CombinedOutput()
if err != nil {
return fmt.Errorf("error when dumping %s: %w", name, err)
return fmt.Errorf("error when dumping %s: %w", strings.Join(args, " "), err)
}
return writeFile(d.fs, filepath.Join(basedir, name), name, output)
return writeFile(d.fs, filepath.Join(basedir, args[0]), args[0], output)
}
commands := [][]string{
{"link"},
{"address"},
{"rule", "show"},
{"route", "show", "table", "all"},
}
for _, item := range []string{"route", "link", "address"} {
if err := dump(item); err != nil {
for _, cmd := range commands {
if err := dump(cmd...); err != nil {
return err
}
}
return nil
}

func (d *agentDumper) dumpInterfaceConfigs(basedir string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a unit test for this function? And ideally for dumpIPToolInfo as well.

interfaces, err := net.Interfaces()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want to consider all interfaces. There can be a lot of them because there is one per local Pod.
We should include the following interfaces:

  • the host gateway (antrea-gw0 by default but is configurable, so it should not be hardcoded, instead you need to use HostGateway from the config)
  • antrea-egress0
  • antrea-ingress0
  • antrea-ext.<VLAN> (there can be several such interfaces, only the prefix stays the same)

I also think this should be best effort, if we fail to get the information (maybe because of a permission issue), let's log the error + write the error to the support bundle, but let's not fail the bundle operation.

Copy link
Author

Choose a reason for hiding this comment

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

would it be useful to fall back to antrea-gw0if we fail to retrieveHostGateway ?

Copy link
Contributor

Choose a reason for hiding this comment

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

would it be useful to fall back to antrea-gw0 if we fail to retrieveHostGateway ?

I don't think this is a possible error scenario

if err != nil {
return fmt.Errorf("error getting network interfaces: %w", err)
}
hostGateway := d.aq.GetNodeConfig().GatewayConfig.Name
isRelevantIface := func(ifaceName string) bool {
return ifaceName == hostGateway ||
ifaceName == "antrea-egress0" ||
ifaceName == "antrea-ingress0" ||
strings.HasPrefix(ifaceName, "antrea-ext.")
}

params := []string{"rp_filter", "arp_ignore", "arp_announce"}
var output bytes.Buffer
for _, iface := range interfaces {
if !isRelevantIface(iface.Name) {
continue
}
output.WriteString(iface.Name)
output.WriteString("\n")
for _, param := range params {
value, err := sysctl.GetSysctlNet(fmt.Sprintf("ipv4/conf/%s/%s", iface.Name, param))
if err != nil {
klog.ErrorS(err, "Failed to get sysctl value", "interface", iface.Name, "param", param)
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should log the error here, even if we don't return it

}
output.WriteString(fmt.Sprintf("%s=%d\n", param, value))
}
output.WriteString("\n")
}
return writeFile(d.fs, filepath.Join(basedir, "interface-config"), "interface-config", output.Bytes())
}

func (d *agentDumper) DumpMemberlist(basedir string) error {
output, err := d.executor.Command("antctl", "-oyaml", "get", "memberlist").CombinedOutput()
if err != nil && !strings.Contains(string(output), "memberlist is not enabled") {
Expand Down
54 changes: 52 additions & 2 deletions pkg/support/dump_others_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,17 @@ import (
"path/filepath"
"testing"

"antrea.io/antrea/pkg/util/logdir"

"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"

"antrea.io/antrea/pkg/agent/config"
aqtest "antrea.io/antrea/pkg/agent/querier/testing"
"antrea.io/antrea/pkg/util/logdir"
)


func TestDumpLog(t *testing.T) {
fs := afero.NewMemMapFs()
logDir := logdir.GetLogDir()
Expand All @@ -49,3 +53,49 @@ func TestDumpLog(t *testing.T) {
require.NoError(t, err)
assert.True(t, ok)
}

func TestDumpIPToolInfo(t *testing.T) {
fs := afero.NewMemMapFs()
exe := new(testExec)

dumper := NewAgentDumper(fs, exe, nil, nil, nil, "7s", true, true)
err := dumper.(*agentDumper).dumpIPToolInfo(baseDir)
require.NoError(t, err)

ok, err := afero.Exists(fs, filepath.Join(baseDir, "link"))
require.NoError(t, err)
assert.True(t, ok)

ok, err = afero.Exists(fs, filepath.Join(baseDir, "address"))
require.NoError(t, err)
assert.True(t, ok)

ok, err = afero.Exists(fs, filepath.Join(baseDir, "rule"))
require.NoError(t, err)
assert.True(t, ok)

ok, err = afero.Exists(fs, filepath.Join(baseDir, "route"))
require.NoError(t, err)
assert.True(t, ok)
}

func TestDumpInterfaceConfigs(t *testing.T) {
ctrl := gomock.NewController(t)

fs := afero.NewMemMapFs()
q := aqtest.NewMockAgentQuerier(ctrl)
q.EXPECT().GetNodeConfig().Return(&config.NodeConfig{
GatewayConfig: &config.GatewayConfig{
Name: "antrea-gw0",
},
}).AnyTimes()

dumper := NewAgentDumper(fs, nil, nil, q, nil, "7s", true, true)
err := dumper.(*agentDumper).dumpInterfaceConfigs(baseDir)
require.NoError(t, err)

ok, err := afero.Exists(fs, filepath.Join(baseDir, "interface-config"))
require.NoError(t, err)
assert.True(t, ok)
}