fix(image): don't treat digest-only images as dangling in list output#6960
fix(image): don't treat digest-only images as dangling in list output#6960mohithshuka wants to merge 6 commits into
Conversation
Fixes #24643 --kernel-memory was deprecated in v20.10 and removed in v23.0. Add deprecation callouts to both the options table and the Kernel memory constraints section of the run reference page.
The --filter flag for 'docker volume prune' previously only showed 'label=<label>' as an example, not mentioning the negation form 'label!=<label>', even though it is valid and supported by the daemon. Fixes docker#6918 Signed-off-by: mohithshuka <153504854+mohithshuka@users.noreply.github.com>
When a port is bound on both 0.0.0.0 and ::, hide the IPv6 entry and show only the IPv4 entry to reduce noise in docker ps output. Before: 0.0.0.0:8080->80/tcp, :::8080->80/tcp After: 0.0.0.0:8080->80/tcp Fixes docker#6869
The --archive (-a) flag description was misleading. Updated to clarify that it preserves uid/gid from the source, and that it only takes effect when copying files TO a container (not from). Fixes docker#6870
Signed-off-by: mohithshuka <153504854+mohithshuka@users.noreply.github.com>
The local isDangling in list.go was missing the RepoDigests check, causing images with a digest but no tag to be incorrectly filtered from default 'docker images' output. Align with formatter.isDangling. Fixes docker#6650
There was a problem hiding this comment.
Pull request overview
Fixes incorrect filtering in docker images so digest-only images are not treated as dangling (and therefore not hidden), alongside several CLI help/doc updates and a docker ps port-formatting tweak.
Changes:
- Fix
cli/command/image/list.godangling detection to require both no tags and no digests. - Adjust
docker psport rendering to suppress duplicate IPv6 wildcard entries when an equivalent IPv4 wildcard mapping exists. - Update CLI help text and docs for
docker cp --archive,docker run --kernel-memorydeprecation, and volume prune filter examples.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/reference/run.md | Marks --kernel-memory as deprecated/removed and adds a deprecation callout in the kernel memory section. |
| docs/reference/commandline/cp.md | Updates generated docs text for docker cp --archive behavior. |
| docs/reference/commandline/container_cp.md | Updates generated docs text for docker container cp --archive behavior. |
| cli/command/volume/prune.go | Expands --filter help example to include label!=.... |
| cli/command/image/list.go | Fixes dangling filtering so digest-only images aren’t removed from default docker images output. |
| cli/command/formatter/container.go | Suppresses duplicate IPv6 wildcard port mappings in DisplayablePorts. |
| cli/command/container/cp.go | Updates --archive flag help text to clarify behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Pre-pass: record which (hostPort, privatePort, proto) tuples have an | ||
| // IPv4 wildcard (0.0.0.0) binding. Used below to suppress the matching | ||
| // IPv6 wildcard (::) entry, avoiding duplicate output such as: | ||
| // 0.0.0.0:8080->80/tcp, :::8080->80/tcp | ||
| // See: https://github.com/docker/cli/issues/6869 | ||
| type mappingKey struct { | ||
| hostPort uint16 | ||
| privatePort uint16 | ||
| proto string | ||
| } | ||
| ipv4Bindings := make(map[mappingKey]bool) | ||
| for _, port := range ports { | ||
| if port.IP.String() == "0.0.0.0" && port.PublicPort != 0 { | ||
| ipv4Bindings[mappingKey{port.PublicPort, port.PrivatePort, port.Type}] = true | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The PR title/description focus on fixing docker images filtering of digest-only images, but this PR also includes unrelated changes (port formatting suppression in DisplayablePorts, docs updates for --kernel-memory, docker cp flag text, and volume prune filter help). Please either split these into separate PRs or update the PR title/description to accurately describe the full scope, to make review and release notes clearer.
| // Pre-pass: record which (hostPort, privatePort, proto) tuples have an | ||
| // IPv4 wildcard (0.0.0.0) binding. Used below to suppress the matching | ||
| // IPv6 wildcard (::) entry, avoiding duplicate output such as: | ||
| // 0.0.0.0:8080->80/tcp, :::8080->80/tcp | ||
| // See: https://github.com/docker/cli/issues/6869 |
There was a problem hiding this comment.
The newly added comment examples show IPv6 wildcard bindings as :::8080->80/tcp, but the actual formatting produced here uses net.JoinHostPort, which renders IPv6 as [::]:8080->80/tcp (see existing tests expecting [::1]:...). Please update the examples so they match real output and avoid confusion when reading this code.
| @@ -373,6 +391,18 @@ func DisplayablePorts(ports []container.PortSummary) string { | |||
| portKey := port.Type | |||
| if port.IP.IsValid() { | |||
| if port.PublicPort != current { | |||
| // Suppress the IPv6 wildcard entry when an IPv4 wildcard | |||
| // entry already covers the same (hostPort, privatePort, proto) | |||
| // tuple. This merges: | |||
| // 0.0.0.0:8080->80/tcp, :::8080->80/tcp | |||
| // into the cleaner: | |||
| // 0.0.0.0:8080->80/tcp | |||
| if port.IP.Is6() && !port.IP.Is4In6() && port.IP.IsUnspecified() { | |||
| key := mappingKey{port.PublicPort, port.PrivatePort, port.Type} | |||
| if ipv4Bindings[key] { | |||
| continue | |||
| } | |||
| } | |||
There was a problem hiding this comment.
This change introduces new behavior (suppressing the IPv6 wildcard :: mapping when an equivalent IPv4 wildcard 0.0.0.0 mapping exists), but there is no test coverage for the duplicate-output case (e.g., 0.0.0.0:8080->80/tcp + [::]:8080->80/tcp). Please add/extend a TestDisplayablePorts case to lock in the intended suppression behavior and prevent regressions.
| if len(img.RepoTags) == 0 && len(img.RepoDigests) == 0 { | ||
| return true | ||
| } | ||
| return len(img.RepoTags) == 1 && img.RepoTags[0] == "<none>:<none>" && len(img.RepoDigests) == 1 && img.RepoDigests[0] == "<none>@<none>" |
There was a problem hiding this comment.
This PR changes the CLI-side dangling filtering so digest-only images are no longer removed from default docker images output. There are existing tests/goldens in cli/command/image/list_test.go (e.g., list-command-filter-dangling.no-dangling-filter.golden) that currently expect digest-only images to be hidden; those fixtures should be updated so the tests reflect the corrected behavior (digest-only images should remain visible when --all is not set and dangling filter is absent/false).
|
This needs a rebase as it has unrelated commits in the PR, but I think this was an intentional change; |
I've read the so-called "intentional" change, and I'm asking myself some serious questions :
Btw, just curious, but what is usually the review process for any breaking change on a critical component ? |
I'd also like to know what the process was for introducing changes like this that fundamentally change the expected behavior of important commands. I also had several hours of tearing my hair out trying to figure out why my untagged images weren't showing, and also have several pipelines using the cli to list images that broke with that change. Who wanted that change? What was the process for deciding if it was necessary? Same questions for the format change that was introduced, although at least that one has a configuration setting to revert back to the better behavior. |
Problem
docker imageswas hiding images that have a digest but no tag, treatingthem as dangling. This caused valid images to silently disappear from
default output.
The local
isDanglingfunction inlist.gowas missing thelen(img.RepoDigests) == 0check, so any image with no tags but a validdigest was incorrectly filtered out.
Fix
Align
list.go'sisDanglingwith the definition already correct informatter/image.go— an image is only dangling if it has both no tagsAND no digests.
Before / After
Before: Images with digest but no tag silently hidden from
docker imagesAfter: Those images appear in output as expected
Fixes #6650