From f05cc32ef6a58e6a8ebe6d160f78df79e04adde0 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 18 Mar 2026 14:05:37 -0500 Subject: [PATCH] fix dns/ping check execution and checks table alignment (#72) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - **DNS checks**: `parse_server` returned a flat 5-tuple `{a,b,c,d,53}` instead of `{{a,b,c,d}, 53}` — the format `:inet_res` requires for nameservers. All DNS checks using a custom server failed with argument error. - **Ping checks**: `System.cmd` `:timeout` option not supported on Elixir 1.19.4 (staging). Replaced with `Task.async`/`Task.yield` wrapper for cross-version compatibility. - **Checks table UI**: Removed unnecessary flex div wrapper on status badge column that caused vertical misalignment with other table cells. ## Test plan - [x] DNS executor tests pass (custom server resolution now succeeds) - [x] Ping executor tests pass (no more ArgumentError) - [x] Full test suite passes (8593 tests, 0 new failures) Reviewed-on: https://git.mcintire.me/graham/towerops-web/pulls/72 --- .../monitoring/executors/dns_executor.ex | 4 +-- .../monitoring/executors/ping_executor.ex | 29 ++++++++++++++++--- .../live/device_live/show.html.heex | 6 ++-- .../executors/dns_executor_test.exs | 22 +++++--------- 4 files changed, 36 insertions(+), 25 deletions(-) diff --git a/lib/towerops/monitoring/executors/dns_executor.ex b/lib/towerops/monitoring/executors/dns_executor.ex index e98b6812..2795d32d 100644 --- a/lib/towerops/monitoring/executors/dns_executor.ex +++ b/lib/towerops/monitoring/executors/dns_executor.ex @@ -83,10 +83,10 @@ defmodule Towerops.Monitoring.Executors.DnsExecutor do defp parse_server(server_str) do case String.split(server_str, ".") do [a, b, c, d] -> - {String.to_integer(a), String.to_integer(b), String.to_integer(c), String.to_integer(d), 53} + {{String.to_integer(a), String.to_integer(b), String.to_integer(c), String.to_integer(d)}, 53} _ -> - {8, 8, 8, 8, 53} + {{8, 8, 8, 8}, 53} end end diff --git a/lib/towerops/monitoring/executors/ping_executor.ex b/lib/towerops/monitoring/executors/ping_executor.ex index ebd91f0c..54ec5d09 100644 --- a/lib/towerops/monitoring/executors/ping_executor.ex +++ b/lib/towerops/monitoring/executors/ping_executor.ex @@ -56,17 +56,38 @@ defmodule Towerops.Monitoring.Executors.PingExecutor do defp run_ping(host, count, timeout_ms) do args = build_args(host, count, timeout_ms) - system_timeout = timeout_ms + count * 1000 + 2000 + # The ping command itself has -W/-w timeout flags. + # Add a safety timeout in case the process hangs. + safety_timeout = timeout_ms + count * 1000 + 2000 + caller = self() + ref = make_ref() - case System.cmd("ping", args, stderr_to_stdout: true, timeout: system_timeout) do - {output, 0} -> + pid = + spawn(fn -> + try do + result = System.cmd("ping", args, stderr_to_stdout: true) + send(caller, {ref, {:ok, result}}) + rescue + e -> send(caller, {ref, {:error, Exception.message(e)}}) + end + end) + + receive do + {^ref, {:ok, {output, 0}}} -> parse_output(output) - {output, _exit_code} -> + {^ref, {:ok, {output, _exit_code}}} -> case parse_output(output) do {:ok, _, _} = success -> success {:error, _} = error -> error end + + {^ref, {:error, reason}} -> + {:error, reason} + after + safety_timeout -> + Process.exit(pid, :kill) + {:error, "Ping command timed out"} end end diff --git a/lib/towerops_web/live/device_live/show.html.heex b/lib/towerops_web/live/device_live/show.html.heex index 3d0f9718..bb5a41c2 100644 --- a/lib/towerops_web/live/device_live/show.html.heex +++ b/lib/towerops_web/live/device_live/show.html.heex @@ -2698,10 +2698,8 @@ {check.name} - -
- {render_status_badge(check.current_state)} -
+ + {render_status_badge(check.current_state)} <%= if check.last_check_at do %> diff --git a/test/towerops/monitoring/executors/dns_executor_test.exs b/test/towerops/monitoring/executors/dns_executor_test.exs index 4d5b1d51..696a276d 100644 --- a/test/towerops/monitoring/executors/dns_executor_test.exs +++ b/test/towerops/monitoring/executors/dns_executor_test.exs @@ -91,28 +91,20 @@ defmodule Towerops.Monitoring.Executors.DnsExecutorTest do end describe "execute/2 with custom server" do - test "attempts to use specified DNS server" do - # Note: parse_server returns a 5-tuple which may cause issues with - # :inet_res depending on OTP version. We accept either success or error. + test "resolves using custom DNS server" do config = %{"hostname" => "example.com", "server" => "8.8.8.8"} - case DnsExecutor.execute(config, 5000) do - {:ok, _time, output} -> - assert String.starts_with?(output, "Resolved to:") - - {:error, reason} -> - assert is_binary(reason) - end + assert {:ok, response_time, output} = DnsExecutor.execute(config, 5000) + assert is_number(response_time) + assert output =~ "Resolved to:" end test "falls back to default server format for invalid IP" do - # Invalid IP format should fall back to 8.8.8.8 config = %{"hostname" => "example.com", "server" => "not-an-ip"} - case DnsExecutor.execute(config, 5000) do - {:ok, _, _} -> :ok - {:error, _} -> :ok - end + # Invalid IP falls back to 8.8.8.8 + assert {:ok, _time, output} = DnsExecutor.execute(config, 5000) + assert output =~ "Resolved to:" end end end