diff --git a/lib/towerops/workers/discovery_worker.ex b/lib/towerops/workers/discovery_worker.ex index 86f0c3d5..9ae08cce 100644 --- a/lib/towerops/workers/discovery_worker.ex +++ b/lib/towerops/workers/discovery_worker.ex @@ -131,6 +131,15 @@ defmodule Towerops.Workers.DiscoveryWorker do end end + defp perform_discovery(%{snmp_enabled: false} = device) do + Logger.info( + "Skipping discovery for #{device.name} (#{device.ip_address}): SNMP disabled", + device_id: device.id + ) + + {:error, :snmp_not_enabled} + end + defp perform_discovery(device) do start_time = System.monotonic_time(:millisecond) @@ -139,7 +148,6 @@ defmodule Towerops.Workers.DiscoveryWorker do device_id: device.id, device_name: device.name, device_ip: device.ip_address, - snmp_enabled: device.snmp_enabled, snmp_port: device.snmp_port || 161 ) @@ -282,6 +290,8 @@ defmodule Towerops.Workers.DiscoveryWorker do def retryable_error?(:invalid_config), do: false def retryable_error?(:no_snmp_credentials), do: false def retryable_error?(:device_deleted), do: false + def retryable_error?(:snmp_not_enabled), do: false + def retryable_error?(:device_unresponsive), do: false def retryable_error?(_reason), do: true # Private helpers diff --git a/test/towerops/workers/discovery_worker_test.exs b/test/towerops/workers/discovery_worker_test.exs index 5a212ff0..e6dabf3c 100644 --- a/test/towerops/workers/discovery_worker_test.exs +++ b/test/towerops/workers/discovery_worker_test.exs @@ -79,15 +79,44 @@ defmodule Towerops.Workers.DiscoveryWorkerTest do end test "returns error for transient failures to allow retry", %{device: device} do - # Mock SNMP failure with a transient error (timeout) - stub(SnmpMock, :get, fn _target, _oid, _opts -> - {:error, :timeout} + # Mock SNMP to pass speed categorization but fail during discovery + 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, "Test"} + "1.3.6.1.2.1.1.2.0" -> {:ok, [1, 3, 6, 1]} + "1.3.6.1.2.1.1.3.0" -> {:ok, 1} + # Other gets fail with transient error + _ -> {:error, :timeout} + end 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 SNMP responses - use stub for flexibility as discovery calls vary stub(SnmpMock, :get, fn _target, oid, _opts -> @@ -198,6 +227,14 @@ defmodule Towerops.Workers.DiscoveryWorkerTest 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