Skip SNMP check execution in CheckExecutorWorker for agent-polled devices
CheckExecutorWorker was missing the phoenix_snmp_disabled and should_phoenix_poll_device guards that DevicePollerWorker and DeviceMonitorWorker already have. This caused SNMP checks to execute through Phoenix even when devices are assigned to agents, creating an infinite loop of failing jobs that fill the Oban queue.
This commit is contained in:
parent
327c981422
commit
1712e7ac9d
3 changed files with 193 additions and 7 deletions
|
|
@ -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})")
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue