Problem: upsert_mac_address was using Repo.one() which crashes with
Ecto.MultipleResultsError when duplicate MAC address records exist
in the database.
Root Cause: Race condition or historical data corruption created
duplicate entries for the same (device_id, mac_address, vlan_id)
tuple, violating the unique constraint.
Fix:
- Changed Repo.one() to Repo.all() to handle duplicates gracefully
- When duplicates found: keep first entry, delete extras, log warning
- Includes device_id, mac_address, vlan_id, and duplicate IDs in log
- Auto-cleans corrupted data while maintaining service availability
Example Log Output:
Found 1 duplicate MAC address entries, cleaning up
device_id=f3ff19d5-b519-4327-9f7d-dc416e494cfa
mac_address=78:9a:18:52:b1:c6
vlan_id=nil
duplicate_ids=[...]
Impact:
- Prevents polling crashes when duplicate MAC entries exist
- Self-healing: automatically cleans up database corruption
- Maintains data integrity while preserving service availability
Related: Issue 2.9 from bugs analysis (ARP/MAC parsing issues)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit addresses Phase 1 (CRITICAL - Data Loss Prevention) issues
from the comprehensive bugs and inconsistencies analysis.
CRITICAL FIXES:
1. Protobuf Schema Inconsistencies
- Added missing job_id field (field 5) to SnmpResult message
- Fixed AgentError message structure (job_id, message, timestamp)
- Removed obsolete transport field from SnmpDevice struct
- Regenerated protobuf code with protoc-gen-elixir
- Prevents crashes when agent sends SNMP results or errors
2. Interface/Sensor Discovery Timeout Data Loss
- Changed timeout behavior to return error instead of empty list
- Prevents deletion of ALL existing interfaces/sensors on slow networks
- Discovery fails gracefully instead of destroying historical data
- Addresses commit 7a57f7c sensor discovery pipeline data loss issue
3. Polling Offset Schedule Mismatch
- Fixed schedule_next_poll() to use offset only (not interval + offset)
- Maintains consistent polling intervals across all executions
- Prevents load bunching that offset was designed to prevent
- Example: 300s interval with offset=94s now consistently polls every 5m
4. Always Reschedule Race Condition
- Added should_continue_polling?() and should_continue_monitoring?()
- Only reschedule if device is still enabled and should be polled by Phoenix
- Prevents zombie jobs from continuing to poll disabled/deleted/reassigned devices
- Stops race condition where stop_polling() is bypassed by in-flight jobs
Files Changed:
- priv/proto/agent.proto
- lib/towerops/proto/agent.pb.ex (regenerated)
- lib/towerops_web/channels/agent_channel.ex
- lib/towerops/snmp/discovery.ex
- lib/towerops/workers/device_poller_worker.ex
- lib/towerops/workers/device_monitor_worker.ex
- test/towerops/workers/device_poller_worker_test.exs
Test Results: All passing
- device_poller_worker_test.exs: 8 tests, 0 failures
- device_monitor_worker_test.exs: 3 tests, 0 failures
- discovery_test.exs: 13 tests, 0 failures
Remaining Phases:
- Phase 2 (HIGH): 8 data integrity issues
- Phase 3 (MEDIUM): 10 reliability issues
- Phase 4 (LOW): 3 cleanup issues
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add logger filter to drop repetitive 'Cannot find module' errors
from net-snmp library. These errors occur when snmptranslate can't
find vendor-specific MIB modules but don't affect functionality
since the system falls back to numeric OIDs.
The errors were flooding production logs (hundreds of identical
messages) during polling of Morningstar devices that reference
TRISTAR MIB modules.
Changes:
- Add drop_snmp_mib_errors/2 filter to LoggerFilters module
- Register filter in production logger configuration
- Filter matches exact error format from snmptranslate stderr
The root cause (missing NIF binary) will be addressed separately
by compiling the towerops_nif.so during Docker build.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
When a device is deleted, now broadcasts an assignment change event
to the agent (if assigned), causing the agent to receive an updated
job list without the deleted device. This ensures agents stop
processing any in-flight jobs for deleted devices.
Changes:
- Get agent assignment before deleting device
- Broadcast :device_deleted event to agent after deletion
- Agent channel receives broadcast and sends updated job list
- Agents naturally stop processing jobs for devices not in new list
Previously only cancelled Oban jobs but didn't notify agents,
leaving potential for agents to continue processing jobs for
deleted devices.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Previously redirected to /dashboard or /orgs based on whether the
superuser had organizations. Now always goes to /devices for
consistency.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The protobuf Elixir module was missing the PING (value 4) enum
entry, causing encoding errors when building jobs for agents.
Manually added the field since protoc-gen-elixir has asdf version
issues. The generated module now matches the updated agent.proto
definition.
Fixes: ** (Protobuf.EncodeError) no function clause matching in
Towerops.Agent.JobType
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Enables Phoenix to send PING jobs to agents and receive monitoring
check results, fixing the "unknown" status for devices assigned to
local (non-cloud) agents.
Changes:
- Rename JobType::MONITOR to JobType::PING in protobuf for clarity
- Add validate_monitoring_check_message() to Validator module
- Add "monitoring_check" channel handler to receive ping results
- Implement store_monitoring_check() to save results to database
- Add build_ping_job() to generate PING jobs for agents
- Update build_jobs_for_device() to include ping jobs when
monitoring_enabled=true
Phoenix now sends PING jobs alongside SNMP/MikroTik jobs, and agents
send back MonitoringCheck results that are stored in the
monitoring_checks table with proper agent_token_id tracking.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
When an organization has sites disabled (use_sites=false), the API
now automatically removes any site_id from device creation requests,
ensuring devices are assigned directly to the organization instead.
This prevents Terraform (and other API clients) from creating devices
in sites when sites functionality is disabled for the organization.
Fixes issue where Terraform could bypass site restrictions via API.
Show the specific cloud poller name (e.g., dfw, lon) alongside "Cloud"
when a device is assigned to a cloud poller. Displays as "Cloud (dfw)"
instead of just "Cloud" for better visibility of which poller is being used.
When no agent is assigned, continues to show just "Cloud".
Display latency values with single decimal precision (e.g., 114.1ms)
for improved readability in graphs. This ensures consistency between
the raw chart data and tooltip formatting.
The previous commit (90e8964) introduced an incorrect LoggerBackends.add/2
call that prevented Oban from starting, causing all discovery jobs to remain
stuck in "available" state without being processed.
Root cause: LoggerBackends.add expects a tuple {module, id} as the first
argument, not the module alone with configuration options.
Fix: Use the correct API pattern:
1. Add backend with tuple identifier: {LoggerFileBackend, :file_log}
2. Configure separately with Logger.configure_backend/2
This resolves device discovery being completely broken - all background
jobs (discovery, polling, monitoring) now process correctly.
Critical fixes:
- Add [:safe] option to binary_to_term to prevent RCE attacks
- Implement whitelist validation for String.to_atom conversions
- Add input validation before String.to_existing_atom usage
Changes:
- MIB compiler and cache: Use safe binary deserialization
- SNMP contexts: Whitelist protocol, device type, and source atoms
- API controllers: Validate error message keys before atom conversion
- Reduce function nesting to comply with Credo standards
All 6,145 tests passing with zero Credo issues.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace Docker-based builds with Nix flakes for reproducible builds.
Changes:
- Use nixos/nix:latest image with `nix` tag requirement
- Build Docker image with `nix build .#dockerImage`
- Add optional Cachix integration for binary caching
- Load image with `docker load < result`
- Keep existing deploy stage unchanged
Benefits:
- Reproducible builds across all environments
- Binary caching via Cachix (faster subsequent builds)
- All dependencies pinned in flake.lock
- Image size similar to Docker builds (~507 MB compressed)
Requirements:
- GitLab Runner with Nix installed and `nix` tag
- Docker socket mounted for loading/pushing images
- Optional: CACHIX_AUTH_TOKEN for binary caching
See docs/gitlab-runner-nix-setup.md for runner setup instructions.
The old Docker-based config is available in git history if rollback needed.
Nix build fixes:
- Override beamPackages.elixir to use 1.19.5 instead of default 1.18.4
- Add explicit buildPhase to compile dependencies before application
- Remove postInstall phase (Mix automatically includes priv/ in release)
- Set HEX_OFFLINE=false to allow dependency fetching in Nix sandbox
Docker image build now succeeds (~507 MB compressed).
Also remove .claude/settings.local.json from git tracking (keep local).
File remains in working directory but is now properly ignored.
Add git and cacert to nativeBuildInputs and set SSL certificate
environment variables. This fixes the SSL certificate verification
error when fetching heroicons from GitHub.
Override configurePhase to explicitly run 'mix deps.get' before
compilation. This ensures hex dependencies (like ecto) are available
when vendored path dependencies (oban_met) are compiled.
The postUnpack step was deleting deps directories which may have
been removing vendored dependencies. cleanSourceWith already handles
source filtering, so postUnpack is redundant and potentially harmful.
Remove buggy conditional that was always true. Now the filter
correctly excludes only build artifacts while including all
source files including vendor directory.
Replace sourceFilesBySuffices with cleanSourceWith to ensure
vendor directory and all dependencies are properly included.
This fixes the oban_met compilation error.
Find the erts include path dynamically instead of relying on
erlang.version which may not match the actual erts version.
This fixes 'erl_nif.h' file not found error.
Instead of hardcoding the version in the path, find the release
directory dynamically. This fixes the 'undefined variable version'
error during Docker image build.
Ignore local Nix development directories and files:
- .nix-mix/ - Mix cache
- .nix-hex/ - Hex cache
- .nix-postgres/ - PostgreSQL data directory
- .nix-redis/ - Redis data directory
- .nix-services-started - Service flag file
These are created by the Nix dev shell and should not be
committed to the repository.
- Check if PostgreSQL/Redis already running before starting
- Install Hex and Rebar3 non-interactively on shell entry
- Only run migrations if deps directory exists
- Better error messages with log file locations
This fixes issues when:
- Port 5432 is already in use (existing PostgreSQL)
- Port 6379 is already in use (existing Redis)
- Mix prompts for Hex installation interactively
Add Nix overlay to pin specific versions matching .tool-versions:
- Erlang: OTP 28.3 (via erlang_28)
- Elixir: 1.19.5-otp-28 (via beam.packages.erlang_28.elixir_1_19)
This ensures the Nix build environment matches the asdf-based
development setup exactly, providing consistency across all
development and build environments.
Use stdenv.isLinux instead of lib.isLinux to properly check if
inotify-tools should be included. This fixes the undefined variable
error when evaluating the flake on macOS.
- Make inotify-tools Linux-only (not available on Darwin)
- Replace nixfmt-rfc-style with nixfmt (deprecated warning)
- Add flake.lock for reproducible builds
Changes:
- nix/shell.nix: Conditionally include inotify-tools only on Linux
- flake.nix: Use pkgs.nixfmt instead of pkgs.nixfmt-rfc-style
- flake.lock: Lock dependencies for reproducibility
