done i think (#177)

Reviewed-on: graham/towerops-web#177

lib/towerops/capacity.ex

@@ -211,7 +211,7 @@ defmodule Towerops.Capacity do
 
   def percentile(values, n) when n >= 0 and n <= 100 do
     sorted = Enum.sort(values)
-    rank = Float.ceil(n / 100 * length(sorted)) |> trunc() |> max(1)
+    rank = (n / 100 * length(sorted)) |> Float.ceil() |> trunc() |> max(1)
     Enum.at(sorted, rank - 1)
   end
 

lib/towerops/monitoring.ex

@@ -556,6 +556,50 @@ defmodule Towerops.Monitoring do
     end
   end
 
+  @doc """
+  Finds an existing auto-discovered check or creates a new one.
+
+  Uses `(device_id, check_type, source_id)` as the uniqueness key.
+  If a check already exists, updates its name and config. Otherwise creates it
+  and schedules a CheckExecutorWorker job.
+
+  Attrs must include: `:device_id`, `:check_type`, `:source_id`, `:source_type`,
+  `:organization_id`, `:name`, `:config`.
+  """
+  def ensure_discovery_check(attrs) do
+    device_id = Map.fetch!(attrs, :device_id)
+    check_type = Map.fetch!(attrs, :check_type)
+    source_id = Map.fetch!(attrs, :source_id)
+
+    existing =
+      Repo.one(
+        from(c in Check,
+          where: c.device_id == ^device_id,
+          where: c.check_type == ^check_type,
+          where: c.source_id == ^source_id,
+          limit: 1
+        )
+      )
+
+    case existing do
+      nil ->
+        with {:ok, check} <- create_check(attrs) do
+          _ = schedule_check(check)
+          {:ok, check}
+        end
+
+      check ->
+        name = Map.get(attrs, :name, check.name)
+        config = Map.get(attrs, :config, check.config)
+
+        if check.name == name and check.config == config do
+          {:ok, check}
+        else
+          update_check(check, %{name: name, config: config})
+        end
+    end
+  end
+
   ## Legacy ping-based monitoring compatibility (stub)
 
   @doc false

lib/towerops/snmp/discovery.ex

@@ -446,8 +446,6 @@ defmodule Towerops.Snmp.Discovery do
   ```
   """
   def create_checks_from_discovery(device, snmp_device) do
-    alias Towerops.Monitoring
-
     # Ensure associations are loaded
     snmp_device = Repo.preload(snmp_device, [:sensors, :interfaces, :processors, :storage])
 
@@ -510,98 +508,74 @@ defmodule Towerops.Snmp.Discovery do
   # Private functions
 
   defp create_sensor_check(device, sensor) do
-    alias Towerops.Monitoring
+    Towerops.Monitoring.ensure_discovery_check(%{
-
+      organization_id: device.organization_id,
-    with {:ok, check} <-
+      device_id: device.id,
-           Monitoring.create_check(%{
+      name: sensor.sensor_descr,
-             organization_id: device.organization_id,
+      check_type: "snmp_sensor",
-             device_id: device.id,
+      source_type: "auto_discovery",
-             name: sensor.sensor_descr,
+      source_id: sensor.id,
-             check_type: "snmp_sensor",
+      interval_seconds: 60,
-             source_type: "auto_discovery",
+      enabled: true,
-             source_id: sensor.id,
+      config: %{
-             interval_seconds: 60,
+        "sensor_type" => sensor.sensor_type,
-             enabled: true,
+        "sensor_class" => sensor.sensor_class,
-             config: %{
+        "sensor_oid" => sensor.sensor_oid,
-               "sensor_type" => sensor.sensor_type,
+        "sensor_divisor" => sensor.sensor_divisor,
-               "sensor_class" => sensor.sensor_class,
+        "sensor_unit" => sensor.sensor_unit
-               "sensor_oid" => sensor.sensor_oid,
+      }
-               "sensor_divisor" => sensor.sensor_divisor,
+    })
-               "sensor_unit" => sensor.sensor_unit
-             }
-           }),
-         {:ok, _job} <- Monitoring.schedule_check(check) do
-      {:ok, check}
-    end
   end
 
   defp create_interface_check(device, interface) do
-    alias Towerops.Monitoring
+    Towerops.Monitoring.ensure_discovery_check(%{
-
+      organization_id: device.organization_id,
-    with {:ok, check} <-
+      device_id: device.id,
-           Monitoring.create_check(%{
+      name: "Interface #{interface.if_descr}",
-             organization_id: device.organization_id,
+      check_type: "snmp_interface",
-             device_id: device.id,
+      source_type: "auto_discovery",
-             name: "Interface #{interface.if_descr}",
+      source_id: interface.id,
-             check_type: "snmp_interface",
+      interval_seconds: 60,
-             source_type: "auto_discovery",
+      enabled: true,
-             source_id: interface.id,
+      config: %{
-             interval_seconds: 60,
+        "if_index" => interface.if_index,
-             enabled: true,
+        "if_descr" => interface.if_descr
-             config: %{
+      }
-               "if_index" => interface.if_index,
+    })
-               "if_descr" => interface.if_descr
-             }
-           }),
-         {:ok, _job} <- Monitoring.schedule_check(check) do
-      {:ok, check}
-    end
   end
 
   defp create_processor_check(device, processor) do
-    alias Towerops.Monitoring
+    Towerops.Monitoring.ensure_discovery_check(%{
-
+      organization_id: device.organization_id,
-    with {:ok, check} <-
+      device_id: device.id,
-           Monitoring.create_check(%{
+      name: "CPU #{processor.processor_index}",
-             organization_id: device.organization_id,
+      check_type: "snmp_processor",
-             device_id: device.id,
+      source_type: "auto_discovery",
-             name: "CPU #{processor.processor_index}",
+      source_id: processor.id,
-             check_type: "snmp_processor",
+      interval_seconds: 60,
-             source_type: "auto_discovery",
+      enabled: true,
-             source_id: processor.id,
+      config: %{
-             interval_seconds: 60,
+        "processor_index" => processor.processor_index,
-             enabled: true,
+        "processor_descr" => processor.processor_descr
-             config: %{
+      }
-               "processor_index" => processor.processor_index,
+    })
-               "processor_descr" => processor.processor_descr
-             }
-           }),
-         {:ok, _job} <- Monitoring.schedule_check(check) do
-      {:ok, check}
-    end
   end
 
   defp create_storage_check(device, storage) do
-    alias Towerops.Monitoring
+    Towerops.Monitoring.ensure_discovery_check(%{
-
+      organization_id: device.organization_id,
-    with {:ok, check} <-
+      device_id: device.id,
-           Monitoring.create_check(%{
+      name: storage.description || storage.device_name || "Storage #{storage.storage_index}",
-             organization_id: device.organization_id,
+      check_type: "snmp_storage",
-             device_id: device.id,
+      source_type: "auto_discovery",
-             name: storage.description || storage.device_name || "Storage #{storage.storage_index}",
+      source_id: storage.id,
-             check_type: "snmp_storage",
+      interval_seconds: 60,
-             source_type: "auto_discovery",
+      enabled: true,
-             source_id: storage.id,
+      config: %{
-             interval_seconds: 60,
+        "storage_index" => storage.storage_index,
-             enabled: true,
+        "storage_descr" => storage.description
-             config: %{
+      }
-               "storage_index" => storage.storage_index,
+    })
-               "storage_descr" => storage.description
-             }
-           }),
-         {:ok, _job} <- Monitoring.schedule_check(check) do
-      {:ok, check}
-    end
   end
 
   defp log_check_creation_results(device_id, check_counts) do

lib/towerops/snmp/profiles/vendors/airfiber.ex

@@ -33,6 +33,7 @@ defmodule Towerops.Snmp.Profiles.Vendors.Airfiber do
     case Client.get(v1_opts, "1.3.6.1.2.1.1.5.0") do
       {:ok, sys_name} when is_binary(sys_name) and sys_name != "" ->
         "Ubiquiti AirFiber (#{sys_name})"
+
       _ ->
         "Ubiquiti AirFiber"
     end

lib/towerops/topology.ex

@@ -6,6 +6,7 @@ defmodule Towerops.Topology do
 
   import Ecto.Query
 
+  alias Towerops.Capacity
   alias Towerops.Devices
   alias Towerops.Devices.Device
   alias Towerops.Repo
@@ -22,7 +23,6 @@ defmodule Towerops.Topology do
   alias Towerops.Topology.DeviceNeighbor
   alias Towerops.Topology.Identifier
   alias Towerops.Topology.Lldp
-  alias Towerops.Capacity
 
   require Logger
 
@@ -1402,57 +1402,57 @@ defmodule Towerops.Topology do
   defp enrich_edges_with_utilization(edges, device_ids) do
     # Get all interfaces with configured capacity for these devices
     interface_utilizations = get_interface_utilizations(device_ids)
-    
+
     Enum.map(edges, fn edge ->
       # Try to find utilization data for the source or target interface
       source_util = get_edge_utilization(edge, :source, interface_utilizations)
       target_util = get_edge_utilization(edge, :target, interface_utilizations)
-      
+
       # Use the higher utilization of the two interfaces
       utilization = combine_interface_utilizations(source_util, target_util)
-      
+
       Map.merge(edge, utilization)
     end)
   end
 
   defp get_interface_utilizations(device_ids) do
     # Query interfaces with capacity configuration and recent stats
-    Repo.all(
+    from(i in Interface,
-      from i in Interface,
+      join: sd in SnmpDevice,
-        join: sd in SnmpDevice, on: i.snmp_device_id == sd.id,
+      on: i.snmp_device_id == sd.id,
-        join: d in Device, on: sd.device_id == d.id,
+      join: d in Device,
-        where: d.id in ^device_ids and not is_nil(i.configured_capacity_bps),
+      on: sd.device_id == d.id,
-        select: %{
+      where: d.id in ^device_ids and not is_nil(i.configured_capacity_bps),
-          interface_id: i.id,
+      select: %{
-          device_id: d.id,
+        interface_id: i.id,
-          interface_name: i.if_name,
+        device_id: d.id,
-          capacity_bps: i.configured_capacity_bps,
+        interface_name: i.if_name,
-          if_index: i.if_index
+        capacity_bps: i.configured_capacity_bps,
-        }
+        if_index: i.if_index
+      }
     )
+    |> Repo.all()
     |> Enum.map(fn interface ->
       # Calculate current utilization using the Capacity module
       interface_struct = %Interface{
         id: interface.interface_id,
         configured_capacity_bps: interface.capacity_bps
       }
-      
+
       utilization = Capacity.get_utilization(interface_struct)
-      
+
-      Map.merge(interface, %{
+      Map.put(interface, :utilization_data, utilization)
-        utilization_data: utilization
-      })
     end)
     |> Map.new(fn interface -> {interface.interface_id, interface} end)
   end
 
   defp get_edge_utilization(edge, direction, interface_utilizations) do
-    interface_id = 
+    interface_id =
       case direction do
         :source -> edge[:source_interface_id]
         :target -> edge[:target_interface_id]
       end
-    
+
     case interface_id do
       nil -> nil
       id -> Map.get(interface_utilizations, id)
@@ -1469,18 +1469,18 @@ defmodule Towerops.Topology do
           capacity_bps: nil,
           utilization_text: nil
         }
-      
+
       {util, nil} ->
         process_single_utilization(util)
-        
+
       {nil, util} ->
         process_single_utilization(util)
-        
+
       {source, target} ->
         # Use the higher utilization of the two interfaces
         source_data = process_single_utilization(source)
         target_data = process_single_utilization(target)
-        
+
         if (source_data.utilization_pct || 0) >= (target_data.utilization_pct || 0) do
           source_data
         else
@@ -1499,11 +1499,11 @@ defmodule Towerops.Topology do
           capacity_bps: util_data.capacity_bps,
           utilization_text: nil
         }
-        
+
       util ->
         utilization_pct = Float.round(util.utilization_pct, 1)
         level = classify_utilization_level(utilization_pct)
-        
+
         %{
           utilization_pct: utilization_pct,
           utilization_level: level,
@@ -1517,7 +1517,7 @@ defmodule Towerops.Topology do
 
   defp classify_utilization_level(nil), do: nil
   defp classify_utilization_level(pct) when pct < 30, do: :low
-  defp classify_utilization_level(pct) when pct < 60, do: :medium  
+  defp classify_utilization_level(pct) when pct < 60, do: :medium
   defp classify_utilization_level(pct) when pct < 80, do: :high
   defp classify_utilization_level(_pct), do: :critical
 
@@ -1528,22 +1528,26 @@ defmodule Towerops.Topology do
   end
 
   defp format_bps(nil), do: "N/A"
+
   defp format_bps(bps) when bps >= 1_000_000_000 do
     "#{Float.round(bps / 1_000_000_000, 1)} Gbps"
   end
+
   defp format_bps(bps) when bps >= 1_000_000 do
     "#{Float.round(bps / 1_000_000, 0)} Mbps"
   end
+
   defp format_bps(bps) when bps >= 1_000 do
     "#{Float.round(bps / 1_000, 0)} Kbps"
   end
+
   defp format_bps(bps) do
     "#{Float.round(bps, 0)} bps"
   end
 
   # Compute utilization statistics for the weathermap dashboard.
   defp compute_utilization_stats(edges) do
-    utilization_counts = 
+    utilization_counts =
       edges
       |> Enum.map(& &1[:utilization_level])
       |> Enum.frequencies()

lib/towerops/workers/device_poller_worker.ex

@@ -1013,7 +1013,9 @@ defmodule Towerops.Workers.DevicePollerWorker do
         fn interface ->
           stat_data =
             case get_airfiber_stats(af_overrides, interface, client_opts) do
-              {:ok, data} -> data
+              {:ok, data} ->
+                data
+
               _ ->
                 {:ok, data} = get_interface_stats(client_opts, interface.if_index)
                 data
@@ -1048,8 +1050,9 @@ defmodule Towerops.Workers.DevicePollerWorker do
     # Ubiquiti uses two enterprise OIDs:
     # - 1.3.6.1.4.1.41112 (new UBNT enterprise OID)
     # - 1.3.6.1.4.1.10002 (old UBNT enterprise OID, used by AirFiber AF11/AF24)
-    is_ubnt = String.starts_with?(sys_oid, "1.3.6.1.4.1.41112") or
+    is_ubnt =
-              String.starts_with?(sys_oid, "1.3.6.1.4.1.10002")
+      String.starts_with?(sys_oid, "1.3.6.1.4.1.41112") or
+        String.starts_with?(sys_oid, "1.3.6.1.4.1.10002")
 
     if is_ubnt do
       # Probe for the AirFiber statistics table — if it exists, this device
@@ -1058,7 +1061,9 @@ defmodule Towerops.Workers.DevicePollerWorker do
 
       # Check for LTU first (more specific)
       case Client.get(v1_opts, "1.3.6.1.4.1.41112.1.10.1.2.2.0") do
-        {:ok, val} when val != nil -> :airfiber_ltu
+        {:ok, val} when val != nil ->
+          :airfiber_ltu
+
         _ ->
           # Check for regular AirFiber stats table (index field)
           case Client.get(v1_opts, "1.3.6.1.4.1.41112.1.3.3.1.1.1") do
@@ -1066,8 +1071,6 @@ defmodule Towerops.Workers.DevicePollerWorker do
             _ -> nil
           end
       end
-    else
-      nil
     end
   end
 
@@ -1077,8 +1080,7 @@ defmodule Towerops.Workers.DevicePollerWorker do
   # since the proprietary counters represent the radio link's actual throughput.
   # Only skip lo and sit0 (loopback/tunnel with no real traffic).
   defp get_airfiber_stats(nil, _interface, _client_opts), do: :not_airfiber
-  defp get_airfiber_stats(_af_type, %{if_descr: descr}, _client_opts)
+  defp get_airfiber_stats(_af_type, %{if_descr: descr}, _client_opts) when descr in ["lo", "sit0"], do: :skip_loopback
-       when descr in ["lo", "sit0"], do: :skip_loopback
 
   defp get_airfiber_stats(:airfiber, _interface, client_opts) do
     # UBNT-AirFIBER-MIB: airFiberStatistics table (1.3.6.1.4.1.41112.1.3.3.1)
@@ -1090,20 +1092,28 @@ defmodule Towerops.Workers.DevicePollerWorker do
     v1_opts = Keyword.put(client_opts, :version, "1")
 
     oids = [
-      {"1.3.6.1.4.1.41112.1.3.3.1.7.1", :if_in_octets},    # rxOctetsOK
+      # rxOctetsOK
-      {"1.3.6.1.4.1.41112.1.3.3.1.6.1", :if_out_octets},   # txOctetsOK
+      {"1.3.6.1.4.1.41112.1.3.3.1.7.1", :if_in_octets},
-      {"1.3.6.1.4.1.41112.1.3.3.1.10.1", :if_in_errors},   # rxErroredFrames
+      # txOctetsOK
-      {"1.3.6.1.4.1.41112.1.3.3.1.11.1", :if_out_errors}   # txErroredFrames
+      {"1.3.6.1.4.1.41112.1.3.3.1.6.1", :if_out_octets},
+      # rxErroredFrames
+      {"1.3.6.1.4.1.41112.1.3.3.1.10.1", :if_in_errors},
+      # txErroredFrames
+      {"1.3.6.1.4.1.41112.1.3.3.1.11.1", :if_out_errors}
     ]
+
     fetch_proprietary_stats(v1_opts, oids, true)
   end
 
   defp get_airfiber_stats(:airfiber_ltu, _interface, client_opts) do
     # UBNT-AFLTU-MIB: afLTUeth table (1.3.6.1.4.1.41112.1.10.1.6)
     oids = [
-      {"1.3.6.1.4.1.41112.1.10.1.6.1.6.0", :if_in_octets},   # afLTUethRxBytes
+      # afLTUethRxBytes
-      {"1.3.6.1.4.1.41112.1.10.1.6.1.4.0", :if_out_octets}    # afLTUethTxBytes
+      {"1.3.6.1.4.1.41112.1.10.1.6.1.6.0", :if_in_octets},
+      # afLTUethTxBytes
+      {"1.3.6.1.4.1.41112.1.10.1.6.1.4.0", :if_out_octets}
     ]
+
     fetch_proprietary_stats(client_opts, oids, true)
   end
 

lib/towerops_web/live/weathermap_live.ex

@@ -132,7 +132,7 @@ defmodule ToweropsWeb.WeathermapLive do
   def handle_event("toggle_fullscreen", _params, socket) do
     new_fullscreen = not socket.assigns.fullscreen
 
-    path_with_params = 
+    path_with_params =
       if new_fullscreen do
         ~p"/weathermap?#{%{tab: socket.assigns.active_tab, fullscreen: true}}"
       else
@@ -192,4 +192,4 @@ defmodule ToweropsWeb.WeathermapLive do
       detail -> assign(socket, :selected_node_detail, detail)
     end
   end
-end
+end

lib/towerops_web/live/weathermap_live.html.heex

@@ -1,4 +1,7 @@
-<div class={["h-screen flex flex-col", if(@fullscreen, do: "fixed inset-0 z-50 bg-white dark:bg-gray-900")]}>
+<div class={[
+  "h-screen flex flex-col",
+  if(@fullscreen, do: "fixed inset-0 z-50 bg-white dark:bg-gray-900")
+]}>
   <%= if not @fullscreen do %>
     <Layouts.authenticated
       flash={@flash}
@@ -64,8 +67,8 @@
       </div>
     </div>
   <% end %>
-
+  
-  <!-- Tab Navigation -->
+<!-- Tab Navigation -->
   <div class="border-b border-gray-200 dark:border-white/10 px-4 bg-white dark:bg-gray-900">
     <nav class="-mb-px flex space-x-8">
       <.link
@@ -232,7 +235,7 @@
       </div>
     </div>
     
-    <!-- Filter Bar -->
+<!-- Filter Bar -->
     <div class="bg-white dark:bg-gray-800 border-b border-gray-200 dark:border-white/10 p-3">
       <div class="flex items-center justify-between gap-4 flex-wrap">
         <div class="flex items-center gap-2">
@@ -263,7 +266,8 @@
             High Utilization (60%+)
             <%= if (@topology.stats[:high_utilization_links] || 0) + (@topology.stats[:overutilized_links] || 0) > 0 do %>
               <span class="ml-1 inline-flex items-center justify-center w-5 h-5 text-xs font-bold text-orange-800 bg-orange-200 rounded-full dark:bg-orange-900 dark:text-orange-300">
-                {(@topology.stats[:high_utilization_links] || 0) + (@topology.stats[:overutilized_links] || 0)}
+                {(@topology.stats[:high_utilization_links] || 0) +
+                  (@topology.stats[:overutilized_links] || 0)}
               </span>
             <% end %>
           </button>
@@ -338,7 +342,7 @@
       </div>
     </div>
     
-    <!-- Network Weathermap Container -->
+<!-- Network Weathermap Container -->
     <div class="flex-1 bg-white dark:bg-gray-800 relative overflow-hidden">
       <div class="absolute inset-0 border-t border-gray-200 dark:border-white/10">
         <div
@@ -349,7 +353,7 @@
         >
         </div>
         
-        <!-- Zoom Controls -->
+<!-- Zoom Controls -->
         <div class="absolute bottom-4 left-4 flex flex-col gap-1 z-10">
           <button
             id="cy-zoom-in"
@@ -376,8 +380,8 @@
             <.icon name="hero-arrows-pointing-out" class="h-4 w-4" />
           </button>
         </div>
-
+        
-        <!-- Utilization Legend -->
+<!-- Utilization Legend -->
         <div class="absolute bottom-4 right-4 bg-white dark:bg-gray-800 border border-gray-200 dark:border-white/10 rounded-lg shadow-lg p-4 z-10">
           <h4 class="text-sm font-medium text-gray-900 dark:text-white mb-3">Link Utilization</h4>
           <div class="space-y-2 text-xs">
@@ -407,7 +411,7 @@
           </div>
         </div>
         
-        <!-- Node Detail Panel (slide-out) -->
+<!-- Node Detail Panel (slide-out) -->
         <%= if @selected_node_detail do %>
           <div
             id="node-detail-panel"
@@ -427,7 +431,7 @@
                 </button>
               </div>
               
-              <!-- Device Info -->
+<!-- Device Info -->
               <div class="space-y-3">
                 <div>
                   <h4 class="text-base font-medium text-gray-900 dark:text-white">
@@ -494,7 +498,7 @@
                 <% end %>
               </div>
               
-              <!-- Utilization Stats (for devices with bandwidth data) -->
+<!-- Utilization Stats (for devices with bandwidth data) -->
               <%= if @selected_node_detail[:utilization_stats] do %>
                 <div class="mt-4 pt-3 border-t border-gray-200 dark:border-white/10">
                   <h4 class="text-xs font-medium text-gray-500 dark:text-gray-400 uppercase tracking-wider mb-2">
@@ -529,7 +533,7 @@
                 </div>
               <% end %>
               
-              <!-- Connections -->
+<!-- Connections -->
               <%= if length(@selected_node_detail.connections) > 0 do %>
                 <div class="mt-5 pt-4 border-t border-gray-200 dark:border-white/10">
                   <h4 class="text-xs font-medium text-gray-500 dark:text-gray-400 uppercase tracking-wider mb-3">
@@ -574,7 +578,7 @@
                 </div>
               <% end %>
               
-              <!-- View Device Link (managed only) -->
+<!-- View Device Link (managed only) -->
               <%= if @selected_node_detail.type == :managed do %>
                 <div class="mt-4">
                   <.link
@@ -592,7 +596,7 @@
       </div>
     </div>
     
-    <!-- Empty State -->
+<!-- Empty State -->
     <%= if @topology.stats.total_devices == 0 do %>
       <div class="flex-1 flex items-center justify-center">
         <div class="text-center py-16">
@@ -601,7 +605,9 @@
             {t("No network data available")}
           </h3>
           <p class="mt-2 text-sm text-gray-600 dark:text-gray-400">
-            {t("Add devices with SNMP enabled and configure interface capacities to view utilization.")}
+            {t(
+              "Add devices with SNMP enabled and configure interface capacities to view utilization."
+            )}
           </p>
           <div class="mt-6">
             <.button navigate={~p"/devices/new"} variant="primary">
@@ -612,4 +618,4 @@
       </div>
     <% end %>
   <% end %>
-</div>
+</div>

mix.lock

@@ -62,7 +62,7 @@
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.2", "8efba0122db06df95bfaa78f791344a89352ba04baedd3849593bfce4d0dc1c6", [:mix], [], "hexpm", "4b21398942dda052b403bbe1da991ccd03a053668d147d53fb8c4e0efe09c973"},
   "nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"},
   "nimble_totp": {:hex, :nimble_totp, "1.0.0", "79753bae6ce59fd7cacdb21501a1dbac249e53a51c4cd22b34fa8438ee067283", [:mix], [], "hexpm", "6ce5e4c068feecdb782e85b18237f86f66541523e6bad123e02ee1adbe48eda9"},
-  "oban": {:hex, :oban, "2.21.0", "25b2d22061628f60ffa93e5c945bdd7dcd90ab3892ae0734da41ec6ede23b9ed", [:mix], [{:ecto_sql, "~> 3.10", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, "~> 0.9", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:igniter, "~> 0.5", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:myxql, "~> 0.7", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.20", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 1.3", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "3209e3d008a2dc46085ed9dce2f9cf0b4e1c58d1ab32615cf64ac1830bdb90bf"},
+  "oban": {:hex, :oban, "2.21.1", "4b6af7b901ef9baca09e239b5a991ef2fa429cf5a13799bc429a131d610ff692", [:mix], [{:ecto_sql, "~> 3.10", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, "~> 0.9", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:igniter, "~> 0.5", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:myxql, "~> 0.7", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.20", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 1.3", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "8162a160924cf4a25905fed2a9242e7787d88e320e3b5b0dcf324eb17c51c4e6"},
   "oban_met": {:hex, :oban_met, "1.0.6", "2a5500aff496b7ac4b830b0b03b08e920625a051bb6890981fbb53b15f1cbdc0", [:mix], [{:oban, "~> 2.19", [hex: :oban, repo: "hexpm", optional: false]}], "hexpm", "15ea3303de76225878a8e6c25a9d62bd1e2e9dd1c46ac8487d873b9f99e8dcee"},
   "parse_trans": {:hex, :parse_trans, "3.4.1", "6e6aa8167cb44cc8f39441d05193be6e6f4e7c2946cb2759f015f8c56b76e5ff", [:rebar3], [], "hexpm", "620a406ce75dada827b82e453c19cf06776be266f5a67cff34e1ef2cbb60e49a"},
   "phoenix": {:hex, :phoenix, "1.8.5", "919db335247e6d4891764dc3063415b0d2457641c5f9b3751b5df03d8e20bbcf", [:mix], [{:bandit, "~> 1.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.1", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.7", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:websock_adapter, "~> 0.5.3", [hex: :websock_adapter, repo: "hexpm", optional: false]}], "hexpm", "83b2bb125127e02e9f475c8e3e92736325b5b01b0b9b05407bcb4083b7a32485"},

priv/repo/migrations/20260325215632_deduplicate_discovery_checks.exs

@@ -0,0 +1,40 @@
+defmodule Towerops.Repo.Migrations.DeduplicateDiscoveryChecks do
+  use Ecto.Migration
+
+  def up do
+    # Delete duplicate auto-discovered checks, keeping the oldest one per
+    # (device_id, check_type, source_id) combination.
+    execute("""
+    DELETE FROM checks
+    WHERE id IN (
+      SELECT id FROM (
+        SELECT id,
+               ROW_NUMBER() OVER (
+                 PARTITION BY device_id, check_type, source_id
+                 ORDER BY inserted_at ASC
+               ) AS rn
+        FROM checks
+        WHERE source_type = 'auto_discovery'
+          AND source_id IS NOT NULL
+      ) ranked
+      WHERE rn > 1
+    )
+    """)
+
+    # Prevent future duplicates for auto-discovered checks.
+    create(
+      unique_index(:checks, [:device_id, :check_type, :source_id],
+        where: "source_type = 'auto_discovery' AND source_id IS NOT NULL",
+        name: :checks_device_type_source_unique_index
+      )
+    )
+  end
+
+  def down do
+    drop_if_exists(
+      index(:checks, [:device_id, :check_type, :source_id],
+        name: :checks_device_type_source_unique_index
+      )
+    )
+  end
+end

test/towerops/monitoring_test.exs

@@ -620,6 +620,112 @@ defmodule Towerops.MonitoringTest do
     end
   end
 
+  describe "ensure_discovery_check/1" do
+    test "creates a new check when none exists", %{organization: org, device: device} do
+      attrs = %{
+        organization_id: org.id,
+        device_id: device.id,
+        name: "eth0 Status",
+        check_type: "snmp_interface",
+        source_type: "auto_discovery",
+        source_id: Ecto.UUID.generate(),
+        interval_seconds: 60,
+        enabled: true,
+        config: %{"if_index" => 1, "if_descr" => "eth0"}
+      }
+
+      assert {:ok, check} = Monitoring.ensure_discovery_check(attrs)
+      assert check.name == "eth0 Status"
+      assert check.check_type == "snmp_interface"
+      assert check.source_type == "auto_discovery"
+    end
+
+    test "is idempotent - returns existing check on repeated calls", %{organization: org, device: device} do
+      source_id = Ecto.UUID.generate()
+
+      attrs = %{
+        organization_id: org.id,
+        device_id: device.id,
+        name: "eth0 Status",
+        check_type: "snmp_interface",
+        source_type: "auto_discovery",
+        source_id: source_id,
+        interval_seconds: 60,
+        enabled: true,
+        config: %{"if_index" => 1, "if_descr" => "eth0"}
+      }
+
+      assert {:ok, first} = Monitoring.ensure_discovery_check(attrs)
+      assert {:ok, second} = Monitoring.ensure_discovery_check(attrs)
+      assert first.id == second.id
+
+      # Verify only one check exists in DB
+      checks =
+        Repo.all(
+          from(c in Check,
+            where: c.device_id == ^device.id,
+            where: c.check_type == "snmp_interface",
+            where: c.source_id == ^source_id
+          )
+        )
+
+      assert length(checks) == 1
+    end
+
+    test "updates name if it changed", %{organization: org, device: device} do
+      source_id = Ecto.UUID.generate()
+
+      attrs = %{
+        organization_id: org.id,
+        device_id: device.id,
+        name: "eth0 Status",
+        check_type: "snmp_interface",
+        source_type: "auto_discovery",
+        source_id: source_id,
+        interval_seconds: 60,
+        enabled: true,
+        config: %{"if_index" => 1, "if_descr" => "eth0"}
+      }
+
+      assert {:ok, first} = Monitoring.ensure_discovery_check(attrs)
+      assert first.name == "eth0 Status"
+
+      updated_attrs = %{attrs | name: "Interface eth0"}
+      assert {:ok, second} = Monitoring.ensure_discovery_check(updated_attrs)
+      assert second.id == first.id
+      assert second.name == "Interface eth0"
+    end
+
+    test "differentiates checks by source_id", %{organization: org, device: device} do
+      base_attrs = %{
+        organization_id: org.id,
+        device_id: device.id,
+        check_type: "snmp_interface",
+        source_type: "auto_discovery",
+        interval_seconds: 60,
+        enabled: true
+      }
+
+      attrs1 =
+        Map.merge(base_attrs, %{
+          name: "eth0 Status",
+          source_id: Ecto.UUID.generate(),
+          config: %{"if_index" => 1, "if_descr" => "eth0"}
+        })
+
+      attrs2 =
+        Map.merge(base_attrs, %{
+          name: "eth1 Status",
+          source_id: Ecto.UUID.generate(),
+          config: %{"if_index" => 2, "if_descr" => "eth1"}
+        })
+
+      assert {:ok, check1} = Monitoring.ensure_discovery_check(attrs1)
+      assert {:ok, check2} = Monitoring.ensure_discovery_check(attrs2)
+      refute check1.id == check2.id
+    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)

test/towerops/snmp/profiles/vendors/airfiber_test.exs

@@ -24,8 +24,7 @@ defmodule Towerops.Snmp.Profiles.Vendors.AirfiberTest do
 
   describe "detect_hardware/1" do
     test "returns hardware string with sysName when available" do
-      SnmpMock
+      expect(SnmpMock, :get, fn _target, "1.3.6.1.2.1.1.5.0", _opts ->
-      |> expect(:get, fn _target, "1.3.6.1.2.1.1.5.0", _opts ->
         {:ok, {:octet_string, "Climax-380 AF24"}}
       end)
 
@@ -33,8 +32,7 @@ defmodule Towerops.Snmp.Profiles.Vendors.AirfiberTest do
     end
 
     test "returns generic hardware string when sysName unavailable" do
-      SnmpMock
+      expect(SnmpMock, :get, fn _target, "1.3.6.1.2.1.1.5.0", _opts ->
-      |> expect(:get, fn _target, "1.3.6.1.2.1.1.5.0", _opts ->
         {:error, :timeout}
       end)