mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-22 03:04:03 +00:00
fix: storage alerts and UI improvements
- Added CheckStorage calls in parallel storage polling (was missing, causing storage alerts to not trigger) - Fixed node cleanup logic to use alert.Node field directly instead of parsing IDs - Removed auto-acknowledge on alert click - now only acknowledge button toggles state - Added unacknowledge button for acknowledged alerts - Fixed double-toggle issue with acknowledge button using race condition prevention - Fixed tab menu width in Alerts and Settings pages (changed flex-shrink-0 to flex-1) addresses #228 (storage alert threshold issue)
This commit is contained in:
Pulse Monitor
@@ -766,7 +766,7 @@ const Settings: Component = () => {
|
||||
<For each={tabs}>
|
||||
{(tab) => (
|
||||
<button type="button"
|
||||
class={`flex-shrink-0 px-2 sm:px-3 py-1.5 sm:py-2 text-xs sm:text-sm font-medium rounded-md transition-all whitespace-nowrap ${
|
||||
class={`flex-1 px-2 sm:px-3 py-1.5 sm:py-2 text-xs sm:text-sm font-medium rounded-md transition-all whitespace-nowrap ${
|
||||
activeTab() === tab.id
|
||||
? 'bg-white dark:bg-gray-800 text-gray-900 dark:text-gray-100 shadow-sm'
|
||||
: 'text-gray-600 dark:text-gray-400 hover:text-gray-900 dark:hover:text-gray-100'
|
||||
|
||||
@@ -620,7 +620,7 @@ export function Alerts() {
|
||||
{/* Tab Navigation - modern style */}
|
||||
<div class="bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 rounded-lg shadow-sm">
|
||||
<div class="p-1">
|
||||
<div class="inline-flex rounded-lg bg-gray-100 dark:bg-gray-700 p-0.5 w-full overflow-x-auto">
|
||||
<div class="flex rounded-lg bg-gray-100 dark:bg-gray-700 p-0.5 w-full overflow-x-auto">
|
||||
<For each={tabs}>
|
||||
{(tab) => (
|
||||
<button type="button"
|
||||
@@ -860,44 +860,15 @@ function OverviewTab(props: {
|
||||
<For each={filteredAlerts()}>
|
||||
{(alert) => (
|
||||
<div
|
||||
onClick={async () => {
|
||||
// Clicking always toggles acknowledge state
|
||||
if (processingAlerts().has(alert.id)) return; // Prevent double-clicks
|
||||
|
||||
setProcessingAlerts(prev => new Set(prev).add(alert.id));
|
||||
try {
|
||||
if (alert.acknowledged) {
|
||||
// Un-acknowledge
|
||||
await AlertsAPI.unacknowledge(alert.id);
|
||||
props.updateAlert(alert.id, { acknowledged: false });
|
||||
showSuccess('Alert restored');
|
||||
} else {
|
||||
// Acknowledge
|
||||
await AlertsAPI.acknowledge(alert.id);
|
||||
props.updateAlert(alert.id, { acknowledged: true });
|
||||
showSuccess('Alert acknowledged');
|
||||
}
|
||||
} catch (err) {
|
||||
console.error('Failed to toggle alert state:', err);
|
||||
showError('Failed to update alert');
|
||||
} finally {
|
||||
setProcessingAlerts(prev => {
|
||||
const next = new Set(prev);
|
||||
next.delete(alert.id);
|
||||
return next;
|
||||
});
|
||||
}
|
||||
}}
|
||||
class={`border rounded-lg p-4 transition-all cursor-pointer hover:shadow-md ${
|
||||
processingAlerts().has(alert.id) ? 'opacity-50 cursor-wait' : ''
|
||||
class={`border rounded-lg p-4 transition-all ${
|
||||
processingAlerts().has(alert.id) ? 'opacity-50' : ''
|
||||
} ${
|
||||
alert.acknowledged
|
||||
? 'opacity-60 border-gray-300 dark:border-gray-700 bg-gray-50 dark:bg-gray-900/20 hover:opacity-80'
|
||||
? 'opacity-60 border-gray-300 dark:border-gray-700 bg-gray-50 dark:bg-gray-900/20'
|
||||
: alert.level === 'critical'
|
||||
? 'border-red-300 dark:border-red-800 bg-red-50 dark:bg-red-900/20'
|
||||
: 'border-yellow-300 dark:border-yellow-800 bg-yellow-50 dark:bg-yellow-900/20'
|
||||
}`}
|
||||
title={alert.acknowledged ? 'Click to restore this alert' : 'Click to acknowledge'}>
|
||||
}`}>
|
||||
<div class="flex flex-col sm:flex-row sm:items-start">
|
||||
<div class="flex items-start flex-1">
|
||||
{/* Status icon */}
|
||||
@@ -945,33 +916,56 @@ function OverviewTab(props: {
|
||||
</div>
|
||||
</div>
|
||||
<div class="flex gap-2 mt-3 sm:mt-0 sm:ml-4 self-end sm:self-start">
|
||||
<Show when={!alert.acknowledged}>
|
||||
<button
|
||||
class="px-3 py-1.5 text-xs font-medium bg-white dark:bg-gray-700 text-yellow-700 dark:text-yellow-300 border border-yellow-300 dark:border-yellow-700 rounded-lg hover:bg-yellow-50 dark:hover:bg-yellow-900/20 transition-all disabled:opacity-50 disabled:cursor-not-allowed"
|
||||
disabled={processingAlerts().has(alert.id)}
|
||||
onClick={async (e) => {
|
||||
e.stopPropagation();
|
||||
setProcessingAlerts(prev => new Set(prev).add(alert.id));
|
||||
try {
|
||||
<button
|
||||
class={`px-3 py-1.5 text-xs font-medium border rounded-lg transition-all disabled:opacity-50 disabled:cursor-not-allowed ${
|
||||
alert.acknowledged
|
||||
? 'bg-white dark:bg-gray-700 text-gray-700 dark:text-gray-300 border-gray-300 dark:border-gray-600 hover:bg-gray-50 dark:hover:bg-gray-600'
|
||||
: 'bg-white dark:bg-gray-700 text-yellow-700 dark:text-yellow-300 border-yellow-300 dark:border-yellow-700 hover:bg-yellow-50 dark:hover:bg-yellow-900/20'
|
||||
}`}
|
||||
disabled={processingAlerts().has(alert.id)}
|
||||
onClick={async (e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
|
||||
// Prevent double-clicks
|
||||
if (processingAlerts().has(alert.id)) return;
|
||||
|
||||
setProcessingAlerts(prev => new Set(prev).add(alert.id));
|
||||
|
||||
// Store current state to avoid race conditions
|
||||
const wasAcknowledged = alert.acknowledged;
|
||||
|
||||
try {
|
||||
if (wasAcknowledged) {
|
||||
await AlertsAPI.unacknowledge(alert.id);
|
||||
props.updateAlert(alert.id, { acknowledged: false });
|
||||
showSuccess('Alert restored');
|
||||
} else {
|
||||
await AlertsAPI.acknowledge(alert.id);
|
||||
// Update the local state immediately
|
||||
props.updateAlert(alert.id, { acknowledged: true });
|
||||
showSuccess('Alert acknowledged');
|
||||
} catch (err) {
|
||||
console.error('Failed to acknowledge alert:', err);
|
||||
showError('Failed to acknowledge alert');
|
||||
} finally {
|
||||
}
|
||||
} catch (err) {
|
||||
console.error(`Failed to ${wasAcknowledged ? 'unacknowledge' : 'acknowledge'} alert:`, err);
|
||||
showError(`Failed to ${wasAcknowledged ? 'restore' : 'acknowledge'} alert`);
|
||||
} finally {
|
||||
// Delay removing from processing to prevent race conditions
|
||||
setTimeout(() => {
|
||||
setProcessingAlerts(prev => {
|
||||
const next = new Set(prev);
|
||||
next.delete(alert.id);
|
||||
return next;
|
||||
});
|
||||
}
|
||||
}}
|
||||
>
|
||||
{processingAlerts().has(alert.id) ? 'Processing...' : 'Acknowledge'}
|
||||
</button>
|
||||
</Show>
|
||||
}, 100);
|
||||
}
|
||||
}}
|
||||
>
|
||||
{processingAlerts().has(alert.id)
|
||||
? 'Processing...'
|
||||
: alert.acknowledged
|
||||
? 'Unacknowledge'
|
||||
: 'Acknowledge'}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -48,6 +48,9 @@ export function createWebSocketStore(url: string) {
|
||||
const [activeAlerts, setActiveAlerts] = createStore<Record<string, Alert>>({});
|
||||
const [recentlyResolved, setRecentlyResolved] = createStore<Record<string, ResolvedAlert>>({});
|
||||
const [updateProgress, setUpdateProgress] = createSignal<any>(null);
|
||||
|
||||
// Track alerts with pending acknowledgment changes to prevent race conditions
|
||||
const pendingAckChanges = new Set<string>();
|
||||
|
||||
let ws: WebSocket | null = null;
|
||||
let reconnectTimeout: number;
|
||||
@@ -179,8 +182,13 @@ export function createWebSocketStore(url: string) {
|
||||
}
|
||||
});
|
||||
|
||||
// Add new alerts
|
||||
// Add new alerts (skip those with pending ack changes)
|
||||
Object.entries(newAlerts).forEach(([id, alert]) => {
|
||||
// Skip updating if this alert has a pending acknowledgment change
|
||||
if (pendingAckChanges.has(id)) {
|
||||
logger.debug(`Skipping update for alert ${id} - has pending ack change`);
|
||||
return;
|
||||
}
|
||||
setActiveAlerts(id, alert);
|
||||
});
|
||||
|
||||
@@ -381,6 +389,14 @@ export function createWebSocketStore(url: string) {
|
||||
updateAlert: (alertId: string, updates: Partial<Alert>) => {
|
||||
const existingAlert = activeAlerts[alertId];
|
||||
if (existingAlert) {
|
||||
// Track this alert as having pending changes if acknowledgment is changing
|
||||
if ('acknowledged' in updates) {
|
||||
pendingAckChanges.add(alertId);
|
||||
// Clear the pending flag after a delay to allow server sync
|
||||
setTimeout(() => {
|
||||
pendingAckChanges.delete(alertId);
|
||||
}, 2000); // 2 seconds should be enough for server to sync
|
||||
}
|
||||
setActiveAlerts(alertId, { ...existingAlert, ...updates });
|
||||
}
|
||||
}
|
||||
|
||||
+14
-25
@@ -684,6 +684,16 @@ func (m *Manager) CheckStorage(storage models.Storage) {
|
||||
// Check usage if storage has valid data (even if not currently active on this node)
|
||||
// In clusters, storage may show as inactive on nodes where it's not currently mounted
|
||||
// but we still want to alert on high usage
|
||||
log.Info().
|
||||
Str("storage", storage.Name).
|
||||
Str("id", storage.ID).
|
||||
Float64("usage", storage.Usage).
|
||||
Str("status", storage.Status).
|
||||
Float64("trigger", threshold.Trigger).
|
||||
Float64("clear", threshold.Clear).
|
||||
Bool("hasOverride", hasOverride).
|
||||
Msg("Checking storage thresholds")
|
||||
|
||||
if storage.Status != "offline" && storage.Status != "unavailable" && storage.Usage > 0 {
|
||||
m.checkMetric(storage.ID, storage.Name, storage.Node, storage.Instance, "Storage", "usage", storage.Usage, &threshold)
|
||||
}
|
||||
@@ -1975,32 +1985,11 @@ func (m *Manager) CleanupAlertsForNodes(existingNodes map[string]bool) {
|
||||
Msg("Starting alert cleanup for non-existent nodes")
|
||||
|
||||
removedCount := 0
|
||||
for alertID := range m.activeAlerts {
|
||||
var node string
|
||||
for alertID, alert := range m.activeAlerts {
|
||||
// Use the Node field from the alert itself, which is more reliable
|
||||
node := alert.Node
|
||||
|
||||
// Extract node from alert ID
|
||||
// Format can be either "node:type/id-metric" or "node-storage-name-usage"
|
||||
if strings.Contains(alertID, ":") {
|
||||
// Guest alert format: "node:type/id-metric"
|
||||
parts := strings.Split(alertID, ":")
|
||||
if len(parts) >= 2 {
|
||||
node = parts[0]
|
||||
}
|
||||
} else if strings.Contains(alertID, "-storage-") {
|
||||
// Storage alert format: "node-storage-name-usage"
|
||||
parts := strings.Split(alertID, "-storage-")
|
||||
if len(parts) >= 1 {
|
||||
node = parts[0]
|
||||
}
|
||||
} else if strings.HasPrefix(alertID, "node-offline-") {
|
||||
// Node offline alert format: "node-offline-node/nodename"
|
||||
// Extract the node name after the last slash
|
||||
if idx := strings.LastIndex(alertID, "/"); idx != -1 {
|
||||
node = alertID[idx+1:]
|
||||
}
|
||||
}
|
||||
|
||||
// If we couldn't extract a node or the node doesn't exist, remove the alert
|
||||
// If we couldn't get a node or the node doesn't exist, remove the alert
|
||||
if node == "" || !existingNodes[node] {
|
||||
delete(m.activeAlerts, alertID)
|
||||
removedCount++
|
||||
|
||||
@@ -2011,6 +2011,12 @@ func (m *Monitor) pollStorageWithNodes(ctx context.Context, instanceName string,
|
||||
m.metricsHistory.AddStorageMetric(modelStorage.ID, "avail", float64(modelStorage.Free), now)
|
||||
|
||||
// Check thresholds for alerts
|
||||
log.Info().
|
||||
Str("storage", modelStorage.Name).
|
||||
Str("id", modelStorage.ID).
|
||||
Float64("usage", modelStorage.Usage).
|
||||
Str("status", modelStorage.Status).
|
||||
Msg("Calling CheckStorage for storage device")
|
||||
m.alertManager.CheckStorage(modelStorage)
|
||||
}
|
||||
}
|
||||
@@ -2982,7 +2988,12 @@ func (m *Monitor) checkMockAlerts() {
|
||||
}
|
||||
|
||||
// Check alerts for storage
|
||||
log.Info().Int("storageCount", len(state.Storage)).Msg("Checking storage alerts")
|
||||
for _, storage := range state.Storage {
|
||||
log.Debug().
|
||||
Str("name", storage.Name).
|
||||
Float64("usage", storage.Usage).
|
||||
Msg("Checking storage for alerts")
|
||||
m.alertManager.CheckStorage(storage)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -582,6 +582,11 @@ func (m *Monitor) pollStorageWithNodesOptimized(ctx context.Context, instanceNam
|
||||
}
|
||||
}
|
||||
|
||||
// Check alerts for all storage devices
|
||||
for _, storage := range allStorage {
|
||||
m.alertManager.CheckStorage(storage)
|
||||
}
|
||||
|
||||
// Update state with all storage
|
||||
m.state.UpdateStorageForInstance(instanceName, allStorage)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user