diff --git a/lib/towerops/workers/check_executor_worker.ex b/lib/towerops/workers/check_executor_worker.ex index 7727a4eb..332b5324 100644 --- a/lib/towerops/workers/check_executor_worker.ex +++ b/lib/towerops/workers/check_executor_worker.ex @@ -23,6 +23,8 @@ defmodule Towerops.Workers.CheckExecutorWorker do queue: :check_executors, max_attempts: 3 + alias Towerops.Agents + alias Towerops.Devices.Device alias Towerops.Monitoring alias Towerops.Monitoring.Executors.DnsExecutor alias Towerops.Monitoring.Executors.HttpExecutor @@ -31,10 +33,13 @@ defmodule Towerops.Workers.CheckExecutorWorker do alias Towerops.Monitoring.Executors.SnmpSensorExecutor alias Towerops.Monitoring.Executors.SnmpStorageExecutor alias Towerops.Monitoring.Executors.TcpExecutor + alias Towerops.Snmp.Client alias Towerops.Workers.PollingOffset require Logger + @snmp_check_types ~w(snmp_sensor snmp_interface snmp_processor snmp_storage) + @impl Oban.Worker def perform(%Oban.Job{args: %{"check_id" => check_id}}) do case Monitoring.get_check(check_id) do @@ -43,17 +48,29 @@ defmodule Towerops.Workers.CheckExecutorWorker do :ok check -> - if check.enabled do - execute_and_record(check) - schedule_next_check(check) - else - Logger.debug("Check #{check_id} disabled, skipping execution") + cond do + not check.enabled -> + Logger.debug("Check #{check_id} disabled, skipping execution") + + should_skip_snmp_check?(check) -> + Logger.debug("Skipping SNMP check #{check_id} - Phoenix SNMP disabled or device assigned to agent") + + true -> + execute_and_record(check) + schedule_next_check(check) end :ok end end + defp should_skip_snmp_check?(%{check_type: check_type} = check) when check_type in @snmp_check_types do + Client.phoenix_snmp_disabled() or + (check.device_id != nil and not Agents.should_phoenix_poll_device?(%Device{id: check.device_id})) + end + + defp should_skip_snmp_check?(_check), do: false + defp execute_and_record(check) do Logger.debug("Executing check #{check.id} (#{check.check_type})") diff --git a/lib/towerops_web/controllers/admin_controller.ex b/lib/towerops_web/controllers/admin_controller.ex index bb886914..2b4087cb 100644 --- a/lib/towerops_web/controllers/admin_controller.ex +++ b/lib/towerops_web/controllers/admin_controller.ex @@ -28,7 +28,7 @@ defmodule ToweropsWeb.AdminController do import Ecto.Query valid_states = ~w(scheduled retryable available) - requested = String.split(states_param, ",") |> Enum.map(&String.trim/1) + requested = states_param |> String.split(",") |> Enum.map(&String.trim/1) states = Enum.filter(requested, &(&1 in valid_states)) if states == [] do @@ -39,7 +39,7 @@ defmodule ToweropsWeb.AdminController do |> where([j], j.state in ^states) |> Towerops.Repo.delete_all() - conn |> json(%{ok: true, deleted: count, states: states}) + json(conn, %{ok: true, deleted: count, states: states}) end end end diff --git a/test/towerops/workers/check_executor_worker_test.exs b/test/towerops/workers/check_executor_worker_test.exs index 32276fcf..abe4bedf 100644 --- a/test/towerops/workers/check_executor_worker_test.exs +++ b/test/towerops/workers/check_executor_worker_test.exs @@ -312,4 +312,173 @@ defmodule Towerops.Workers.CheckExecutorWorkerTest do assert :ok = perform_job(CheckExecutorWorker, %{check_id: Ecto.UUID.generate()}) end end + + describe "perform/1 - SNMP check skipping" do + test "skips SNMP check when device is assigned to a local agent", %{ + device: device, + organization: organization, + snmp_device: snmp_device + } do + # Create a local agent and assign device to it + {:ok, agent_token, _token} = Towerops.Agents.create_agent_token(organization.id, "Local Agent") + {:ok, _assignment} = Towerops.Agents.assign_device_to_agent(agent_token.id, device.id) + + sensor = + %Sensor{} + |> Sensor.changeset(%{ + snmp_device_id: snmp_device.id, + sensor_descr: "Agent Sensor", + sensor_type: "temperature", + sensor_index: "1", + sensor_oid: ".1.2.3.4", + sensor_unit: "°C", + sensor_divisor: 1 + }) + |> Repo.insert!() + + {:ok, check} = + Monitoring.create_check(%{ + organization_id: device.organization_id, + device_id: device.id, + name: "Agent Sensor Check", + check_type: "snmp_sensor", + source_type: "auto_discovery", + source_id: sensor.id, + interval_seconds: 60, + enabled: true, + config: %{} + }) + + # Clear existing jobs + Repo.delete_all(Oban.Job) + + # Should NOT call SNMP - device is agent-polled + assert :ok = perform_job(CheckExecutorWorker, %{check_id: check.id}) + + # No results should be recorded + results = Repo.all(from(r in CheckResult, where: r.check_id == ^check.id)) + assert results == [] + + # Should NOT reschedule + jobs = Repo.all(from(j in Oban.Job, where: j.worker == "Towerops.Workers.CheckExecutorWorker")) + assert jobs == [] + end + + test "skips snmp_interface check when device is assigned to agent", %{ + device: device, + organization: organization + } do + {:ok, agent_token, _token} = Towerops.Agents.create_agent_token(organization.id, "Local Agent") + {:ok, _assignment} = Towerops.Agents.assign_device_to_agent(agent_token.id, device.id) + + {:ok, check} = + Monitoring.create_check(%{ + organization_id: device.organization_id, + device_id: device.id, + name: "Interface Check", + check_type: "snmp_interface", + source_type: "auto_discovery", + source_id: Ecto.UUID.generate(), + interval_seconds: 60, + enabled: true, + config: %{"if_index" => "1", "if_descr" => "eth0"} + }) + + Repo.delete_all(Oban.Job) + + assert :ok = perform_job(CheckExecutorWorker, %{check_id: check.id}) + + results = Repo.all(from(r in CheckResult, where: r.check_id == ^check.id)) + assert results == [] + end + + test "still executes non-SNMP checks when device is assigned to agent", %{ + device: device, + organization: organization + } do + {:ok, agent_token, _token} = Towerops.Agents.create_agent_token(organization.id, "Local Agent") + {:ok, _assignment} = Towerops.Agents.assign_device_to_agent(agent_token.id, device.id) + + {:ok, check} = + Monitoring.create_check(%{ + organization_id: device.organization_id, + device_id: device.id, + name: "HTTP Check", + check_type: "http", + source_type: "manual", + interval_seconds: 60, + enabled: true, + config: %{"url" => "https://example.com"} + }) + + Repo.delete_all(Oban.Job) + + # HTTP checks should still execute even when device has an agent + assert :ok = perform_job(CheckExecutorWorker, %{check_id: check.id}) + + # Should have a result (even if error from network) + results = Repo.all(from(r in CheckResult, where: r.check_id == ^check.id)) + assert length(results) == 1 + end + + test "skips SNMP check when phoenix_snmp_disabled is true", %{ + device: device, + snmp_device: snmp_device + } do + # Override env to simulate non-test environment with SNMP disabled + old_env = Application.get_env(:towerops, :env) + old_disable = Application.get_env(:towerops, :disable_phoenix_snmp) + Application.put_env(:towerops, :env, :prod) + Application.put_env(:towerops, :disable_phoenix_snmp, true) + + on_exit(fn -> + Application.put_env(:towerops, :env, old_env) + + if old_disable == nil do + Application.delete_env(:towerops, :disable_phoenix_snmp) + else + Application.put_env(:towerops, :disable_phoenix_snmp, old_disable) + end + end) + + sensor = + %Sensor{} + |> Sensor.changeset(%{ + snmp_device_id: snmp_device.id, + sensor_descr: "Disabled SNMP Sensor", + sensor_type: "temperature", + sensor_index: "2", + sensor_oid: ".1.2.3.5", + sensor_unit: "°C", + sensor_divisor: 1 + }) + |> Repo.insert!() + + {:ok, check} = + Monitoring.create_check(%{ + organization_id: device.organization_id, + device_id: device.id, + name: "Disabled SNMP Check", + check_type: "snmp_sensor", + source_type: "auto_discovery", + source_id: sensor.id, + interval_seconds: 60, + enabled: true, + config: %{} + }) + + Repo.delete_all(Oban.Job) + + # Should skip without calling SNMP + assert :ok = perform_job(CheckExecutorWorker, %{check_id: check.id}) + + # No results recorded + results = Repo.all(from(r in CheckResult, where: r.check_id == ^check.id)) + assert results == [] + + # No rescheduling + jobs = Repo.all(from(j in Oban.Job, where: j.worker == "Towerops.Workers.CheckExecutorWorker")) + assert jobs == [] + end + end end