diff --git a/lib/towerops/devices.ex b/lib/towerops/devices.ex index 71837cda..ce0cae70 100644 --- a/lib/towerops/devices.ex +++ b/lib/towerops/devices.ex @@ -646,25 +646,52 @@ defmodule Towerops.Devices do Resolves the effective SNMP community string for a device. Resolution order: - 1. If device has its own community string (source = "device"), use it - 2. If source = "site", inherit from site's community string + 1. If device has its own community string (source = "device" AND community is non-empty), use it + 2. Otherwise, inherit from site's community string 3. If site has no community, inherit from organization's community string 4. If no community found anywhere, return nil + + Note: Even if source is "device", we fall back to inheritance if the device community is empty. + This handles cases where the source field is incorrect or the community was cleared. """ def resolve_snmp_community(%DeviceSchema{} = device) do device = Repo.preload(device, site: :organization) - case device.snmp_community_source do - "device" -> - # Use device-specific community + # Try device-specific community first (if source is "device" AND it's non-empty) + device_community = + if device.snmp_community_source == "device" && present?(device.snmp_community) do device.snmp_community + end - _ -> - # Inherit from site or organization - device.site.snmp_community || device.site.organization.snmp_community + # Fall back to inheritance chain + community = + device_community || + device.site.snmp_community || + device.site.organization.snmp_community + + # Debug logging to track empty community strings + if is_nil(community) or community == "" do + require Logger + + Logger.warning("Empty SNMP community resolved for device", + device_id: device.id, + device_name: device.name, + community_source: device.snmp_community_source, + device_community: device.snmp_community, + site_community: device.site.snmp_community, + org_community: device.site.organization.snmp_community, + site_id: device.site_id, + org_id: device.site.organization_id + ) end + + community end + # Helper to check if a string is present (not nil and not empty) + defp present?(value) when is_binary(value), do: String.trim(value) != "" + defp present?(_), do: false + @doc """ Propagates SNMP community string changes from a site to all devices that inherit from it (source = "site"). diff --git a/lib/towerops/devices/device.ex b/lib/towerops/devices/device.ex index 24730979..9dde5b0d 100644 --- a/lib/towerops/devices/device.ex +++ b/lib/towerops/devices/device.ex @@ -34,7 +34,7 @@ defmodule Towerops.Devices.Device do field :snmp_enabled, :boolean, default: true field :snmp_version, :string, default: "2c" field :snmp_community, :string - field :snmp_community_source, :string, default: "device" + field :snmp_community_source, :string, default: "organization" field :snmp_port, :integer, default: 161 field :last_discovery_at, :utc_datetime field :last_snmp_poll_at, :utc_datetime diff --git a/lib/towerops/snmp/profiles/vendors/routeros.ex b/lib/towerops/snmp/profiles/vendors/routeros.ex index 28d4e14d..ca482597 100644 --- a/lib/towerops/snmp/profiles/vendors/routeros.ex +++ b/lib/towerops/snmp/profiles/vendors/routeros.ex @@ -869,7 +869,7 @@ defmodule Towerops.Snmp.Profiles.Vendors.Routeros do # Helper: Get integer value from grouped results defp get_int_value(values, sub_oid) do case Map.get(values, sub_oid) do - value when is_integer(value) and value != 0 -> value + value when is_integer(value) -> value _ -> nil end end diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index 7f6f7a1f..a9bc2a1c 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -248,9 +248,10 @@ defmodule ToweropsWeb.AgentChannel do build_polling_job(device) end - # Build MikroTik job if device is MikroTik and MikroTik API is enabled + # Build MikroTik job only during discovery (not during regular polling) + # MikroTik commands are discovery-type operations that should run once per 24h mikrotik_job = - if mikrotik_device?(device) do + if needs_discovery?(device) and mikrotik_device?(device) do build_mikrotik_job(device) end @@ -273,13 +274,22 @@ defmodule ToweropsWeb.AgentChannel do end defp build_discovery_job(device) do + community = Devices.resolve_snmp_community(device) + + maybe_debug_log(%{assigns: %{agent_token_id: "system"}}, "Building discovery job", + device_id: device.id, + device_name: device.name, + community_present: !is_nil(community) && community != "", + community_source: device.snmp_community_source + ) + %AgentJob{ job_id: "discover:#{device.id}", job_type: :DISCOVER, device_id: device.id, snmp_device: %SnmpDevice{ ip: device.ip_address, - community: Devices.resolve_snmp_community(device), + community: community, version: device.snmp_version, port: device.snmp_port || 161 }, @@ -289,6 +299,22 @@ defmodule ToweropsWeb.AgentChannel do defp build_polling_job(device) do _snmp_device = device.snmp_device + community = Devices.resolve_snmp_community(device) + + maybe_debug_log(%{assigns: %{agent_token_id: "system"}}, "Building polling job", + device_id: device.id, + device_name: device.name, + community_present: !is_nil(community) && community != "", + community_source: device.snmp_community_source, + site_id: device.site_id, + site_loaded: not is_nil(Map.get(device, :site)), + site_community_present: + case Map.get(device, :site) do + nil -> false + %Ecto.Association.NotLoaded{} -> false + site -> !is_nil(site.snmp_community) && site.snmp_community != "" + end + ) %AgentJob{ job_id: "poll:#{device.id}", @@ -296,7 +322,7 @@ defmodule ToweropsWeb.AgentChannel do device_id: device.id, snmp_device: %SnmpDevice{ ip: device.ip_address, - community: Devices.resolve_snmp_community(device), + community: community, version: device.snmp_version, port: device.snmp_port || 161 }, diff --git a/lib/towerops_web/live/device_live/show.html.heex b/lib/towerops_web/live/device_live/show.html.heex index 39bd2fc2..a1c7e7c6 100644 --- a/lib/towerops_web/live/device_live/show.html.heex +++ b/lib/towerops_web/live/device_live/show.html.heex @@ -1488,7 +1488,10 @@ <%= if length(@mikrotik_backups) > 1 do %>
- <.icon name="hero-information-circle" class="h-5 w-5 text-blue-600 dark:text-blue-400 flex-shrink-0 mt-0.5" /> + <.icon + name="hero-information-circle" + class="h-5 w-5 text-blue-600 dark:text-blue-400 flex-shrink-0 mt-0.5" + />
Compare configurations: Select any 2 backups using the checkboxes to compare their differences @@ -1500,7 +1503,10 @@ - @@ -1614,7 +1620,10 @@
<%!-- Selection status --%>
- <.icon name="hero-check-circle" class="h-5 w-5 text-blue-600 dark:text-blue-400" /> + <.icon + name="hero-check-circle" + class="h-5 w-5 text-blue-600 dark:text-blue-400" + />
{MapSet.size(@selected_backup_ids)} of 2 backups selected diff --git a/priv/repo/migrations/20260202225627_fix_mikrotik_gauge_sensors_with_wrong_divisor.exs b/priv/repo/migrations/20260202225627_fix_mikrotik_gauge_sensors_with_wrong_divisor.exs new file mode 100644 index 00000000..967b7992 --- /dev/null +++ b/priv/repo/migrations/20260202225627_fix_mikrotik_gauge_sensors_with_wrong_divisor.exs @@ -0,0 +1,130 @@ +defmodule Towerops.Repo.Migrations.FixMikrotikGaugeSensorsWithWrongDivisor do + use Ecto.Migration + + def up do + # Fix MikroTik gauge table sensors that were discovered with divisor 1 + # due to the bug in get_int_value() that rejected 0 values. + # + # This affects sensors where: + # - mtxrGaugeUnit was 0, nil, or an unknown value + # - Name-based override should have set divisor to 10 + # - But the override didn't run or failed, leaving divisor at 1 + # + # We identify these by: + # - OID matches gauge table pattern (1.3.6.1.4.1.14988.1.1.3.100.1.3.*) + # - sensor_type should be temperature/voltage/current/power (tenths of unit) + # - sensor_divisor is currently 1 (wrong) + + # Temperature sensors (should be tenths of degrees) + execute """ + UPDATE snmp_sensors + SET sensor_divisor = 10, + sensor_unit = '°C', + last_value = CASE + WHEN last_value IS NOT NULL THEN last_value / 10.0 + ELSE NULL + END + WHERE sensor_oid LIKE '1.3.6.1.4.1.14988.1.1.3.100.1.3.%' + AND sensor_type = 'temperature' + AND sensor_divisor = 1 + """ + + # Voltage sensors (should be tenths of volts) + execute """ + UPDATE snmp_sensors + SET sensor_divisor = 10, + sensor_unit = 'V', + last_value = CASE + WHEN last_value IS NOT NULL THEN last_value / 10.0 + ELSE NULL + END + WHERE sensor_oid LIKE '1.3.6.1.4.1.14988.1.1.3.100.1.3.%' + AND sensor_type = 'voltage' + AND sensor_divisor = 1 + """ + + # Current sensors (should be tenths of amps) + execute """ + UPDATE snmp_sensors + SET sensor_divisor = 10, + sensor_unit = 'A', + last_value = CASE + WHEN last_value IS NOT NULL THEN last_value / 10.0 + ELSE NULL + END + WHERE sensor_oid LIKE '1.3.6.1.4.1.14988.1.1.3.100.1.3.%' + AND sensor_type = 'current' + AND sensor_divisor = 1 + """ + + # Power sensors (should be tenths of watts) + execute """ + UPDATE snmp_sensors + SET sensor_divisor = 10, + sensor_unit = 'W', + last_value = CASE + WHEN last_value IS NOT NULL THEN last_value / 10.0 + ELSE NULL + END + WHERE sensor_oid LIKE '1.3.6.1.4.1.14988.1.1.3.100.1.3.%' + AND sensor_type = 'power' + AND sensor_divisor = 1 + """ + end + + def down do + # Revert to divisor 1 (incorrect, but reversible) + + execute """ + UPDATE snmp_sensors + SET sensor_divisor = 1, + sensor_unit = '', + last_value = CASE + WHEN last_value IS NOT NULL THEN last_value * 10.0 + ELSE NULL + END + WHERE sensor_oid LIKE '1.3.6.1.4.1.14988.1.1.3.100.1.3.%' + AND sensor_type = 'temperature' + AND sensor_divisor = 10 + """ + + execute """ + UPDATE snmp_sensors + SET sensor_divisor = 1, + sensor_unit = '', + last_value = CASE + WHEN last_value IS NOT NULL THEN last_value * 10.0 + ELSE NULL + END + WHERE sensor_oid LIKE '1.3.6.1.4.1.14988.1.1.3.100.1.3.%' + AND sensor_type = 'voltage' + AND sensor_divisor = 10 + """ + + execute """ + UPDATE snmp_sensors + SET sensor_divisor = 1, + sensor_unit = '', + last_value = CASE + WHEN last_value IS NOT NULL THEN last_value * 10.0 + ELSE NULL + END + WHERE sensor_oid LIKE '1.3.6.1.4.1.14988.1.1.3.100.1.3.%' + AND sensor_type = 'current' + AND sensor_divisor = 10 + """ + + execute """ + UPDATE snmp_sensors + SET sensor_divisor = 1, + sensor_unit = '', + last_value = CASE + WHEN last_value IS NOT NULL THEN last_value * 10.0 + ELSE NULL + END + WHERE sensor_oid LIKE '1.3.6.1.4.1.14988.1.1.3.100.1.3.%' + AND sensor_type = 'power' + AND sensor_divisor = 10 + """ + end +end diff --git a/priv/repo/migrations/20260202230847_fix_snmp_community_source_for_empty_communities.exs b/priv/repo/migrations/20260202230847_fix_snmp_community_source_for_empty_communities.exs new file mode 100644 index 00000000..e6eeb4b9 --- /dev/null +++ b/priv/repo/migrations/20260202230847_fix_snmp_community_source_for_empty_communities.exs @@ -0,0 +1,35 @@ +defmodule Towerops.Repo.Migrations.FixSnmpCommunitySourceForEmptyCommunities do + use Ecto.Migration + + def up do + # Fix devices that have snmp_community_source = "device" but no actual community string. + # These should inherit from organization/site instead. + # + # Root cause: The changeset logic in update_community_source/1 was setting source = "device" + # as the default, even when the device had no community string. + # + # This migration changes devices with: + # - snmp_community_source = "device" + # - snmp_community IS NULL or empty string + # + # To: + # - snmp_community_source = "organization" (will cascade through site → org inheritance) + + execute """ + UPDATE devices + SET snmp_community_source = 'organization' + WHERE snmp_community_source = 'device' + AND (snmp_community IS NULL OR snmp_community = '') + """ + end + + def down do + # Revert to "device" source (not ideal, but reversible) + execute """ + UPDATE devices + SET snmp_community_source = 'device' + WHERE snmp_community_source = 'organization' + AND (snmp_community IS NULL OR snmp_community = '') + """ + end +end
+ <.icon name="hero-check-circle" class="h-4 w-4 mx-auto" /> Date