graham/towerops
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

add auto ICMP ping check for all devices (#68)

Browse source 
## Summary

- Every device now automatically gets an ICMP ping check (`source_type: "auto_discovery"`) when created via `Monitoring.ensure_default_ping_check/1`
- When a device IP changes, the ping check host config is synced automatically
- Backfill migration inserts ICMP Ping checks for all existing devices that don't have one
- Fixes checkbox alignment in the Add Service Check modal (verify SSL / follow redirects)

## Test plan

- [x] 4 new tests for `ensure_default_ping_check/1` (create, idempotent, IP update, no-op)
- [x] 3 new tests for device lifecycle (auto-create on device create, sync on IP change, no-op when IP unchanged)
- [x] 13 existing tests updated to account for auto-created ping check
- [x] Full test suite passes (`mix precommit`)
- [ ] Verify backfill migration on staging
- [ ] Create device in UI, confirm ping check appears in service checks list
- [ ] Verify checkbox alignment in Add Service Check modal

Reviewed-on: graham/towerops-web#68
This commit is contained in:
Graham McIntire 2026-03-17 17:55:23 -05:00 • committed by graham
parent 3c935ed56e
commit c8a092196f
9 changed files with 253 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
lib/towerops/devices.ex
View file

@ -611,8 +611,8 @@ defmodule Towerops.Devices do
DeviceMonitorWorker.start_monitoring(device.id)
end
# Note: Checks will be created and scheduled when discovery runs
# No need to start polling here
# Auto-create ICMP ping check for all devices
_ = Monitoring.ensure_default_ping_check(device)
broadcast_device_change(device.organization_id, :device_created)
@ -705,6 +705,12 @@ defmodule Towerops.Devices do
{:ok, updated_device} = result ->
_ = handle_monitoring_changes(updated_device, old_monitoring)
_ = handle_snmp_changes(updated_device, old_snmp, old_snmp_version, old_snmp_port)
# Sync ping check if IP changed
if updated_device.ip_address != device.ip_address do
_ = Monitoring.ensure_default_ping_check(updated_device)
end
broadcast_device_change(updated_device.organization_id, :device_updated)
result

43
lib/towerops/monitoring.ex
View file

@ -7,6 +7,7 @@ defmodule Towerops.Monitoring do
import Ecto.Query
alias Towerops.Devices.Device
alias Towerops.Monitoring.Check
alias Towerops.Monitoring.CheckResult
alias Towerops.Monitoring.MonitoringCheck
@ -412,6 +413,48 @@ defmodule Towerops.Monitoring do
end
end
## Auto ICMP ping checks
@doc """
Ensures a default ICMP ping check exists for the given device.
Creates an auto_discovery ping check if none exists, or updates the host
config if the device IP has changed.
"""
def ensure_default_ping_check(%Device{} = device) do
existing =
Repo.one(
from(c in Check,
where: c.device_id == ^device.id,
where: c.check_type == "ping",
where: c.source_type == "auto_discovery",
limit: 1
)
)
case existing do
nil ->
create_check(%{
organization_id: device.organization_id,
device_id: device.id,
name: "ICMP Ping",
check_type: "ping",
config: %{"host" => device.ip_address, "count" => 3},
enabled: true,
interval_seconds: 60,
timeout_ms: 5000,
source_type: "auto_discovery"
})
check ->
if check.config["host"] == device.ip_address do
{:ok, check}
else
update_check(check, %{config: Map.put(check.config, "host", device.ip_address)})
end
end
end
## Legacy ping-based monitoring compatibility (stub)
@doc false

13
lib/towerops_web/live/check_live/form_component.ex
View file

@ -150,16 +150,9 @@ defmodule ToweropsWeb.CheckLive.FormComponent do
</div>
</div>
<div class="flex items-center gap-4">
<label class="flex items-center gap-2">
<.input field={@form[:verify_ssl]} type="checkbox" />
<span class="text-sm text-gray-700 dark:text-gray-300">Verify SSL Certificate</span>
</label>
<label class="flex items-center gap-2">
<.input field={@form[:follow_redirects]} type="checkbox" />
<span class="text-sm text-gray-700 dark:text-gray-300">Follow Redirects</span>
</label>
<div class="flex items-center gap-6">
<.input field={@form[:verify_ssl]} type="checkbox" label="Verify SSL Certificate" />
<.input field={@form[:follow_redirects]} type="checkbox" label="Follow Redirects" />
</div>
<details class="border border-gray-200 dark:border-gray-700 rounded-lg p-3">

43
priv/repo/migrations/20260317224403_backfill_default_ping_checks.exs Normal file
View file

@ -0,0 +1,43 @@
defmodule Towerops.Repo.Migrations.BackfillDefaultPingChecks do
use Ecto.Migration
def up do
execute """
INSERT INTO checks (id, organization_id, device_id, name, check_type, config, enabled,
interval_seconds, timeout_ms, source_type, current_state,
current_state_type, current_check_attempt, inserted_at, updated_at)
SELECT
gen_random_uuid(),
d.organization_id,
d.id,
'ICMP Ping',
'ping',
jsonb_build_object('host', d.ip_address, 'count', 3),
true,
60,
5000,
'auto_discovery',
3,
'soft',
1,
NOW(),
NOW()
FROM devices d
WHERE NOT EXISTS (
SELECT 1 FROM checks c
WHERE c.device_id = d.id
AND c.check_type = 'ping'
AND c.source_type = 'auto_discovery'
)
"""
end
def down do
execute """
DELETE FROM checks
WHERE check_type = 'ping'
AND source_type = 'auto_discovery'
AND name = 'ICMP Ping'
"""
end
end

69
test/towerops/devices_test.exs
View file

@ -2354,4 +2354,73 @@ defmodule Towerops.EquipmentTest do
assert result == []
end
end
describe "auto ICMP ping check" do
import Towerops.AccountsFixtures
alias Towerops.Monitoring
setup do
user = user_fixture()
{:ok, organization} = Towerops.Organizations.create_organization(%{name: "Test Org"}, user.id)
{:ok, site} =
Towerops.Sites.create_site(%{
name: "Test Site",
organization_id: organization.id
})
%{organization: organization, site: site}
end
test "create_device/1 auto-creates an ICMP ping check", %{organization: organization, site: site} do
{:ok, device} =
Devices.create_device(%{
name: "Router",
ip_address: "192.168.1.1",
site_id: site.id,
organization_id: organization.id
})
checks = Monitoring.list_checks(organization.id, device_id: device.id, check_type: "ping")
assert [ping_check] = checks
assert ping_check.source_type == "auto_discovery"
assert ping_check.config["host"] == "192.168.1.1"
end
test "update_device/2 syncs ping check when IP changes", %{organization: organization, site: site} do
{:ok, device} =
Devices.create_device(%{
name: "Router",
ip_address: "192.168.1.1",
site_id: site.id,
organization_id: organization.id
})
{:ok, updated_device} = Devices.update_device(device, %{ip_address: "10.0.0.1"})
checks = Monitoring.list_checks(organization.id, device_id: updated_device.id, check_type: "ping")
assert [ping_check] = checks
assert ping_check.config["host"] == "10.0.0.1"
end
test "update_device/2 does not sync ping check when IP unchanged", %{organization: organization, site: site} do
{:ok, device} =
Devices.create_device(%{
name: "Router",
ip_address: "192.168.1.1",
site_id: site.id,
organization_id: organization.id
})
checks_before = Monitoring.list_checks(organization.id, device_id: device.id, check_type: "ping")
assert [check_before] = checks_before
{:ok, _updated} = Devices.update_device(device, %{name: "Updated Router"})
checks_after = Monitoring.list_checks(organization.id, device_id: device.id, check_type: "ping")
assert [check_after] = checks_after
assert check_after.updated_at == check_before.updated_at
end
end
end

81
test/towerops/monitoring_test.exs
View file

@ -46,8 +46,8 @@ defmodule Towerops.MonitoringTest do
describe "list_checks/2" do
test "returns checks for organization", %{organization: org} do
{:ok, check} = Monitoring.create_check(valid_check_attrs(org.id))
assert [found] = Monitoring.list_checks(org.id)
assert found.id == check.id
checks = Monitoring.list_checks(org.id)
assert Enum.any?(checks, &(&1.id == check.id))
end
test "does not return checks from other organizations", %{organization: org} do
@ -55,7 +55,10 @@ defmodule Towerops.MonitoringTest do
other_org = organization_fixture(other_user.id)
{:ok, _check} = Monitoring.create_check(valid_check_attrs(other_org.id))
assert Monitoring.list_checks(org.id) == []
# Only the auto-created ping check from setup device should be present
checks = Monitoring.list_checks(org.id)
assert Enum.all?(checks, &(&1.organization_id == org.id))
refute Enum.any?(checks, &(&1.check_type == "http"))
end
test "filters by device_id", %{organization: org, device: device} do
@ -64,8 +67,9 @@ defmodule Towerops.MonitoringTest do
{:ok, _check_without} = Monitoring.create_check(valid_check_attrs(org.id, %{name: "Other"}))
assert [found] = Monitoring.list_checks(org.id, device_id: device.id)
assert found.id == check_with_device.id
checks = Monitoring.list_checks(org.id, device_id: device.id)
assert Enum.any?(checks, &(&1.id == check_with_device.id))
refute Enum.any?(checks, &(&1.name == "Other"))
end
test "filters by check_type", %{organization: org} do
@ -90,8 +94,9 @@ defmodule Towerops.MonitoringTest do
{:ok, _disabled} =
Monitoring.create_check(valid_check_attrs(org.id, %{name: "Disabled", enabled: false}))
assert [found] = Monitoring.list_checks(org.id, enabled: true)
assert found.id == enabled.id
enabled_checks = Monitoring.list_checks(org.id, enabled: true)
assert Enum.any?(enabled_checks, &(&1.id == enabled.id))
refute Enum.any?(enabled_checks, &(&1.name == "Disabled"))
end
test "orders by name ascending", %{organization: org} do
@ -100,7 +105,10 @@ defmodule Towerops.MonitoringTest do
{:ok, _c} = Monitoring.create_check(valid_check_attrs(org.id, %{name: "Charlie"}))
names = org.id |> Monitoring.list_checks() |> Enum.map(& &1.name)
assert names == ["Alpha", "Bravo", "Charlie"]
assert names == Enum.sort(names)
assert "Alpha" in names
assert "Bravo" in names
assert "Charlie" in names
end
end
@ -441,11 +449,11 @@ defmodule Towerops.MonitoringTest do
test "returns health summary for devices with checks", %{organization: org, device: device} do
now = DateTime.truncate(DateTime.utc_now(), :second)
{:ok, check} =
{:ok, _check} =
Monitoring.create_check(valid_check_attrs(org.id, %{device_id: device.id}))
# Use Repo.update_all since Check.changeset doesn't cast current_state
Repo.update_all(from(c in Check, where: c.id == ^check.id), set: [current_state: 0, last_check_at: now])
# Set ALL device checks to state 0 (including auto-created ping check)
Repo.update_all(from(c in Check, where: c.device_id == ^device.id), set: [current_state: 0, last_check_at: now])
summary = Monitoring.get_device_health_summary([device.id])
assert Map.has_key?(summary, device.id)
assert summary[device.id].worst_status == 0
@ -456,15 +464,15 @@ defmodule Towerops.MonitoringTest do
end
test "returns worst status across multiple checks", %{organization: org, device: device} do
{:ok, check1} =
{:ok, _check1} =
Monitoring.create_check(valid_check_attrs(org.id, %{name: "OK Check", device_id: device.id}))
{:ok, check2} =
{:ok, crit_check} =
Monitoring.create_check(valid_check_attrs(org.id, %{name: "Crit Check", device_id: device.id}))
# Use Repo.update_all since Check.changeset doesn't cast current_state
Repo.update_all(from(c in Check, where: c.id == ^check1.id), set: [current_state: 0])
Repo.update_all(from(c in Check, where: c.id == ^check2.id), set: [current_state: 2])
# Set all device checks to OK first, then set crit_check to CRITICAL
Repo.update_all(from(c in Check, where: c.device_id == ^device.id), set: [current_state: 0])
Repo.update_all(from(c in Check, where: c.id == ^crit_check.id), set: [current_state: 2])
summary = Monitoring.get_device_health_summary([device.id])
assert summary[device.id].worst_status == 2
end
@ -537,4 +545,45 @@ defmodule Towerops.MonitoringTest do
assert Monitoring.get_latency_data(Ecto.UUID.generate(), []) == []
end
end
describe "ensure_default_ping_check/1" do
test "creates a ping check for a device that has none", %{organization: org, device: device} do
assert {:ok, check} = Monitoring.ensure_default_ping_check(device)
assert check.check_type == "ping"
assert check.name == "ICMP Ping"
assert check.source_type == "auto_discovery"
assert check.device_id == device.id
assert check.organization_id == org.id
assert check.config["host"] == device.ip_address
assert check.config["count"] == 3
assert check.enabled == true
assert check.interval_seconds == 60
assert check.timeout_ms == 5000
end
test "is idempotent - does not create duplicate", %{device: device} do
{:ok, first} = Monitoring.ensure_default_ping_check(device)
{:ok, second} = Monitoring.ensure_default_ping_check(device)
assert first.id == second.id
end
test "updates host config when device IP changed", %{device: device} do
{:ok, check} = Monitoring.ensure_default_ping_check(device)
assert check.config["host"] == "192.168.1.1"
# Simulate device with changed IP
updated_device = %{device | ip_address: "10.0.0.1"}
{:ok, updated_check} = Monitoring.ensure_default_ping_check(updated_device)
assert updated_check.id == check.id
assert updated_check.config["host"] == "10.0.0.1"
end
test "does not update check when IP is unchanged", %{device: device} do
{:ok, check} = Monitoring.ensure_default_ping_check(device)
{:ok, same_check} = Monitoring.ensure_default_ping_check(device)
assert same_check.id == check.id
assert same_check.updated_at == check.updated_at
end
end
end

23
test/towerops/snmp_test.exs
View file

@ -2727,11 +2727,10 @@ defmodule Towerops.SnmpTest do
assert {:ok, counts} = Snmp.create_checks_from_discovery(device, snmp_device)
assert counts.sensors == 1
# Verify check was created
# Verify check was created (auto ping check + sensor check)
checks = Towerops.Monitoring.list_checks(device.organization_id, device_id: device.id)
assert length(checks) == 1
check = hd(checks)
check = Enum.find(checks, &(&1.check_type == "snmp_sensor"))
assert check
assert check.name == "CPU Temperature"
assert check.check_type == "snmp_sensor"
assert check.source_type == "auto_discovery"
@ -2764,9 +2763,8 @@ defmodule Towerops.SnmpTest do
assert counts.interfaces == 1
checks = Towerops.Monitoring.list_checks(device.organization_id, device_id: device.id)
assert length(checks) == 1
check = hd(checks)
check = Enum.find(checks, &(&1.check_type == "snmp_interface"))
assert check
assert check.name == "Ethernet0 Status"
assert check.check_type == "snmp_interface"
assert check.source_type == "auto_discovery"
@ -2791,7 +2789,8 @@ defmodule Towerops.SnmpTest do
assert counts.processors == 1
checks = Towerops.Monitoring.list_checks(device.organization_id, device_id: device.id)
check = hd(checks)
check = Enum.find(checks, &(&1.check_type == "snmp_processor"))
assert check
assert check.name == "CPU 1"
assert check.check_type == "snmp_processor"
assert check.source_id == processor.id
@ -2815,7 +2814,8 @@ defmodule Towerops.SnmpTest do
assert counts.storage == 1
checks = Towerops.Monitoring.list_checks(device.organization_id, device_id: device.id)
check = hd(checks)
check = Enum.find(checks, &(&1.check_type == "snmp_storage"))
assert check
assert check.name == "/ (root) Usage"
assert check.check_type == "snmp_storage"
assert check.source_id == storage.id
@ -2853,9 +2853,10 @@ defmodule Towerops.SnmpTest do
assert counts.storage == 0
checks = Towerops.Monitoring.list_checks(device.organization_id, device_id: device.id)
assert length(checks) == 2
snmp_checks = Enum.reject(checks, &(&1.check_type == "ping"))
assert length(snmp_checks) == 2
check_types = checks |> Enum.map(& &1.check_type) |> Enum.sort()
check_types = snmp_checks |> Enum.map(& &1.check_type) |> Enum.sort()
assert check_types == ["snmp_interface", "snmp_sensor"]
end

15
test/towerops_web/controllers/api/v1/check_results_controller_test.exs
View file

@ -49,21 +49,22 @@ defmodule ToweropsWeb.Api.V1.CheckResultsControllerTest do
# Create a check for this device
{:ok, check} =
Towerops.Monitoring.create_check(%{
name: "Ping Check",
check_type: "ping",
name: "HTTP Check",
check_type: "http",
enabled: true,
interval_seconds: 60,
device_id: device.id,
organization_id: organization.id,
config: %{"host" => "10.0.0.1"}
config: %{"url" => "https://example.com"}
})
conn = get(conn, ~p"/api/v1/devices/#{device.id}/checks")
assert %{"data" => [check_data]} = json_response(conn, 200)
assert check_data["id"] == check.id
assert check_data["name"] == "Ping Check"
assert check_data["check_type"] == "ping"
assert %{"data" => data} = json_response(conn, 200)
# Device has auto-created ping check + the HTTP check we just created
check_data = Enum.find(data, &(&1["id"] == check.id))
assert check_data["name"] == "HTTP Check"
assert check_data["check_type"] == "http"
assert check_data["enabled"] == true
assert check_data["interval_seconds"] == 60
end

4
test/towerops_web/live/device_live/show_test.exs
View file

@ -60,11 +60,11 @@ defmodule ToweropsWeb.DeviceLive.ShowTest do
assert html =~ "Checks"
end
test "loads checks tab with empty state", %{conn: conn, user: user, device: device} do
test "loads checks tab showing auto-created ping check", %{conn: conn, user: user, device: device} do
conn = log_in_user(conn, user)
{:ok, _view, html} = live(conn, ~p"/devices/#{device.id}?tab=checks")
assert html =~ "No checks configured"
assert html =~ "ICMP Ping"
assert html =~ "Add Check"
end
end