skip all server-side checks when any agent is assigned (#77)
## Summary - Add `device_has_effective_agent?/1` to Agents context that checks full cascade (device → site → org → global default) without distinguishing between cloud pollers and local agents - Update check_executor_worker, device_poller_worker, and device_monitor_worker to use new function - Phoenix never executes checks (DNS, ping, HTTP, TCP, SSL, SNMP) when any agent handles the device ## Problem The global default cloud poller was not being checked in `should_phoenix_poll_device?/1`, causing Phoenix to run DNS/ping/service checks server-side even when the Go agent was handling the device. This produced duplicate results with NXDOMAIN errors from the Erlang DNS resolver. ## Test plan - [x] 6 new tests for `device_has_effective_agent?/1` covering all cascade levels - [x] Full test suite passes (8601 tests) - [ ] Verify staging DNS check stops showing NXDOMAIN errors after deploy Reviewed-on: graham/towerops-web#77
This commit is contained in:
parent
5493a83114
commit
f3cf1a9b3c
5 changed files with 130 additions and 17 deletions
|
|
@ -1011,6 +1011,42 @@ defmodule Towerops.Agents do
|
|||
end
|
||||
end
|
||||
|
||||
@doc """
|
||||
Returns true if the device has any effective agent assigned (cloud or local)
|
||||
at any level: device, site, organization, or global default.
|
||||
|
||||
Unlike `should_phoenix_poll_device?/1`, this makes no distinction between
|
||||
cloud pollers and local agents. Any agent means the Go agent handles polling.
|
||||
|
||||
Used by the check executor to decide whether to skip server-side execution.
|
||||
"""
|
||||
@spec device_has_effective_agent?(Ecto.UUID.t()) :: boolean()
|
||||
def device_has_effective_agent?(device_id) do
|
||||
query =
|
||||
from d in Device,
|
||||
left_join: aa in AgentAssignment,
|
||||
on: aa.device_id == d.id and aa.enabled == true,
|
||||
left_join: s in assoc(d, :site),
|
||||
left_join: o in assoc(d, :organization),
|
||||
where: d.id == ^device_id,
|
||||
select: %{
|
||||
has_device_assignment: not is_nil(aa.agent_token_id),
|
||||
has_site_token: not is_nil(s.agent_token_id),
|
||||
has_org_token: not is_nil(o.default_agent_token_id)
|
||||
}
|
||||
|
||||
case Repo.one(query) do
|
||||
nil ->
|
||||
false
|
||||
|
||||
result ->
|
||||
result.has_device_assignment or
|
||||
result.has_site_token or
|
||||
result.has_org_token or
|
||||
Towerops.Settings.get_global_default_cloud_poller() != nil
|
||||
end
|
||||
end
|
||||
|
||||
## PubSub notifications
|
||||
|
||||
@doc """
|
||||
|
|
|
|||
|
|
@ -24,7 +24,6 @@ defmodule Towerops.Workers.CheckExecutorWorker do
|
|||
max_attempts: 3
|
||||
|
||||
alias Towerops.Agents
|
||||
alias Towerops.Devices.Device
|
||||
alias Towerops.Monitoring
|
||||
alias Towerops.Monitoring.Executors.DnsExecutor
|
||||
alias Towerops.Monitoring.Executors.HttpExecutor
|
||||
|
|
@ -78,21 +77,22 @@ defmodule Towerops.Workers.CheckExecutorWorker do
|
|||
|
||||
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}))
|
||||
(check.device_id != nil and Agents.device_has_effective_agent?(check.device_id))
|
||||
end
|
||||
|
||||
defp should_skip_snmp_check?(_check), do: false
|
||||
|
||||
defp should_skip_service_check?(%{check_type: check_type} = check) when check_type in @service_check_types do
|
||||
# Skip service checks (HTTP/TCP/DNS) if device is assigned to an agent
|
||||
# The agent will execute these checks from the device's network location
|
||||
check.device_id != nil and not Agents.should_phoenix_poll_device?(%Device{id: check.device_id})
|
||||
# Skip service checks (HTTP/TCP/DNS/SSL) if device has any agent (cloud or local)
|
||||
# The agent handles all check execution - no fallback to Phoenix
|
||||
check.device_id != nil and Agents.device_has_effective_agent?(check.device_id)
|
||||
end
|
||||
|
||||
defp should_skip_service_check?(_check), do: false
|
||||
|
||||
defp should_skip_ping_check?(%{check_type: "ping"} = check) do
|
||||
check.device_id != nil and not Agents.should_phoenix_poll_device?(%Device{id: check.device_id})
|
||||
# Skip ping if device has any agent (cloud or local)
|
||||
check.device_id != nil and Agents.device_has_effective_agent?(check.device_id)
|
||||
end
|
||||
|
||||
defp should_skip_ping_check?(_check), do: false
|
||||
|
|
|
|||
|
|
@ -60,10 +60,10 @@ defmodule Towerops.Workers.DeviceMonitorWorker do
|
|||
end
|
||||
|
||||
# Check if device should continue being monitored
|
||||
# Returns false if Phoenix SNMP is disabled, monitoring is disabled, or device is assigned to an agent
|
||||
# Returns false if Phoenix SNMP is disabled, monitoring is disabled, or device has any agent
|
||||
defp should_continue_monitoring?(device) do
|
||||
!Client.phoenix_snmp_disabled() && device.monitoring_enabled &&
|
||||
Agents.should_phoenix_poll_device?(device)
|
||||
!Agents.device_has_effective_agent?(device.id)
|
||||
end
|
||||
|
||||
defp maybe_perform_check(device) do
|
||||
|
|
@ -74,8 +74,8 @@ defmodule Towerops.Workers.DeviceMonitorWorker do
|
|||
not device.monitoring_enabled ->
|
||||
:ok
|
||||
|
||||
not Agents.should_phoenix_poll_device?(device) ->
|
||||
Logger.debug("Skipping Phoenix monitoring for device #{device.name} - assigned to non-cloud-poller agent")
|
||||
Agents.device_has_effective_agent?(device.id) ->
|
||||
Logger.debug("Skipping Phoenix monitoring for device #{device.name} - device has agent assigned")
|
||||
:ok
|
||||
|
||||
true ->
|
||||
|
|
|
|||
|
|
@ -77,10 +77,10 @@ defmodule Towerops.Workers.DevicePollerWorker do
|
|||
end
|
||||
|
||||
# Check if device should continue being polled
|
||||
# Returns false if Phoenix SNMP is disabled, SNMP is disabled on device, or device is assigned to an agent
|
||||
# Returns false if Phoenix SNMP is disabled, SNMP is disabled on device, or device has any agent
|
||||
defp should_continue_polling?(device) do
|
||||
!Client.phoenix_snmp_disabled() && device.snmp_enabled &&
|
||||
Agents.should_phoenix_poll_device?(device)
|
||||
!Agents.device_has_effective_agent?(device.id)
|
||||
end
|
||||
|
||||
defp maybe_poll_device(device) do
|
||||
|
|
@ -91,8 +91,8 @@ defmodule Towerops.Workers.DevicePollerWorker do
|
|||
not device.snmp_enabled ->
|
||||
:ok
|
||||
|
||||
not Agents.should_phoenix_poll_device?(device) ->
|
||||
Logger.debug("Skipping Phoenix poll for device #{device.name} - assigned to non-cloud-poller agent")
|
||||
Agents.device_has_effective_agent?(device.id) ->
|
||||
Logger.debug("Skipping Phoenix poll for device #{device.name} - device has agent assigned")
|
||||
:ok
|
||||
|
||||
true ->
|
||||
|
|
@ -272,10 +272,10 @@ defmodule Towerops.Workers.DevicePollerWorker do
|
|||
:reassigned
|
||||
|
||||
device ->
|
||||
if Agents.should_phoenix_poll_device?(device) do
|
||||
:ok
|
||||
else
|
||||
if Agents.device_has_effective_agent?(device.id) do
|
||||
:reassigned
|
||||
else
|
||||
:ok
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -1814,6 +1814,83 @@ defmodule Towerops.AgentsTest do
|
|||
end
|
||||
end
|
||||
|
||||
describe "device_has_effective_agent?/1" do
|
||||
setup %{organization: org} do
|
||||
{:ok, site} =
|
||||
Towerops.Sites.create_site(%{
|
||||
name: "Test Site",
|
||||
organization_id: org.id
|
||||
})
|
||||
|
||||
{:ok, device} =
|
||||
Towerops.Devices.create_device(%{
|
||||
name: "Test Device",
|
||||
ip_address: "192.168.1.1",
|
||||
site_id: site.id,
|
||||
organization_id: org.id
|
||||
})
|
||||
|
||||
{:ok, local_agent, _} = Agents.create_agent_token(org.id, "Local Agent")
|
||||
{:ok, cloud_poller, _} = Agents.create_cloud_poller("Cloud Poller")
|
||||
|
||||
%{
|
||||
site: site,
|
||||
device: device,
|
||||
local_agent: local_agent,
|
||||
cloud_poller: cloud_poller
|
||||
}
|
||||
end
|
||||
|
||||
test "returns false when device has no agent assigned", %{device: device} do
|
||||
assert Agents.device_has_effective_agent?(device.id) == false
|
||||
end
|
||||
|
||||
test "returns true when device is assigned to local agent", %{
|
||||
device: device,
|
||||
local_agent: local_agent
|
||||
} do
|
||||
{:ok, _} = Agents.assign_device_to_agent(local_agent.id, device.id)
|
||||
assert Agents.device_has_effective_agent?(device.id) == true
|
||||
end
|
||||
|
||||
test "returns true when device is assigned to cloud poller", %{
|
||||
device: device,
|
||||
cloud_poller: cloud_poller
|
||||
} do
|
||||
{:ok, _} = Agents.assign_device_to_agent(cloud_poller.id, device.id)
|
||||
assert Agents.device_has_effective_agent?(device.id) == true
|
||||
end
|
||||
|
||||
test "returns true when site has agent assigned", %{
|
||||
device: device,
|
||||
site: site,
|
||||
local_agent: local_agent
|
||||
} do
|
||||
{:ok, _} = Towerops.Sites.update_site(site, %{agent_token_id: local_agent.id})
|
||||
assert Agents.device_has_effective_agent?(device.id) == true
|
||||
end
|
||||
|
||||
test "returns true when organization has default agent", %{
|
||||
organization: org,
|
||||
device: device,
|
||||
local_agent: local_agent
|
||||
} do
|
||||
{:ok, _} = Towerops.Organizations.update_organization(org, %{default_agent_token_id: local_agent.id})
|
||||
assert Agents.device_has_effective_agent?(device.id) == true
|
||||
end
|
||||
|
||||
test "returns true when global default cloud poller is set", %{
|
||||
device: device,
|
||||
cloud_poller: cloud_poller
|
||||
} do
|
||||
{:ok, _} = Towerops.Settings.set_global_default_cloud_poller(cloud_poller.id)
|
||||
assert Agents.device_has_effective_agent?(device.id) == true
|
||||
|
||||
# Clean up
|
||||
{:ok, _} = Towerops.Settings.set_global_default_cloud_poller(nil)
|
||||
end
|
||||
end
|
||||
|
||||
describe "list_updatable_agents/0" do
|
||||
test "returns agents with recent last_seen_at", %{organization: org} do
|
||||
{:ok, agent, _token} = Agents.create_agent_token(org.id, "Active Agent")
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue