discovery no longer deletes historical data

This commit is contained in:
Graham McIntire 2026-01-20 17:29:58 -06:00
parent c54b5d81bb
commit 1d45699240
No known key found for this signature in database
2 changed files with 477 additions and 25 deletions

View file

@ -376,14 +376,13 @@ defmodule Towerops.Snmp.Discovery do
# Upsert Device
snmp_device = upsert_device(device, device_info)
# Delete old interfaces/sensors and insert new ones
_ = delete_old_data(snmp_device)
new_interfaces = insert_interfaces(snmp_device, interfaces)
_ = insert_sensors(snmp_device, sensors)
# Sync interfaces and sensors (preserving historical data)
synced_interfaces = sync_interfaces(snmp_device, interfaces)
_ = sync_sensors(snmp_device, sensors)
# Return device with interfaces loaded and device_id added
# Use snmp_device.device_id which references the Equipment table
%{snmp_device | interfaces: Enum.map(new_interfaces, &Map.put(&1, :device_id, snmp_device.device_id))}
%{snmp_device | interfaces: Enum.map(synced_interfaces, &Map.put(&1, :device_id, snmp_device.device_id))}
end)
end
@ -421,32 +420,95 @@ defmodule Towerops.Snmp.Discovery do
end
end
@spec delete_old_data(Device.t()) :: {non_neg_integer(), nil | [term()]}
defp delete_old_data(device) do
# Delete old interfaces and sensors (cascade will handle stats/readings)
Repo.delete_all(from i in Interface, where: i.snmp_device_id == ^device.id)
Repo.delete_all(from s in Sensor, where: s.snmp_device_id == ^device.id)
end
@spec sync_interfaces(Device.t(), [interface_data()]) :: [Interface.t()]
defp sync_interfaces(device, discovered_interfaces) do
# Get existing interfaces for this device, indexed by if_index
existing_interfaces =
from(i in Interface, where: i.snmp_device_id == ^device.id)
|> Repo.all()
|> Map.new(&{&1.if_index, &1})
@spec insert_interfaces(Device.t(), [interface_data()]) :: [Interface.t()]
defp insert_interfaces(device, interfaces) do
Enum.map(interfaces, fn interface_data ->
%Interface{}
|> Interface.changeset(Map.put(interface_data, :snmp_device_id, device.id))
|> Repo.insert!()
# Get the set of discovered if_indices
discovered_indices = MapSet.new(discovered_interfaces, & &1.if_index)
# Delete interfaces that no longer exist on the device
existing_indices = MapSet.new(Map.keys(existing_interfaces))
removed_indices = MapSet.difference(existing_indices, discovered_indices)
if MapSet.size(removed_indices) != 0 do
removed_list = MapSet.to_list(removed_indices)
Repo.delete_all(
from i in Interface,
where: i.snmp_device_id == ^device.id and i.if_index in ^removed_list
)
Logger.debug("Deleted #{MapSet.size(removed_indices)} removed interfaces")
end
# Upsert each discovered interface
Enum.map(discovered_interfaces, fn interface_data ->
case Map.get(existing_interfaces, interface_data.if_index) do
nil ->
# New interface - insert
%Interface{}
|> Interface.changeset(Map.put(interface_data, :snmp_device_id, device.id))
|> Repo.insert!()
existing ->
# Existing interface - update (preserves ID and associated stats)
existing
|> Interface.changeset(interface_data)
|> Repo.update!()
end
end)
end
@spec insert_sensors(Device.t(), [sensor_data()]) :: [Sensor.t()]
defp insert_sensors(device, sensors) do
@spec sync_sensors(Device.t(), [sensor_data()]) :: [Sensor.t()]
defp sync_sensors(device, discovered_sensors) do
# Deduplicate sensors by sensor_index (keep first occurrence)
# This prevents duplicate key errors when profiles generate duplicate indices
sensors
|> Enum.uniq_by(& &1.sensor_index)
|> Enum.map(fn sensor_data ->
%Sensor{}
|> Sensor.changeset(Map.put(sensor_data, :snmp_device_id, device.id))
|> Repo.insert!()
discovered_sensors = Enum.uniq_by(discovered_sensors, & &1.sensor_index)
# Get existing sensors for this device, indexed by sensor_index
existing_sensors =
from(s in Sensor, where: s.snmp_device_id == ^device.id)
|> Repo.all()
|> Map.new(&{&1.sensor_index, &1})
# Get the set of discovered sensor_indices
discovered_indices = MapSet.new(discovered_sensors, & &1.sensor_index)
# Delete sensors that no longer exist on the device
existing_indices = MapSet.new(Map.keys(existing_sensors))
removed_indices = MapSet.difference(existing_indices, discovered_indices)
if MapSet.size(removed_indices) != 0 do
removed_list = MapSet.to_list(removed_indices)
Repo.delete_all(
from s in Sensor,
where: s.snmp_device_id == ^device.id and s.sensor_index in ^removed_list
)
Logger.debug("Deleted #{MapSet.size(removed_indices)} removed sensors")
end
# Upsert each discovered sensor
Enum.map(discovered_sensors, fn sensor_data ->
case Map.get(existing_sensors, sensor_data.sensor_index) do
nil ->
# New sensor - insert
%Sensor{}
|> Sensor.changeset(Map.put(sensor_data, :snmp_device_id, device.id))
|> Repo.insert!()
existing ->
# Existing sensor - update (preserves ID and associated readings)
existing
|> Sensor.changeset(sensor_data)
|> Repo.update!()
end
end)
end

View file

@ -405,6 +405,396 @@ defmodule Towerops.Snmp.DiscoveryTest do
assert updated_device.model == "Linux"
assert updated_device.sys_name == "updated-name"
end
test "preserves sensor readings when rediscovering device with same sensor_index", %{
site: site
} do
alias Towerops.Snmp.SensorReading
{:ok, device} =
Towerops.Devices.create_device(%{
name: "Device With Sensors",
ip_address: "192.168.1.50",
site_id: site.id,
snmp_enabled: true,
snmp_version: "2c",
snmp_community: "public",
snmp_port: 161
})
# Create initial SNMP device with a sensor
{:ok, snmp_device} =
%Device{}
|> Device.changeset(%{
device_id: device.id,
manufacturer: "Cisco",
model: "C2960",
sys_name: "test-switch"
})
|> Repo.insert()
{:ok, sensor} =
%Sensor{}
|> Sensor.changeset(%{
snmp_device_id: snmp_device.id,
sensor_type: "temperature",
sensor_index: "1001",
sensor_oid: "1.3.6.1.4.1.9.9.91.1.1.1.1.4.1001",
sensor_descr: "CPU Temperature",
sensor_unit: "celsius",
sensor_divisor: 1,
last_value: 45.0
})
|> Repo.insert()
# Create historical sensor readings
readings_to_create = [
%{
sensor_id: sensor.id,
value: 42.0,
status: "ok",
checked_at: DateTime.add(DateTime.utc_now(), -3600, :second)
},
%{
sensor_id: sensor.id,
value: 44.0,
status: "ok",
checked_at: DateTime.add(DateTime.utc_now(), -1800, :second)
},
%{
sensor_id: sensor.id,
value: 45.0,
status: "ok",
checked_at: DateTime.utc_now()
}
]
for reading_attrs <- readings_to_create do
%SensorReading{}
|> SensorReading.changeset(reading_attrs)
|> Repo.insert!()
end
# Verify readings exist before rediscovery
initial_reading_count =
Repo.aggregate(from(r in SensorReading, where: r.sensor_id == ^sensor.id), :count)
assert initial_reading_count == 3
# Mock SNMP responses for rediscovery
stub(SnmpMock, :get, fn _, oid, _ ->
case oid do
"1.3.6.1.2.1.1.3.0" -> {:ok, {:timeticks, 54_321}}
"1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Cisco IOS Software, C2960"}}
"1.3.6.1.2.1.1.2.0" -> {:ok, {:object_identifier, [1, 3, 6, 1, 4, 1, 9, 1, 1208]}}
"1.3.6.1.2.1.1.4.0" -> {:ok, {:octet_string, "admin@example.com"}}
"1.3.6.1.2.1.1.5.0" -> {:ok, {:octet_string, "test-switch"}}
"1.3.6.1.2.1.1.6.0" -> {:ok, {:octet_string, "Data Center"}}
_ -> {:error, :no_such_object}
end
end)
stub(SnmpMock, :walk, fn _, oid, _ ->
# CISCO-ENTITY-SENSOR-MIB - sensor type table (ent_sensor_type)
# This is what Cisco profile walks first to find sensors
if oid == "1.3.6.1.4.1.9.9.91.1.1.1.1.1" do
{:ok, [%{oid: "1.3.6.1.4.1.9.9.91.1.1.1.1.1.1001", value: {:integer, 8}}]}
else
{:ok, []}
end
end)
# Mock the individual sensor GETs that Cisco profile does after finding sensor indices
stub(SnmpMock, :get, fn _, oid, _ ->
case oid do
"1.3.6.1.2.1.1.3.0" -> {:ok, {:timeticks, 54_321}}
"1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Cisco IOS Software, C2960"}}
"1.3.6.1.2.1.1.2.0" -> {:ok, {:object_identifier, [1, 3, 6, 1, 4, 1, 9, 1, 1208]}}
"1.3.6.1.2.1.1.4.0" -> {:ok, {:octet_string, "admin@example.com"}}
"1.3.6.1.2.1.1.5.0" -> {:ok, {:octet_string, "test-switch"}}
"1.3.6.1.2.1.1.6.0" -> {:ok, {:octet_string, "Data Center"}}
# Cisco sensor details for index 1001
"1.3.6.1.4.1.9.9.91.1.1.1.1.1.1001" -> {:ok, {:integer, 8}}
"1.3.6.1.4.1.9.9.91.1.1.1.1.2.1001" -> {:ok, {:integer, 9}}
"1.3.6.1.4.1.9.9.91.1.1.1.1.3.1001" -> {:ok, {:integer, 0}}
"1.3.6.1.4.1.9.9.91.1.1.1.1.4.1001" -> {:ok, {:integer, 47}}
"1.3.6.1.4.1.9.9.91.1.1.1.1.5.1001" -> {:ok, {:integer, 1}}
"1.3.6.1.2.1.47.1.1.1.1.2.1001" -> {:ok, {:octet_string, "CPU Temperature"}}
"1.3.6.1.2.1.47.1.1.1.1.7.1001" -> {:ok, {:octet_string, "TempSensor1"}}
_ -> {:error, :no_such_object}
end
end)
# Run rediscovery
assert {:ok, rediscovered_device} = Discovery.discover_device(device)
# Verify the SNMP device was updated (same ID)
assert rediscovered_device.id == snmp_device.id
# Find the sensor - it should still exist (same ID preserved)
sensors = Repo.all(from s in Sensor, where: s.snmp_device_id == ^snmp_device.id)
assert length(sensors) == 1
[updated_sensor] = sensors
# The key assertion: sensor ID should be the same, preserving readings
assert updated_sensor.id == sensor.id
# Verify all historical readings are still there
final_reading_count =
Repo.aggregate(from(r in SensorReading, where: r.sensor_id == ^sensor.id), :count)
assert final_reading_count == 3
# Verify readings still have the correct values
readings = Repo.all(from r in SensorReading, where: r.sensor_id == ^sensor.id, order_by: r.checked_at)
assert length(readings) == 3
assert Enum.map(readings, & &1.value) == [42.0, 44.0, 45.0]
end
test "preserves interface stats when rediscovering device with same if_index", %{site: site} do
alias Towerops.Snmp.InterfaceStat
{:ok, device} =
Towerops.Devices.create_device(%{
name: "Device With Interfaces",
ip_address: "192.168.1.51",
site_id: site.id,
snmp_enabled: true,
snmp_version: "2c",
snmp_community: "public",
snmp_port: 161
})
# Create initial SNMP device with an interface
{:ok, snmp_device} =
%Device{}
|> Device.changeset(%{
device_id: device.id,
manufacturer: "Cisco",
model: "C2960",
sys_name: "test-switch"
})
|> Repo.insert()
{:ok, interface} =
%Interface{}
|> Interface.changeset(%{
snmp_device_id: snmp_device.id,
if_index: 1,
if_descr: "GigabitEthernet0/1",
if_name: "Gi0/1",
if_type: 6,
if_speed: 1_000_000_000
})
|> Repo.insert()
# Create historical interface stats
stats_to_create = [
%{
interface_id: interface.id,
if_in_octets: 1_000_000,
if_out_octets: 500_000,
if_in_errors: 0,
if_out_errors: 0,
checked_at: DateTime.add(DateTime.utc_now(), -3600, :second)
},
%{
interface_id: interface.id,
if_in_octets: 2_000_000,
if_out_octets: 1_000_000,
if_in_errors: 0,
if_out_errors: 0,
checked_at: DateTime.add(DateTime.utc_now(), -1800, :second)
},
%{
interface_id: interface.id,
if_in_octets: 3_000_000,
if_out_octets: 1_500_000,
if_in_errors: 1,
if_out_errors: 0,
checked_at: DateTime.utc_now()
}
]
for stat_attrs <- stats_to_create do
%InterfaceStat{}
|> InterfaceStat.changeset(stat_attrs)
|> Repo.insert!()
end
# Verify stats exist before rediscovery
initial_stat_count =
Repo.aggregate(from(s in InterfaceStat, where: s.interface_id == ^interface.id), :count)
assert initial_stat_count == 3
# Mock SNMP responses for rediscovery
stub(SnmpMock, :get, fn _, oid, _ ->
case oid do
"1.3.6.1.2.1.1.3.0" -> {:ok, {:timeticks, 54_321}}
"1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Cisco IOS Software, C2960"}}
"1.3.6.1.2.1.1.2.0" -> {:ok, {:object_identifier, [1, 3, 6, 1, 4, 1, 9, 1, 1208]}}
"1.3.6.1.2.1.1.4.0" -> {:ok, {:octet_string, "admin@example.com"}}
"1.3.6.1.2.1.1.5.0" -> {:ok, {:octet_string, "test-switch"}}
"1.3.6.1.2.1.1.6.0" -> {:ok, {:octet_string, "Data Center"}}
# Interface details for if_index 1
"1.3.6.1.2.1.2.2.1.2.1" -> {:ok, {:octet_string, "GigabitEthernet0/1"}}
"1.3.6.1.2.1.2.2.1.3.1" -> {:ok, {:integer, 6}}
"1.3.6.1.2.1.2.2.1.5.1" -> {:ok, {:gauge32, 1_000_000_000}}
"1.3.6.1.2.1.2.2.1.6.1" -> {:ok, {:octet_string, <<0x00, 0x11, 0x22, 0x33, 0x44, 0x55>>}}
"1.3.6.1.2.1.2.2.1.7.1" -> {:ok, {:integer, 1}}
"1.3.6.1.2.1.2.2.1.8.1" -> {:ok, {:integer, 1}}
"1.3.6.1.2.1.31.1.1.1.1.1" -> {:ok, {:octet_string, "Gi0/1"}}
"1.3.6.1.2.1.31.1.1.1.18.1" -> {:ok, {:octet_string, "Uplink"}}
_ -> {:error, :no_such_object}
end
end)
stub(SnmpMock, :walk, fn _, oid, _ ->
# Interface discovery - return same interface with if_index 1
if oid == "1.3.6.1.2.1.2.2.1.1" do
{:ok, [%{oid: "1.3.6.1.2.1.2.2.1.1.1", value: {:integer, 1}}]}
else
{:ok, []}
end
end)
# Run rediscovery
assert {:ok, rediscovered_device} = Discovery.discover_device(device)
# Verify the SNMP device was updated (same ID)
assert rediscovered_device.id == snmp_device.id
# Find the interface - it should still exist (same ID preserved)
interfaces = Repo.all(from i in Interface, where: i.snmp_device_id == ^snmp_device.id)
assert length(interfaces) == 1
[updated_interface] = interfaces
# The key assertion: interface ID should be the same, preserving stats
assert updated_interface.id == interface.id
# Verify all historical stats are still there
final_stat_count =
Repo.aggregate(from(s in InterfaceStat, where: s.interface_id == ^interface.id), :count)
assert final_stat_count == 3
# Verify stats still have the correct values
stats = Repo.all(from s in InterfaceStat, where: s.interface_id == ^interface.id, order_by: s.checked_at)
assert length(stats) == 3
assert Enum.map(stats, & &1.if_in_octets) == [1_000_000, 2_000_000, 3_000_000]
end
test "deletes sensors that no longer exist during rediscovery", %{site: site} do
{:ok, device} =
Towerops.Devices.create_device(%{
name: "Device With Removed Sensor",
ip_address: "192.168.1.52",
site_id: site.id,
snmp_enabled: true,
snmp_version: "2c",
snmp_community: "public",
snmp_port: 161
})
# Create initial SNMP device with two sensors
{:ok, snmp_device} =
%Device{}
|> Device.changeset(%{
device_id: device.id,
manufacturer: "Cisco",
model: "C2960",
sys_name: "test-switch"
})
|> Repo.insert()
{:ok, sensor1} =
%Sensor{}
|> Sensor.changeset(%{
snmp_device_id: snmp_device.id,
sensor_type: "temperature",
sensor_index: "1001",
sensor_oid: "1.3.6.1.4.1.9.9.91.1.1.1.1.4.1001",
sensor_descr: "CPU Temperature",
sensor_unit: "celsius",
sensor_divisor: 1
})
|> Repo.insert()
{:ok, _sensor2} =
%Sensor{}
|> Sensor.changeset(%{
snmp_device_id: snmp_device.id,
sensor_type: "temperature",
sensor_index: "1002",
sensor_oid: "1.3.6.1.4.1.9.9.91.1.1.1.1.4.1002",
sensor_descr: "Chassis Temperature",
sensor_unit: "celsius",
sensor_divisor: 1
})
|> Repo.insert()
# Verify two sensors exist before rediscovery
initial_sensor_count =
Repo.aggregate(from(s in Sensor, where: s.snmp_device_id == ^snmp_device.id), :count)
assert initial_sensor_count == 2
# Mock SNMP responses - only return sensor 1001, not 1002
stub(SnmpMock, :get, fn _, oid, _ ->
case oid do
"1.3.6.1.2.1.1.3.0" -> {:ok, {:timeticks, 54_321}}
"1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Cisco IOS Software, C2960"}}
"1.3.6.1.2.1.1.2.0" -> {:ok, {:object_identifier, [1, 3, 6, 1, 4, 1, 9, 1, 1208]}}
"1.3.6.1.2.1.1.4.0" -> {:ok, {:octet_string, "admin@example.com"}}
"1.3.6.1.2.1.1.5.0" -> {:ok, {:octet_string, "test-switch"}}
"1.3.6.1.2.1.1.6.0" -> {:ok, {:octet_string, "Data Center"}}
_ -> {:error, :no_such_object}
end
end)
stub(SnmpMock, :walk, fn _, oid, _ ->
# CISCO-ENTITY-SENSOR-MIB - only return sensor 1001 (1002 is "removed")
if oid == "1.3.6.1.4.1.9.9.91.1.1.1.1.1" do
{:ok, [%{oid: "1.3.6.1.4.1.9.9.91.1.1.1.1.1.1001", value: {:integer, 8}}]}
else
{:ok, []}
end
end)
# Mock the individual sensor GETs for Cisco profile
stub(SnmpMock, :get, fn _, oid, _ ->
case oid do
"1.3.6.1.2.1.1.3.0" -> {:ok, {:timeticks, 54_321}}
"1.3.6.1.2.1.1.1.0" -> {:ok, {:octet_string, "Cisco IOS Software, C2960"}}
"1.3.6.1.2.1.1.2.0" -> {:ok, {:object_identifier, [1, 3, 6, 1, 4, 1, 9, 1, 1208]}}
"1.3.6.1.2.1.1.4.0" -> {:ok, {:octet_string, "admin@example.com"}}
"1.3.6.1.2.1.1.5.0" -> {:ok, {:octet_string, "test-switch"}}
"1.3.6.1.2.1.1.6.0" -> {:ok, {:octet_string, "Data Center"}}
# Cisco sensor details for index 1001 only
"1.3.6.1.4.1.9.9.91.1.1.1.1.1.1001" -> {:ok, {:integer, 8}}
"1.3.6.1.4.1.9.9.91.1.1.1.1.2.1001" -> {:ok, {:integer, 9}}
"1.3.6.1.4.1.9.9.91.1.1.1.1.3.1001" -> {:ok, {:integer, 0}}
"1.3.6.1.4.1.9.9.91.1.1.1.1.4.1001" -> {:ok, {:integer, 47}}
"1.3.6.1.4.1.9.9.91.1.1.1.1.5.1001" -> {:ok, {:integer, 1}}
"1.3.6.1.2.1.47.1.1.1.1.2.1001" -> {:ok, {:octet_string, "CPU Temperature"}}
"1.3.6.1.2.1.47.1.1.1.1.7.1001" -> {:ok, {:octet_string, "TempSensor1"}}
_ -> {:error, :no_such_object}
end
end)
# Run rediscovery
assert {:ok, _rediscovered_device} = Discovery.discover_device(device)
# Verify only sensor 1001 remains
sensors = Repo.all(from s in Sensor, where: s.snmp_device_id == ^snmp_device.id)
assert length(sensors) == 1
[remaining_sensor] = sensors
# The preserved sensor should be sensor1 (index 1001)
assert remaining_sensor.id == sensor1.id
assert remaining_sensor.sensor_index == "1001"
end
end
describe "discover_all/1" do