v0.74.0: fix controller->agent connection leak (reuse one agentapi client)

agentClient() built a new agentapi.Client (new bare http.Transport, IdleConnTimeout:0)
per call and discarded it without closing idle conns -> one leaked idle ESTABLISHED
socket per call to the agent :8443, exhausting the ephemeral port range after ~5 days
(EADDRNOTAVAIL). Memoize one shared client via sync.Once; harden Transport
(MaxIdleConns/PerHost + IdleConnTimeout 90s). Agent/firewall untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-22 17:19:24 +02:00
parent 0bfb481c2a
commit 2a5b88fbf9
6 changed files with 141 additions and 4 deletions
+25
View File
@@ -1,5 +1,30 @@
## Changelog
### v0.74.0 — fix the controller→agent connection leak (per-call agentapi client) (2026-06-22)
**Bugfix — agent local-API socket leak that took down the whole agent-backed feature set after ~5 days.**
`Server.agentClient()` built a fresh `agentapi.Client` (hence a fresh bare `http.Transport` with
`IdleConnTimeout:0`) on **every** call and discarded it without closing idle connections. The agent's
keep-alive left one idle ESTABLISHED socket per call to `192.168.0.162:8443`; these accumulated
(~5.8k/day, measured 206 in 47 min) until the ephemeral source-port range for that tuple exhausted →
`connect: cannot assign requested address` (EADDRNOTAVAIL), killing storage UI, host-metrics, and
whole-guest backup. (`:8006`/pveproxy was immune — the controller never dials it.) Diagnosis:
`felhom.eu/documentation/tests/unattended-test-campaign-2026-06-22-8443-diagnosis.md`.
- `internal/web/server.go``Server` gains `agentCli *agentapi.Client` + `agentCliErr error` +
`agentCliOnce sync.Once` (and the `agentapi` import).
- `internal/web/agent_disk_handlers.go``agentClient()` now memoizes the build via `agentCliOnce`
and **reuses one shared client** (cfg.LocalAPI is static per process — a config-apply self-restarts).
The empty-endpoint "not configured" guard stays OUTSIDE the Once. All 19 call sites unchanged.
- `internal/agentapi/client.go``New` Transport hardened: `MaxIdleConns:4`, `MaxIdleConnsPerHost:2`,
`IdleConnTimeout:90s` (was a bare Transport, `IdleConnTimeout:0`). Added optional `Client.Close()`
(CloseIdleConnections) hygiene helper.
- Tests: `TestAgentClient_ReusesSameInstance` (+ `TestAgentClient_UnconfiguredErrors`) and
`TestNew_TransportIdlePoolBounded` — both red-proofed against the pre-fix code.
- Agent, its bridge-IP bind, and firewall rules were **not** touched (controller-only fix).
Separate open item: the defense-in-depth host firewall rule scoping `:8443` to the guest bridge
subnet is still absent (pve-firewall disabled) — to be closed independently.
### v0.73.0 — DR recipe: emit the secret-free customer+apps half in the hub report (2026-06-16)
**DR recipe slice (controller half).** Additive `dr_recipe` section on the controller's hub report — the
+16 -2
View File
@@ -84,12 +84,26 @@ func New(endpoint, token, fingerprintHex string) (*Client, error) {
baseURL: "https://" + endpoint,
token: token,
hc: &http.Client{
Timeout: 15 * time.Second,
Transport: &http.Transport{TLSClientConfig: tlsCfg},
Timeout: 15 * time.Second,
// Bound + expire the idle-conn pool. With the controller reusing one Client (so the pool
// stays ~2), IdleConnTimeout also lets idle conns to a RESTARTED agent drain instead of
// lingering as stale ESTABLISHED entries, and caps any future per-call misuse. (The earlier
// bare Transport had IdleConnTimeout:0 = idle keep-alives never expire → the leak.)
Transport: &http.Transport{
TLSClientConfig: tlsCfg,
MaxIdleConns: 4,
MaxIdleConnsPerHost: 2,
IdleConnTimeout: 90 * time.Second,
},
},
}, nil
}
// Close releases the client's idle keep-alive connections. Optional hygiene for any caller that builds
// a short-lived client; the controller reuses one long-lived client, so it relies on the bounded,
// expiring idle pool (above) rather than calling this.
func (c *Client) Close() { c.hc.CloseIdleConnections() }
// Storage calls GET /storage and returns this guest's mounts (connectivity + placement view).
func (c *Client) Storage(ctx context.Context) (StorageResponse, error) {
var out StorageResponse
@@ -0,0 +1,30 @@
package agentapi
import (
"net/http"
"strings"
"testing"
)
// T2: New must build a Transport with a BOUNDED, EXPIRING idle-conn pool. The earlier bare
// &http.Transport{TLSClientConfig:…} had IdleConnTimeout==0 (idle keep-alives never expire) — the
// leak. White-box (package agentapi) so we can read the unexported hc.
//
// Companion red-proof: the bare Transport gives IdleConnTimeout==0 → this test fails (demonstrated,
// then reverted, during implementation).
func TestNew_TransportIdlePoolBounded(t *testing.T) {
c, err := New("127.0.0.1:8443", "tok", strings.Repeat("a", 64))
if err != nil {
t.Fatalf("New: %v", err)
}
tr, ok := c.hc.Transport.(*http.Transport)
if !ok {
t.Fatalf("unexpected transport type %T", c.hc.Transport)
}
if tr.IdleConnTimeout <= 0 {
t.Fatalf("IdleConnTimeout must be > 0 (idle keep-alives must expire); got %v", tr.IdleConnTimeout)
}
if tr.MaxIdleConnsPerHost <= 0 {
t.Fatalf("MaxIdleConnsPerHost must be > 0 (bounded pool); got %d", tr.MaxIdleConnsPerHost)
}
}
@@ -0,0 +1,47 @@
package web
import (
"strings"
"testing"
"gitea.dooplex.hu/admin/felhom-controller/internal/config"
)
// T1: agentClient() must return the SAME *agentapi.Client across calls — one Client ⇒ one pooled
// http.Transport ⇒ the per-call Transport leak (idle ESTABLISHED sockets to the agent's :8443 that
// exhausted the ephemeral port range after ~5 days) is structurally impossible. cfg.LocalAPI is
// valid-but-non-network (agentapi.New builds the struct without dialing), so this is hermetic.
//
// Companion red-proof: reverting agentClient() to `return agentapi.New(...)` makes the two pointers
// differ → this test fails (demonstrated, then reverted, during implementation).
func TestAgentClient_ReusesSameInstance(t *testing.T) {
s := &Server{cfg: &config.Config{LocalAPI: config.LocalAPIConfig{
Endpoint: "127.0.0.1:8443",
Token: "tok",
Fingerprint: strings.Repeat("a", 64), // syntactically valid 64-hex; no network touched
}}}
c1, err := s.agentClient()
if err != nil {
t.Fatalf("agentClient() #1: %v", err)
}
c2, err := s.agentClient()
if err != nil {
t.Fatalf("agentClient() #2: %v", err)
}
if c1 == nil || c2 == nil {
t.Fatal("agentClient() returned a nil client")
}
if c1 != c2 {
t.Fatalf("agentClient() must return the SAME instance (leak fix); got %p then %p", c1, c2)
}
}
// T1b: the "not configured" semantics are preserved — an empty endpoint still errors, OUTSIDE the
// memoizing Once (so an unprovisioned guest never caches a client).
func TestAgentClient_UnconfiguredErrors(t *testing.T) {
s := &Server{cfg: &config.Config{}} // LocalAPI.Endpoint == ""
if _, err := s.agentClient(); err == nil {
t.Fatal("expected an 'agent not configured' error when Endpoint is empty")
}
}
+14 -2
View File
@@ -38,13 +38,25 @@ func (s *Server) ServeDiskAPI(w http.ResponseWriter, r *http.Request) {
}
}
// agentClient builds a pinned client for the host agent's per-guest local API.
// agentClient returns the shared pinned client for the host agent's per-guest local API.
// Returns a clear error if the local API is not configured (unprovisioned guest).
//
// The client is built ONCE and reused (cfg.LocalAPI is static per process — a config-apply triggers a
// graceful self-restart). Reusing one *agentapi.Client (one pooled http.Transport) eliminates the
// per-call Transport leak that accumulated idle ESTABLISHED sockets to the agent's :8443 and exhausted
// the ephemeral source-port range after ~5 days of uptime. *agentapi.Client/*http.Client are safe for
// concurrent use, so the (19) callers need no extra locking. The empty-endpoint guard stays OUTSIDE the
// Once so it is re-checked each call (defensive; the endpoint is static); only a real build result
// (client, or a construction error like a bad fingerprint — neither changes without a restart) is memoized.
func (s *Server) agentClient() (*agentapi.Client, error) {
if s.cfg.LocalAPI.Endpoint == "" {
return nil, errors.New("agent not configured")
}
return agentapi.New(s.cfg.LocalAPI.Endpoint, s.cfg.LocalAPI.Token, s.cfg.LocalAPI.Fingerprint)
s.agentCliOnce.Do(func() {
s.agentCli, s.agentCliErr = agentapi.New(
s.cfg.LocalAPI.Endpoint, s.cfg.LocalAPI.Token, s.cfg.LocalAPI.Fingerprint)
})
return s.agentCli, s.agentCliErr
}
// writeDiskJSON writes the standard {ok,data,error} envelope used by the disk API.
+9
View File
@@ -13,6 +13,7 @@ import (
"sync/atomic"
"time"
"gitea.dooplex.hu/admin/felhom-controller/internal/agentapi"
"gitea.dooplex.hu/admin/felhom-controller/internal/appexport"
"gitea.dooplex.hu/admin/felhom-controller/internal/assets"
"gitea.dooplex.hu/admin/felhom-controller/internal/backup"
@@ -51,6 +52,14 @@ type Server struct {
// Guard for FileBrowser sync — prevents concurrent file writes (H5 fix)
fileBrowserMu sync.Mutex
// Shared agent local-API client (built once, reused). cfg.LocalAPI is static per process (a
// config-apply triggers a graceful self-restart), so the client is memoized via agentCliOnce —
// this kills the per-call http.Transport leak that exhausted the controller's ephemeral ports to
// the agent's :8443 after ~5 days of uptime (see agentClient()).
agentCli *agentapi.Client
agentCliErr error
agentCliOnce sync.Once
// Hub push status callback — set via SetHubPushStatus for monitoring page
hubPushStatusFn func() HubPushStatusData