From 9285dd1b07de21b1bdcd26cdbff0be1b32e1dfb2 Mon Sep 17 00:00:00 2001 From: Thomas Casteleyn Date: Wed, 25 May 2022 20:47:51 +0200 Subject: [PATCH] fix(inputs/snmp): Reconnect TCP agents if needed (#11163) --- internal/snmp/wrapper.go | 8 ++++++++ plugins/inputs/snmp/snmp.go | 9 +++++++-- plugins/inputs/snmp/snmp_test.go | 15 +++++++++------ plugins/processors/ifname/ifname.go | 6 +++--- plugins/processors/ifname/ifname_test.go | 2 +- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/internal/snmp/wrapper.go b/internal/snmp/wrapper.go index 49595d4fdc8a5..ec0638d488075 100644 --- a/internal/snmp/wrapper.go +++ b/internal/snmp/wrapper.go @@ -170,3 +170,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 +} diff --git a/plugins/inputs/snmp/snmp.go b/plugins/inputs/snmp/snmp.go index aab01fc32d4b9..d48f92aadff5a 100644 --- a/plugins/inputs/snmp/snmp.go +++ b/plugins/inputs/snmp/snmp.go @@ -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 @@ -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 } @@ -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 diff --git a/plugins/inputs/snmp/snmp_test.go b/plugins/inputs/snmp/snmp_test.go index 25346c1259be5..68a1747899fda 100644 --- a/plugins/inputs/snmp/snmp_test.go +++ b/plugins/inputs/snmp/snmp_test.go @@ -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", @@ -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) @@ -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) @@ -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) @@ -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()) @@ -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()) diff --git a/plugins/processors/ifname/ifname.go b/plugins/processors/ifname/ifname.go index 9c5c39dd0fa94..df97beab73344 100644 --- a/plugins/processors/ifname/ifname.go +++ b/plugins/processors/ifname/ifname.go @@ -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 } @@ -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) diff --git a/plugins/processors/ifname/ifname_test.go b/plugins/processors/ifname/ifname_test.go index c639fc2f21af2..56eed24c1802c 100644 --- a/plugins/processors/ifname/ifname_test.go +++ b/plugins/processors/ifname/ifname_test.go @@ -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) }