Keep Docker update alerts open when the local digest is unknown

Docker update alerts resolved and re-fired a full delay later with no
fresh registry check in between (#2353). Image inspect shares the
15-second per-container collection budget with the sized container
inspect, stats and filesystem calls, so on a busy host it can time out.
The agent then compared an empty local digest, which reads as "not
different", and reported UpdateAvailable false with no error. The server
took that as recovery, resolved the alert and restarted the delay.

Agent: fall back to the last successful inspect of the same image ID
(an image ID names immutable content), pruned to the images in use. With
no earlier inspect, send no update status, which the server already
treats as unknown.

Server: a no-update status without a current digest is not a comparison,
so treat it like an absent or failed check. This also covers agents that
predate the fix. Only the agent produces these statuses and it always
sends the digest it compared; locally built images never assert an
update. The retention tests' clear fixtures now carry a current digest,
as real agents do.
This commit is contained in:
rcourtman
2026-10-06 09:59:07 +01:00
parent 1896f1f2ba
commit 2bd9000faf
10 changed files with 325 additions and 10 deletions
@@ -8752,3 +8752,12 @@ update or removal path changed.
webhook header, custom field and signing secret values per key on update and
form test, and to mask create and update responses. No agent registration,
enrolment, install, update or removal path changed.
### Docker update-check digest fallback only
`internal/dockeragent/collect.go` changed only so a failed image inspect
reuses the last successful local digest lookup for the same image ID, or sends
no update status, instead of reporting "no update available" (#2353). Agent
registration, enrolment, install, update, removal and report identity are
unchanged; the per-image memo lives in the registry checker and is pruned
each collection cycle to the images in use.
@@ -927,6 +927,26 @@ The saved observation is aged deliberately; this is deterministic source-level
restart/housekeeping proof, not a naturally elapsed day, live registry or
destination result, or reporter confirmation of #2353's full daily cycle.
### A "no update" report without a local digest is unknown
An image-update clear is a comparison: the agent must have known the local
digest it compared. Agents before #2353 reported `UpdateAvailable: false` with
an empty `CurrentDigest` when an image inspect failed, because an empty digest
compares as "not different". The server took that as recovery, resolving the
alert (attributed to `legacy-alert-recovery-adapter`, the default for clears
without explicit recovery evidence) and restarting the delay, so the alert
re-fired a full delay later with no fresh registry check in between.
`checkDockerContainerImageUpdate` now treats a no-update status without a
current digest like an absent or failed check: it refreshes `LastSeen` and
keeps the occurrence and its pending age. Only the agent produces these
statuses and it always sends the current digest when it knows it; locally
built images, which have none, never assert an update, so they lose nothing.
The "no update without a current digest" case of
`TestCheckDockerContainerImageUpdate` pins the guard and that a real
comparison still resolves; the retention and restart tests' clear fixtures
carry a current digest as real agents do. The agent side is in the monitoring
contract.
### Continuing unacknowledged alerts survive age-based cleanup
`MaxAlertAgeDays` removes an unacknowledged alert only when both its occurrence
@@ -5023,6 +5023,25 @@ reported as outdated. `TestRegistryChecker_MultipleLocalRepoDigestsSuppressFalse
`TestRegistryChecker_DigestsDiffer` cases pin the comparison. This is synthetic
registry-transport evidence, not reporter acceptance.
### A failed image inspect leaves the update status unknown
Image inspect shares the 15-second per-container collection budget with the
container inspect (sized when disk metrics are on), the one-shot stats call and
filesystem collection, so it can time out on a busy host. It previously
returned an empty local digest and no platform, and comparing that empty
digest reported `UpdateAvailable: false` with no error, which the alerts
runtime took as an affirmative clear of a pending update (#2353).
`localImageDigestsForUpdateCheck` now falls back to the last successful
inspect of the same image ID, kept by the registry checker and pruned to the
images of the collected containers; an image ID names immutable content. With
no earlier inspect the container carries no update status that cycle, which
the server treats as unknown. A locally built image (inspect succeeds, no
RepoDigests) keeps its existing status. `getImageRepoDigests` keeps its
signature for its other callers.
`TestCollectContainerKeepsUpdateStatusWhenImageInspectFails` pins both the
fallback and the unknown case, with the registry answer seeded offline; on the
previous code it reproduces the false no-update status.
### Forked host reports cannot bypass a base-identity removal block
Host report admission resolves identity before consulting removal blocks, so a
@@ -128,7 +128,7 @@ func TestCleanupContinuingDockerUpdateKeepsOccurrence(t *testing.T) {
if len(restarted.GetActiveAlerts()) != 1 || afterRestartFirings.Load() != 0 || afterRestartDeliveries.Load() != 0 {
t.Fatal("restart/unknown report lost or re-fired the occurrence")
}
container.UpdateStatus = &models.DockerContainerUpdateStatus{LastChecked: time.Now()}
container.UpdateStatus = &models.DockerContainerUpdateStatus{CurrentDigest: "sha256:new", LatestDigest: "sha256:new", LastChecked: time.Now()}
restarted.checkDockerContainerImageUpdate(host, container, resourceID, container.Name, host.Hostname, host.Hostname)
if len(restarted.GetActiveAlerts()) != 0 || afterRestartResolutions.Load() != 1 || afterRestartDeliveries.Load() != 0 {
t.Fatal("affirmative clear did not resolve once without another firing notification")
+9
View File
@@ -1369,6 +1369,15 @@ func (m *Manager) checkDockerContainerImageUpdate(host models.DockerHost, contai
return
}
// "No update" is a comparison result only when the agent knew the local
// digest. Agents before #2353 compared an empty digest after a failed image
// inspect and reported no update, which resolved a pending alert and
// restarted its delay. Without a current digest the result is unknown.
if !container.UpdateStatus.UpdateAvailable && strings.TrimSpace(container.UpdateStatus.CurrentDigest) == "" {
m.touchDockerContainerUpdateAlert(canonicalAlertID)
return
}
// Check if an update is available
if !container.UpdateStatus.UpdateAvailable {
// No update available - clear tracking and alert
@@ -48,7 +48,7 @@ func TestDockerUpdateTrackingSurvivesDailyCleanup(t *testing.T) {
t.Fatal("failed check reset pending age")
}
// An affirmative clear remains effective.
container.UpdateStatus = &models.DockerContainerUpdateStatus{UpdateAvailable: false, LastChecked: time.Now()}
container.UpdateStatus = &models.DockerContainerUpdateStatus{UpdateAvailable: false, CurrentDigest: "sha256:new", LatestDigest: "sha256:new", LastChecked: time.Now()}
check()
if len(m.GetActiveAlerts()) != 0 || len(m.dockerUpdateFirstSeen) != 0 || len(m.dockerUpdateFirstSeenByIdentity) != 0 || len(m.dockerUpdateLastObserved) != 0 {
t.Fatal("affirmative clear did not retire update")
+69 -1
View File
@@ -280,6 +280,74 @@ func TestCheckDockerContainerImageUpdate(t *testing.T) {
}
})
// Agents before #2353 reported "no update" with no current digest when an
// image inspect failed. That is not a comparison, so it must not resolve the
// alert and restart a whole delay window.
t.Run("no update without a current digest preserves existing alert and tracking", func(t *testing.T) {
testResourceID := "docker:" + hostID + "/container-no-local-digest"
container := models.DockerContainer{
ID: "container-no-local-digest",
Name: "no-local-digest-container",
Image: "mongo:7",
UpdateStatus: &models.DockerContainerUpdateStatus{
UpdateAvailable: true,
CurrentDigest: "sha256:old",
LatestDigest: "sha256:new",
LastChecked: time.Now(),
},
}
trackingKey := dockerUpdateTrackingKey(host, container)
firstSeen := time.Now().Add(-25 * time.Hour)
m.mu.Lock()
m.dockerUpdateFirstSeen[testResourceID] = firstSeen
m.dockerUpdateFirstSeenByIdentity[trackingKey] = firstSeen
m.mu.Unlock()
m.checkDockerContainerImageUpdate(host, container, testResourceID, "no-local-digest-container", instanceName, nodeName)
canonicalAlertID := buildCanonicalStateID(testResourceID, testResourceID+"-image-update")
m.mu.RLock()
alert := testRequireActiveAlert(t, m, canonicalAlertID)
if alert == nil {
m.mu.RUnlock()
t.Fatalf("expected update alert %q to be active", canonicalAlertID)
}
startTime := alert.StartTime
m.mu.RUnlock()
container.UpdateStatus = &models.DockerContainerUpdateStatus{
UpdateAvailable: false,
LatestDigest: "sha256:new",
LastChecked: time.Now(),
}
m.checkDockerContainerImageUpdate(host, container, testResourceID, "no-local-digest-container", instanceName, nodeName)
m.mu.RLock()
alert = testRequireActiveAlert(t, m, canonicalAlertID)
var stillStarted time.Time
if alert != nil {
stillStarted = alert.StartTime
}
trackedAge := m.dockerUpdateFirstSeenByIdentity[trackingKey]
m.mu.RUnlock()
if alert == nil {
t.Fatal("a no-update report without a current digest resolved the alert")
}
if !stillStarted.Equal(startTime) || !trackedAge.Equal(firstSeen) {
t.Fatalf("pending age restarted: alert start %s -> %s, tracking %s", startTime, stillStarted, trackedAge)
}
container.UpdateStatus = &models.DockerContainerUpdateStatus{
UpdateAvailable: false,
CurrentDigest: "sha256:new",
LatestDigest: "sha256:new",
LastChecked: time.Now(),
}
m.checkDockerContainerImageUpdate(host, container, testResourceID, "no-local-digest-container", instanceName, nodeName)
if hasAlertWithPrefix(m.GetActiveAlerts(), canonicalAlertID) {
t.Fatal("a real comparison showing the image current did not resolve the alert")
}
})
t.Run("update detection errors preserve existing alert and tracking", func(t *testing.T) {
testResourceID := "docker:" + hostID + "/container-error-status"
container := models.DockerContainer{
@@ -910,7 +978,7 @@ func TestDockerUpdateRestartRestoresActivePendingAge(t *testing.T) {
// Explicit recovery retires age; another pending update must wait
// its normal delay rather than reuse the resolved occurrence.
host.Containers[0].UpdateStatus = &models.DockerContainerUpdateStatus{LastChecked: time.Now()}
host.Containers[0].UpdateStatus = &models.DockerContainerUpdateStatus{CurrentDigest: "sha256:new", LatestDigest: "sha256:new", LastChecked: time.Now()}
restarted.CheckDockerHost(host)
if len(restarted.GetActiveAlerts()) != 0 || resolutions.Load() != 1 {
t.Fatal("affirmative recovery did not resolve exactly once")
@@ -803,6 +803,97 @@ func TestAgentClose(t *testing.T) {
}
}
// A failed image inspect leaves the local digest unknown. Comparing an empty
// digest reported "no update available", which the server treats as an
// affirmative clear, so a pending update alert resolved and re-fired a whole
// delay later (#2353). The last good inspect of the same image ID stands in;
// without one the status stays unknown.
func TestCollectContainerKeepsUpdateStatusWhenImageInspectFails(t *testing.T) {
const imageID = "sha256:mongo7local"
inspect := baseInspect()
inspect.State = &containertypes.State{Running: false, Status: "exited"}
inspect.Config.Image = "mongo:7"
summary := containertypes.Summary{
ID: "container-mongo",
Names: []string{"/unifi-db"},
Image: "mongo:7",
ImageID: imageID,
State: "exited",
Status: "Exited (0) 1 minute ago",
}
newChecker := func(t *testing.T) *RegistryChecker {
checker := NewRegistryChecker(zerolog.Nop())
checker.httpClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
t.Errorf("unexpected registry request to %s", req.URL)
return nil, errors.New("offline")
})}
registry, repository, tag := parseImageReference("mongo:7")
checker.cacheDigestResult(registry+"/"+repository+":"+tag+"|amd64/linux/", "sha256:new", "sha256:new")
// A failed inspect also loses the platform; seed that lookup too so the
// registry answer is available offline, as it would be in production.
checker.cacheDigestResult(registry+"/"+repository+":"+tag+"|//", "sha256:new", "sha256:new")
return checker
}
inspectFails := false
newAgent := func(checker *RegistryChecker) *Agent {
return &Agent{
logger: zerolog.Nop(),
prevContainerCPU: make(map[string]cpuSample),
registryChecker: checker,
docker: &fakeDockerClient{
containerInspectWithRawFn: func(context.Context, string, bool) (containertypes.InspectResponse, []byte, error) {
return inspect, nil, nil
},
imageInspectWithRawFn: func(context.Context, string) (imagetypes.InspectResponse, []byte, error) {
if inspectFails {
return imagetypes.InspectResponse{}, nil, context.DeadlineExceeded
}
return imagetypes.InspectResponse{
RepoDigests: []string{"docker.io/library/mongo@sha256:old"},
Architecture: "amd64",
Os: "linux",
}, nil, nil
},
},
}
}
checker := newChecker(t)
agent := newAgent(checker)
container, err := agent.collectContainer(context.Background(), summary)
if err != nil {
t.Fatalf("collect: %v", err)
}
if status := container.UpdateStatus; status == nil || !status.UpdateAvailable || status.CurrentDigest != "sha256:old" {
t.Fatalf("baseline update status = %+v, want an available update from sha256:old", status)
}
inspectFails = true
container, err = agent.collectContainer(context.Background(), summary)
if err != nil {
t.Fatalf("collect with failed image inspect: %v", err)
}
if status := container.UpdateStatus; status == nil || !status.UpdateAvailable || status.Error != "" || status.CurrentDigest != "sha256:old" {
t.Fatalf("failed image inspect reported %+v, want the same available update", status)
}
// A fresh agent has no earlier inspect to fall back on.
container, err = newAgent(newChecker(t)).collectContainer(context.Background(), summary)
if err != nil {
t.Fatalf("collect without a remembered image: %v", err)
}
if container.UpdateStatus != nil {
t.Fatalf("unknown local digest reported %+v, want no update status", container.UpdateStatus)
}
// Images no longer used by a collected container are forgotten.
checker.pruneLocalImages(map[string]struct{}{"sha256:other": {}})
if _, ok := checker.lastLocalImage(imageID); ok {
t.Fatal("pruned image digests are still remembered")
}
}
func TestAgentCollectImageRepoDigestsIncludesEveryLocalDigest(t *testing.T) {
agent := &Agent{
docker: &fakeDockerClient{
+48 -7
View File
@@ -299,6 +299,7 @@ func (a *Agent) collectContainers(ctx context.Context) ([]agentsdocker.Container
containers := make([]agentsdocker.Container, 0, len(list))
active := make(map[string]struct{}, len(list))
activeImages := make(map[string]struct{}, len(list))
for _, summary := range list {
if len(a.allowedStates) > 0 {
if _, ok := a.allowedStates[strings.ToLower(string(summary.State))]; !ok {
@@ -312,6 +313,7 @@ func (a *Agent) collectContainers(ctx context.Context) ([]agentsdocker.Container
}
active[summary.ID] = struct{}{}
activeImages[summary.ImageID] = struct{}{}
container, err := a.collectContainer(ctx, summary)
if err != nil {
@@ -322,6 +324,7 @@ func (a *Agent) collectContainers(ctx context.Context) ([]agentsdocker.Container
}
a.pruneStaleCPUSamples(active)
a.pruneInactiveContainerInspectCache(active)
a.registryChecker.pruneLocalImages(activeImages)
return containers, nil
}
@@ -779,7 +782,8 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum
// Get the actual manifest digest (RepoDigest) from the image for accurate comparison.
// The ImageID is a local content-addressable ID that differs from the registry manifest digest.
// We also get the architecture details to correctly resolve manifest lists from the registry.
currentDigests, arch, os, variant := a.getImageRepoDigests(containerCtx, summary.ImageID, summary.Image)
local, localKnown := a.localImageDigestsForUpdateCheck(containerCtx, summary.ImageID, summary.Image)
currentDigests, arch, os, variant := local.digests, local.arch, local.os, local.variant
digestForComparison := ""
if len(currentDigests) > 0 {
digestForComparison = currentDigests[0]
@@ -806,6 +810,15 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum
Error: "digest-pinned image",
}
// Skip to end of update check block - don't call registry
} else if !localKnown {
// Without the local digest no comparison is possible. Comparing an
// empty digest reports "no update", which the server takes as an
// affirmative clear: a pending update alert resolved and re-fired a
// full delay later (#2353). Leave the status unknown this cycle.
a.logger.Debug().
Str("container", container.Name).
Str("image", imageToCheck).
Msg("Skipping update check: local image digest unavailable")
} else {
a.logger.Debug().
Str("container", container.Name).
@@ -982,6 +995,31 @@ func (a *Agent) getImageRepoDigest(ctx context.Context, imageID, imageName strin
// tag and a platform manifest), so an update check must treat the whole set as
// the local identity rather than trusting one arbitrary entry (#2110).
func (a *Agent) getImageRepoDigests(ctx context.Context, imageID, imageName string) ([]string, string, string, string) {
local, err := a.inspectLocalImageDigests(ctx, imageID, imageName)
if err != nil {
return nil, "", "", ""
}
return local.digests, local.arch, local.os, local.variant
}
// localImageDigestsForUpdateCheck returns the local digests to compare against
// the registry. Image inspect shares the per-container collection budget, so it
// can time out on a busy host; the last successful inspect of the same image
// ID stands in, since an image ID names immutable content. known is false only
// when neither is available.
func (a *Agent) localImageDigestsForUpdateCheck(ctx context.Context, imageID, imageName string) (localImageDigests, bool) {
local, err := a.inspectLocalImageDigests(ctx, imageID, imageName)
if err == nil {
a.registryChecker.rememberLocalImage(imageID, local)
return local, true
}
return a.registryChecker.lastLocalImage(imageID)
}
// inspectLocalImageDigests is getImageRepoDigests with the inspect failure
// kept distinct from an image that simply has no RepoDigests (a locally built
// image), so update checks can treat the first as unknown.
func (a *Agent) inspectLocalImageDigests(ctx context.Context, imageID, imageName string) (localImageDigests, error) {
imageInspect, _, err := a.docker.ImageInspectWithRaw(ctx, imageID)
if err != nil {
a.logger.Debug().
@@ -989,16 +1027,18 @@ func (a *Agent) getImageRepoDigests(ctx context.Context, imageID, imageName stri
Str("imageID", imageID).
Str("imageName", imageName).
Msg("Failed to inspect image for RepoDigest")
return nil, "", "", ""
return localImageDigests{}, err
}
arch := imageInspect.Architecture
os := imageInspect.Os
variant := imageInspect.Variant
local := localImageDigests{
arch: imageInspect.Architecture,
os: imageInspect.Os,
variant: imageInspect.Variant,
}
if len(imageInspect.RepoDigests) == 0 {
// Locally built images won't have RepoDigests
return nil, arch, os, variant
return local, nil
}
matched := make([]string, 0, len(imageInspect.RepoDigests))
@@ -1020,7 +1060,8 @@ func (a *Agent) getImageRepoDigests(ctx context.Context, imageID, imageName stri
}
}
return append(matched, unmatched...), arch, os, variant
local.digests = append(matched, unmatched...)
return local, nil
}
func addrString(addr netip.Addr) string {
+58
View File
@@ -23,6 +23,7 @@ type RegistryChecker struct {
// credentials optionally resolves host Docker credentials so private
// registries can be checked; nil keeps every lookup anonymous.
credentials registryCredentialSource
localImages localImageMemo
logger zerolog.Logger
mu sync.RWMutex
@@ -32,6 +33,63 @@ type RegistryChecker struct {
lastFullCheck time.Time
}
// localImageDigests is the local side of an update comparison: the image's
// RepoDigests and the platform used to resolve a manifest list.
type localImageDigests struct {
digests []string
arch string
os string
variant string
}
// localImageMemo keeps the last successful local digest lookup per image ID,
// the fallback when an inspect fails. It holds only images in use by the
// collected containers.
type localImageMemo struct {
entries map[string]localImageDigests
mu sync.Mutex
}
func (r *RegistryChecker) rememberLocalImage(imageID string, local localImageDigests) {
if r == nil || imageID == "" {
return
}
r.localImages.mu.Lock()
defer r.localImages.mu.Unlock()
if r.localImages.entries == nil {
r.localImages.entries = make(map[string]localImageDigests)
}
local.digests = append([]string(nil), local.digests...)
r.localImages.entries[imageID] = local
}
func (r *RegistryChecker) lastLocalImage(imageID string) (localImageDigests, bool) {
if r == nil || imageID == "" {
return localImageDigests{}, false
}
r.localImages.mu.Lock()
defer r.localImages.mu.Unlock()
local, ok := r.localImages.entries[imageID]
if !ok {
return localImageDigests{}, false
}
local.digests = append([]string(nil), local.digests...)
return local, true
}
func (r *RegistryChecker) pruneLocalImages(active map[string]struct{}) {
if r == nil {
return
}
r.localImages.mu.Lock()
defer r.localImages.mu.Unlock()
for imageID := range r.localImages.entries {
if _, ok := active[imageID]; !ok {
delete(r.localImages.entries, imageID)
}
}
}
// digestCache provides thread-safe caching of digest lookups.
type digestCache struct {
entries map[string]cacheEntry