From f442c59ad07d4fef93415d77b8b8771f6cc9fa54 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 25 Mar 2026 16:01:05 -0500 Subject: [PATCH] fix/ping-monitoring-without-snmp (#171) Reviewed-on: https://git.mcintire.me/graham/towerops-web/pulls/171 --- CHANGELOG.txt | 15 +++++++++ lib/towerops/agents.ex | 5 +-- .../live/device_live/show.html.heex | 28 ++++++++-------- priv/static/changelog.txt | 4 +++ test/towerops/agents_test.exs | 32 ++++++++++++++----- 5 files changed, 61 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index e6da6a97..725298f3 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,18 @@ +2026-03-25 +ui: hide neighbors tab when SNMP is disabled + - Neighbors tab (LLDP/CDP discovery) now only shows when device has SNMP enabled + - Prevents confusion for non-SNMP devices that can't discover neighbors + Files: lib/towerops_web/live/device_live/show.html.heex + +2026-03-25 +fix: agents now monitor devices with ping-only monitoring (SNMP disabled) + - Previously, agents only polled devices with snmp_enabled=true + - This meant devices with monitoring_enabled=true but snmp_enabled=false received no ping checks + - Fixed: agents now poll devices where EITHER snmp_enabled OR monitoring_enabled is true + - Enables pure ICMP monitoring for non-SNMP devices (DNS servers, web servers, etc.) + - Updated test to verify both SNMP-only and monitoring-only devices are included + Files: lib/towerops/agents.ex, test/towerops/agents_test.exs + 2026-03-22 fix: correct capacity calculation for backhaul devices with sensor-based capacity - Sensor capacity (Rx/Tx Capacity) now represents total device capacity, not per-interface diff --git a/lib/towerops/agents.ex b/lib/towerops/agents.ex index 5e6c4787..36df2c53 100644 --- a/lib/towerops/agents.ex +++ b/lib/towerops/agents.ex @@ -673,7 +673,8 @@ defmodule Towerops.Agents do 3. In an organization that has this agent as default (and not overridden by site or device) 4. This agent is the global default cloud poller (and device has no assignment at any level) - Returns device records with preloaded associations, filtered to only SNMP-enabled devices. + Returns device records with preloaded associations, filtered to devices where + SNMP polling is enabled OR ICMP monitoring is enabled (or both). ## Examples @@ -698,7 +699,7 @@ defmodule Towerops.Agents do on: disabled.device_id == e.id and disabled.agent_token_id == ^agent_token_id and disabled.enabled == false, - where: e.snmp_enabled == true, + where: e.snmp_enabled == true or e.monitoring_enabled == true, preload: [ :agent_assignments, :organization, diff --git a/lib/towerops_web/live/device_live/show.html.heex b/lib/towerops_web/live/device_live/show.html.heex index 4c3a9324..3ce0de11 100644 --- a/lib/towerops_web/live/device_live/show.html.heex +++ b/lib/towerops_web/live/device_live/show.html.heex @@ -186,19 +186,21 @@ <% end %> - <.link - patch={~p"/devices/#{@device.id}?tab=neighbors"} - class={[ - "whitespace-nowrap py-4 px-1 border-b-2 text-sm transition-colors", - if @active_tab == "neighbors" do - "border-blue-500 text-blue-600 dark:text-blue-400 font-semibold" - else - "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300 dark:text-gray-400 dark:hover:text-gray-300 font-medium" - end - ]} - > - {t("Neighbors")} - + <%= if @device.snmp_enabled do %> + <.link + patch={~p"/devices/#{@device.id}?tab=neighbors"} + class={[ + "whitespace-nowrap py-4 px-1 border-b-2 text-sm transition-colors", + if @active_tab == "neighbors" do + "border-blue-500 text-blue-600 dark:text-blue-400 font-semibold" + else + "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300 dark:text-gray-400 dark:hover:text-gray-300 font-medium" + end + ]} + > + {t("Neighbors")} + + <% end %> <%= if @arp_entries && length(@arp_entries) > 0 do %> <.link diff --git a/priv/static/changelog.txt b/priv/static/changelog.txt index d0eb73e9..f7da13df 100644 --- a/priv/static/changelog.txt +++ b/priv/static/changelog.txt @@ -1,3 +1,7 @@ +2026-03-25 — Bug Fixes +* Fixed ping monitoring not working for devices with SNMP disabled but monitoring enabled +* Neighbors tab now hidden for devices with SNMP disabled (since neighbor discovery requires SNMP) + 2026-03-22 — Bug Fixes * Fixed capacity reference lines showing inflated values (5-6x higher than actual) on backhaul device traffic charts * Fixed wireless sensor values (frequency, power, etc.) displaying in scientific notation instead of standard format diff --git a/test/towerops/agents_test.exs b/test/towerops/agents_test.exs index 6d05c4f9..ea7d95ce 100644 --- a/test/towerops/agents_test.exs +++ b/test/towerops/agents_test.exs @@ -1168,7 +1168,7 @@ defmodule Towerops.AgentsTest do assert Agents.list_agent_polling_targets(agent1.id) == [] end - test "only returns SNMP-enabled devices", %{ + test "only returns SNMP-enabled or monitoring-enabled devices", %{ organization: organization, agent1: agent1, site1: site1 @@ -1185,25 +1185,41 @@ defmodule Towerops.AgentsTest do snmp_community: "public" }) - # Equipment with SNMP disabled + # Equipment with SNMP disabled but monitoring enabled (ping checks) {:ok, device2} = Towerops.Devices.create_device(%{ name: "Equipment 2", ip_address: "192.168.1.2", site_id: site1.id, organization_id: organization.id, - snmp_enabled: false + snmp_enabled: false, + monitoring_enabled: true }) - # Assign both to agent1 + # Equipment with both SNMP and monitoring disabled + {:ok, device3} = + Towerops.Devices.create_device(%{ + name: "Equipment 3", + ip_address: "192.168.1.3", + site_id: site1.id, + organization_id: organization.id, + snmp_enabled: false, + monitoring_enabled: false + }) + + # Assign all to agent1 {:ok, _} = Agents.assign_device_to_agent(agent1.id, device1.id) {:ok, _} = Agents.assign_device_to_agent(agent1.id, device2.id) + {:ok, _} = Agents.assign_device_to_agent(agent1.id, device3.id) - # Agent1 should only see SNMP-enabled devices + # Agent1 should see device1 (SNMP enabled) and device2 (monitoring enabled), but not device3 targets = Agents.list_agent_polling_targets(agent1.id) - assert length(targets) == 1 - assert hd(targets).id == device1.id - assert hd(targets).snmp_enabled == true + assert length(targets) == 2 + + target_ids = targets |> Enum.map(& &1.id) |> Enum.sort() + assert device1.id in target_ids + assert device2.id in target_ids + refute device3.id in target_ids end test "returns device from multiple sites with different assignment levels", %{