From 7af19a973eb808e193999d7fa27ecdbe449e04c5 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 19:02:48 +0100 Subject: [PATCH] Bound stopped-container Docker inspection cadence Cache unchanged inactive container detail between live telemetry cycles so NAS hosts with large historical inventories do not re-inspect every stopped container every 30 seconds. Keep running state live, fail open to fresh inspection when lifecycle evidence is incomplete, and clear daemon-scoped caches on reconnect. Refs #1729. Contract-Neutral: Bounds internal daemon call cadence without changing report or operator contracts --- internal/dockeragent/agent.go | 10 ++ .../dockeragent/agent_report_coverage_test.go | 105 +++++++++++++++++- internal/dockeragent/collect.go | 79 ++++++++++++- internal/dockeragent/hardening.go | 15 ++- internal/dockeragent/runtime_reconnect.go | 1 + 5 files changed, 201 insertions(+), 9 deletions(-) diff --git a/internal/dockeragent/agent.go b/internal/dockeragent/agent.go index 3c6865932..940c8ace9 100644 --- a/internal/dockeragent/agent.go +++ b/internal/dockeragent/agent.go @@ -13,6 +13,7 @@ import ( "sync" "time" + containertypes "github.com/moby/moby/api/types/container" systemtypes "github.com/moby/moby/api/types/system" "github.com/moby/moby/client" "github.com/rcourtman/pulse-go-rewrite/internal/agenttarget" @@ -135,6 +136,8 @@ type Agent struct { cpuMu sync.Mutex // protects prevContainerCPU storageUsageMu sync.Mutex storageUsageCache dockerStorageUsageCache + inactiveInspectMu sync.Mutex + inactiveInspects map[string]inactiveContainerInspectCacheEntry reportBuffer *utils.Queue[agentsdocker.Report] reportBuffers map[string]*utils.Queue[agentsdocker.Report] registryChecker *RegistryChecker // For checking container image updates @@ -163,6 +166,13 @@ type dockerStorageUsageCache struct { valid bool } +type inactiveContainerInspectCacheEntry struct { + inspect containertypes.InspectResponse + fingerprint string + withSize bool + expiresAt time.Time +} + // ErrStopRequested indicates the agent should terminate gracefully after acknowledging a stop command. var ErrStopRequested = errors.New("docker host stop requested") diff --git a/internal/dockeragent/agent_report_coverage_test.go b/internal/dockeragent/agent_report_coverage_test.go index c9aa08024..e999c3c62 100644 --- a/internal/dockeragent/agent_report_coverage_test.go +++ b/internal/dockeragent/agent_report_coverage_test.go @@ -30,18 +30,21 @@ func TestBuildReportSynologySizedInventoryBoundsStorageComputations(t *testing.T for i := 0; i < containerCount; i++ { id := fmt.Sprintf("container-%02d", i) state := containertypes.ContainerState("exited") + status := "Exited (0) 2 hours ago" if i < runningCount { state = "running" + status = "Up 2 hours" running[id] = true } containers = append(containers, containertypes.Summary{ - ID: id, - Names: []string{"/" + id}, - State: state, + ID: id, + Names: []string{"/" + id}, + State: state, + Status: status, }) } - var diskUsageCalls, imageListCalls, sharedSizeRequests int + var diskUsageCalls, imageListCalls, sharedSizeRequests, inspectCalls int agent := &Agent{ cfg: Config{ Interval: 30 * time.Second, @@ -58,6 +61,7 @@ func TestBuildReportSynologySizedInventoryBoundsStorageComputations(t *testing.T return containers, nil }, containerInspectWithRawFn: func(_ context.Context, id string, size bool) (containertypes.InspectResponse, []byte, error) { + inspectCalls++ if size { t.Fatal("normal unified-agent reports must not request per-container size walks") } @@ -102,6 +106,99 @@ func TestBuildReportSynologySizedInventoryBoundsStorageComputations(t *testing.T if sharedSizeRequests != 0 { t.Fatalf("live image inventory requested %d shared-size computations, want none", sharedSizeRequests) } + if want := containerCount + runningCount; inspectCalls != want { + t.Fatalf("container detail calls = %d, want %d (all on first cycle plus running only on second)", inspectCalls, want) + } +} + +func TestInactiveContainerInspectCacheInvalidatesOnLifecycleChangeAndExpiry(t *testing.T) { + var inspectCalls int + agent := &Agent{ + docker: &fakeDockerClient{ + containerInspectWithRawFn: func(context.Context, string, bool) (containertypes.InspectResponse, []byte, error) { + inspectCalls++ + return containertypes.InspectResponse{ + State: &containertypes.State{}, + Config: &containertypes.Config{}, + }, nil, nil + }, + }, + } + + summary := containertypes.Summary{ID: "stopped", State: "exited", Status: "Exited (0) 2 hours ago"} + if _, err := agent.inspectContainer(context.Background(), summary, false); err != nil { + t.Fatal(err) + } + if _, err := agent.inspectContainer(context.Background(), summary, false); err != nil { + t.Fatal(err) + } + if inspectCalls != 1 { + t.Fatalf("unchanged inactive container inspect calls = %d, want 1", inspectCalls) + } + + summary.Status = "Exited (1) 1 second ago" + if _, err := agent.inspectContainer(context.Background(), summary, false); err != nil { + t.Fatal(err) + } + if inspectCalls != 2 { + t.Fatalf("lifecycle-changed inactive container inspect calls = %d, want 2", inspectCalls) + } + + agent.inactiveInspectMu.Lock() + entry := agent.inactiveInspects[summary.ID] + entry.expiresAt = time.Now().Add(-time.Second) + agent.inactiveInspects[summary.ID] = entry + agent.inactiveInspectMu.Unlock() + if _, err := agent.inspectContainer(context.Background(), summary, false); err != nil { + t.Fatal(err) + } + if inspectCalls != 3 { + t.Fatalf("expired inactive container inspect calls = %d, want 3", inspectCalls) + } + if _, err := agent.inspectContainer(context.Background(), summary, true); err != nil { + t.Fatal(err) + } + if inspectCalls != 4 { + t.Fatalf("disk-metric mode change inspect calls = %d, want 4", inspectCalls) + } + + summary.Status = "" + if _, err := agent.inspectContainer(context.Background(), summary, false); err != nil { + t.Fatal(err) + } + if _, err := agent.inspectContainer(context.Background(), summary, false); err != nil { + t.Fatal(err) + } + if inspectCalls != 6 { + t.Fatalf("inactive container without lifecycle evidence inspect calls = %d, want 6", inspectCalls) + } +} + +func TestRuntimeAdoptionClearsDaemonScopedCollectionCaches(t *testing.T) { + oldClient := &fakeDockerClient{} + newClient := &fakeDockerClient{daemonHost: "unix:///run/new-docker.sock"} + agent := &Agent{ + docker: oldClient, + runtime: RuntimeDocker, + storageUsageCache: dockerStorageUsageCache{ + valid: true, + nextRefresh: time.Now().Add(time.Hour), + }, + inactiveInspects: map[string]inactiveContainerInspectCacheEntry{ + "old-container": {expiresAt: time.Now().Add(time.Hour)}, + }, + } + + previous := agent.adoptRuntimeConnection(newClient, systemtypes.Info{ServerVersion: "28.0.0"}, RuntimeDocker) + if previous != oldClient { + t.Fatalf("previous client = %T, want old client", previous) + } + if agent.storageUsageCache.valid || !agent.storageUsageCache.nextRefresh.IsZero() { + t.Fatalf("storage cache survived runtime adoption: %+v", agent.storageUsageCache) + } + if len(agent.inactiveInspects) != 0 { + t.Fatalf("inactive inspect cache survived runtime adoption: %+v", agent.inactiveInspects) + } } func TestBuildReport_RuntimeChangePodman(t *testing.T) { diff --git a/internal/dockeragent/collect.go b/internal/dockeragent/collect.go index 269255dbc..9bcad7b75 100644 --- a/internal/dockeragent/collect.go +++ b/internal/dockeragent/collect.go @@ -314,6 +314,7 @@ func (a *Agent) collectContainers(ctx context.Context) ([]agentsdocker.Container containers = append(containers, container) } a.pruneStaleCPUSamples(active) + a.pruneInactiveContainerInspectCache(active) return containers, nil } @@ -557,7 +558,7 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum defer cancel() requestSize := a.cfg.CollectDiskMetrics - inspect, _, err := a.docker.ContainerInspectWithRaw(containerCtx, summary.ID, requestSize) + inspect, err := a.inspectContainer(containerCtx, summary, requestSize) if err != nil { return agentsdocker.Container{}, fmt.Errorf("inspect: %w", err) } @@ -794,6 +795,82 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum return container, nil } +func (a *Agent) inspectContainer(ctx context.Context, summary containertypes.Summary, requestSize bool) (containertypes.InspectResponse, error) { + // Empty IDs/statuses do not provide enough evidence to detect a lifecycle + // change between reports (some Podman compatibility endpoints omit Status), + // so keep those on the live path rather than accepting bounded stale state. + cacheable := strings.TrimSpace(summary.ID) != "" && strings.TrimSpace(summary.Status) != "" && isInactiveContainerState(summary.State) + fingerprint := inactiveContainerInspectFingerprint(summary) + if cacheable { + a.inactiveInspectMu.Lock() + entry, ok := a.inactiveInspects[summary.ID] + if ok && entry.withSize == requestSize && entry.fingerprint == fingerprint && time.Now().Before(entry.expiresAt) { + inspect := entry.inspect + a.inactiveInspectMu.Unlock() + return inspect, nil + } + a.inactiveInspectMu.Unlock() + } + + inspect, _, err := a.docker.ContainerInspectWithRaw(ctx, summary.ID, requestSize) + if err != nil { + return containertypes.InspectResponse{}, err + } + + a.inactiveInspectMu.Lock() + defer a.inactiveInspectMu.Unlock() + if !cacheable || inspect.State == nil || inspect.State.Running || inspect.State.Paused || inspect.State.Restarting { + delete(a.inactiveInspects, summary.ID) + return inspect, nil + } + if a.inactiveInspects == nil { + a.inactiveInspects = make(map[string]inactiveContainerInspectCacheEntry) + } + a.inactiveInspects[summary.ID] = inactiveContainerInspectCacheEntry{ + inspect: inspect, + fingerprint: fingerprint, + withSize: requestSize, + expiresAt: time.Now().Add(dockerInactiveInspectRefreshInterval), + } + return inspect, nil +} + +func isInactiveContainerState(state containertypes.ContainerState) bool { + switch strings.ToLower(strings.TrimSpace(string(state))) { + case "created", "exited", "dead": + return true + default: + return false + } +} + +func inactiveContainerInspectFingerprint(summary containertypes.Summary) string { + // Status includes the latest exit result/time, so an inactive container + // that ran and stopped entirely between two reports cannot reuse its old + // exit code, restart count, or lifecycle timestamps. + return strings.ToLower(strings.TrimSpace(string(summary.State))) + "\x00" + strings.TrimSpace(summary.Status) +} + +func (a *Agent) pruneInactiveContainerInspectCache(active map[string]struct{}) { + a.inactiveInspectMu.Lock() + defer a.inactiveInspectMu.Unlock() + for containerID := range a.inactiveInspects { + if _, ok := active[containerID]; !ok { + delete(a.inactiveInspects, containerID) + } + } +} + +func (a *Agent) clearDockerCollectionCaches() { + a.storageUsageMu.Lock() + a.storageUsageCache = dockerStorageUsageCache{} + a.storageUsageMu.Unlock() + + a.inactiveInspectMu.Lock() + a.inactiveInspects = nil + a.inactiveInspectMu.Unlock() +} + var ( healthcheckURLPattern = regexp.MustCompile(`(?i)https?://[^\s'"<>]+`) healthcheckTargetPattern = regexp.MustCompile(`(?i)^[a-z0-9][a-z0-9._-]{0,252}$`) diff --git a/internal/dockeragent/hardening.go b/internal/dockeragent/hardening.go index d563f9f20..cd76b6a6a 100644 --- a/internal/dockeragent/hardening.go +++ b/internal/dockeragent/hardening.go @@ -22,10 +22,17 @@ const ( // daemons such as Synology DSM may traverse every stopped container layer // for this call, so immediate repetition can saturate dockerd (#1729). dockerStorageUsageRefreshInterval = 15 * time.Minute - dockerSwarmListCallTimeout = 20 * time.Second - dockerCleanupCallTimeout = 15 * time.Second - dockerUpdateCallTimeout = 2 * time.Minute - dockerUpdateOverallTimeout = 15 * time.Minute + // dockerInactiveInspectRefreshInterval keeps immutable container detail off + // the live telemetry cadence. A stopped-container-heavy NAS otherwise pays + // one /containers/{id}/json request per historical container every 30 + // seconds even though only running/paused containers need live state and + // stats. Summary changes invalidate the entry immediately; this ceiling + // bounds staleness for out-of-band network changes (#1729). + dockerInactiveInspectRefreshInterval = 15 * time.Minute + dockerSwarmListCallTimeout = 20 * time.Second + dockerCleanupCallTimeout = 15 * time.Second + dockerUpdateCallTimeout = 2 * time.Minute + dockerUpdateOverallTimeout = 15 * time.Minute // dockerCollectCycleTimeout bounds one whole collection cycle // (buildReport). Every docker call inside the cycle already carries its diff --git a/internal/dockeragent/runtime_reconnect.go b/internal/dockeragent/runtime_reconnect.go index 08e5eebac..87e1399c9 100644 --- a/internal/dockeragent/runtime_reconnect.go +++ b/internal/dockeragent/runtime_reconnect.go @@ -212,6 +212,7 @@ func (a *Agent) adoptRuntimeConnection(cli dockerClient, info systemtypes.Info, a.cfg.IncludeTasks = false } a.cfg.Runtime = string(runtimeKind) + a.clearDockerCollectionCaches() return previous }