fix(docker): clear the update-available flag after an out-of-band image update - #2723
Conversation
…ge update reloadUpdateStatus() took the local digest from the on-disk cache and only inspected the image when that entry was missing, so once an image had been checked its cached digest was trusted forever. Replacing an image outside the Docker tab (docker pull, or a recreate through the API/CLI) never refreshed it, so the tab stayed on "update available" and the update check kept notifying. Check for Updates could not clear it either, since it runs the same code. Inspect the local image on every check and fall back to the cached digest only for images that carry no repo digest, such as locally built ones. Making the live inspect authoritative also exposed which digest inspectLocalVersion() picks. An image can carry several digests for the same repo, because a multiarch tag that was re-pushed keeps the older manifest list as well, and the tag does not always resolve to the last entry. redis:latest currently resolves to RepoDigests[0], so always taking the last one reported a spurious update. Prefer the digest the tag resolves to when the image carries it, and keep the last entry as the fallback. Fixes unraid#2710
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughDocker update checks now retrieve the remote digest first, re-inspect local images on every reload, preserve cached values only when inspection returns null, and match remote digests against all valid local repository digests. ChangesDocker update status
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Mergeability Score: ⚪ Minimal · up to The change refreshes Docker image digests so out-of-band updates correctly clear stale update notices; no actionable merge-blocking risk remains beyond normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Fixes #2710.
DockerUpdate::reloadUpdateStatus()read the local digest out of/var/lib/docker/unraid-update-status.jsonand only calledinspectLocalVersion()when that entry was missing. Once an image had been checked, its cached digest was trusted forever. Replacing the image outside the Docker tab (adocker pull, or a recreate driven through the API or CLI) never refreshed it, so the tab stayed on "update available" andscripts/dockerupdatekept sending a version notification on every run. "Check for Updates" could not clear it either, because it reaches the same code throughgetAllInfo(true).The fix is to inspect the local image on every check and keep the cached digest only as a fallback for images that carry no repo digest, such as locally built ones. The inspect is a local Docker socket call, so this costs nothing next to
getRemoteVersionV2(), which the same loop already calls unconditionally.Making the live inspect authoritative uncovered a second problem in
inspectLocalVersion(). An image can carry several digests for the same repo, because a multiarch tag that was re-pushed keeps the older manifest list as well, and the tag does not always resolve to the last one. On my serverredis:latesthas two:and
docker buildx imagetools inspect redis:latestresolves tosha256:344e3945a0b4, the first entry. Taking the last entry unconditionally (#2581) therefore reports a spurious update as soon as the digest is read live instead of from the cache. SoinspectLocalVersion()now takes the digest the tag actually resolves to when the local image carries it, and keeps the last entry as the fallback when it does not. Membership is the right test here: if the registry's current digest is among the image's repo digests, the local image is what the tag points at.inspectLocalVersion()gained an optional second parameter, so existing one-argument callers are unaffected.Verification
Tested on Unraid 7.3.2, PHP 8.4.23, against the real Docker socket and the real registry, with the update-status cache redirected to a scratch file.
End to end, with no mocking at all: tag a throwaway
alpine:3.19to an older image, run the check, then update the image out of band withdocker pulland run the check again, which is exactly what "Check for Updates" does.A case matrix driven through
reloadUpdateStatus()with the registry digest as a controlled input passes on the patched code and fails on master for the out-of-band cases:To check for false positives I compared every image on the server, 53 in total, cached digest against live inspect: 48 identical, so the patch changes nothing for them, 3 with a stale cache entry, and 2 without a repo digest that correctly keep their cached value. Resolving those against the real registry, master and the patched code report the same status for all of them, including
redis:latest, which stays up-to-date only because of the digest preference described above.Summary by CodeRabbit