diff --git a/config/runtime.exs b/config/runtime.exs index a7f13ed5..32daa9fe 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -236,9 +236,11 @@ if config_env() == :prod do # for details about using IPv6 vs IPv4 and loopback vs public addresses. ip: {0, 0, 0, 0, 0, 0, 0, 0}, # Bandit uses ThousandIsland for connection management - # Timeout for reading client data before closing the connection (30 seconds) + # Timeout for reading client data before closing the connection. + # Set high enough to accommodate long-lived WebSocket connections + # (agents send Phoenix transport heartbeats every 25s). thousand_island_options: [ - read_timeout: 30_000 + read_timeout: 120_000 ] ], secret_key_base: secret_key_base diff --git a/lib/towerops/agents.ex b/lib/towerops/agents.ex index 3b904e86..0f8d57b4 100644 --- a/lib/towerops/agents.ex +++ b/lib/towerops/agents.ex @@ -616,6 +616,43 @@ defmodule Towerops.Agents do end end + @doc """ + Checks if a device should be polled by Phoenix directly. + + Returns false if the device is assigned to a non-cloud-poller agent + (i.e., a customer/local agent). Phoenix should never poll devices that + are assigned to local agents - those devices should only be polled by + their assigned agent. + + Returns true if: + - No agent is assigned to the device + - The device is assigned to a cloud poller (Phoenix-hosted agent) + + ## Examples + + iex> should_phoenix_poll_device?(device_with_local_agent) + false + + iex> should_phoenix_poll_device?(device_with_cloud_poller) + true + + iex> should_phoenix_poll_device?(device_no_agent) + true + + """ + def should_phoenix_poll_device?(%Device{} = device) do + case get_effective_agent_token(device) do + nil -> + # No agent assigned - Phoenix can poll + true + + agent_token_id -> + # Check if the agent is a cloud poller + agent_token = get_agent_token!(agent_token_id) + agent_token.is_cloud_poller + end + end + ## PubSub notifications @doc """ diff --git a/lib/towerops/workers/device_monitor_worker.ex b/lib/towerops/workers/device_monitor_worker.ex index cbf3cf05..68371b99 100644 --- a/lib/towerops/workers/device_monitor_worker.ex +++ b/lib/towerops/workers/device_monitor_worker.ex @@ -14,6 +14,7 @@ defmodule Towerops.Workers.DeviceMonitorWorker do states: [:available, :scheduled, :executing, :retryable] ] + alias Towerops.Agents alias Towerops.Alerts alias Towerops.Devices alias Towerops.Monitoring @@ -34,22 +35,33 @@ defmodule Towerops.Workers.DeviceMonitorWorker do :ok device -> - _ = - if device.monitoring_enabled do - perform_check(device) - end + maybe_perform_check(device) + schedule_next_check_with_error_handling(device_id) + :ok + end + end - # Schedule next check - case schedule_next_check(device_id) do - {:ok, _job} -> - :ok + defp maybe_perform_check(device) do + cond do + not device.monitoring_enabled -> + :ok - {:error, changeset} -> - Logger.error("Failed to schedule next monitoring check for device #{device_id}: #{inspect(changeset.errors)}") + not Agents.should_phoenix_poll_device?(device) -> + Logger.debug("Skipping Phoenix monitoring for device #{device.name} - assigned to non-cloud-poller agent") + :ok - :ok - end + true -> + perform_check(device) + end + end + defp schedule_next_check_with_error_handling(device_id) do + case schedule_next_check(device_id) do + {:ok, _job} -> + :ok + + {:error, changeset} -> + Logger.error("Failed to schedule next monitoring check for device #{device_id}: #{inspect(changeset.errors)}") :ok end end diff --git a/lib/towerops/workers/device_poller_worker.ex b/lib/towerops/workers/device_poller_worker.ex index 1a4f99e4..b007b6c2 100644 --- a/lib/towerops/workers/device_poller_worker.ex +++ b/lib/towerops/workers/device_poller_worker.ex @@ -20,6 +20,7 @@ defmodule Towerops.Workers.DevicePollerWorker do states: [:available, :scheduled, :executing, :retryable] ] + alias Towerops.Agents alias Towerops.Devices alias Towerops.Snmp alias Towerops.Snmp.ArpDiscovery @@ -40,23 +41,35 @@ defmodule Towerops.Workers.DevicePollerWorker do :ok device -> - if device.snmp_enabled do - poll_device(device) - end + maybe_poll_device(device) + schedule_next_poll_with_error_handling(device_id, device) + :ok + end + end - # Schedule next poll - poll_interval = get_poll_interval(device) + defp maybe_poll_device(device) do + cond do + not device.snmp_enabled -> + :ok - case schedule_next_poll(device_id, poll_interval) do - {:ok, _job} -> - :ok + not Agents.should_phoenix_poll_device?(device) -> + Logger.debug("Skipping Phoenix poll for device #{device.name} - assigned to non-cloud-poller agent") + :ok - {:error, changeset} -> - Logger.error("Failed to schedule next poll for device #{device_id}: #{inspect(changeset.errors)}") + true -> + poll_device(device) + end + end - :ok - end + defp schedule_next_poll_with_error_handling(device_id, device) do + poll_interval = get_poll_interval(device) + case schedule_next_poll(device_id, poll_interval) do + {:ok, _job} -> + :ok + + {:error, changeset} -> + Logger.error("Failed to schedule next poll for device #{device_id}: #{inspect(changeset.errors)}") :ok end end diff --git a/lib/towerops_web/live/device_live/show.ex b/lib/towerops_web/live/device_live/show.ex index 2fbbb11d..e4ede254 100644 --- a/lib/towerops_web/live/device_live/show.ex +++ b/lib/towerops_web/live/device_live/show.ex @@ -367,19 +367,22 @@ defmodule ToweropsWeb.DeviceLive.Show do type: :cloud, name: "Cloud Polling", source: nil, - last_seen_at: nil + last_seen_at: nil, + is_offline: false } end defp load_agent_info(agent_token_id, source) do agent_token = Agents.get_agent_token!(agent_token_id) + is_offline = agent_offline?(agent_token) %{ agent_token_id: agent_token_id, type: if(agent_token.is_cloud_poller, do: :cloud, else: :agent), name: agent_token.name, source: source, - last_seen_at: agent_token.last_seen_at + last_seen_at: agent_token.last_seen_at, + is_offline: is_offline } rescue Ecto.NoResultsError -> @@ -389,10 +392,20 @@ defmodule ToweropsWeb.DeviceLive.Show do type: :cloud, name: "Cloud Polling (agent not found)", source: nil, - last_seen_at: nil + last_seen_at: nil, + is_offline: false } end + # Agent is considered offline if it hasn't been seen in the last 5 minutes + defp agent_offline?(%{is_cloud_poller: true}), do: false + defp agent_offline?(%{last_seen_at: nil}), do: true + + defp agent_offline?(%{last_seen_at: last_seen_at}) do + five_minutes_ago = DateTime.add(DateTime.utc_now(), -5, :minute) + DateTime.before?(last_seen_at, five_minutes_ago) + end + defp calculate_metrics(checks, _equipment) do total_checks = length(checks) diff --git a/lib/towerops_web/live/device_live/show.html.heex b/lib/towerops_web/live/device_live/show.html.heex index 90c5990d..313e5617 100644 --- a/lib/towerops_web/live/device_live/show.html.heex +++ b/lib/towerops_web/live/device_live/show.html.heex @@ -44,6 +44,14 @@ <.icon name="hero-server" class="h-3.5 w-3.5" /> {@agent_info.name} + <%= if @agent_info.is_offline do %> + + <.icon name="hero-exclamation-triangle" class="h-3 w-3" /> Offline + + <% end %> <%= if @agent_info.last_seen_at do %> ({format_relative_time(@agent_info.last_seen_at)}) diff --git a/test/towerops/agents_test.exs b/test/towerops/agents_test.exs index 5844750e..6edf136c 100644 --- a/test/towerops/agents_test.exs +++ b/test/towerops/agents_test.exs @@ -1574,4 +1574,159 @@ defmodule Towerops.AgentsTest do # No assertion needed - just verify it doesn't crash end end + + describe "should_phoenix_poll_device?/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 true when device has no agent assigned", %{device: device} do + device = Repo.preload(device, site: [organization: :default_agent_token]) + + assert Agents.should_phoenix_poll_device?(device) == true + end + + test "returns true when device is assigned to cloud poller via direct assignment", %{ + device: device, + cloud_poller: cloud_poller + } do + {:ok, _} = Agents.assign_device_to_agent(cloud_poller.id, device.id) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + assert Agents.should_phoenix_poll_device?(device) == true + end + + test "returns true when device inherits cloud poller from site", %{ + device: device, + site: site, + cloud_poller: cloud_poller + } do + {:ok, _} = Towerops.Sites.update_site(site, %{agent_token_id: cloud_poller.id}) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + assert Agents.should_phoenix_poll_device?(device) == true + end + + test "returns true when device inherits cloud poller from organization", %{ + organization: org, + device: device, + cloud_poller: cloud_poller + } do + {:ok, _} = Towerops.Organizations.update_organization(org, %{default_agent_token_id: cloud_poller.id}) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + assert Agents.should_phoenix_poll_device?(device) == true + end + + test "returns true when device inherits global cloud poller", %{device: device, cloud_poller: cloud_poller} do + {:ok, _} = Towerops.Settings.set_global_default_cloud_poller(cloud_poller.id) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + assert Agents.should_phoenix_poll_device?(device) == true + + # Clean up + {:ok, _} = Towerops.Settings.set_global_default_cloud_poller(nil) + end + + test "returns false when device is assigned to local agent via direct assignment", %{ + device: device, + local_agent: local_agent + } do + {:ok, _} = Agents.assign_device_to_agent(local_agent.id, device.id) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + assert Agents.should_phoenix_poll_device?(device) == false + end + + test "returns false when device inherits local agent from site", %{ + device: device, + site: site, + local_agent: local_agent + } do + {:ok, _} = Towerops.Sites.update_site(site, %{agent_token_id: local_agent.id}) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + assert Agents.should_phoenix_poll_device?(device) == false + end + + test "returns false when device inherits local agent from organization", %{ + organization: org, + device: device, + local_agent: local_agent + } do + {:ok, _} = Towerops.Organizations.update_organization(org, %{default_agent_token_id: local_agent.id}) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + assert Agents.should_phoenix_poll_device?(device) == false + end + + test "device-level assignment takes precedence over site/org (local overrides cloud)", %{ + organization: org, + site: site, + device: device, + local_agent: local_agent, + cloud_poller: cloud_poller + } do + # Set cloud poller at org and site level + {:ok, _} = Towerops.Organizations.update_organization(org, %{default_agent_token_id: cloud_poller.id}) + {:ok, _} = Towerops.Sites.update_site(site, %{agent_token_id: cloud_poller.id}) + + # But assign local agent at device level + {:ok, _} = Agents.assign_device_to_agent(local_agent.id, device.id) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + # Should return false because device is assigned to local agent + assert Agents.should_phoenix_poll_device?(device) == false + end + + test "device-level assignment takes precedence over site/org (cloud overrides local)", %{ + organization: org, + site: site, + device: device, + local_agent: local_agent, + cloud_poller: cloud_poller + } do + # Set local agent at org and site level + {:ok, _} = Towerops.Organizations.update_organization(org, %{default_agent_token_id: local_agent.id}) + {:ok, _} = Towerops.Sites.update_site(site, %{agent_token_id: local_agent.id}) + + # But assign cloud poller at device level + {:ok, _} = Agents.assign_device_to_agent(cloud_poller.id, device.id) + + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + + # Should return true because device is assigned to cloud poller + assert Agents.should_phoenix_poll_device?(device) == true + end + end end