Fix persistent temperature monitoring issues for standalone Proxmox nodes (addresses #571)
This commit resolves the recurring temperature monitoring failures that have plagued multiple releases: 1. **Fix user mismatch (v4.27.1 regression)**: - Changed binary default user from 'pulse-sensor' to 'pulse-sensor-proxy' - Aligns with the user created by install-sensor-proxy.sh (line 389) - Prevents panic when binary is run outside systemd context - Systemd unit already uses User=pulse-sensor-proxy, so this makes manual runs work too 2. **Fix standalone node validation (v4.25.0+ regression)**: - pvecm status exits with code 2 on standalone nodes (not in a cluster) - This caused validation to fail, rejecting all temperature requests - Added discoverLocalHostAddresses() helper that discovers actual host IPs/hostnames - On standalone nodes, cluster membership list is populated with host's own addresses - Maintains SSRF protection while allowing standalone operation - Added comprehensive test coverage 3. **Make installer fail loudly on proxy setup failure**: - Previously, failed proxy installation only printed a warning - Install script then claimed "Pulse installation complete!" (confusing for users) - Now exits with clear error message and remediation steps - Forces operators to fix proxy issues before claiming success - Users who skip temperature monitoring are unaffected 4. **Add test coverage to prevent future regressions**: - Added TestDiscoverLocalHostAddresses to verify local address discovery - Validates no loopback or link-local addresses are returned - All existing tests pass with new changes Pattern of failures across releases: - v4.23.0: Missing proxy binaries in release - v4.24.0-rc.3: AMD CPU sensor naming (Tctl vs Tdie) - v4.25.0: Single-node pvecm status exit code - v4.27.1: User mismatch (pulse-sensor vs pulse-sensor-proxy) This comprehensive fix addresses the root causes rather than applying another tactical patch. Related to #571
This commit is contained in:
parent
62a9f40cc7
commit
c9d1671afd
4 changed files with 138 additions and 7 deletions
|
|
@ -39,7 +39,7 @@ const (
|
|||
defaultConfigPath = "/etc/pulse-sensor-proxy/config.yaml"
|
||||
defaultAuditLogPath = "/var/log/pulse/sensor-proxy/audit.log"
|
||||
maxRequestBytes = 16 * 1024 // 16 KiB max request size
|
||||
defaultRunAsUser = "pulse-sensor"
|
||||
defaultRunAsUser = "pulse-sensor-proxy"
|
||||
)
|
||||
|
||||
func defaultWorkDir() string {
|
||||
|
|
|
|||
|
|
@ -557,6 +557,7 @@ func (p *Proxy) getTemperatureViaSSH(nodeHost string) (string, error) {
|
|||
|
||||
// discoverClusterNodes discovers all nodes in the Proxmox cluster
|
||||
// Returns IP addresses of cluster nodes
|
||||
// For standalone nodes (no cluster), returns the host's own addresses
|
||||
func discoverClusterNodes() ([]string, error) {
|
||||
// Check if pvecm is available (only on Proxmox hosts)
|
||||
if _, err := exec.LookPath("pvecm"); err != nil {
|
||||
|
|
@ -568,9 +569,21 @@ func discoverClusterNodes() ([]string, error) {
|
|||
var out, stderr bytes.Buffer
|
||||
cmd.Stdout = &out
|
||||
cmd.Stderr = &stderr
|
||||
if err := cmd.Run(); err != nil {
|
||||
log.Warn().Str("stderr", stderr.String()).Msg("pvecm status failed")
|
||||
return nil, fmt.Errorf("failed to get cluster status: %w (stderr: %s)", err, stderr.String())
|
||||
err := cmd.Run()
|
||||
|
||||
// pvecm status exits with code 2 on standalone nodes (not in a cluster)
|
||||
// Treat this as a valid case and discover local host addresses
|
||||
if err != nil {
|
||||
stderrStr := stderr.String()
|
||||
// Check if this is the "not part of a cluster" error
|
||||
if strings.Contains(stderrStr, "does not exist") ||
|
||||
strings.Contains(stderrStr, "not part of a cluster") {
|
||||
log.Info().Msg("Standalone Proxmox node detected (not in cluster) - discovering local host addresses")
|
||||
return discoverLocalHostAddresses()
|
||||
}
|
||||
// For other errors, fail
|
||||
log.Warn().Str("stderr", stderrStr).Msg("pvecm status failed")
|
||||
return nil, fmt.Errorf("failed to get cluster status: %w (stderr: %s)", err, stderrStr)
|
||||
}
|
||||
|
||||
// Parse output to extract IP addresses
|
||||
|
|
@ -607,6 +620,81 @@ func discoverClusterNodes() ([]string, error) {
|
|||
return nodes, nil
|
||||
}
|
||||
|
||||
// discoverLocalHostAddresses discovers all addresses of the local host
|
||||
// Used for standalone nodes that aren't part of a cluster
|
||||
func discoverLocalHostAddresses() ([]string, error) {
|
||||
addresses := make(map[string]struct{})
|
||||
|
||||
// Get hostname and FQDN
|
||||
if hostname, err := os.Hostname(); err == nil && hostname != "" {
|
||||
addresses[strings.ToLower(hostname)] = struct{}{}
|
||||
|
||||
// Try to get FQDN
|
||||
cmd := exec.Command("hostname", "-f")
|
||||
if out, err := cmd.Output(); err == nil {
|
||||
fqdn := strings.TrimSpace(string(out))
|
||||
if fqdn != "" && fqdn != hostname {
|
||||
addresses[strings.ToLower(fqdn)] = struct{}{}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Get all non-loopback IP addresses using ip command
|
||||
cmd := exec.Command("ip", "-o", "addr", "show")
|
||||
out, err := cmd.Output()
|
||||
if err != nil {
|
||||
log.Warn().Err(err).Msg("Failed to get IP addresses via 'ip addr'")
|
||||
} else {
|
||||
// Parse output lines like:
|
||||
// 2: eth0 inet 192.168.0.100/24 brd 192.168.0.255 scope global eth0
|
||||
// 2: eth0 inet6 fe80::a00:27ff:fe4e:66a1/64 scope link
|
||||
lines := strings.Split(string(out), "\n")
|
||||
for _, line := range lines {
|
||||
fields := strings.Fields(line)
|
||||
if len(fields) < 4 {
|
||||
continue
|
||||
}
|
||||
|
||||
// Field 2 is interface, field 3 is inet/inet6, field 4 is addr/prefix
|
||||
if fields[2] != "inet" && fields[2] != "inet6" {
|
||||
continue
|
||||
}
|
||||
|
||||
// Split addr/prefix (e.g., "192.168.0.100/24")
|
||||
addrWithPrefix := fields[3]
|
||||
addr := strings.Split(addrWithPrefix, "/")[0]
|
||||
|
||||
// Skip loopback addresses
|
||||
if strings.HasPrefix(addr, "127.") || addr == "::1" {
|
||||
continue
|
||||
}
|
||||
|
||||
// Skip link-local IPv6
|
||||
if strings.HasPrefix(addr, "fe80:") {
|
||||
continue
|
||||
}
|
||||
|
||||
addresses[addr] = struct{}{}
|
||||
}
|
||||
}
|
||||
|
||||
// Convert map to slice
|
||||
result := make([]string, 0, len(addresses))
|
||||
for addr := range addresses {
|
||||
result = append(result, addr)
|
||||
}
|
||||
|
||||
if len(result) == 0 {
|
||||
return nil, fmt.Errorf("no local host addresses found")
|
||||
}
|
||||
|
||||
log.Info().
|
||||
Strs("addresses", result).
|
||||
Msg("Discovered local host addresses for standalone node validation")
|
||||
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// isProxmoxHost checks if we're running on a Proxmox host
|
||||
func isProxmoxHost() bool {
|
||||
// Check for pvecm command
|
||||
|
|
|
|||
|
|
@ -172,3 +172,34 @@ func TestReadAllWithLimit(t *testing.T) {
|
|||
t.Fatalf("expected unlimited read to return full data without exceeding")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiscoverLocalHostAddresses(t *testing.T) {
|
||||
// This test verifies that discoverLocalHostAddresses returns valid addresses
|
||||
// It will vary by host but should always return at least hostname or IP addresses
|
||||
addresses, err := discoverLocalHostAddresses()
|
||||
if err != nil {
|
||||
t.Fatalf("discoverLocalHostAddresses failed: %v", err)
|
||||
}
|
||||
|
||||
if len(addresses) == 0 {
|
||||
t.Fatal("expected at least one address from discoverLocalHostAddresses")
|
||||
}
|
||||
|
||||
// Verify addresses are non-empty and don't contain loopback
|
||||
for _, addr := range addresses {
|
||||
if addr == "" {
|
||||
t.Error("got empty address in results")
|
||||
}
|
||||
if addr == "127.0.0.1" || addr == "::1" {
|
||||
t.Errorf("discoverLocalHostAddresses should not return loopback address: %s", addr)
|
||||
}
|
||||
if strings.HasPrefix(addr, "127.") {
|
||||
t.Errorf("discoverLocalHostAddresses should not return loopback range: %s", addr)
|
||||
}
|
||||
if strings.HasPrefix(addr, "fe80:") {
|
||||
t.Errorf("discoverLocalHostAddresses should not return link-local IPv6: %s", addr)
|
||||
}
|
||||
}
|
||||
|
||||
t.Logf("Discovered %d local addresses: %v", len(addresses), addresses)
|
||||
}
|
||||
|
|
|
|||
18
install.sh
18
install.sh
|
|
@ -1427,9 +1427,21 @@ fi'; then
|
|||
# Clean up temporary binary if it was copied
|
||||
[[ -f "$local_proxy_binary" ]] && rm -f "$local_proxy_binary"
|
||||
else
|
||||
print_warn "Proxy installation failed - temperature monitoring will use fallback method"
|
||||
print_info "Check logs: /tmp/proxy-install-${CTID}.log"
|
||||
print_info "Or run manually: curl -fsSL https://raw.githubusercontent.com/rcourtman/Pulse/main/scripts/install-sensor-proxy.sh | bash -s -- --ctid $CTID"
|
||||
# Proxy installation failed - this is a fatal error since user opted in
|
||||
rm -f "$proxy_script"
|
||||
echo
|
||||
print_error "Temperature proxy installation failed"
|
||||
echo
|
||||
echo "Temperature monitoring was enabled but the proxy setup failed."
|
||||
echo "Check logs: /tmp/proxy-install-${CTID}.log"
|
||||
echo
|
||||
echo "To fix, you can either:"
|
||||
echo " 1. Fix the issue and run the proxy installer manually:"
|
||||
echo " curl -fsSL https://raw.githubusercontent.com/rcourtman/Pulse/main/scripts/install-sensor-proxy.sh | bash -s -- --ctid $CTID"
|
||||
echo
|
||||
echo " 2. Re-run the Pulse installer and skip temperature monitoring when prompted"
|
||||
echo
|
||||
exit 1
|
||||
fi
|
||||
rm -f "$proxy_script"
|
||||
fi
|
||||
|
|
|
|||
Loading…
Reference in a new issue