diff --git a/internal/api/config_handlers.go b/internal/api/config_handlers.go index 003dacd..c568547 100644 --- a/internal/api/config_handlers.go +++ b/internal/api/config_handlers.go @@ -290,7 +290,7 @@ type NodeConfigRequest struct { MonitorContainers bool `json:"monitorContainers,omitempty"` // PVE only MonitorStorage bool `json:"monitorStorage,omitempty"` // PVE only MonitorBackups bool `json:"monitorBackups,omitempty"` // PVE only - MonitorPhysicalDisks bool `json:"monitorPhysicalDisks,omitempty"` // PVE only + MonitorPhysicalDisks *bool `json:"monitorPhysicalDisks,omitempty"` // PVE only (nil = enabled by default) MonitorDatastores bool `json:"monitorDatastores,omitempty"` // PBS only MonitorSyncJobs bool `json:"monitorSyncJobs,omitempty"` // PBS only MonitorVerifyJobs bool `json:"monitorVerifyJobs,omitempty"` // PBS only @@ -318,7 +318,7 @@ type NodeResponse struct { MonitorContainers bool `json:"monitorContainers,omitempty"` MonitorStorage bool `json:"monitorStorage,omitempty"` MonitorBackups bool `json:"monitorBackups,omitempty"` - MonitorPhysicalDisks bool `json:"monitorPhysicalDisks,omitempty"` + MonitorPhysicalDisks *bool `json:"monitorPhysicalDisks,omitempty"` MonitorDatastores bool `json:"monitorDatastores,omitempty"` MonitorSyncJobs bool `json:"monitorSyncJobs,omitempty"` MonitorVerifyJobs bool `json:"monitorVerifyJobs,omitempty"` @@ -720,7 +720,7 @@ func (h *ConfigHandlers) HandleGetNodes(w http.ResponseWriter, r *http.Request) MonitorContainers: true, MonitorStorage: true, MonitorBackups: true, - MonitorPhysicalDisks: true, + MonitorPhysicalDisks: nil, // nil = enabled by default Status: "connected", IsCluster: true, ClusterName: "mock-cluster", @@ -746,7 +746,7 @@ func (h *ConfigHandlers) HandleGetNodes(w http.ResponseWriter, r *http.Request) MonitorContainers: true, MonitorStorage: true, MonitorBackups: true, - MonitorPhysicalDisks: true, + MonitorPhysicalDisks: nil, // nil = enabled by default Status: "connected", IsCluster: false, // Not part of a cluster ClusterName: "", @@ -1437,7 +1437,11 @@ func (h *ConfigHandlers) HandleUpdateNode(w http.ResponseWriter, r *http.Request // Update the node if nodeType == "pve" && index < len(h.config.PVEInstances) { pve := &h.config.PVEInstances[index] - pve.Name = req.Name + + // Only update name if provided + if req.Name != "" { + pve.Name = req.Name + } if req.Host != "" { host := req.Host @@ -1456,30 +1460,37 @@ func (h *ConfigHandlers) HandleUpdateNode(w http.ResponseWriter, r *http.Request pve.Host = host } + // Handle authentication updates - only switch auth method if explicitly provided if req.TokenName != "" || req.TokenValue != "" { + // Switching to or updating token authentication if req.TokenName != "" { pve.TokenName = req.TokenName } if req.TokenValue != "" { pve.TokenValue = req.TokenValue } - // When using token authentication, clear password to avoid conflicts + // Clear password to avoid conflicts pve.Password = "" if req.User != "" { pve.User = req.User } - } else { + } else if req.Password != "" { + // Explicitly switching to password authentication if req.User != "" { pve.User = normalizePVEUser(req.User) } else if pve.User != "" { pve.User = normalizePVEUser(pve.User) } - if req.Password != "" { - pve.Password = req.Password - } - // Clear token fields when using password authentication + pve.Password = req.Password + // Clear token fields when switching to password auth pve.TokenName = "" pve.TokenValue = "" + } else { + // No authentication changes - preserve existing auth fields + // Only normalize user if it exists + if pve.User != "" { + pve.User = normalizePVEUser(pve.User) + } } pve.Fingerprint = req.Fingerprint @@ -1511,22 +1522,22 @@ func (h *ConfigHandlers) HandleUpdateNode(w http.ResponseWriter, r *http.Request } pbs.Host = host - // Determine authentication method and clear the unused fields + // Handle authentication updates - only switch auth method if explicitly provided if req.TokenName != "" && req.TokenValue != "" { - // Using token authentication - clear user/password - pbs.User = "" - pbs.Password = "" + // Switching to token authentication pbs.TokenName = req.TokenName pbs.TokenValue = req.TokenValue - } else if req.TokenName != "" { - // Token name provided without new value - keep existing token value + // Clear user/password when switching to token auth pbs.User = "" pbs.Password = "" + } else if req.TokenName != "" { + // Token name provided without new value - keep existing token value pbs.TokenName = req.TokenName + // Clear user/password when using token auth + pbs.User = "" + pbs.Password = "" } else if req.Password != "" { - // Using password authentication - clear token fields - pbs.TokenName = "" - pbs.TokenValue = "" + // Switching to password authentication pbs.Password = req.Password // Ensure user has realm for PBS pbsUser := req.User @@ -1534,17 +1545,22 @@ func (h *ConfigHandlers) HandleUpdateNode(w http.ResponseWriter, r *http.Request pbsUser = req.User + "@pbs" // Default to @pbs realm if not specified } pbs.User = pbsUser - } else if req.User != "" { - // User provided without password - keep existing password if any + // Clear token fields when switching to password auth pbs.TokenName = "" pbs.TokenValue = "" + } else if req.User != "" { + // User provided - assume password auth but keep existing password // Ensure user has realm for PBS pbsUser := req.User if !strings.Contains(req.User, "@") { pbsUser = req.User + "@pbs" // Default to @pbs realm if not specified } pbs.User = pbsUser + // Clear token fields when using password auth + pbs.TokenName = "" + pbs.TokenValue = "" } + // else: No authentication changes - preserve existing auth fields pbs.Fingerprint = req.Fingerprint pbs.VerifySSL = req.VerifySSL @@ -1575,12 +1591,16 @@ func (h *ConfigHandlers) HandleUpdateNode(w http.ResponseWriter, r *http.Request pmgInst.Host = host } + // Handle authentication updates - only switch auth method if explicitly provided if req.TokenName != "" && req.TokenValue != "" { - pmgInst.User = "" - pmgInst.Password = "" + // Switching to token authentication pmgInst.TokenName = req.TokenName pmgInst.TokenValue = req.TokenValue - } else { + // Clear user/password when switching to token auth + pmgInst.User = "" + pmgInst.Password = "" + } else if req.Password != "" { + // Switching to password authentication if req.User != "" { user := req.User if !strings.Contains(user, "@") { @@ -1588,14 +1608,22 @@ func (h *ConfigHandlers) HandleUpdateNode(w http.ResponseWriter, r *http.Request } pmgInst.User = user } - if req.Password != "" { - pmgInst.Password = req.Password - } - if req.TokenName == "" && req.TokenValue == "" { - pmgInst.TokenName = "" - pmgInst.TokenValue = "" + pmgInst.Password = req.Password + // Clear token fields when switching to password auth + pmgInst.TokenName = "" + pmgInst.TokenValue = "" + } else if req.User != "" { + // User provided - assume password auth but keep existing password + user := req.User + if !strings.Contains(user, "@") { + user = user + "@pmg" } + pmgInst.User = user + // Clear token fields when using password auth + pmgInst.TokenName = "" + pmgInst.TokenValue = "" } + // else: No authentication changes - preserve existing auth fields pmgInst.Fingerprint = req.Fingerprint pmgInst.VerifySSL = req.VerifySSL diff --git a/internal/config/config.go b/internal/config/config.go index 851336b..5c0e04b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -145,8 +145,8 @@ type PVEInstance struct { MonitorContainers bool MonitorStorage bool MonitorBackups bool - MonitorPhysicalDisks bool // Monitor physical disks (polled less frequently to avoid spinning up HDDs) - PhysicalDiskPollingMinutes int // How often to poll physical disks (0 = use default) + MonitorPhysicalDisks *bool // Monitor physical disks (nil = enabled by default, can be explicitly disabled) + PhysicalDiskPollingMinutes int // How often to poll physical disks (0 = use default) // Cluster support IsCluster bool // True if this is a cluster diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 69b52cc..b43c545 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -2145,183 +2145,186 @@ func (m *Monitor) pollPVEInstance(ctx context.Context, instanceName string, clie } } - // Poll physical disks for health monitoring (only if enabled and interval elapsed) - if instanceCfg.MonitorPhysicalDisks { + // Poll physical disks for health monitoring (enabled by default unless explicitly disabled) + // Skip if MonitorPhysicalDisks is explicitly set to false + if instanceCfg.MonitorPhysicalDisks != nil && !*instanceCfg.MonitorPhysicalDisks { + log.Debug().Str("instance", instanceName).Msg("Physical disk monitoring explicitly disabled") + // Keep any existing disk data visible (don't clear it) + } else { + // Enabled by default (when nil or true) // Determine polling interval (default 5 minutes to avoid spinning up HDDs too frequently) pollingInterval := 5 * time.Minute - if instanceCfg.PhysicalDiskPollingMinutes > 0 { - pollingInterval = time.Duration(instanceCfg.PhysicalDiskPollingMinutes) * time.Minute + if instanceCfg.PhysicalDiskPollingMinutes > 0 { + pollingInterval = time.Duration(instanceCfg.PhysicalDiskPollingMinutes) * time.Minute + } + + // Check if enough time has elapsed since last poll + m.mu.Lock() + lastPoll, exists := m.lastPhysicalDiskPoll[instanceName] + shouldPoll := !exists || time.Since(lastPoll) >= pollingInterval + if shouldPoll { + m.lastPhysicalDiskPoll[instanceName] = time.Now() + } + m.mu.Unlock() + + if !shouldPoll { + log.Debug(). + Str("instance", instanceName). + Dur("sinceLastPoll", time.Since(lastPoll)). + Dur("interval", pollingInterval). + Msg("Skipping physical disk poll - interval not elapsed") + // Refresh NVMe temperatures using the latest sensor data even when we skip the disk poll + currentState := m.state.GetSnapshot() + existing := make([]models.PhysicalDisk, 0) + for _, disk := range currentState.PhysicalDisks { + if disk.Instance == instanceName { + existing = append(existing, disk) + } + } + if len(existing) > 0 { + updated := mergeNVMeTempsIntoDisks(existing, modelNodes) + m.state.UpdatePhysicalDisks(instanceName, updated) + } + } else { + log.Debug(). + Int("nodeCount", len(nodes)). + Dur("interval", pollingInterval). + Msg("Starting disk health polling") + + // Get existing disks from state to preserve data for offline nodes + currentState := m.state.GetSnapshot() + existingDisksMap := make(map[string]models.PhysicalDisk) + for _, disk := range currentState.PhysicalDisks { + if disk.Instance == instanceName { + existingDisksMap[disk.ID] = disk + } } - // Check if enough time has elapsed since last poll - m.mu.Lock() - lastPoll, exists := m.lastPhysicalDiskPoll[instanceName] - shouldPoll := !exists || time.Since(lastPoll) >= pollingInterval - if shouldPoll { - m.lastPhysicalDiskPoll[instanceName] = time.Now() - } - m.mu.Unlock() + var allDisks []models.PhysicalDisk + polledNodes := make(map[string]bool) // Track which nodes we successfully polled + + for _, node := range nodes { + // Skip offline nodes but preserve their existing disk data + if node.Status != "online" { + log.Debug().Str("node", node.Node).Msg("Skipping disk poll for offline node - preserving existing data") + continue + } + + // Get disk list for this node + log.Debug().Str("node", node.Node).Msg("Getting disk list for node") + disks, err := client.GetDisks(ctx, node.Node) + if err != nil { + // Check if it's a permission error or if the endpoint doesn't exist + if strings.Contains(err.Error(), "401") || strings.Contains(err.Error(), "403") { + log.Warn(). + Str("node", node.Node). + Err(err). + Msg("Insufficient permissions to access disk information - check API token permissions") + } else if strings.Contains(err.Error(), "404") || strings.Contains(err.Error(), "501") { + log.Info(). + Str("node", node.Node). + Msg("Disk monitoring not available on this node (may be using non-standard storage)") + } else { + log.Warn(). + Str("node", node.Node). + Err(err). + Msg("Failed to get disk list") + } + continue + } - if !shouldPoll { log.Debug(). - Str("instance", instanceName). - Dur("sinceLastPoll", time.Since(lastPoll)). - Dur("interval", pollingInterval). - Msg("Skipping physical disk poll - interval not elapsed") - // Refresh NVMe temperatures using the latest sensor data even when we skip the disk poll - currentState := m.state.GetSnapshot() - existing := make([]models.PhysicalDisk, 0) - for _, disk := range currentState.PhysicalDisks { - if disk.Instance == instanceName { - existing = append(existing, disk) - } - } - if len(existing) > 0 { - updated := mergeNVMeTempsIntoDisks(existing, modelNodes) - m.state.UpdatePhysicalDisks(instanceName, updated) - } - } else { - log.Debug(). - Int("nodeCount", len(nodes)). - Dur("interval", pollingInterval). - Msg("Starting disk health polling") + Str("node", node.Node). + Int("diskCount", len(disks)). + Msg("Got disk list for node") - // Get existing disks from state to preserve data for offline nodes - currentState := m.state.GetSnapshot() - existingDisksMap := make(map[string]models.PhysicalDisk) - for _, disk := range currentState.PhysicalDisks { - if disk.Instance == instanceName { - existingDisksMap[disk.ID] = disk - } - } + // Mark this node as successfully polled + polledNodes[node.Node] = true - var allDisks []models.PhysicalDisk - polledNodes := make(map[string]bool) // Track which nodes we successfully polled - - for _, node := range nodes { - // Skip offline nodes but preserve their existing disk data - if node.Status != "online" { - log.Debug().Str("node", node.Node).Msg("Skipping disk poll for offline node - preserving existing data") - continue + // Check each disk for health issues and add to state + for _, disk := range disks { + // Create PhysicalDisk model + diskID := fmt.Sprintf("%s-%s-%s", instanceName, node.Node, strings.ReplaceAll(disk.DevPath, "/", "-")) + physicalDisk := models.PhysicalDisk{ + ID: diskID, + Node: node.Node, + Instance: instanceName, + DevPath: disk.DevPath, + Model: disk.Model, + Serial: disk.Serial, + Type: disk.Type, + Size: disk.Size, + Health: disk.Health, + Wearout: disk.Wearout, + RPM: disk.RPM, + Used: disk.Used, + LastChecked: time.Now(), } - // Get disk list for this node - log.Debug().Str("node", node.Node).Msg("Getting disk list for node") - disks, err := client.GetDisks(ctx, node.Node) - if err != nil { - // Check if it's a permission error or if the endpoint doesn't exist - if strings.Contains(err.Error(), "401") || strings.Contains(err.Error(), "403") { - log.Warn(). - Str("node", node.Node). - Err(err). - Msg("Insufficient permissions to access disk information - check API token permissions") - } else if strings.Contains(err.Error(), "404") || strings.Contains(err.Error(), "501") { - log.Info(). - Str("node", node.Node). - Msg("Disk monitoring not available on this node (may be using non-standard storage)") - } else { - log.Warn(). - Str("node", node.Node). - Err(err). - Msg("Failed to get disk list") - } - continue - } + allDisks = append(allDisks, physicalDisk) log.Debug(). Str("node", node.Node). - Int("diskCount", len(disks)). - Msg("Got disk list for node") + Str("disk", disk.DevPath). + Str("model", disk.Model). + Str("health", disk.Health). + Int("wearout", disk.Wearout). + Msg("Checking disk health") - // Mark this node as successfully polled - polledNodes[node.Node] = true - - // Check each disk for health issues and add to state - for _, disk := range disks { - // Create PhysicalDisk model - diskID := fmt.Sprintf("%s-%s-%s", instanceName, node.Node, strings.ReplaceAll(disk.DevPath, "/", "-")) - physicalDisk := models.PhysicalDisk{ - ID: diskID, - Node: node.Node, - Instance: instanceName, - DevPath: disk.DevPath, - Model: disk.Model, - Serial: disk.Serial, - Type: disk.Type, - Size: disk.Size, - Health: disk.Health, - Wearout: disk.Wearout, - RPM: disk.RPM, - Used: disk.Used, - LastChecked: time.Now(), - } - - allDisks = append(allDisks, physicalDisk) - - log.Debug(). + normalizedHealth := strings.ToUpper(strings.TrimSpace(disk.Health)) + if normalizedHealth != "" && normalizedHealth != "UNKNOWN" && normalizedHealth != "PASSED" && normalizedHealth != "OK" { + // Disk has failed or is failing - alert manager will handle this + log.Warn(). Str("node", node.Node). Str("disk", disk.DevPath). Str("model", disk.Model). Str("health", disk.Health). Int("wearout", disk.Wearout). - Msg("Checking disk health") + Msg("Disk health issue detected") - normalizedHealth := strings.ToUpper(strings.TrimSpace(disk.Health)) - if normalizedHealth != "" && normalizedHealth != "UNKNOWN" && normalizedHealth != "PASSED" && normalizedHealth != "OK" { - // Disk has failed or is failing - alert manager will handle this - log.Warn(). - Str("node", node.Node). - Str("disk", disk.DevPath). - Str("model", disk.Model). - Str("health", disk.Health). - Int("wearout", disk.Wearout). - Msg("Disk health issue detected") + // Pass disk info to alert manager + m.alertManager.CheckDiskHealth(instanceName, node.Node, disk) + } else if disk.Wearout > 0 && disk.Wearout < 10 { + // Low wearout warning (less than 10% life remaining) + log.Warn(). + Str("node", node.Node). + Str("disk", disk.DevPath). + Str("model", disk.Model). + Int("wearout", disk.Wearout). + Msg("SSD wearout critical - less than 10% life remaining") - // Pass disk info to alert manager - m.alertManager.CheckDiskHealth(instanceName, node.Node, disk) - } else if disk.Wearout > 0 && disk.Wearout < 10 { - // Low wearout warning (less than 10% life remaining) - log.Warn(). - Str("node", node.Node). - Str("disk", disk.DevPath). - Str("model", disk.Model). - Int("wearout", disk.Wearout). - Msg("SSD wearout critical - less than 10% life remaining") - - // Pass to alert manager for wearout alert - m.alertManager.CheckDiskHealth(instanceName, node.Node, disk) - } + // Pass to alert manager for wearout alert + m.alertManager.CheckDiskHealth(instanceName, node.Node, disk) } } - - // Preserve existing disk data for nodes that weren't polled (offline or error) - for _, existingDisk := range existingDisksMap { - // Only preserve if we didn't poll this node - if !polledNodes[existingDisk.Node] { - // Keep the existing disk data but update the LastChecked to indicate it's stale - allDisks = append(allDisks, existingDisk) - log.Debug(). - Str("node", existingDisk.Node). - Str("disk", existingDisk.DevPath). - Msg("Preserving existing disk data for unpolled node") - } - } - - allDisks = mergeNVMeTempsIntoDisks(allDisks, modelNodes) - - // Update physical disks in state - log.Debug(). - Str("instance", instanceName). - Int("diskCount", len(allDisks)). - Int("preservedCount", len(existingDisksMap)-len(polledNodes)). - Msg("Updating physical disks in state") - m.state.UpdatePhysicalDisks(instanceName, allDisks) } - } else { - // Physical disk monitoring is disabled - clear any existing disk data for this instance - log.Debug().Str("instance", instanceName).Msg("Physical disk monitoring disabled - clearing disk data") - m.state.UpdatePhysicalDisks(instanceName, []models.PhysicalDisk{}) + + // Preserve existing disk data for nodes that weren't polled (offline or error) + for _, existingDisk := range existingDisksMap { + // Only preserve if we didn't poll this node + if !polledNodes[existingDisk.Node] { + // Keep the existing disk data but update the LastChecked to indicate it's stale + allDisks = append(allDisks, existingDisk) + log.Debug(). + Str("node", existingDisk.Node). + Str("disk", existingDisk.DevPath). + Msg("Preserving existing disk data for unpolled node") + } + } + + allDisks = mergeNVMeTempsIntoDisks(allDisks, modelNodes) + + // Update physical disks in state + log.Debug(). + Str("instance", instanceName). + Int("diskCount", len(allDisks)). + Int("preservedCount", len(existingDisksMap)-len(polledNodes)). + Msg("Updating physical disks in state") + m.state.UpdatePhysicalDisks(instanceName, allDisks) + } } + // Note: Physical disk monitoring is now enabled by default with a 5-minute polling interval. + // Users can explicitly disable it in node settings. Disk data is preserved between polls. // Update nodes with storage fallback if rootfs was not available for i := range modelNodes {