Skip to content

Commit

Permalink
fix(inputs/snmp): Reconnect TCP agents if needed (#11163)
Browse files Browse the repository at this point in the history
  • Loading branch information
Hipska authored May 25, 2022
1 parent 22efc25 commit 885444a
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 12 deletions.
8 changes: 8 additions & 0 deletions internal/snmp/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,11 @@ func (gs *GosnmpWrapper) SetAgent(agent string) error {
gs.Port = uint16(port)
return nil
}

func (gs *GosnmpWrapper) Reconnect() error {
if gs.Conn == nil {
return gs.Connect()
}

return nil
}
9 changes: 7 additions & 2 deletions plugins/inputs/snmp/snmp.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ type snmpConnection interface {
//BulkWalkAll(string) ([]gosnmp.SnmpPDU, error)
Walk(string, gosnmp.WalkFunc) error
Get(oids []string) (*gosnmp.SnmpPacket, error)
Reconnect() error
}

// getConnection creates a snmpConnection (*gosnmp.GoSNMP) object and caches the
Expand All @@ -568,6 +569,10 @@ type snmpConnection interface {
// more than one goroutine.
func (s *Snmp) getConnection(idx int) (snmpConnection, error) {
if gs := s.connectionCache[idx]; gs != nil {
if err := gs.Reconnect(); err != nil {
return gs, fmt.Errorf("reconnecting: %w", err)
}

return gs, nil
}

Expand All @@ -585,13 +590,13 @@ func (s *Snmp) getConnection(idx int) (snmpConnection, error) {
return nil, err
}

s.connectionCache[idx] = gs
s.connectionCache[idx] = &gs

if err := gs.Connect(); err != nil {
return nil, fmt.Errorf("setting up connection: %w", err)
}

return gs, nil
return &gs, nil
}

// fieldConvert converts from any type according to the conv specification
Expand Down
15 changes: 9 additions & 6 deletions plugins/inputs/snmp/snmp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ func (tsc *testSNMPConnection) Walk(oid string, wf gosnmp.WalkFunc) error {
}
return nil
}
func (tsc *testSNMPConnection) Reconnect() error {
return nil
}

var tsc = &testSNMPConnection{
host: "tsc",
Expand Down Expand Up @@ -261,7 +264,7 @@ func TestGetSNMPConnection_v2(t *testing.T) {

gsc, err := s.getConnection(0)
require.NoError(t, err)
gs := gsc.(snmp.GosnmpWrapper)
gs := gsc.(*snmp.GosnmpWrapper)
assert.Equal(t, "1.2.3.4", gs.Target)
assert.EqualValues(t, 567, gs.Port)
assert.Equal(t, gosnmp.Version2c, gs.Version)
Expand All @@ -270,14 +273,14 @@ func TestGetSNMPConnection_v2(t *testing.T) {

gsc, err = s.getConnection(1)
require.NoError(t, err)
gs = gsc.(snmp.GosnmpWrapper)
gs = gsc.(*snmp.GosnmpWrapper)
assert.Equal(t, "1.2.3.4", gs.Target)
assert.EqualValues(t, 161, gs.Port)
assert.Equal(t, "udp", gs.Transport)

gsc, err = s.getConnection(2)
require.NoError(t, err)
gs = gsc.(snmp.GosnmpWrapper)
gs = gsc.(*snmp.GosnmpWrapper)
assert.Equal(t, "127.0.0.1", gs.Target)
assert.EqualValues(t, 161, gs.Port)
assert.Equal(t, "udp", gs.Transport)
Expand All @@ -301,7 +304,7 @@ func TestGetSNMPConnectionTCP(t *testing.T) {
wg.Add(1)
gsc, err := s.getConnection(0)
require.NoError(t, err)
gs := gsc.(snmp.GosnmpWrapper)
gs := gsc.(*snmp.GosnmpWrapper)
assert.Equal(t, "127.0.0.1", gs.Target)
assert.EqualValues(t, 56789, gs.Port)
assert.Equal(t, "tcp", gs.Transport)
Expand Down Expand Up @@ -348,7 +351,7 @@ func TestGetSNMPConnection_v3(t *testing.T) {

gsc, err := s.getConnection(0)
require.NoError(t, err)
gs := gsc.(snmp.GosnmpWrapper)
gs := gsc.(*snmp.GosnmpWrapper)
assert.Equal(t, gs.Version, gosnmp.Version3)
sp := gs.SecurityParameters.(*gosnmp.UsmSecurityParameters)
assert.Equal(t, "1.2.3.4", gsc.Host())
Expand Down Expand Up @@ -469,7 +472,7 @@ func TestGetSNMPConnection_v3_blumenthal(t *testing.T) {

gsc, err := s.getConnection(0)
require.NoError(t, err)
gs := gsc.(snmp.GosnmpWrapper)
gs := gsc.(*snmp.GosnmpWrapper)
assert.Equal(t, gs.Version, gosnmp.Version3)
sp := gs.SecurityParameters.(*gosnmp.UsmSecurityParameters)
assert.Equal(t, "1.2.3.4", gsc.Host())
Expand Down
6 changes: 3 additions & 3 deletions plugins/processors/ifname/ifname.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,11 @@ func (d *IfName) getMapRemoteNoMock(agent string) (nameMap, error) {
//try ifXtable and ifName first. if that fails, fall back to
//ifTable and ifDescr
var m nameMap
if m, err = d.buildMap(gs, d.ifXTable); err == nil {
if m, err = d.buildMap(&gs, d.ifXTable); err == nil {
return m, nil
}

if m, err = d.buildMap(gs, d.ifTable); err == nil {
if m, err = d.buildMap(&gs, d.ifTable); err == nil {
return m, nil
}

Expand Down Expand Up @@ -308,7 +308,7 @@ func (d *IfName) makeTableNoMock(oid string) (*si.Table, error) {
return &tab, nil
}

func (d *IfName) buildMap(gs snmp.GosnmpWrapper, tab *si.Table) (nameMap, error) {
func (d *IfName) buildMap(gs *snmp.GosnmpWrapper, tab *si.Table) (nameMap, error) {
var err error

rtab, err := tab.Build(gs, true, d.translator)
Expand Down
2 changes: 1 addition & 1 deletion plugins/processors/ifname/ifname_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestTable(t *testing.T) {
require.NoError(t, err)

// Could use ifIndex but oid index is always the same
m, err := d.buildMap(gs, tab)
m, err := d.buildMap(&gs, tab)
require.NoError(t, err)
require.NotEmpty(t, m)
}
Expand Down

0 comments on commit 885444a

Please sign in to comment.