From 735acbb05a435a2c767f3dc0cc26ec7900ee3385 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:33:14 +0100 Subject: [PATCH] fix(dockeragent): bound image storage computation Keep live image identity fresh without recomputing shared layer sizes on every report. Reuse the throttled storage snapshot and qualify the reported Synology inventory shape. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 7 +- .../v6/internal/subsystems/monitoring.md | 8 +- internal/dockeragent/agent_collect_test.go | 29 +++--- .../dockeragent/agent_report_coverage_test.go | 91 +++++++++++++++++++ internal/dockeragent/collect.go | 31 ++++++- 5 files changed, 147 insertions(+), 19 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 3e0257670..e088f8533 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -6632,8 +6632,11 @@ telemetry at its configured interval, while the daemon-wide verbose storage inventory is cached for 15 minutes. A storage refresh is single-attempt and retains the last successful aggregate after failure, so a slow appliance daemon cannot be held busy by an immediate retry followed by another scan on -the next report tick. This changes neither agent enrollment nor report -authority; it bounds the collection work attached to that lifecycle (#1729). +the next report tick. Live image-list calls do not request Docker's optional +shared-size computation; they keep image identity fresh while projecting size +and container-count detail from that cached storage snapshot. This changes +neither agent enrollment nor report authority; it bounds the collection work +attached to that lifecycle (#1729). ### Host removal blocks clear from every store on re-enroll diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index f1d7d73f6..6b8cfa515 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -3245,7 +3245,13 @@ daemons such as Synology DSM when many stopped containers exist. It runs once per refresh window with no immediate transient retry, preserves the last good aggregate across a failed refresh, and suppresses a failed cold-start scan until the next window instead of starting it again on every 30-second report -(#1729). `TestCollectStorageUsageDecouplesFullDaemonScanFromLiveTelemetry` and +(#1729). Live image-list requests also leave Docker's optional `shared-size` +calculation disabled; image IDs, tags, and digests remain fresh each report, +while shared-layer bytes and container counts come from the cached storage +snapshot. `TestBuildReportSynologySizedInventoryBoundsStorageComputations` +qualifies two live cycles over the reported 47-container/4-running inventory +shape and pins one full storage walk with no additional shared-size request. +`TestCollectStorageUsageDecouplesFullDaemonScanFromLiveTelemetry` and `TestCollectStorageUsageThrottlesInitialTransientFailureWithoutRetry` pin the cadence, stale-result continuity, and no-retry boundary. diff --git a/internal/dockeragent/agent_collect_test.go b/internal/dockeragent/agent_collect_test.go index 82b725b5c..5e7f2cc63 100644 --- a/internal/dockeragent/agent_collect_test.go +++ b/internal/dockeragent/agent_collect_test.go @@ -133,14 +133,17 @@ func TestCollectDockerNativeInventory(t *testing.T) { agent := &Agent{ logger: zerolog.Nop(), docker: &fakeDockerClient{ - imageListFn: func(context.Context, dockerImageListOptions) ([]imagetypes.Summary, error) { + imageListFn: func(_ context.Context, opts dockerImageListOptions) ([]imagetypes.Summary, error) { + if opts.SharedSize { + t.Fatal("live image inventory must not request shared-size computation") + } return []imagetypes.Summary{{ ID: " sha256:image1 ", RepoTags: []string{"repo/app:latest", " "}, RepoDigests: []string{"repo/app@sha256:abc"}, Size: 1024, - SharedSize: 256, - Containers: 2, + SharedSize: -1, + Containers: -1, Created: createdAt.Unix(), Labels: map[string]string{"tier": "web"}, }}, nil @@ -176,6 +179,7 @@ func TestCollectDockerNativeInventory(t *testing.T) { return dockerclient.DiskUsageResult{ Images: dockerclient.ImagesDiskUsage{ TotalCount: 3, ActiveCount: 2, TotalSize: 4096, Reclaimable: 512, + Items: []imagetypes.Summary{{ID: "sha256:image1", SharedSize: 256, Containers: 2}}, }, Volumes: dockerclient.VolumesDiskUsage{ TotalCount: 1, @@ -189,7 +193,15 @@ func TestCollectDockerNativeInventory(t *testing.T) { }, } - images, err := agent.collectImages(context.Background()) + usageResult, storageUsage, err := agent.collectStorageUsage(context.Background()) + if err != nil { + t.Fatalf("collectStorageUsage: %v", err) + } + if storageUsage.Images.TotalCount != 3 || storageUsage.Images.ReclaimableBytes != 512 { + t.Fatalf("unexpected storage usage: %+v", storageUsage) + } + + images, err := agent.collectImages(context.Background(), usageResult.Images.Items) if err != nil { t.Fatalf("collectImages: %v", err) } @@ -199,13 +211,8 @@ func TestCollectDockerNativeInventory(t *testing.T) { if len(images[0].RepoTags) != 1 || images[0].RepoTags[0] != "repo/app:latest" { t.Fatalf("expected normalized repo tags, got %#v", images[0].RepoTags) } - - usageResult, storageUsage, err := agent.collectStorageUsage(context.Background()) - if err != nil { - t.Fatalf("collectStorageUsage: %v", err) - } - if storageUsage.Images.TotalCount != 3 || storageUsage.Images.ReclaimableBytes != 512 { - t.Fatalf("unexpected storage usage: %+v", storageUsage) + if images[0].SharedSizeBytes != 256 || images[0].Containers != 2 { + t.Fatalf("image storage projection = %+v, want cached shared-size and container counts", images[0]) } volumes, err := agent.collectVolumes(context.Background(), usageResult.Volumes.Items) diff --git a/internal/dockeragent/agent_report_coverage_test.go b/internal/dockeragent/agent_report_coverage_test.go index 790221444..c9aa08024 100644 --- a/internal/dockeragent/agent_report_coverage_test.go +++ b/internal/dockeragent/agent_report_coverage_test.go @@ -3,16 +3,107 @@ package dockeragent import ( "context" "errors" + "fmt" "testing" "time" containertypes "github.com/moby/moby/api/types/container" + imagetypes "github.com/moby/moby/api/types/image" swarmtypes "github.com/moby/moby/api/types/swarm" systemtypes "github.com/moby/moby/api/types/system" + dockerclient "github.com/moby/moby/client" "github.com/rcourtman/pulse-go-rewrite/internal/hostmetrics" "github.com/rs/zerolog" ) +func TestBuildReportSynologySizedInventoryBoundsStorageComputations(t *testing.T) { + swap(t, &hostmetricsCollect, func(context.Context, []string) (hostmetrics.Snapshot, error) { + return hostmetrics.Snapshot{}, nil + }) + + const ( + containerCount = 47 + runningCount = 4 + ) + containers := make([]containertypes.Summary, 0, containerCount) + running := make(map[string]bool, runningCount) + for i := 0; i < containerCount; i++ { + id := fmt.Sprintf("container-%02d", i) + state := containertypes.ContainerState("exited") + if i < runningCount { + state = "running" + running[id] = true + } + containers = append(containers, containertypes.Summary{ + ID: id, + Names: []string{"/" + id}, + State: state, + }) + } + + var diskUsageCalls, imageListCalls, sharedSizeRequests int + agent := &Agent{ + cfg: Config{ + Interval: 30 * time.Second, + IncludeContainers: true, + }, + runtime: RuntimeDocker, + logger: zerolog.Nop(), + prevContainerCPU: make(map[string]cpuSample), + docker: &fakeDockerClient{ + infoFunc: func(context.Context) (systemtypes.Info, error) { + return systemtypes.Info{ID: "synology-daemon", Name: "synology", ServerVersion: "24.0.0"}, nil + }, + containerListFunc: func(context.Context, dockerContainerListOptions) ([]containertypes.Summary, error) { + return containers, nil + }, + containerInspectWithRawFn: func(_ context.Context, id string, size bool) (containertypes.InspectResponse, []byte, error) { + if size { + t.Fatal("normal unified-agent reports must not request per-container size walks") + } + return containertypes.InspectResponse{ + State: &containertypes.State{Running: running[id]}, + Config: &containertypes.Config{}, + }, nil, nil + }, + containerStatsOneShotFn: func(context.Context, string) (dockerStatsResponseReader, error) { + return statsReader(t, containertypes.StatsResponse{}), nil + }, + diskUsageFn: func(context.Context, dockerDiskUsageOptions) (dockerclient.DiskUsageResult, error) { + diskUsageCalls++ + return dockerclient.DiskUsageResult{}, nil + }, + imageListFn: func(_ context.Context, opts dockerImageListOptions) ([]imagetypes.Summary, error) { + imageListCalls++ + if opts.SharedSize { + sharedSizeRequests++ + } + return nil, nil + }, + }, + } + + for cycle := 1; cycle <= 2; cycle++ { + report, err := agent.buildReport(context.Background()) + if err != nil { + t.Fatalf("build report cycle %d: %v", cycle, err) + } + if len(report.Containers) != containerCount { + t.Fatalf("cycle %d containers = %d, want %d", cycle, len(report.Containers), containerCount) + } + } + + if diskUsageCalls != 1 { + t.Fatalf("full daemon storage walks = %d, want one across two live telemetry cycles", diskUsageCalls) + } + if imageListCalls != 2 { + t.Fatalf("fresh image inventory calls = %d, want one per live telemetry cycle", imageListCalls) + } + if sharedSizeRequests != 0 { + t.Fatalf("live image inventory requested %d shared-size computations, want none", sharedSizeRequests) + } +} + func TestBuildReport_RuntimeChangePodman(t *testing.T) { swap(t, &hostmetricsCollect, func(context.Context, []string) (hostmetrics.Snapshot, error) { return hostmetrics.Snapshot{ diff --git a/internal/dockeragent/collect.go b/internal/dockeragent/collect.go index b7221c624..269255dbc 100644 --- a/internal/dockeragent/collect.go +++ b/internal/dockeragent/collect.go @@ -136,7 +136,7 @@ func (a *Agent) buildReport(ctx context.Context) (agentsdocker.Report, error) { if err != nil { a.logger.Warn().Err(err).Msg("failed to collect Docker storage usage") } - images, err := a.collectImages(ctx) + images, err := a.collectImages(ctx, diskUsageResult.Images.Items) if err != nil { a.logger.Warn().Err(err).Msg("failed to collect Docker images") } @@ -317,9 +317,20 @@ func (a *Agent) collectContainers(ctx context.Context) ([]agentsdocker.Container return containers, nil } -func (a *Agent) collectImages(ctx context.Context) ([]agentsdocker.Image, error) { +func (a *Agent) collectImages(ctx context.Context, usageImages []image.Summary) ([]agentsdocker.Image, error) { + usageByID := make(map[string]image.Summary, len(usageImages)) + for _, summary := range usageImages { + if id := strings.TrimSpace(summary.ID); id != "" { + usageByID[id] = summary + } + } + list, err := dockerCallWithRetry(ctx, dockerInventoryCallTimeout, func(callCtx context.Context) ([]image.Summary, error) { - return a.docker.ImageList(callCtx, dockerImageListOptions{All: true, SharedSize: true}) + // Keep image identity and tags on the live telemetry cadence without + // asking the daemon to recompute shared layer sizes every 30 seconds. + // The throttled /system/df snapshot above already owns that expensive + // calculation and supplies its cached values here (#1729). + return a.docker.ImageList(callCtx, dockerImageListOptions{All: true}) }) if err != nil { return nil, annotateDockerConnectionError(err) @@ -327,13 +338,23 @@ func (a *Agent) collectImages(ctx context.Context) ([]agentsdocker.Image, error) images := make([]agentsdocker.Image, 0, len(list)) for _, summary := range list { + sharedSize := summary.SharedSize + containers := summary.Containers + if usage, ok := usageByID[strings.TrimSpace(summary.ID)]; ok { + sharedSize = usage.SharedSize + containers = usage.Containers + } + // Docker uses -1 when these optional calculations were not requested. + // Do not project that sentinel as a real negative byte/container count. + sharedSize = max(sharedSize, 0) + containers = max(containers, 0) images = append(images, agentsdocker.Image{ ID: strings.TrimSpace(summary.ID), RepoTags: normalizedNonEmptyStrings(summary.RepoTags), RepoDigests: normalizedNonEmptyStrings(summary.RepoDigests), SizeBytes: summary.Size, - SharedSizeBytes: summary.SharedSize, - Containers: summary.Containers, + SharedSizeBytes: sharedSize, + Containers: containers, CreatedAt: dockerUnixTimestamp(summary.Created), Labels: cloneStringMap(summary.Labels), })