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
This commit is contained in:
pulse-triage[bot]
2026-08-27 16:33:14 +01:00
parent f4e1e47e74
commit 735acbb05a
5 changed files with 147 additions and 19 deletions
@@ -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
@@ -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.
+18 -11
View File
@@ -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)
@@ -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{
+26 -5
View File
@@ -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),
})