Replace sequential multi-GET with true batched SNMP GET support.
Previously, Client.get_multiple/2 sent N individual SNMP GET PDUs
for N OIDs. Now sends a single PDU with multiple varbinds, reducing
network round-trips by ~80% for multi-OID operations.
Implementation:
- Add get_multiple/3 callback to SnmpBehaviour
- Implement batched GET in Manager using PDU.build_get_request_multi/2
- Update Client.get_multiple/2 to use batched GET with sequential fallback
- Return map format %{oid => {type, value}} for batched results
- Convert to ordered list for backward compatibility
- Handle SNMP exceptions (noSuchObject, noSuchInstance) in result map
Testing:
- Update all test mocks to use get_multiple expectations
- Add individual get stubs for categorize_device_speed and optional fields
- 2249/2251 tests passing (99.87%)
This addresses F1 from LibreNMS feature parity analysis:
"Replace sequential multi-get with batched GET support."
Impact:
- Discovery system info (6 OIDs): 1 SNMP request instead of 6
- Storage polling (3 OIDs/entry): 1 request per entry instead of 3
- Network round-trips reduced by ~80% for all get_multiple operations
Reviewed-on: graham/towerops-web#183
280 lines
9.2 KiB
Elixir
280 lines
9.2 KiB
Elixir
defmodule Towerops.Workers.DiscoveryWorkerTest do
|
|
use Towerops.DataCase, async: true
|
|
|
|
import Mox
|
|
import Towerops.AccountsFixtures
|
|
|
|
alias Towerops.Snmp
|
|
alias Towerops.Snmp.SnmpMock
|
|
alias Towerops.Workers.DiscoveryWorker
|
|
|
|
setup :verify_on_exit!
|
|
|
|
describe "perform/1" do
|
|
setup do
|
|
user = user_fixture()
|
|
{:ok, organization} = Towerops.Organizations.create_organization(%{name: "Test Org"}, user.id)
|
|
|
|
{:ok, site} =
|
|
Towerops.Sites.create_site(%{
|
|
name: "Test Site",
|
|
organization_id: organization.id
|
|
})
|
|
|
|
{:ok, device} =
|
|
Towerops.Devices.create_device(%{
|
|
name: "Test Device",
|
|
ip_address: "192.168.1.1",
|
|
snmp_enabled: true,
|
|
snmp_version: "2c",
|
|
snmp_community: "public",
|
|
snmp_port: 161,
|
|
site_id: site.id,
|
|
organization_id: organization.id
|
|
})
|
|
|
|
{:ok, device: device, org: organization}
|
|
end
|
|
|
|
test "successfully discovers a device", %{device: device} do
|
|
# Mock batched GET for system info (discovery now uses get_multiple)
|
|
stub(SnmpMock, :get_multiple, fn _target, oids, _opts ->
|
|
result_map =
|
|
Map.new(oids, fn oid ->
|
|
value =
|
|
case oid do
|
|
"1.3.6.1.2.1.1.1.0" -> {:octet_string, "Test Device"}
|
|
"1.3.6.1.2.1.1.2.0" -> {:object_identifier, [1, 3, 6, 1, 4, 1, 9]}
|
|
"1.3.6.1.2.1.1.3.0" -> {:timeticks, 12_345}
|
|
"1.3.6.1.2.1.1.4.0" -> {:octet_string, "admin@test.com"}
|
|
"1.3.6.1.2.1.1.5.0" -> {:octet_string, "test-device"}
|
|
"1.3.6.1.2.1.1.6.0" -> {:octet_string, "Test Location"}
|
|
_ -> {:octet_string, ""}
|
|
end
|
|
|
|
{oid, value}
|
|
end)
|
|
|
|
{:ok, result_map}
|
|
end)
|
|
|
|
# Mock individual GET calls (for categorize_device_speed, test_connection, and optional fields)
|
|
stub(SnmpMock, :get, fn _target, oid, _opts ->
|
|
case oid do
|
|
"1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Test Device"}}
|
|
# sysUptime for test_connection
|
|
"1.3.6.1.2.1.1.3.0" -> {:ok, {:timeticks, 12_345}}
|
|
# UCD-SNMP-MIB CPU stats (processor discovery fallback)
|
|
"1.3.6.1.4.1.2021.11.9.0" -> {:ok, {:integer, 10}}
|
|
"1.3.6.1.4.1.2021.11.10.0" -> {:ok, {:integer, 5}}
|
|
"1.3.6.1.4.1.2021.11.11.0" -> {:ok, {:integer, 85}}
|
|
_ -> {:error, :no_such_object}
|
|
end
|
|
end)
|
|
|
|
# Mock interface discovery (if_index walk + other IF-MIB walks) and neighbor discovery
|
|
# Total: if_index + if_descr + if_type + if_speed + if_phys_address + if_admin_status +
|
|
# if_oper_status + if_name + if_alias (9 walks) + LLDP (6 walks) + CDP (1 walk) = 16 walks
|
|
# Simplify: just allow any number of walk calls to return empty
|
|
stub(SnmpMock, :walk, fn _target, _oid, _opts ->
|
|
{:ok, []}
|
|
end)
|
|
|
|
assert :ok = DiscoveryWorker.perform(%Oban.Job{args: %{"device_id" => device.id}})
|
|
|
|
# Verify SNMP device was created
|
|
snmp_device = Snmp.get_device(device.id)
|
|
assert snmp_device
|
|
assert snmp_device.sys_descr == "Test Device"
|
|
assert snmp_device.sys_name == "test-device"
|
|
end
|
|
|
|
test "returns :discard when device not found" do
|
|
non_existent_id = Ecto.UUID.generate()
|
|
|
|
assert :discard =
|
|
DiscoveryWorker.perform(%Oban.Job{args: %{"device_id" => non_existent_id}})
|
|
end
|
|
|
|
test "returns error for transient failures to allow retry", %{device: device} do
|
|
# Mock individual GET to pass speed categorization
|
|
stub(SnmpMock, :get, fn _target, oid, _opts ->
|
|
case oid do
|
|
# Speed categorization probe succeeds (device is responsive)
|
|
"1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Test"}}
|
|
_ -> {:error, :timeout}
|
|
end
|
|
end)
|
|
|
|
# Mock batched GET to fail (transient error during system info discovery)
|
|
stub(SnmpMock, :get_multiple, fn _target, _oids, _opts ->
|
|
{:error, :timeout}
|
|
end)
|
|
|
|
stub(SnmpMock, :walk, fn _target, _oid, _opts -> {:error, :timeout} end)
|
|
|
|
# Transient failures should return {:error, reason} so Oban can retry
|
|
assert {:error, _reason} = DiscoveryWorker.perform(%Oban.Job{args: %{"device_id" => device.id}})
|
|
end
|
|
|
|
test "skips discovery when snmp_enabled is false", %{org: organization} do
|
|
{:ok, site} =
|
|
Towerops.Sites.create_site(%{
|
|
name: "No SNMP Site",
|
|
organization_id: organization.id
|
|
})
|
|
|
|
{:ok, device} =
|
|
Towerops.Devices.create_device(%{
|
|
name: "No SNMP Device",
|
|
ip_address: "192.168.1.99",
|
|
snmp_enabled: false,
|
|
site_id: site.id,
|
|
organization_id: organization.id
|
|
})
|
|
|
|
# Should discard immediately without attempting SNMP
|
|
assert :discard = DiscoveryWorker.perform(%Oban.Job{args: %{"device_id" => device.id}})
|
|
end
|
|
|
|
test "successfully completes discovery with proper mocks", %{device: device} do
|
|
# Mock batched GET for system info
|
|
stub(SnmpMock, :get_multiple, fn _target, oids, _opts ->
|
|
result_map =
|
|
Map.new(oids, fn oid ->
|
|
value =
|
|
case oid do
|
|
"1.3.6.1.2.1.1.1.0" -> {:octet_string, "Test"}
|
|
"1.3.6.1.2.1.1.2.0" -> {:object_identifier, [1, 3, 6, 1]}
|
|
"1.3.6.1.2.1.1.3.0" -> {:timeticks, 1}
|
|
"1.3.6.1.2.1.1.4.0" -> {:octet_string, ""}
|
|
"1.3.6.1.2.1.1.5.0" -> {:octet_string, "test"}
|
|
"1.3.6.1.2.1.1.6.0" -> {:octet_string, ""}
|
|
_ -> {:octet_string, ""}
|
|
end
|
|
|
|
{oid, value}
|
|
end)
|
|
|
|
{:ok, result_map}
|
|
end)
|
|
|
|
# Mock individual GET calls (for categorize_device_speed, test_connection, and optional fields)
|
|
stub(SnmpMock, :get, fn _target, oid, _opts ->
|
|
case oid do
|
|
"1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Test"}}
|
|
# sysUptime for test_connection
|
|
"1.3.6.1.2.1.1.3.0" -> {:ok, {:timeticks, 1}}
|
|
# UCD-SNMP-MIB CPU stats (processor discovery fallback)
|
|
"1.3.6.1.4.1.2021.11.9.0" -> {:ok, {:integer, 10}}
|
|
"1.3.6.1.4.1.2021.11.10.0" -> {:ok, {:integer, 5}}
|
|
"1.3.6.1.4.1.2021.11.11.0" -> {:ok, {:integer, 85}}
|
|
_ -> {:error, :no_such_object}
|
|
end
|
|
end)
|
|
|
|
# Allow any number of walk calls (interface + neighbor discovery)
|
|
stub(SnmpMock, :walk, fn _target, _oid, _opts -> {:ok, []} end)
|
|
|
|
assert :ok = DiscoveryWorker.perform(%Oban.Job{args: %{"device_id" => device.id}})
|
|
|
|
# Verify SNMP device was created
|
|
snmp_device = Snmp.get_device(device.id)
|
|
assert snmp_device
|
|
end
|
|
|
|
test "logs error when discovery fails", %{device: device} do
|
|
import ExUnit.CaptureLog
|
|
|
|
stub(SnmpMock, :get, fn _target, _oid, _opts ->
|
|
{:error, :network_unreachable}
|
|
end)
|
|
|
|
log =
|
|
capture_log(fn ->
|
|
DiscoveryWorker.perform(%Oban.Job{args: %{"device_id" => device.id}})
|
|
end)
|
|
|
|
assert log =~ "Direct SNMP discovery failed:"
|
|
end
|
|
end
|
|
|
|
describe "enqueue/1" do
|
|
setup do
|
|
user = user_fixture()
|
|
{:ok, organization} = Towerops.Organizations.create_organization(%{name: "Enqueue Org"}, user.id)
|
|
|
|
{:ok, site} =
|
|
Towerops.Sites.create_site(%{
|
|
name: "Enqueue Site",
|
|
organization_id: organization.id
|
|
})
|
|
|
|
{:ok, device} =
|
|
Towerops.Devices.create_device(%{
|
|
name: "Enqueue Device",
|
|
ip_address: "192.168.1.50",
|
|
snmp_enabled: true,
|
|
site_id: site.id,
|
|
organization_id: organization.id
|
|
})
|
|
|
|
{:ok, device: device}
|
|
end
|
|
|
|
test "enqueues a discovery job", %{device: device} do
|
|
assert :ok = DiscoveryWorker.enqueue(device.id)
|
|
end
|
|
|
|
test "deduplicates jobs within 60 seconds", %{device: device} do
|
|
assert :ok = DiscoveryWorker.enqueue(device.id)
|
|
assert :ok = DiscoveryWorker.enqueue(device.id)
|
|
end
|
|
end
|
|
|
|
describe "retryable_error?/1" do
|
|
test "timeout is retryable" do
|
|
assert DiscoveryWorker.retryable_error?(:timeout)
|
|
end
|
|
|
|
test "network_unreachable is retryable" do
|
|
assert DiscoveryWorker.retryable_error?(:network_unreachable)
|
|
end
|
|
|
|
test "connection_refused is retryable" do
|
|
assert DiscoveryWorker.retryable_error?(:connection_refused)
|
|
end
|
|
|
|
test "econnrefused is retryable" do
|
|
assert DiscoveryWorker.retryable_error?(:econnrefused)
|
|
end
|
|
|
|
test "device_not_found is not retryable" do
|
|
refute DiscoveryWorker.retryable_error?(:device_not_found)
|
|
end
|
|
|
|
test "invalid_config is not retryable" do
|
|
refute DiscoveryWorker.retryable_error?(:invalid_config)
|
|
end
|
|
|
|
test "no_snmp_credentials is not retryable" do
|
|
refute DiscoveryWorker.retryable_error?(:no_snmp_credentials)
|
|
end
|
|
|
|
test "device_deleted is not retryable" do
|
|
refute DiscoveryWorker.retryable_error?(:device_deleted)
|
|
end
|
|
|
|
test "snmp_not_enabled is not retryable" do
|
|
refute DiscoveryWorker.retryable_error?(:snmp_not_enabled)
|
|
end
|
|
|
|
test "device_unresponsive is not retryable" do
|
|
refute DiscoveryWorker.retryable_error?(:device_unresponsive)
|
|
end
|
|
|
|
test "unknown errors are retryable by default" do
|
|
assert DiscoveryWorker.retryable_error?(:some_unknown_error)
|
|
end
|
|
end
|
|
end
|