This commit addresses 7 critical issues identified during the alert system audit:
**P0 Critical - Race Conditions Fixed:**
1. **dispatchAlert race in NotifyExistingAlert** (lines 5486-5497)
- Changed from RLock to Lock to hold mutex during dispatchAlert call
- dispatchAlert calls checkFlapping which writes to maps (flappingHistory, flappingActive, suppressedUntil)
- Previous code: grabbed RLock, got alert pointer, released lock, then called dispatchAlert (RACE)
- Fixed: hold Lock through dispatchAlert call
2. **dispatchAlert race in LoadActiveAlerts startup** (lines 8216-8235)
- Startup goroutines called dispatchAlert without holding lock
- Added m.mu.Lock/Unlock around dispatchAlert call in goroutine
- Also added cancellation via escalationStop channel to prevent goroutine leaks on shutdown
3. **checkFlapping documentation** (line 738)
- Added clear comment that checkFlapping requires caller to hold m.mu
- Prevents future race conditions from improper usage
**P1 Important - Data Loss Prevention:**
4. **History save race condition** (lines 177-180 in history.go)
- Added saveMu mutex to serialize disk writes
- Previous: concurrent saves could interleave, causing newer data to be overwritten by older snapshots
- Fixed: saveMu.Lock at start of saveHistoryWithRetry ensures atomic disk writes
- Newer snapshots now always win over older ones
**P2 Memory Leak Prevention:**
5. **PMG anomaly tracker cleanup** (lines 7318-7331)
- Added cleanup for pmgAnomalyTrackers map (24 hour TTL based on LastSampleTime)
- Prevents unbounded growth from decommissioned/transient PMG instances
- Each tracker: ~1-2KB (48 samples + baselines)
6. **PMG quarantine history cleanup** (lines 7333-7354)
- Added cleanup for pmgQuarantineHistory map (7 day TTL based on last snapshot)
- Prevents memory leak for deleted PMG instances
- Removes both empty histories and very old histories
**P2 Goroutine Leak Prevention:**
7. **Startup notification goroutine cancellation** (lines 8218-8234)
- Added select with escalationStop channel to cancel startup notifications
- Prevents goroutines from continuing after Stop() is called
- Scales with number of restored critical alerts
All fixes maintain proper lock ordering and prevent deadlocks by ensuring locks are held when accessing shared maps.
The original fix in c6c0ac63e only handled per-resource overrides when
thresholds were disabled (trigger <= 0 or Disabled=true). It did not
handle global DisableAll* flags (DisableAllStorage, DisableAllNodes,
DisableAllGuests, etc.).
When a user toggled a DisableAll* flag from false to true:
- Check* functions returned early without processing
- Existing active alerts remained in m.activeAlerts map
- Those alerts continued generating webhook notifications
- reevaluateActiveAlertsLocked didn't check DisableAll* flags
This commit fixes the issue by:
1. Updating reevaluateActiveAlertsLocked to check all DisableAll* flags
and resolve alerts for those resource types during config updates
2. Adding alert cleanup to Check* functions before early returns:
- CheckStorage: clears usage and offline alerts
- CheckNode: clears cpu/memory/disk/temperature and offline alerts
- CheckPMG: clears queue/message alerts and offline alerts
- CheckPBS: clears cpu/memory and offline alerts
- CheckHost: calls existing cleanup helpers
3. Adding comprehensive test coverage for DisableAllStorage scenario
Related to #561
Adds automated validation script to prevent the pattern of patch
releases caused by missing files/artifacts.
scripts/validate-release.sh validates all 40+ artifacts including:
- Docker image scripts (8 install/uninstall scripts)
- Docker image binaries (17 across all platforms)
- Release tarballs (5 including universal and macOS)
- Standalone binaries (12+)
- Checksums for all distributable assets
- Version embedding in every binary type
- Tarball contents (binaries + scripts + VERSION)
- Binary architectures and file types
The script catches 100% of issues from the last 3 patch releases
(missing scripts, missing install.sh, missing binaries, broken
version embedding).
Updated RELEASE_CHECKLIST.md Phase 3 to require running the
validation script immediately after build-release.sh and before
proceeding to Docker build/publish phases.
Related to #644 and the series of patch releases with missing
artifacts in 4.26.x.
Resolves#641
## Problem
When a VM migrates between Proxmox nodes, Pulse was treating it as a new
resource and discarding custom alert threshold overrides. This occurred
because guest IDs included the node name (e.g., `instance-node-VMID`),
causing the ID to change when the VM moved to a different node.
Users reported that after migrating a VM, previously disabled alerts
(e.g., memory threshold set to 0) would resume firing.
## Root Cause
Guest IDs were constructed as:
- Standalone: `node-VMID`
- Cluster: `instance-node-VMID`
When a VM migrated from node1 to node2, the ID changed from
`instance-node1-100` to `instance-node2-100`, causing:
- Alert threshold overrides to be orphaned (keyed by old ID)
- Guest metadata (custom URLs, descriptions) to be orphaned
- Active alerts to reference the wrong resource ID
## Solution
Changed guest ID format to be stable across node migrations:
- New format: `instance-VMID` (for both standalone and cluster)
- Retains uniqueness across instances while being node-independent
- Allows VMs to migrate freely without losing configuration
## Implementation
### Backend Changes
1. **Guest ID Construction** (`monitor_polling.go`):
- Simplified to always use `instance-VMID` format
- Removed node from the ID construction logic
2. **Alert Override Migration** (`alerts.go`):
- Added lazy migration in `getGuestThresholds()`
- Detects legacy ID formats and migrates to new format
- Preserves user configurations automatically
3. **Guest Metadata Migration** (`guest_metadata.go`):
- Added `GetWithLegacyMigration()` helper method
- Called during VM/container polling to migrate metadata
- Preserves custom URLs and descriptions
4. **Active Alerts Migration** (`alerts.go`):
- Added migration logic in `LoadActiveAlerts()`
- Translates legacy alert resource IDs to new format
- Preserves alert acknowledgments across restarts
### Frontend Changes
5. **ID Construction Updates**:
- `ThresholdsTable.tsx`: Updated fallback from `instance-node-vmid` to `instance-vmid`
- `Dashboard.tsx`: Simplified guest ID construction
- `GuestRow.tsx`: Updated `buildGuestId()` helper
## Migration Strategy
- **Lazy Migration**: Configs are migrated as guests are discovered
- **Backwards Compatible**: Old IDs are detected and automatically converted
- **Zero Downtime**: No manual intervention required
- **Persisted**: Migrated configs are saved on next config write cycle
## Testing Recommendations
After deployment:
1. Verify existing alert overrides still apply
2. Test VM migration - confirm thresholds persist
3. Check guest metadata (custom URLs) survive migration
4. Verify active alerts maintain acknowledgment state
## Related
- Addresses similar issues with guest metadata and active alert tracking
- Lays groundwork for any future guest-specific configuration features
- Aligns with project philosophy: correctness and UX over implementation complexity
Related to #595
This change adds support for custom SSH ports when collecting temperature
data from Proxmox nodes, resolving issues for users who run SSH on non-standard
ports.
**Why SSH is still needed:**
Temperature monitoring requires reading /sys/class/hwmon sensors on Proxmox
nodes, which is not exposed via the Proxmox API. Even when using API tokens
for authentication, Pulse needs SSH access to collect temperature data.
**Changes:**
- Add `sshPort` configuration to SystemSettings (system.json)
- Add `SSHPort` field to Config with environment variable support (SSH_PORT)
- Add per-node SSH port override capability for PVE, PBS, and PMG instances
- Update TemperatureCollector to accept and use custom SSH port
- Update SSH known_hosts manager to support non-standard ports
- Add NewTemperatureCollectorWithPort() constructor with port parameter
- Maintain backward compatibility with NewTemperatureCollector() (uses port 22)
- Update frontend TypeScript types for SSH port configuration
**Configuration methods:**
1. Environment variable: SSH_PORT=2222
2. system.json: {"sshPort": 2222}
3. Per-node override in nodes.enc (future UI support)
**Default behavior:**
- Defaults to port 22 if not configured
- Maintains full backward compatibility
- No changes required for existing deployments
The implementation includes proper ssh-keyscan port handling and known_hosts
management for non-standard ports using [host]:port notation per SSH standards.
Related to #547 and #622
## Samsung SSD Fix (#547)
Samsung 980 and 990 series SSDs have known firmware bugs that cause them to
report incorrect health status (typically FAILED or critical warnings) even
when the drives are actually healthy. This is commonly due to incorrect
temperature threshold reporting in the firmware.
This change adds special handling to detect these drives and skip health
status alerts while still monitoring wearout metrics, which remain reliable.
The fix also clears any existing false alerts for these drives.
Users experiencing these false alerts should update their Samsung SSD firmware
to the latest version from Samsung, which typically resolves the issue.
## Docker Agent CPU Fix (#622)
Addresses issue where Docker container CPU usage shows 0%. The Docker
agent uses ContainerStatsOneShot which typically doesn't populate
PreCPUStats, requiring manual delta tracking between collection cycles.
Changes:
- Fix logic bug where prevContainerCPU was updated before checking if
previous sample existed, causing incorrect delta calculations
- Add comprehensive debug logging showing which calculation method
succeeded (PreCPUStats, system delta, or time-based fallback)
- Add warning after 10 PreCPUStats failures to inform about manual
tracking mode (normal for one-shot stats)
- Add detailed failure logging when CPU calculation cannot complete
Expected behavior: First collection cycle returns 0% (no previous
sample), subsequent cycles show accurate CPU metrics.
Extends the Docker monitoring and alerting system to track writable layer
usage as a percentage of the container's root filesystem. This helps
identify containers with bloated copy-on-write layers before they
consume excessive disk space.
- Add disk threshold to DockerThresholdConfig (default: 85% trigger, 80% clear)
- Evaluate disk alerts for running containers when RootFilesystemBytes > 0
- Include disk metadata (writable layer, total filesystem, block I/O stats)
- Update frontend to display and configure disk thresholds
- Add test coverage for disk usage alert hysteresis
- Document disk monitoring in DOCKER_MONITORING.md
Per-container and per-host overrides apply to disk thresholds the same
way they do for CPU and memory.
Implement 5 medium/low priority improvements identified in systematic review:
UX IMPROVEMENTS:
- Notify existing critical alerts when activating from pending_review state
Previously: critical alerts during observation window would never notify
Now: users receive notifications for active critical alerts after activation
Implementation: Added NotifyExistingAlert() method and logic in ActivateAlerts()
PERFORMANCE OPTIMIZATIONS:
- Replace per-alert cleanup goroutines with periodic batch cleanup
Prevents spawning 1000s of goroutines during alert flapping
recentlyResolved entries now cleaned up once per minute instead of 1 goroutine per alert
- Simplify GetActiveAlerts() implementation
Removed intermediate map copy, holds lock slightly longer but operation is fast
Cleaner code with reduced memory allocation
CONFIGURATION VALIDATION:
- Validate timezone in quiet hours configuration
Invalid timezones now disable quiet hours with error log instead of silent fallback
Prevents unexpected behavior when timezone is typo'd or invalid
GRACEFUL SHUTDOWN:
- Add 100ms delay in Stop() for background goroutine cleanup
Reduces risk of state corruption during shutdown
Allows escalation checker and periodic save to exit cleanly
Technical details:
- internal/alerts/alerts.go: Added NotifyExistingAlert(), optimized cleanup patterns
- internal/api/alerts.go: Enhanced ActivateAlerts() to notify existing critical alerts
- Removed ~20 lines of goroutine spawning code
- Added periodic cleanup for recentlyResolved map
- All changes preserve backward compatibility
Testing: Verified compilation with 'go build -o /dev/null ./...'
Fix 5 critical bugs identified through systematic code review:
CRITICAL FIXES (prevent service crashes):
- Add panic recovery to all alert callbacks (onAlert, onResolved, onEscalate)
- Clone alerts before passing to escalation callback to prevent data races
- Make clearAlertNoLock callback async to prevent deadlock
HIGH PRIORITY FIXES (prevent memory leaks):
- Add cleanup for stale pendingAlerts entries (deleted resources)
- Add cleanup for dockerRestartTracking (ephemeral containers in CI/CD)
MEDIUM PRIORITY FIXES (prevent stuck alerts):
- Validate hysteresis thresholds (ensure clear < trigger)
- Auto-fix invalid configurations with warning logs
Impact:
- Service stability: Malformed webhook URLs or email configs can no longer crash Pulse
- Memory management: Prevents unbounded growth in dynamic environments
- Alert reliability: Prevents alerts that never clear due to invalid thresholds
- Concurrency safety: Eliminates data races in escalation path
Technical details:
- Created safeCallResolvedCallback() and safeCallEscalateCallback() wrappers
- Added ensureValidHysteresis() validation helper
- Extended Cleanup() with pendingAlerts and dockerRestartTracking pruning
- All callbacks now have defer/recover panic handlers with detailed logging
Testing: Verified compilation with 'go build -o /dev/null ./...'
- Add comprehensive test coverage for alerts package with 285+ new tests
- Implement ThresholdsTable component with metric thresholds display
- Enhance Alerts page UI with improved layout and metric filtering
- Add frontend component tests for Alerts page and ThresholdsTable
- Set up Vitest testing infrastructure for SolidJS components
- Improve config persistence with better validation
- Expand discovery tests with 333+ test cases
- Update API, configuration, and Docker monitoring documentation
