diff --git a/lib/towerops/devices.ex b/lib/towerops/devices.ex index a12be4d1..b94009cb 100644 --- a/lib/towerops/devices.ex +++ b/lib/towerops/devices.ex @@ -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 diff --git a/lib/towerops/monitoring.ex b/lib/towerops/monitoring.ex index e59719b6..db80e12b 100644 --- a/lib/towerops/monitoring.ex +++ b/lib/towerops/monitoring.ex @@ -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 diff --git a/lib/towerops_web/live/check_live/form_component.ex b/lib/towerops_web/live/check_live/form_component.ex index 32655caf..0ba6d4f8 100644 --- a/lib/towerops_web/live/check_live/form_component.ex +++ b/lib/towerops_web/live/check_live/form_component.ex @@ -150,16 +150,9 @@ defmodule ToweropsWeb.CheckLive.FormComponent do -
- - - +
+ <.input field={@form[:verify_ssl]} type="checkbox" label="Verify SSL Certificate" /> + <.input field={@form[:follow_redirects]} type="checkbox" label="Follow Redirects" />
diff --git a/priv/repo/migrations/20260317224403_backfill_default_ping_checks.exs b/priv/repo/migrations/20260317224403_backfill_default_ping_checks.exs new file mode 100644 index 00000000..882f6ca1 --- /dev/null +++ b/priv/repo/migrations/20260317224403_backfill_default_ping_checks.exs @@ -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 diff --git a/test/towerops/devices_test.exs b/test/towerops/devices_test.exs index 5025df89..c8948ba0 100644 --- a/test/towerops/devices_test.exs +++ b/test/towerops/devices_test.exs @@ -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 diff --git a/test/towerops/monitoring_test.exs b/test/towerops/monitoring_test.exs index 13217201..14f1c0e7 100644 --- a/test/towerops/monitoring_test.exs +++ b/test/towerops/monitoring_test.exs @@ -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 diff --git a/test/towerops/snmp_test.exs b/test/towerops/snmp_test.exs index 2aa1134b..032d5dd7 100644 --- a/test/towerops/snmp_test.exs +++ b/test/towerops/snmp_test.exs @@ -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 diff --git a/test/towerops_web/controllers/api/v1/check_results_controller_test.exs b/test/towerops_web/controllers/api/v1/check_results_controller_test.exs index 24c002e5..ef34e188 100644 --- a/test/towerops_web/controllers/api/v1/check_results_controller_test.exs +++ b/test/towerops_web/controllers/api/v1/check_results_controller_test.exs @@ -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 diff --git a/test/towerops_web/live/device_live/show_test.exs b/test/towerops_web/live/device_live/show_test.exs index 75f0421d..9ba90890 100644 --- a/test/towerops_web/live/device_live/show_test.exs +++ b/test/towerops_web/live/device_live/show_test.exs @@ -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