Fix VM migration issue where custom alert thresholds are lost
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
This commit is contained in:
rcourtman
parent
dfe960deb4
commit
20854256c3
7 changed files with 219 additions and 25 deletions
|
|
@ -1136,7 +1136,7 @@ const [editingThresholds, setEditingThresholds] = createSignal<
|
|||
const overridesMap = new Map((props.overrides() ?? []).map((o) => [o.id, o]));
|
||||
|
||||
const guests = (props.allGuests() ?? []).map((guest) => {
|
||||
const guestId = guest.id || `${guest.instance}-${guest.node}-${guest.vmid}`;
|
||||
const guestId = guest.id || `${guest.instance}-${guest.vmid}`;
|
||||
const override = overridesMap.get(guestId);
|
||||
const overrideSeverity = override?.poweredOffSeverity;
|
||||
|
||||
|
|
|
|||
|
|
@ -1011,12 +1011,9 @@ export function Dashboard(props: DashboardProps) {
|
|||
</Show>
|
||||
<For each={guests} fallback={<></>}>
|
||||
{(guest) => {
|
||||
// Match backend ID generation logic: standalone nodes use "node-vmid", clusters use "instance-node-vmid"
|
||||
// Match backend ID generation logic: stable format is "instance-vmid"
|
||||
const guestId =
|
||||
guest.id ||
|
||||
(guest.instance === guest.node
|
||||
? `${guest.node}-${guest.vmid}`
|
||||
: `${guest.instance}-${guest.node}-${guest.vmid}`);
|
||||
guest.id || `${guest.instance}-${guest.vmid}`;
|
||||
const metadata =
|
||||
guestMetadata()[guestId] ||
|
||||
guestMetadata()[`${guest.node}-${guest.vmid}`];
|
||||
|
|
|
|||
|
|
@ -26,10 +26,7 @@ const DEFAULT_FIRST_CELL_INDENT = 'pl-4';
|
|||
|
||||
const buildGuestId = (guest: Guest) => {
|
||||
if (guest.id) return guest.id;
|
||||
if (guest.instance === guest.node) {
|
||||
return `${guest.node}-${guest.vmid}`;
|
||||
}
|
||||
return `${guest.instance}-${guest.node}-${guest.vmid}`;
|
||||
return `${guest.instance}-${guest.vmid}`;
|
||||
};
|
||||
|
||||
// Type guard for VM vs Container
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import (
|
|||
"math"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
|
@ -7494,7 +7495,72 @@ func (m *Manager) getGuestThresholds(guest interface{}, guestID string) Threshol
|
|||
}
|
||||
|
||||
// Finally check guest-specific overrides (highest priority)
|
||||
if override, exists := m.config.Overrides[guestID]; exists {
|
||||
// First try the new stable ID format (instance-VMID)
|
||||
override, exists := m.config.Overrides[guestID]
|
||||
|
||||
// If not found, try legacy ID formats for migration
|
||||
if !exists {
|
||||
var legacyID string
|
||||
var node string
|
||||
var vmid int
|
||||
var instance string
|
||||
|
||||
// Extract node, vmid, and instance from the guest object
|
||||
switch g := guest.(type) {
|
||||
case models.VM:
|
||||
node = g.Node
|
||||
vmid = g.VMID
|
||||
instance = g.Instance
|
||||
case models.Container:
|
||||
node = g.Node
|
||||
vmid = g.VMID
|
||||
instance = g.Instance
|
||||
default:
|
||||
// Not a VM or container, skip legacy migration
|
||||
goto skipLegacyMigration
|
||||
}
|
||||
|
||||
// Try legacy format: instance-node-VMID
|
||||
if instance != node {
|
||||
legacyID = fmt.Sprintf("%s-%s-%d", instance, node, vmid)
|
||||
if legacyOverride, legacyExists := m.config.Overrides[legacyID]; legacyExists {
|
||||
log.Info().
|
||||
Str("legacyID", legacyID).
|
||||
Str("newID", guestID).
|
||||
Msg("Migrating guest override from legacy ID format")
|
||||
|
||||
// Move to new ID
|
||||
m.config.Overrides[guestID] = legacyOverride
|
||||
delete(m.config.Overrides, legacyID)
|
||||
|
||||
// Config will be persisted on next save cycle
|
||||
override = legacyOverride
|
||||
exists = true
|
||||
}
|
||||
}
|
||||
|
||||
// If still not found, try standalone format: node-VMID
|
||||
if !exists && instance == node {
|
||||
legacyID = fmt.Sprintf("%s-%d", node, vmid)
|
||||
if legacyOverride, legacyExists := m.config.Overrides[legacyID]; legacyExists {
|
||||
log.Info().
|
||||
Str("legacyID", legacyID).
|
||||
Str("newID", guestID).
|
||||
Msg("Migrating guest override from legacy standalone ID format")
|
||||
|
||||
// Move to new ID
|
||||
m.config.Overrides[guestID] = legacyOverride
|
||||
delete(m.config.Overrides, legacyID)
|
||||
|
||||
// Config will be persisted on next save cycle
|
||||
override = legacyOverride
|
||||
exists = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
skipLegacyMigration:
|
||||
if exists {
|
||||
// Apply the disabled flag if set
|
||||
if override.Disabled {
|
||||
thresholds.Disabled = true
|
||||
|
|
@ -7734,6 +7800,57 @@ func (m *Manager) LoadActiveAlerts() error {
|
|||
seen := make(map[string]bool)
|
||||
|
||||
for _, alert := range alerts {
|
||||
// Migrate legacy guest alert IDs (instance-node-VMID -> instance-VMID)
|
||||
// Check if this is a guest-related alert by looking at common alert types
|
||||
isGuestAlert := strings.Contains(alert.Type, "cpu") || strings.Contains(alert.Type, "memory") ||
|
||||
strings.Contains(alert.Type, "disk") || strings.Contains(alert.Type, "network") ||
|
||||
alert.Type == "guest-offline"
|
||||
if isGuestAlert {
|
||||
// Try to extract instance, node, and VMID from resource ID
|
||||
// Legacy format: instance-node-VMID or node-VMID (standalone)
|
||||
parts := strings.Split(alert.ResourceID, "-")
|
||||
|
||||
// Check if this looks like a legacy format (has node in the ID)
|
||||
// We can detect this if we have Node field and it appears in the ResourceID
|
||||
if alert.Node != "" && len(parts) >= 2 {
|
||||
var newResourceID string
|
||||
var vmidStr string
|
||||
|
||||
// Try to extract VMID (should be last part)
|
||||
vmidStr = parts[len(parts)-1]
|
||||
if _, err := strconv.Atoi(vmidStr); err == nil {
|
||||
// VMID is valid, now check if we need to migrate
|
||||
if len(parts) == 3 && alert.Instance != "" && alert.Instance != alert.Node {
|
||||
// Format: instance-node-VMID -> instance-VMID
|
||||
newResourceID = fmt.Sprintf("%s-%s", alert.Instance, vmidStr)
|
||||
} else if len(parts) == 2 && alert.Instance == alert.Node {
|
||||
// Format: node-VMID -> instance-VMID (standalone)
|
||||
newResourceID = fmt.Sprintf("%s-%s", alert.Instance, vmidStr)
|
||||
}
|
||||
|
||||
if newResourceID != "" && newResourceID != alert.ResourceID {
|
||||
log.Info().
|
||||
Str("oldID", alert.ResourceID).
|
||||
Str("newID", newResourceID).
|
||||
Str("alertType", alert.Type).
|
||||
Msg("Migrating active alert from legacy guest ID format")
|
||||
|
||||
oldAlertID := alert.ID
|
||||
|
||||
// Update resource ID
|
||||
alert.ResourceID = newResourceID
|
||||
|
||||
// Update alert ID (usually contains resource ID)
|
||||
alert.ID = strings.Replace(alert.ID, alert.ResourceID, newResourceID, 1)
|
||||
|
||||
// Update seen map to use new ID
|
||||
seen[alert.ID] = true
|
||||
delete(seen, oldAlertID)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Skip duplicates
|
||||
if seen[alert.ID] {
|
||||
duplicateCount++
|
||||
|
|
|
|||
|
|
@ -51,6 +51,85 @@ func (s *GuestMetadataStore) Get(guestID string) *GuestMetadata {
|
|||
return nil
|
||||
}
|
||||
|
||||
// GetWithLegacyMigration retrieves metadata for a guest, attempting legacy ID formats if needed
|
||||
// and migrating them to the new stable format. This should be called when full guest context
|
||||
// (node, instance, vmid) is available.
|
||||
func (s *GuestMetadataStore) GetWithLegacyMigration(guestID, instance, node string, vmid int) *GuestMetadata {
|
||||
s.mu.RLock()
|
||||
meta, exists := s.metadata[guestID]
|
||||
s.mu.RUnlock()
|
||||
|
||||
if exists {
|
||||
return meta
|
||||
}
|
||||
|
||||
// Try legacy formats and migrate if found
|
||||
var legacyID string
|
||||
var legacyMeta *GuestMetadata
|
||||
|
||||
// Try legacy format: instance-node-VMID
|
||||
if instance != node {
|
||||
legacyID = fmt.Sprintf("%s-%s-%d", instance, node, vmid)
|
||||
s.mu.RLock()
|
||||
legacyMeta = s.metadata[legacyID]
|
||||
s.mu.RUnlock()
|
||||
|
||||
if legacyMeta != nil {
|
||||
log.Info().
|
||||
Str("legacyID", legacyID).
|
||||
Str("newID", guestID).
|
||||
Msg("Migrating guest metadata from legacy ID format")
|
||||
|
||||
s.mu.Lock()
|
||||
// Move to new ID
|
||||
s.metadata[guestID] = legacyMeta
|
||||
legacyMeta.ID = guestID
|
||||
delete(s.metadata, legacyID)
|
||||
// Save asynchronously
|
||||
go func() {
|
||||
if err := s.save(); err != nil {
|
||||
log.Error().Err(err).Msg("Failed to save guest metadata after migration")
|
||||
}
|
||||
}()
|
||||
s.mu.Unlock()
|
||||
|
||||
return legacyMeta
|
||||
}
|
||||
}
|
||||
|
||||
// Try standalone format: node-VMID
|
||||
if instance == node {
|
||||
legacyID = fmt.Sprintf("%s-%d", node, vmid)
|
||||
s.mu.RLock()
|
||||
legacyMeta = s.metadata[legacyID]
|
||||
s.mu.RUnlock()
|
||||
|
||||
if legacyMeta != nil {
|
||||
log.Info().
|
||||
Str("legacyID", legacyID).
|
||||
Str("newID", guestID).
|
||||
Msg("Migrating guest metadata from legacy standalone ID format")
|
||||
|
||||
s.mu.Lock()
|
||||
// Move to new ID
|
||||
s.metadata[guestID] = legacyMeta
|
||||
legacyMeta.ID = guestID
|
||||
delete(s.metadata, legacyID)
|
||||
// Save asynchronously
|
||||
go func() {
|
||||
if err := s.save(); err != nil {
|
||||
log.Error().Err(err).Msg("Failed to save guest metadata after migration")
|
||||
}
|
||||
}()
|
||||
s.mu.Unlock()
|
||||
|
||||
return legacyMeta
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// GetAll retrieves all guest metadata
|
||||
func (s *GuestMetadataStore) GetAll() map[string]*GuestMetadata {
|
||||
s.mu.RLock()
|
||||
|
|
|
|||
|
|
@ -442,6 +442,7 @@ type Monitor struct {
|
|||
rng *rand.Rand
|
||||
maxRetryAttempts int
|
||||
tempCollector *TemperatureCollector // SSH-based temperature collector
|
||||
guestMetadataStore *config.GuestMetadataStore
|
||||
mu sync.RWMutex
|
||||
startTime time.Time
|
||||
rateTracker *RateTracker
|
||||
|
|
@ -3199,6 +3200,7 @@ func New(cfg *config.Config) (*Monitor, error) {
|
|||
rng: rand.New(rand.NewSource(time.Now().UnixNano())),
|
||||
maxRetryAttempts: 5,
|
||||
tempCollector: tempCollector,
|
||||
guestMetadataStore: config.NewGuestMetadataStore(cfg.DataPath),
|
||||
startTime: time.Now(),
|
||||
rateTracker: NewRateTracker(),
|
||||
metricsHistory: NewMetricsHistory(1000, 24*time.Hour), // Keep up to 1000 points or 24 hours
|
||||
|
|
|
|||
|
|
@ -253,13 +253,9 @@ func (m *Monitor) pollVMsWithNodes(ctx context.Context, instanceName string, cli
|
|||
tags = strings.Split(vm.Tags, ";")
|
||||
}
|
||||
|
||||
// Create guest ID
|
||||
var guestID string
|
||||
if instanceName == n.Node {
|
||||
guestID = fmt.Sprintf("%s-%d", n.Node, vm.VMID)
|
||||
} else {
|
||||
guestID = fmt.Sprintf("%s-%s-%d", instanceName, n.Node, vm.VMID)
|
||||
}
|
||||
// Create guest ID (stable across node migrations)
|
||||
// Format: instance-VMID
|
||||
guestID := fmt.Sprintf("%s-%d", instanceName, vm.VMID)
|
||||
|
||||
guestRaw := VMMemoryRaw{
|
||||
ListingMem: vm.Mem,
|
||||
|
|
@ -770,6 +766,11 @@ func (m *Monitor) pollVMsWithNodes(ctx context.Context, instanceName string, cli
|
|||
modelVM.DiskWrite = 0
|
||||
}
|
||||
|
||||
// Trigger guest metadata migration if old format exists
|
||||
if m.guestMetadataStore != nil {
|
||||
m.guestMetadataStore.GetWithLegacyMigration(guestID, instanceName, n.Node, vm.VMID)
|
||||
}
|
||||
|
||||
nodeVMs = append(nodeVMs, modelVM)
|
||||
|
||||
m.recordGuestSnapshot(instanceName, modelVM.Type, n.Node, vm.VMID, GuestMemorySnapshot{
|
||||
|
|
@ -929,13 +930,9 @@ func (m *Monitor) pollContainersWithNodes(ctx context.Context, instanceName stri
|
|||
tags = strings.Split(container.Tags, ";")
|
||||
}
|
||||
|
||||
// Create guest ID
|
||||
var guestID string
|
||||
if instanceName == n.Node {
|
||||
guestID = fmt.Sprintf("%s-%d", n.Node, container.VMID)
|
||||
} else {
|
||||
guestID = fmt.Sprintf("%s-%s-%d", instanceName, n.Node, container.VMID)
|
||||
}
|
||||
// Create guest ID (stable across node migrations)
|
||||
// Format: instance-VMID
|
||||
guestID := fmt.Sprintf("%s-%d", instanceName, container.VMID)
|
||||
|
||||
// Calculate I/O rates
|
||||
currentMetrics := IOMetrics{
|
||||
|
|
@ -1045,6 +1042,11 @@ func (m *Monitor) pollContainersWithNodes(ctx context.Context, instanceName stri
|
|||
modelContainer.DiskWrite = 0
|
||||
}
|
||||
|
||||
// Trigger guest metadata migration if old format exists
|
||||
if m.guestMetadataStore != nil {
|
||||
m.guestMetadataStore.GetWithLegacyMigration(guestID, instanceName, n.Node, int(container.VMID))
|
||||
}
|
||||
|
||||
nodeContainers = append(nodeContainers, modelContainer)
|
||||
|
||||
// Check alerts
|
||||
|
|
|
|||
Loading…
Reference in a new issue