- Refactor collector to support mocking
- Fix ZFS detection to support 'fuse.zfs' and case-insensitivity
- Add regression tests for ZFS dataset deduplication
Fixes#727. Previously, if temperature monitoring was enabled and a node wasn't found in ClusterEndpoints, the entire node processing was skipped. This change ensures we only skip temperature collection.
This commit contains multiple fixes for temperature proxy registration,
but the core issue remains unresolved.
## What's Fixed:
1. Added config pointer and reloadFunc to TemperatureProxyHandlers
2. Added SetConfig method to keep handler in sync with router config changes
3. Added config reload after registration to prevent monitor from overwriting
4. Fixed installer port conflict detection and duplicate YAML key issues
5. Added comprehensive debug logging throughout registration flow
## What's Still Broken:
The TemperatureProxyURL, TemperatureProxyToken, and TemperatureProxyControlToken
fields are NOT persisting to nodes.enc after SaveNodesConfig is called.
Debug logs confirm:
- HandleRegister correctly updates nodesConfig.PVEInstances[matchedIndex]
- The correct data is passed to SaveNodesConfig (verified in logs)
- SaveNodesConfig completes without errors
- Config reload executes successfully
- BUT after Pulse restart, the fields are empty when loaded from disk
The bug is in SaveNodesConfig serialization or file writing logic itself.
Related files:
- internal/api/temperature_proxy.go: Registration handler
- internal/config/persistence.go: SaveNodesConfig implementation
- internal/config/config.go: PVEInstance struct definition
Added TestPVESetupScriptArgumentAlignment to prevent future fmt.Sprintf
argument mismatch bugs in the PVE quick setup script template.
The test uses sentinel values (SENTINEL_URL, SENTINEL_HOST, deadbeef...)
to verify that critical placeholders receive the correct argument types:
✓ Repair block INSTALLER_URL uses pulseURL (not authToken)
✓ Repair --pulse-server flags use pulseURL (not authToken)
✓ Authorization headers use runtime $AUTH_TOKEN variable (not hardcoded)
✓ Token ID uses tokenName (pulse-*) (not pulseURL or authToken)
This test would have caught the bugs fixed in commits 2bb73d3c7 and
2053bc5e2, where:
- authToken appeared in --pulse-server URLs (argument shift)
- Authorization headers were hardcoded instead of using runtime variable
Recommended by Codex as a safeguard against this class of regression.
Changed Authorization headers in ssh-config and verify-temperature-ssh API
calls to use the runtime $AUTH_TOKEN variable instead of compile-time
hardcoded authToken.
This fixes a bug where users who override the auth token via:
- PULSE_SETUP_TOKEN environment variable
- Interactive prompt (when auth_token URL param omitted)
...would still send an empty Bearer token in the Authorization headers,
causing API calls to fail with 401 Unauthorized.
Changes:
- Line 4748: -H "Authorization: Bearer %s" → -H "Authorization: Bearer $AUTH_TOKEN"
- Line 4937: -H "Authorization: Bearer %s" → -H "Authorization: Bearer $AUTH_TOKEN"
- Removed 2 authToken arguments from fmt.Sprintf (lines 5059)
Now the script respects runtime token overrides in all code paths.
Identified by Codex during fmt.Sprintf argument alignment review.
Fixed critical argument mismatch bug where fmt.Sprintf arguments didn't align
with template placeholders. This caused:
- authToken being passed where pulseURL expected (curl errors)
- pulseURL being passed where authToken expected (empty Authorization headers)
- tokenName misalignment (Token ID placeholder broken)
Root cause: Template has 51 %s placeholders (54 total - 3 escaped %%s), but
argument list had wrong count and ordering.
Solution: Rebuilt argument list (lines 5049-5059) with correct mapping:
- 27 pulseURL (all installer URLs, --pulse-server flags, API endpoints)
- 11 tokenName (token creation, checks, final Token ID)
- 3 authToken (AUTH_TOKEN variable + 2 Authorization headers)
- 3 serverHost (error message rerun hints)
- 1 each: serverName, time, pulseIP, storagePerms, SSH keys, minProxyReadyVersion
Verified with go vet (passes). Mapping confirmed by walking each placeholder
in template and matching to correct argument type.
Related to #TBD (user will test)
Issue: When deployment type cannot be determined, error message referenced
$PROXY_INSTALLER but deleted it immediately, making instructions unusable.
Fix: Provide complete curl commands that users can copy-paste directly:
curl -fsSL $PULSE_URL/api/install/install-sensor-proxy.sh | bash -s -- ...
This ensures users have a working repair path even when auto-detection fails.
Identified by Codex final review.
Addresses all remaining issues from Codex final review:
Issue 1: SUMMARY_PROXY_INSTALLED unreliable (only set by install.sh)
Fix: Use PROXY_SOCKET_EXISTED_AT_START flag set at script start - works
for all installation methods (manual, older installers, etc.)
Issue 2: CTID detection fails when container offline/renamed
Fix: Read SUMMARY_CTID from install_summary.json as fallback. Priority:
1) Live PULSE_CTID detection
2) SUMMARY_CTID from json file
3) Standalone node detection
Issue 3: Failed repair disables working proxy (TEMPERATURE_ENABLED=false)
Fix: Keep TEMPERATURE_ENABLED=true in all failure paths. Comments explain:
proxy was working before, keep it enabled even if repair fails.
This ensures turnkey repair works reliably across all deployment scenarios
without breaking existing working proxies.
Addresses all issues found in Codex review:
1. Prevent double-install: Check SUMMARY_PROXY_INSTALLED to distinguish
between fresh installs (skip repair) vs existing installs (run repair)
2. Fix clustered node failures: Explicitly detect deployment type and bail
out with clear error message if neither --ctid nor --standalone can be
determined
3. Add health validation: Mirror main install path - verify service active,
socket exists, and fetch SSH public key after repair
4. Capture installer output: Show full diagnostics on failure (tail -20)
5. Better error messages: Provide specific manual repair commands when
deployment type cannot be auto-detected
This ensures the turnkey repair experience works reliably without regressing
fresh install UX.
The previous attempt (ed04926) was ineffective - it only set TEMPERATURE_ENABLED=true
which was redundant (already set at line 4051) and didn't trigger the auto-install block
because that block is gated by SKIP_TEMPERATURE_PROMPT != true.
This fix actually downloads and runs install-sensor-proxy.sh when an existing
socket is detected, which:
- Refreshes control plane tokens (fixes 401 errors)
- Updates control plane URL to correct Pulse instance
- Rewrites config atomically (Phase 2 installer is idempotent)
- Maintains turnkey UX - rerunning setup script now actually works
Detected by Codex final review.
When sensor-proxy socket is detected, the setup script was skipping
temperature monitoring setup with 'already configured' message. This
left stale control plane URLs/tokens, breaking temperature monitoring.
Now follows Codex recommendation: treat existing installations as
upgrade/repair opportunities. The installer is idempotent (Phase 2),
so rerunning it safely refreshes tokens, updates URLs, and ensures
turnkey operation even on hosts with existing installations.
Changes:
- Remove early return when sensor-proxy socket detected
- Set TEMPERATURE_ENABLED=true to proceed with reinstall
- Update message to clarify repair/upgrade behavior
- Maintains turnkey promise: rerun setup and it just works
