From 95eafd0c47a656ebe8e357e7b9cfd7621275ba44 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Mon, 9 Feb 2026 17:30:06 -0600 Subject: [PATCH] routeros fixes and latency rounding --- lib/towerops/monitoring/check.ex | 9 + lib/towerops/snmp/client.ex | 10 +- lib/towerops/workers/device_monitor_worker.ex | 9 +- lib/towerops/workers/device_poller_worker.ex | 8 +- lib/towerops_web/channels/agent_channel.ex | 5 +- priv/profiles/os_discovery/routeros.yaml | 1 + priv/profiles/os_discovery/swos.yaml | 1 + test/integration/discovery_parity_test.exs | 991 ++++++++++++++++++ test/towerops/monitoring/check_test.exs | 2 +- 9 files changed, 1026 insertions(+), 10 deletions(-) create mode 100644 test/integration/discovery_parity_test.exs diff --git a/lib/towerops/monitoring/check.ex b/lib/towerops/monitoring/check.ex index df7faa0a..ef1bd675 100644 --- a/lib/towerops/monitoring/check.ex +++ b/lib/towerops/monitoring/check.ex @@ -27,7 +27,16 @@ defmodule Towerops.Monitoring.Check do check |> cast(attrs, [:device_id, :agent_token_id, :status, :response_time_ms, :checked_at]) |> validate_required([:device_id, :status, :checked_at]) + |> round_response_time() |> foreign_key_constraint(:device_id, match: :suffix) |> foreign_key_constraint(:agent_token_id, match: :suffix) end + + defp round_response_time(changeset) do + case get_change(changeset, :response_time_ms) do + nil -> changeset + ms when is_float(ms) -> put_change(changeset, :response_time_ms, Float.round(ms, 1)) + _ -> changeset + end + end end diff --git a/lib/towerops/snmp/client.ex b/lib/towerops/snmp/client.ex index 5765eee9..72a1bc8f 100644 --- a/lib/towerops/snmp/client.ex +++ b/lib/towerops/snmp/client.ex @@ -392,9 +392,15 @@ defmodule Towerops.Snmp.Client do end end - # Private functions + @doc """ + Returns true if Phoenix-side SNMP operations are disabled. - defp phoenix_snmp_disabled do + When true, all direct SNMP operations from Phoenix (get, walk, etc.) are blocked. + SNMP polling should be handled by connected agents instead. + Always returns false in test environment. + """ + @spec phoenix_snmp_disabled() :: boolean() + def phoenix_snmp_disabled do # Only disable in non-test environments (allow tests to run normally) if Application.get_env(:towerops, :env) == :test do false diff --git a/lib/towerops/workers/device_monitor_worker.ex b/lib/towerops/workers/device_monitor_worker.ex index 9779dd24..8764ba6c 100644 --- a/lib/towerops/workers/device_monitor_worker.ex +++ b/lib/towerops/workers/device_monitor_worker.ex @@ -18,6 +18,7 @@ defmodule Towerops.Workers.DeviceMonitorWorker do alias Towerops.Alerts alias Towerops.Devices alias Towerops.Monitoring + alias Towerops.Snmp.Client alias Towerops.Workers.PollingOffset require Logger @@ -52,13 +53,17 @@ defmodule Towerops.Workers.DeviceMonitorWorker do end # Check if device should continue being monitored - # Returns false if monitoring is disabled or device is assigned to an agent + # Returns false if Phoenix SNMP is disabled, monitoring is disabled, or device is assigned to an agent defp should_continue_monitoring?(device) do - device.monitoring_enabled && Agents.should_phoenix_poll_device?(device) + !Client.phoenix_snmp_disabled() && device.monitoring_enabled && + Agents.should_phoenix_poll_device?(device) end defp maybe_perform_check(device) do cond do + Client.phoenix_snmp_disabled() -> + :ok + not device.monitoring_enabled -> :ok diff --git a/lib/towerops/workers/device_poller_worker.ex b/lib/towerops/workers/device_poller_worker.ex index 70d93489..86493674 100644 --- a/lib/towerops/workers/device_poller_worker.ex +++ b/lib/towerops/workers/device_poller_worker.ex @@ -69,13 +69,17 @@ defmodule Towerops.Workers.DevicePollerWorker do end # Check if device should continue being polled - # Returns false if SNMP is disabled or device is assigned to an agent + # Returns false if Phoenix SNMP is disabled, SNMP is disabled on device, or device is assigned to an agent defp should_continue_polling?(device) do - device.snmp_enabled && Agents.should_phoenix_poll_device?(device) + !Client.phoenix_snmp_disabled() && device.snmp_enabled && + Agents.should_phoenix_poll_device?(device) end defp maybe_poll_device(device) do cond do + Client.phoenix_snmp_disabled() -> + :ok + not device.snmp_enabled -> :ok diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index 831ab8a2..a223feb2 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -109,9 +109,8 @@ defmodule ToweropsWeb.AgentChannel do socket = assign(socket, :last_heartbeat_at, DateTime.utc_now()) Process.send_after(self(), :check_heartbeat, @heartbeat_check_interval_ms) - # Send initial job list synchronously during join to avoid race condition - # where agent disconnects before the async :send_jobs message is processed - build_and_push_jobs(socket) + # Send initial job list after join completes (push/3 cannot be called during join) + send(self(), :send_jobs) {:ok, socket} diff --git a/priv/profiles/os_discovery/routeros.yaml b/priv/profiles/os_discovery/routeros.yaml index d629b161..f96b0245 100644 --- a/priv/profiles/os_discovery/routeros.yaml +++ b/priv/profiles/os_discovery/routeros.yaml @@ -28,6 +28,7 @@ modules: num_oid: '.1.3.6.1.4.1.14988.1.1.3.100.1.3.{{ $index }}' descr: MIKROTIK-MIB::mtxrGaugeName group: System + divisor: 10 index: 'mtxrGaugeTemperature.{{ $index }}' skip_values: - oid: MIKROTIK-MIB::mtxrGaugeUnit diff --git a/priv/profiles/os_discovery/swos.yaml b/priv/profiles/os_discovery/swos.yaml index 125b8df4..1c6beb85 100644 --- a/priv/profiles/os_discovery/swos.yaml +++ b/priv/profiles/os_discovery/swos.yaml @@ -70,6 +70,7 @@ modules: num_oid: '.1.3.6.1.4.1.14988.1.1.3.100.1.3.{{ $index }}' descr: 'CPU Temperature' group: System + divisor: 10 index: 'mtxrGaugeTemperature.{{ $index }}' voltage: data: diff --git a/test/integration/discovery_parity_test.exs b/test/integration/discovery_parity_test.exs new file mode 100644 index 00000000..2300350c --- /dev/null +++ b/test/integration/discovery_parity_test.exs @@ -0,0 +1,991 @@ +defmodule Towerops.Integration.DiscoveryParityTest do + @moduledoc """ + Integration tests for SNMP discovery/polling parity. + + Runs against a real SNMP device (MikroTik RB5009UG+S+) using the actual + SnmpKit library (not mocks) and verifies our discovery pipeline captures + everything that snmpwalk/snmpget returns. + + These tests are excluded from normal `mix test` runs via the :integration tag. + Run with: mix test test/integration/discovery_parity_test.exs --include integration + """ + use Towerops.DataCase, async: false + + import Towerops.DevicesFixtures + + alias Towerops.Snmp + alias Towerops.Snmp.Client + alias Towerops.Snmp.Discovery + alias Towerops.Snmp.Profiles.Base + alias Towerops.Snmp.Profiles.Dynamic + + require Logger + + @moduletag :integration + + # Test device: MikroTik RB5009UG+S+ running RouterOS + @test_ip "204.110.191.1" + @test_community "testlocal" + @test_version "2c" + + @client_opts [ + ip: @test_ip, + community: @test_community, + version: @test_version, + port: 161, + timeout: 10_000 + ] + + # Standard OIDs for ground truth comparisons + @sys_uptime_oid "1.3.6.1.2.1.1.3.0" + @if_index_oid "1.3.6.1.2.1.2.2.1.1" + @if_descr_oid "1.3.6.1.2.1.2.2.1.2" + @if_type_oid "1.3.6.1.2.1.2.2.1.3" + @if_speed_oid "1.3.6.1.2.1.2.2.1.5" + @if_phys_address_oid "1.3.6.1.2.1.2.2.1.6" + @if_admin_status_oid "1.3.6.1.2.1.2.2.1.7" + @if_oper_status_oid "1.3.6.1.2.1.2.2.1.8" + @if_name_oid "1.3.6.1.2.1.31.1.1.1.1" + @if_high_speed_oid "1.3.6.1.2.1.31.1.1.1.15" + @if_alias_oid "1.3.6.1.2.1.31.1.1.1.18" + @if_in_octets_oid "1.3.6.1.2.1.2.2.1.10" + @if_out_octets_oid "1.3.6.1.2.1.2.2.1.16" + @entity_sensor_type_oid "1.3.6.1.2.1.99.1.1.1.1" + @entity_phys_descr_oid "1.3.6.1.2.1.47.1.1.1.1.2" + + # IP-MIB OIDs + @ip_ad_ent_addr_oid "1.3.6.1.2.1.4.20.1.1" + @ip_ad_ent_if_index_oid "1.3.6.1.2.1.4.20.1.2" + @ip_ad_ent_net_mask_oid "1.3.6.1.2.1.4.20.1.3" + + # HR-MIB OIDs + @hr_processor_load_oid "1.3.6.1.2.1.25.3.3.1.2" + @hr_storage_descr_oid "1.3.6.1.2.1.25.2.3.1.3" + @hr_storage_size_oid "1.3.6.1.2.1.25.2.3.1.5" + @hr_storage_used_oid "1.3.6.1.2.1.25.2.3.1.6" + + # MikroTik enterprise OIDs + @mtxr_gauge_name_oid "1.3.6.1.4.1.14988.1.1.3.100.1.2" + @mtxr_gauge_value_oid "1.3.6.1.4.1.14988.1.1.3.100.1.3" + @mtxr_serial_oid "1.3.6.1.4.1.14988.1.1.4.1.0" + @mtxr_firmware_oid "1.3.6.1.4.1.14988.1.1.4.4.0" + @mtxr_model_oid "1.3.6.1.4.1.14988.1.1.7.8.0" + @mtxr_dhcp_lease_count_oid "1.3.6.1.4.1.14988.1.1.6.1.0" + @mtxr_hl_voltage_oid "1.3.6.1.4.1.14988.1.1.3.11.0" + + setup do + # Override SNMP adapter from mock to real SnmpKit + original_adapter = Application.get_env(:towerops, :snmp_adapter) + Application.put_env(:towerops, :snmp_adapter, SnmpKit) + + # Check device connectivity with a quick sysUpTime GET + reachable = + case Client.get(@client_opts, @sys_uptime_oid) do + {:ok, _} -> + true + + {:error, reason} -> + Logger.warning("Test device #{@test_ip} unreachable: #{inspect(reason)}") + false + end + + on_exit(fn -> + if original_adapter do + Application.put_env(:towerops, :snmp_adapter, original_adapter) + else + Application.delete_env(:towerops, :snmp_adapter) + end + end) + + %{reachable: reachable} + end + + describe "raw SNMP connectivity" do + @tag timeout: 30_000 + test "GET sysUpTime.0 returns integer", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, uptime} = Client.get(@client_opts, @sys_uptime_oid) + assert is_integer(uptime), "Expected integer uptime, got: #{inspect(uptime)}" + assert uptime >= 0 + end + + @tag timeout: 60_000 + test "WALK ifIndex returns non-empty results", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, results} = Client.walk(@client_opts, @if_index_oid) + assert is_map(results) + assert map_size(results) > 0, "Expected at least one interface" + + for {_oid, value} <- results do + assert is_integer(value) and value > 0, + "ifIndex values must be positive integers, got: #{inspect(value)}" + end + end + end + + describe "system info parity" do + @tag timeout: 60_000 + test "discover_system_info returns all SNMPv2-MIB fields", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, system_info} = Base.discover_system_info(@client_opts) + + # All 6 standard system fields should be present and populated + assert is_binary(system_info.sys_descr) + assert String.length(system_info.sys_descr) > 0 + + assert is_binary(system_info.sys_object_id) + assert String.starts_with?(system_info.sys_object_id, "1.3.6.1") + + assert is_binary(system_info.sys_name) + assert String.length(system_info.sys_name) > 0 + + assert is_integer(system_info.sys_uptime) + assert system_info.sys_uptime >= 0 + + # Contact and location should be present (may be empty strings) + assert Map.has_key?(system_info, :sys_contact) + assert Map.has_key?(system_info, :sys_location) + + Logger.info( + "System: descr=#{String.slice(system_info.sys_descr, 0..80)}, " <> + "name=#{system_info.sys_name}, oid=#{system_info.sys_object_id}, " <> + "contact=#{system_info.sys_contact}, location=#{system_info.sys_location}" + ) + end + + @tag timeout: 60_000 + test "raw SNMP values match discovery output", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, raw_descr} = Client.get(@client_opts, "1.3.6.1.2.1.1.1.0") + {:ok, raw_name} = Client.get(@client_opts, "1.3.6.1.2.1.1.5.0") + {:ok, raw_contact} = Client.get(@client_opts, "1.3.6.1.2.1.1.4.0") + {:ok, raw_location} = Client.get(@client_opts, "1.3.6.1.2.1.1.6.0") + {:ok, raw_object_id} = Client.get(@client_opts, "1.3.6.1.2.1.1.2.0") + + {:ok, system_info} = Base.discover_system_info(@client_opts) + + assert is_binary(raw_descr) + assert is_binary(system_info.sys_descr) + assert system_info.sys_name == to_string(raw_name) + + # sysObjectID comes as OID list from raw SNMP, discovery converts to dotted string + assert is_list(raw_object_id) or is_binary(raw_object_id) + assert is_binary(system_info.sys_object_id) + assert String.starts_with?(system_info.sys_object_id, "1.3.6.1") + + # Contact and location should match raw values + assert system_info.sys_contact == to_string(raw_contact) + assert system_info.sys_location == to_string(raw_location) + end + end + + describe "profile selection" do + @tag timeout: 60_000 + test "selects RouterOS YAML profile for MikroTik device", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, system_info} = Base.discover_system_info(@client_opts) + profile = Discovery.select_profile(system_info, @client_opts) + + # This device has sysObjectID .1.3.6.1.4.1.14988.1 which should match routeros profile + assert match?({:yaml, %{name: _}}, profile), + "Expected YAML profile match for MikroTik device, got: #{inspect(profile)}" + + {:yaml, yaml_profile} = profile + Logger.info("Selected YAML profile: #{yaml_profile.name}") + + # Verify profile has sensor definitions (routeros.yaml has temperature, voltage, etc.) + assert Map.has_key?(yaml_profile, :sensors) or Map.has_key?(yaml_profile, :table_sensor_oids), + "RouterOS profile should have sensor definitions" + end + end + + describe "interface discovery parity" do + @tag timeout: 120_000 + test "interface count matches raw ifIndex walk", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, raw_if_indices} = Client.walk(@client_opts, @if_index_oid) + expected_count = map_size(raw_if_indices) + + {:ok, interfaces} = Base.discover_interfaces(@client_opts) + + assert length(interfaces) == expected_count, + "Discovery found #{length(interfaces)} interfaces, " <> + "raw walk found #{expected_count}" + + Logger.info("Interface count matches: #{expected_count}") + end + + @tag timeout: 120_000 + test "ifIndex values match raw walk exactly", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, raw_if_indices} = Client.walk(@client_opts, @if_index_oid) + {:ok, interfaces} = Base.discover_interfaces(@client_opts) + + raw_indices = raw_if_indices |> Map.values() |> MapSet.new() + discovered_indices = MapSet.new(interfaces, & &1.if_index) + + assert MapSet.equal?(raw_indices, discovered_indices), + "ifIndex mismatch - raw: #{inspect(MapSet.to_list(raw_indices))}, " <> + "discovered: #{inspect(MapSet.to_list(discovered_indices))}" + end + + @tag timeout: 120_000 + test "all ifTable fields populated per interface", %{reachable: reachable} do + skip_unless_reachable(reachable) + + # Get raw ground truth for all standard IF-MIB fields + {:ok, raw_descrs} = Client.walk(@client_opts, @if_descr_oid) + {:ok, raw_types} = Client.walk(@client_opts, @if_type_oid) + {:ok, raw_admin} = Client.walk(@client_opts, @if_admin_status_oid) + {:ok, raw_oper} = Client.walk(@client_opts, @if_oper_status_oid) + + {:ok, interfaces} = Base.discover_interfaces(@client_opts) + + # Count matches for all fields + assert length(interfaces) == map_size(raw_descrs), + "ifDescr count mismatch: #{length(interfaces)} vs #{map_size(raw_descrs)}" + + assert length(interfaces) == map_size(raw_types), + "ifType count mismatch: #{length(interfaces)} vs #{map_size(raw_types)}" + + assert length(interfaces) == map_size(raw_admin), + "ifAdminStatus count mismatch: #{length(interfaces)} vs #{map_size(raw_admin)}" + + assert length(interfaces) == map_size(raw_oper), + "ifOperStatus count mismatch: #{length(interfaces)} vs #{map_size(raw_oper)}" + + for iface <- interfaces do + # ifDescr must be present on every interface + assert is_binary(iface.if_descr), + "Interface #{iface.if_index} missing if_descr" + + assert String.length(iface.if_descr) > 0, + "Interface #{iface.if_index} has empty if_descr" + + # ifType must be a positive integer (IANAifType) + assert is_integer(iface.if_type) and iface.if_type > 0, + "Interface #{iface.if_index} invalid ifType: #{inspect(iface.if_type)}" + + # Admin/oper status - device returns string atoms or integers + valid_admin = [1, 2, 3, "up", "down", "testing"] + + valid_oper = [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + "up", + "down", + "testing", + "unknown", + "dormant", + "notPresent", + "lowerLayerDown" + ] + + assert iface.if_admin_status in valid_admin, + "Interface #{iface.if_index} invalid admin status: #{inspect(iface.if_admin_status)}" + + assert iface.if_oper_status in valid_oper, + "Interface #{iface.if_index} invalid oper status: #{inspect(iface.if_oper_status)}" + end + end + + @tag timeout: 120_000 + test "ifXTable extended fields populated", %{reachable: reachable} do + skip_unless_reachable(reachable) + + # Ground truth for ifXTable + {:ok, raw_names} = Client.walk(@client_opts, @if_name_oid) + {:ok, raw_aliases} = Client.walk(@client_opts, @if_alias_oid) + {:ok, raw_high_speed} = Client.walk(@client_opts, @if_high_speed_oid) + {:ok, raw_speeds} = Client.walk(@client_opts, @if_speed_oid) + + {:ok, interfaces} = Base.discover_interfaces(@client_opts) + + # ifName should be discovered for all interfaces + assert map_size(raw_names) == length(interfaces), + "Raw ifName count (#{map_size(raw_names)}) != interface count (#{length(interfaces)})" + + interfaces_with_name = Enum.count(interfaces, & &1[:if_name]) + + Logger.info( + "ifXTable: #{interfaces_with_name}/#{length(interfaces)} have ifName, " <> + "#{map_size(raw_aliases)} have ifAlias, " <> + "#{map_size(raw_high_speed)} have ifHighSpeed, " <> + "#{map_size(raw_speeds)} have ifSpeed" + ) + + assert interfaces_with_name == map_size(raw_names), + "Discovery missed #{map_size(raw_names) - interfaces_with_name} ifName entries" + + # Verify ifPhysAddress (MAC address) is populated where available + {:ok, raw_macs} = Client.walk(@client_opts, @if_phys_address_oid) + + interfaces_with_mac = + Enum.count(interfaces, fn iface -> + iface[:if_phys_address] != nil and iface[:if_phys_address] != "" + end) + + # Count non-empty MACs from raw walk (PPP interfaces have empty MACs) + raw_nonempty_macs = + Enum.count(raw_macs, fn {_oid, val} -> + val != "" and val != nil and val != <<0, 0, 0, 0, 0, 0>> + end) + + Logger.info("MAC addresses: #{interfaces_with_mac} discovered, #{raw_nonempty_macs} non-empty in raw walk") + end + end + + describe "sensor discovery parity" do + @tag timeout: 120_000 + test "ENTITY-SENSOR-MIB sensor count matches raw walk", %{reachable: reachable} do + skip_unless_reachable(reachable) + + case Client.walk(@client_opts, @entity_sensor_type_oid) do + {:ok, raw_sensors} when map_size(raw_sensors) > 0 -> + expected_count = map_size(raw_sensors) + {:ok, sensors} = Base.discover_sensors(@client_opts) + + Logger.info("ENTITY-SENSOR-MIB: raw=#{expected_count}, discovered=#{length(sensors)}") + + assert length(sensors) == expected_count, + "Sensor count mismatch: raw=#{expected_count}, discovered=#{length(sensors)}" + + {:ok, raw_sensors} when map_size(raw_sensors) == 0 -> + Logger.info("Device has no ENTITY-SENSOR-MIB sensors (expected for MikroTik)") + {:ok, sensors} = Base.discover_sensors(@client_opts) + assert sensors == [] + + {:error, _reason} -> + Logger.info("ENTITY-SENSOR-MIB not supported (expected for MikroTik)") + {:ok, sensors} = Base.discover_sensors(@client_opts) + assert is_list(sensors) + end + end + + @tag timeout: 120_000 + test "MikroTik gauge sensors discovered via YAML profile", %{reachable: reachable} do + skip_unless_reachable(reachable) + + # Ground truth: raw MikroTik gauge table + {:ok, raw_gauge_names} = Client.walk(@client_opts, @mtxr_gauge_name_oid) + {:ok, raw_gauge_values} = Client.walk(@client_opts, @mtxr_gauge_value_oid) + + Logger.info( + "Raw MikroTik gauges: #{map_size(raw_gauge_names)} names, " <> + "#{map_size(raw_gauge_values)} values" + ) + + for {oid, name} <- raw_gauge_names do + index = oid |> String.split(".") |> List.last() + value_oid = @mtxr_gauge_value_oid <> ".#{index}" + value = Map.get(raw_gauge_values, value_oid) + Logger.info(" Gauge #{index}: name=#{inspect(name)}, value=#{inspect(value)}") + end + + # Now check that YAML profile discovery finds these + {:ok, system_info} = Base.discover_system_info(@client_opts) + profile = Discovery.select_profile(system_info, @client_opts) + + assert match?({:yaml, _}, profile), "Expected YAML profile for MikroTik" + {:yaml, yaml_profile} = profile + + {:ok, yaml_sensors} = Dynamic.discover_sensors(yaml_profile, @client_opts) + + # Log all discovered sensors for comparison + Logger.info("YAML profile discovered #{length(yaml_sensors)} sensors:") + + for sensor <- yaml_sensors do + Logger.info( + " #{sensor.sensor_type}/#{sensor.sensor_index}: " <> + "#{sensor.sensor_descr} = #{inspect(sensor[:last_value])} " <> + "(oid=#{sensor.sensor_oid}, unit=#{sensor.sensor_unit}, div=#{sensor.sensor_divisor})" + ) + end + + # MikroTik gauges should produce at least one temperature sensor + temp_sensors = Enum.filter(yaml_sensors, &(&1.sensor_type == "temperature")) + + assert temp_sensors != [], + "Expected MikroTik temperature sensors (mtxrGaugeTable has cpu-temperature)" + + # Verify raw gauge value matches a discovered sensor + if map_size(raw_gauge_names) > 0 do + gauge_sensor_oids = + yaml_sensors + |> Enum.map(& &1.sensor_oid) + |> Enum.filter(&String.contains?(&1, "14988.1.1.3.100")) + + assert gauge_sensor_oids != [], + "Expected sensors from mtxrGaugeTable (1.3.6.1.4.1.14988.1.1.3.100)" + end + end + + @tag timeout: 120_000 + test "MikroTik voltage sensors discovered", %{reachable: reachable} do + skip_unless_reachable(reachable) + + # Raw voltage value + case Client.get(@client_opts, @mtxr_hl_voltage_oid) do + {:ok, raw_voltage} -> + Logger.info("Raw mtxrHlVoltage: #{inspect(raw_voltage)}") + + {:ok, system_info} = Base.discover_system_info(@client_opts) + {:yaml, yaml_profile} = Discovery.select_profile(system_info, @client_opts) + {:ok, sensors} = Dynamic.discover_sensors(yaml_profile, @client_opts) + + voltage_sensors = Enum.filter(sensors, &(&1.sensor_type == "voltage")) + + Logger.info("Voltage sensors discovered: #{length(voltage_sensors)}") + + for vs <- voltage_sensors do + Logger.info(" #{vs.sensor_descr}: value=#{inspect(vs[:last_value])}, oid=#{vs.sensor_oid}") + end + + assert voltage_sensors != [], "Expected voltage sensors from MikroTik" + + {:error, reason} -> + Logger.warning("mtxrHlVoltage not available: #{inspect(reason)}") + end + end + + @tag timeout: 120_000 + test "MikroTik count sensors discovered (DHCP, firewall)", %{reachable: reachable} do + skip_unless_reachable(reachable) + + # Check raw DHCP lease count + case Client.get(@client_opts, @mtxr_dhcp_lease_count_oid) do + {:ok, raw_dhcp} -> + Logger.info("Raw mtxrDHCPLeaseCount: #{inspect(raw_dhcp)}") + + {:error, reason} -> + Logger.warning("mtxrDHCPLeaseCount not available: #{inspect(reason)}") + end + + {:ok, system_info} = Base.discover_system_info(@client_opts) + {:yaml, yaml_profile} = Discovery.select_profile(system_info, @client_opts) + {:ok, sensors} = Dynamic.discover_sensors(yaml_profile, @client_opts) + + count_sensors = Enum.filter(sensors, &(&1.sensor_type == "count")) + + Logger.info("Count sensors discovered: #{length(count_sensors)}") + + for cs <- count_sensors do + Logger.info(" #{cs.sensor_descr}: value=#{inspect(cs[:last_value])}, oid=#{cs.sensor_oid}") + end + + # MikroTik should have DHCP lease count and/or firewall connection counts + assert count_sensors != [], + "Expected count sensors (DHCP leases, firewall connections)" + end + + @tag timeout: 120_000 + test "all discovered sensor fields have valid types", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, system_info} = Base.discover_system_info(@client_opts) + profile = Discovery.select_profile(system_info, @client_opts) + + sensors = + case profile do + {:yaml, yaml_profile} -> + {:ok, s} = Dynamic.discover_sensors(yaml_profile, @client_opts) + s + + Base -> + {:ok, s} = Base.discover_sensors(@client_opts) + s + end + + for sensor <- sensors do + assert is_binary(sensor.sensor_type), "sensor_type must be string: #{inspect(sensor)}" + assert is_binary(sensor.sensor_index), "sensor_index must be string: #{inspect(sensor)}" + assert is_binary(sensor.sensor_oid), "sensor_oid must be string: #{inspect(sensor)}" + + assert String.starts_with?(sensor.sensor_oid, "1.3.6.1") or + String.starts_with?(sensor.sensor_oid, ".1.3.6.1"), + "sensor_oid must be valid OID: #{sensor.sensor_oid}" + + assert is_binary(sensor.sensor_descr), "sensor_descr must be string: #{inspect(sensor)}" + assert is_binary(sensor.sensor_unit), "sensor_unit must be string: #{inspect(sensor)}" + assert is_number(sensor.sensor_divisor), "sensor_divisor must be number: #{inspect(sensor)}" + assert sensor.sensor_divisor > 0, "sensor_divisor must be positive: #{sensor.sensor_divisor}" + end + + # Log sensor type breakdown + by_type = Enum.group_by(sensors, & &1.sensor_type) + + for {type, group} <- by_type do + Logger.info(" #{type}: #{length(group)} sensors") + end + + Logger.info("Validated #{length(sensors)} sensors across #{map_size(by_type)} types") + end + end + + describe "device identification parity" do + @tag timeout: 120_000 + test "MikroTik-specific OIDs provide serial, firmware, model", %{reachable: reachable} do + skip_unless_reachable(reachable) + + # Raw ground truth from MikroTik enterprise OIDs + {:ok, raw_serial} = Client.get(@client_opts, @mtxr_serial_oid) + {:ok, raw_firmware} = Client.get(@client_opts, @mtxr_firmware_oid) + {:ok, raw_model} = Client.get(@client_opts, @mtxr_model_oid) + + Logger.info( + "Raw MikroTik: serial=#{inspect(raw_serial)}, " <> + "firmware=#{inspect(raw_firmware)}, model=#{inspect(raw_model)}" + ) + + assert is_binary(raw_serial) and String.length(raw_serial) > 0 + assert is_binary(raw_firmware) and String.length(raw_firmware) > 0 + assert is_binary(raw_model) and String.length(raw_model) > 0 + + # Now verify discovery captures these + {:ok, system_info} = Base.discover_system_info(@client_opts) + {:yaml, yaml_profile} = Discovery.select_profile(system_info, @client_opts) + device_info = Dynamic.identify_device(yaml_profile, @client_opts, system_info) + + Logger.info( + "Discovery device_info: manufacturer=#{inspect(device_info[:manufacturer])}, " <> + "model=#{inspect(device_info[:model])}, " <> + "firmware=#{inspect(device_info[:firmware_version])}, " <> + "serial=#{inspect(device_info[:serial_number])}" + ) + + assert is_binary(device_info.manufacturer) + assert is_binary(device_info.model) + + # Serial and firmware should come from MikroTik-specific OIDs + if device_info[:serial_number] do + assert is_binary(device_info.serial_number) + assert String.length(device_info.serial_number) > 0 + Logger.info("Serial number captured: #{device_info.serial_number}") + else + Logger.warning("Serial number NOT captured from MikroTik OIDs (parity gap)") + end + + if device_info[:firmware_version] do + assert is_binary(device_info.firmware_version) + assert String.length(device_info.firmware_version) > 0 + Logger.info("Firmware version captured: #{device_info.firmware_version}") + else + Logger.warning("Firmware version NOT captured from MikroTik OIDs (parity gap)") + end + end + end + + describe "ENTITY-MIB physical entities" do + @tag timeout: 120_000 + test "physical entity descriptions discovered", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, raw_entities} = Client.walk(@client_opts, @entity_phys_descr_oid) + + Logger.info("ENTITY-MIB physical entities: #{map_size(raw_entities)}") + + for {oid, descr} <- raw_entities do + index = oid |> String.split(".") |> List.last() + Logger.info(" Entity #{index}: #{inspect(descr)}") + end + + # Verify our discovery can read these too + {:ok, entities} = Base.discover_physical_entities(@client_opts) + assert is_list(entities) + + Logger.info("Discovery found #{length(entities)} physical entities") + + # MikroTik has ENTITY-MIB entries (RouterOS info, USB controllers, etc.) + if map_size(raw_entities) > 0 do + assert entities != [], + "Expected physical entities - raw walk found #{map_size(raw_entities)}" + end + end + end + + describe "secondary discovery parity" do + @tag timeout: 120_000 + test "VLANs from Q-BRIDGE-MIB", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, vlans} = Base.discover_vlans(@client_opts) + assert is_list(vlans) + + Logger.info("Discovered #{length(vlans)} VLANs (MikroTik typically returns 0)") + end + + @tag timeout: 120_000 + test "IP addresses match raw IP-MIB walk", %{reachable: reachable} do + skip_unless_reachable(reachable) + + # Raw ground truth + {:ok, raw_ip_addrs} = Client.walk(@client_opts, @ip_ad_ent_addr_oid) + {:ok, raw_ip_if_index} = Client.walk(@client_opts, @ip_ad_ent_if_index_oid) + {:ok, raw_ip_masks} = Client.walk(@client_opts, @ip_ad_ent_net_mask_oid) + + Logger.info( + "Raw IP-MIB: #{map_size(raw_ip_addrs)} addresses, " <> + "#{map_size(raw_ip_if_index)} if-indices, #{map_size(raw_ip_masks)} netmasks" + ) + + for {oid, addr} <- raw_ip_addrs do + index = String.replace_prefix(oid, "1.3.6.1.2.1.4.20.1.1.", "") + if_idx_oid = "1.3.6.1.2.1.4.20.1.2.#{index}" + mask_oid = "1.3.6.1.2.1.4.20.1.3.#{index}" + if_idx = Map.get(raw_ip_if_index, if_idx_oid) + mask = Map.get(raw_ip_masks, mask_oid) + Logger.info(" #{inspect(addr)} (ifIndex=#{inspect(if_idx)}, mask=#{inspect(mask)})") + end + + # Discovery output + {:ok, ip_addresses} = Base.discover_all_ip_addresses(@client_opts) + assert is_list(ip_addresses) + + # Should discover at least as many IPs as raw walk + assert length(ip_addresses) >= map_size(raw_ip_addrs), + "Discovery found #{length(ip_addresses)} IPs, raw walk found #{map_size(raw_ip_addrs)}" + + # Target IP must be among discovered addresses + all_ips = Enum.map(ip_addresses, & &1[:ip_address]) + + Logger.info("Discovered IPs: #{inspect(all_ips)}") + + assert @test_ip in all_ips, + "Target IP #{@test_ip} not found among discovered IPs: #{inspect(all_ips)}" + end + + @tag timeout: 120_000 + test "processors match raw HR-MIB walk", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, raw_proc_loads} = Client.walk(@client_opts, @hr_processor_load_oid) + + Logger.info("Raw HR-MIB processors: #{map_size(raw_proc_loads)}") + + for {oid, load} <- raw_proc_loads do + index = oid |> String.split(".") |> List.last() + Logger.info(" Processor #{index}: load=#{load}%") + end + + {:ok, processors} = Base.discover_processors(@client_opts) + assert is_list(processors) + + # Count should match raw walk + assert length(processors) == map_size(raw_proc_loads), + "Processor count mismatch: discovered=#{length(processors)}, " <> + "raw=#{map_size(raw_proc_loads)}" + + for proc <- processors do + assert is_map(proc) + + if proc[:load] do + assert is_number(proc.load) + + assert proc.load >= 0 and proc.load <= 100, + "CPU load out of range: #{proc.load}" + end + end + + Logger.info("Discovered #{length(processors)} processors (matches raw walk)") + end + + @tag timeout: 120_000 + test "storage matches raw HR-MIB walk (disk types only)", %{reachable: reachable} do + skip_unless_reachable(reachable) + + # Raw HR-MIB includes ALL storage types: RAM, FixedDisk, Other, etc. + {:ok, raw_storage_descr} = Client.walk(@client_opts, @hr_storage_descr_oid) + {:ok, raw_storage_size} = Client.walk(@client_opts, @hr_storage_size_oid) + {:ok, raw_storage_used} = Client.walk(@client_opts, @hr_storage_used_oid) + {:ok, raw_storage_types} = Client.walk(@client_opts, "1.3.6.1.2.1.25.2.3.1.2") + + Logger.info("Raw HR-MIB storage entries (all types): #{map_size(raw_storage_descr)}") + + # Disk-type OIDs that discover_storage keeps (RAM and Other are filtered out) + disk_type_oids = [ + # fixedDisk + [1, 3, 6, 1, 2, 1, 25, 2, 1, 4], + # removableDisk + [1, 3, 6, 1, 2, 1, 25, 2, 1, 5], + # floppyDisk + [1, 3, 6, 1, 2, 1, 25, 2, 1, 6], + # compactDisc + [1, 3, 6, 1, 2, 1, 25, 2, 1, 7], + # ramDisk + [1, 3, 6, 1, 2, 1, 25, 2, 1, 8], + # flashMemory + [1, 3, 6, 1, 2, 1, 25, 2, 1, 9], + # networkDisk + [1, 3, 6, 1, 2, 1, 25, 2, 1, 10] + ] + + raw_disk_count = + Enum.count(raw_storage_types, fn {_oid, type_val} -> + type_val in disk_type_oids + end) + + for {oid, descr} <- raw_storage_descr do + index = oid |> String.split(".") |> List.last() + type_oid = "1.3.6.1.2.1.25.2.3.1.2.#{index}" + size_oid = "1.3.6.1.2.1.25.2.3.1.5.#{index}" + used_oid = "1.3.6.1.2.1.25.2.3.1.6.#{index}" + type_val = Map.get(raw_storage_types, type_oid) + is_disk = type_val in disk_type_oids + size = Map.get(raw_storage_size, size_oid) + used = Map.get(raw_storage_used, used_oid) + + Logger.info( + " #{inspect(descr)}: type=#{inspect(type_val)}, disk=#{is_disk}, " <> + "size=#{inspect(size)}, used=#{inspect(used)}" + ) + end + + {:ok, storage} = Base.discover_storage(@client_opts) + assert is_list(storage) + + # discover_storage only returns disk-type entries (not RAM or Other) + assert length(storage) == raw_disk_count, + "Storage count mismatch: discovered=#{length(storage)}, " <> + "raw disk-type=#{raw_disk_count} (total raw=#{map_size(raw_storage_descr)})" + + for entry <- storage do + assert is_map(entry) + assert entry[:total_bytes] != nil, "Storage entry missing total_bytes" + assert is_number(entry.total_bytes) + assert entry.total_bytes >= 0 + end + + Logger.info( + "Discovered #{length(storage)} disk storage entries (raw has #{raw_disk_count} disk, #{map_size(raw_storage_descr)} total)" + ) + end + end + + describe "full discovery pipeline" do + @tag timeout: 300_000 + test "discover_device saves complete data to DB", %{reachable: reachable} do + skip_unless_reachable(reachable) + + device = + device_fixture(%{ + ip_address: @test_ip, + snmp_enabled: true, + snmp_version: @test_version, + snmp_community: @test_community, + snmp_port: 161 + }) + + {:ok, snmp_device} = Discovery.discover_device(device) + + # Verify SNMP device record has all fields + assert snmp_device.id + assert is_binary(snmp_device.sys_descr) + assert is_binary(snmp_device.sys_object_id) + assert is_binary(snmp_device.sys_name) + assert is_binary(snmp_device.manufacturer) + assert is_binary(snmp_device.model) + assert snmp_device.last_discovery_at + + Logger.info( + "Discovery: manufacturer=#{snmp_device.manufacturer}, " <> + "model=#{snmp_device.model}, firmware=#{snmp_device.firmware_version}, " <> + "serial=#{snmp_device.serial_number}" + ) + + # Verify interfaces persisted and match raw walk count + interfaces = Snmp.list_interfaces(snmp_device.id) + {:ok, raw_if_indices} = Client.walk(@client_opts, @if_index_oid) + + assert length(interfaces) == map_size(raw_if_indices), + "Persisted #{length(interfaces)} interfaces, raw walk has #{map_size(raw_if_indices)}" + + Logger.info("Persisted #{length(interfaces)} interfaces (matches raw walk)") + + # Verify sensors persisted (MikroTik should have profile-specific sensors) + sensors = Snmp.list_sensors(snmp_device.id) + assert is_list(sensors) + + Logger.info("Persisted #{length(sensors)} sensors") + + if sensors != [] do + by_type = Enum.group_by(sensors, & &1.sensor_type) + + for {type, group} <- by_type do + Logger.info(" #{type}: #{length(group)} sensors") + + for s <- group do + Logger.info(" #{s.sensor_descr} (#{s.sensor_oid})") + end + end + end + + # Verify processors persisted + processors = Snmp.list_processors(snmp_device.id) + {:ok, raw_proc} = Client.walk(@client_opts, @hr_processor_load_oid) + + assert length(processors) == map_size(raw_proc), + "Persisted #{length(processors)} processors, raw walk has #{map_size(raw_proc)}" + + Logger.info("Persisted #{length(processors)} processors (matches raw walk)") + + # Verify storage persisted (only disk-type entries, not RAM/Other) + storage = Snmp.list_storage(snmp_device.id) + assert is_list(storage) + assert storage != [], "Expected at least one disk storage entry" + + Logger.info("Persisted #{length(storage)} disk storage entries") + + # Verify IP addresses persisted + ip_addresses = Snmp.list_ip_addresses(snmp_device.id) + {:ok, raw_ips} = Client.walk(@client_opts, @ip_ad_ent_addr_oid) + + Logger.info("Persisted #{length(ip_addresses)} IP addresses (raw walk has #{map_size(raw_ips)})") + end + + @tag timeout: 300_000 + test "rediscovery updates without creating duplicates", %{reachable: reachable} do + skip_unless_reachable(reachable) + + device = + device_fixture(%{ + ip_address: @test_ip, + snmp_enabled: true, + snmp_version: @test_version, + snmp_community: @test_community, + snmp_port: 161 + }) + + # First discovery + {:ok, snmp_device_1} = Discovery.discover_device(device) + interfaces_1 = Snmp.list_interfaces(snmp_device_1.id) + sensors_1 = Snmp.list_sensors(snmp_device_1.id) + processors_1 = Snmp.list_processors(snmp_device_1.id) + storage_1 = Snmp.list_storage(snmp_device_1.id) + + # Second discovery (should update, not duplicate) + {:ok, snmp_device_2} = Discovery.discover_device(device) + interfaces_2 = Snmp.list_interfaces(snmp_device_2.id) + sensors_2 = Snmp.list_sensors(snmp_device_2.id) + processors_2 = Snmp.list_processors(snmp_device_2.id) + storage_2 = Snmp.list_storage(snmp_device_2.id) + + # Same SNMP device record (upserted, not duplicated) + assert snmp_device_1.id == snmp_device_2.id + + # Same counts (synced, not appended) + assert length(interfaces_1) == length(interfaces_2), + "Interface count changed: #{length(interfaces_1)} -> #{length(interfaces_2)}" + + assert length(sensors_1) == length(sensors_2), + "Sensor count changed: #{length(sensors_1)} -> #{length(sensors_2)}" + + assert length(processors_1) == length(processors_2), + "Processor count changed: #{length(processors_1)} -> #{length(processors_2)}" + + assert length(storage_1) == length(storage_2), + "Storage count changed: #{length(storage_1)} -> #{length(storage_2)}" + + Logger.info( + "Rediscovery preserved: #{length(interfaces_2)} interfaces, " <> + "#{length(sensors_2)} sensors, #{length(processors_2)} processors, " <> + "#{length(storage_2)} storage" + ) + end + end + + describe "polling after discovery" do + @tag timeout: 300_000 + test "sensor OIDs return numeric values after discovery", %{reachable: reachable} do + skip_unless_reachable(reachable) + + device = + device_fixture(%{ + ip_address: @test_ip, + snmp_enabled: true, + snmp_version: @test_version, + snmp_community: @test_community, + snmp_port: 161 + }) + + {:ok, snmp_device} = Discovery.discover_device(device) + sensors = Snmp.list_sensors(snmp_device.id) + + if Enum.empty?(sensors) do + Logger.warning("No sensors to poll - skipping sensor polling test") + else + # Poll each sensor's OID and verify we get a readable value + results = + Enum.map(sensors, fn sensor -> + case Client.get(@client_opts, sensor.sensor_oid) do + {:ok, value} when is_number(value) -> + {:numeric, sensor.sensor_descr, value} + + {:ok, value} -> + {:non_numeric, sensor.sensor_descr, value} + + {:error, reason} -> + {:error, sensor.sensor_descr, reason} + end + end) + + numeric = Enum.count(results, &(elem(&1, 0) == :numeric)) + non_numeric = Enum.count(results, &(elem(&1, 0) == :non_numeric)) + errors = Enum.count(results, &(elem(&1, 0) == :error)) + + Logger.info( + "Polled #{length(sensors)} sensors: " <> + "#{numeric} numeric, #{non_numeric} non-numeric, #{errors} errors" + ) + + for {status, descr, val} <- results do + Logger.info(" [#{status}] #{descr}: #{inspect(val)}") + end + + assert numeric > 0, "Expected at least one sensor to return numeric data" + end + end + + @tag timeout: 120_000 + test "interface counters (ifInOctets/ifOutOctets) are readable", %{reachable: reachable} do + skip_unless_reachable(reachable) + + {:ok, in_octets} = Client.walk(@client_opts, @if_in_octets_oid) + {:ok, out_octets} = Client.walk(@client_opts, @if_out_octets_oid) + + assert map_size(in_octets) > 0, "Expected ifInOctets data" + assert map_size(out_octets) > 0, "Expected ifOutOctets data" + + # Counter values should be non-negative integers + for {oid, value} <- in_octets do + assert is_integer(value) and value >= 0, + "ifInOctets #{oid} invalid: #{inspect(value)}" + end + + for {oid, value} <- out_octets do + assert is_integer(value) and value >= 0, + "ifOutOctets #{oid} invalid: #{inspect(value)}" + end + + Logger.info("Interface counters: #{map_size(in_octets)} in, #{map_size(out_octets)} out") + end + end + + # Helper to skip tests when device is unreachable. + defp skip_unless_reachable(true), do: :ok + + defp skip_unless_reachable(false) do + flunk("Device #{@test_ip} unreachable - skipping") + end +end diff --git a/test/towerops/monitoring/check_test.exs b/test/towerops/monitoring/check_test.exs index b8efa7de..a5300850 100644 --- a/test/towerops/monitoring/check_test.exs +++ b/test/towerops/monitoring/check_test.exs @@ -155,7 +155,7 @@ defmodule Towerops.Monitoring.CheckTest do changeset = Check.changeset(%Check{}, attrs) assert changeset.valid? - assert get_change(changeset, :response_time_ms) == 123.456 + assert get_change(changeset, :response_time_ms) == 123.5 end test "checked_at accepts DateTime", %{device: device} do