Add reference filter and deprecated filter param… #27872

vdemeester · 2016-10-29T00:13:47Z

Add reference filter and deprecated filter param for /images/json – it was a little bit too confusing for me between filter and filters.

This deprecates the filter param for the /images/json endpoint and make a new filter called reference to replace it. It does change the CLI side (still possible to do docker images busybox:musl) but changes the cli code to use the filter instead (so that docker images --filter reference=busybox:musl and docker images busybox:musl act the same).

$ docker images busybox
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
busybox             uclibc              e02e811dd08f        3 weeks ago         1.093 MB
busybox             musl                733eb3059dce        3 weeks ago         1.213 MB
busybox             glibc               21c16b6787c6        3 weeks ago         4.187 MB
busybox             latest              2b8fd9751c4c        4 months ago        1.093 MB
$ docker images --filter reference=busybox
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
busybox             uclibc              e02e811dd08f        3 weeks ago         1.093 MB
busybox             musl                733eb3059dce        3 weeks ago         1.213 MB
busybox             glibc               21c16b6787c6        3 weeks ago         4.187 MB
busybox             latest              2b8fd9751c4c        4 months ago        1.093 MB
# […]
$ docker images busybox:uclibc
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
busybox             uclibc              e02e811dd08f        3 weeks ago         1.093 MB
$ docker images --filter reference=busybox:uclibc
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
busybox             uclibc              e02e811dd08f        3 weeks ago         1.093 MB
$ docker images busyb
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
$ docker images --filter reference=busyb
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE

Validate the new filter (name, …) 👼
Add tests for the new filter
Add docs for the new filter

/cc @thaJeztah @cpuguy83 @tiborvass @stevvooe

🐸

Signed-off-by: Vincent Demeester vincent@sbr.pm

thaJeztah · 2016-10-29T00:15:03Z

design LGTM

vdemeester · 2016-10-29T05:04:47Z

All right, so docker images … support a little bit of globbing…

λ docker images "centos/*a*"                                                                                                                          
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
centos/mariadb      latest              23d3ef33ce06        3 months ago        370.6 MB
centos/etherpad     latest              097d06617332        3 months ago        378.1 MB

I need to update the PR code to re-use path instead of the reference package 😓

stevvooe · 2016-11-03T02:37:12Z

@vdemeester Might be worthwhile to bolster the reference package with glob-based matching:

references.Match(ref, "foo/**/ba[rz]")

This has use in scope-matching and other applications.

vdemeester

Updated, than should fix the build but I have a question 👼

vdemeester · 2016-11-03T13:08:53Z

reference/reference.go

@@ -168,6 +169,20 @@ func ParseIDOrReference(idOrRef string) (digest.Digest, Named, error) {
 	return "", ref, err
 }

+// Match reports whether ref matches the specified pattern
+func Match(pattern string, ref Named) bool {
+	// FIXME: should we support pattern for NamedTagged or not ?


The lines below are to keep the current behaviour, not sure I like that… We might want to implement supporting foo/**/[ai]:ta* and keep the current behavior in images.go 👼 @stevvooe wdyt ?

First, we should implement this in the upstream distribution package. These packages keep diverging and it is extremely disappointing.

As far as support goes, we should support globbing over any of the reference types.

First, we should implement this in the upstream distribution package. These packages keep diverging and it is extremely disappointing.

❤️

stevvooe · 2016-11-03T19:32:43Z

reference/reference.go

+	if _, ok := patternRef.(NamedTagged); err == nil && ok {
+		return ref.String() == pattern
+	}
+	matched, err := path.Match(pattern, ref.Name())


Does this need to use ref.String()?

If we want to support globbing over any of the reference types, yes we should do that.

stevvooe · 2016-11-03T19:35:33Z

reference/reference_test.go

+	}{
+		{
+			reference: "foo",
+			pattern:   "foo/**/ba[rz]",


The globbing in golang's stdlib doesn't support ** for multi-component match. This case is still valid but this pattern will only match foo/a/ba[rz], not foo/a/b/ba[rz]. I don't think we need to do more for this PR.

vdemeester · 2016-11-04T13:21:23Z

Updated and depends on distribution/distribution#2039 👼

vdemeester · 2016-11-10T21:04:09Z

I think this is blocked by #28235 for now 👼

stevvooe · 2016-11-11T03:56:33Z

@vdemeester Should be unblocked, now!

vdemeester · 2016-11-11T11:17:57Z

@stevvooe It is, PR rebased 👼

cpuguy83

LGTM

Glad to see this one go.

cpuguy83 · 2016-11-11T14:22:56Z

api/server/router/image/image_routes.go

+	}
+
+	version := httputils.VersionFromContext(ctx)
+	if versions.LessThan(version, "1.28") && r.Form.Get("filter") != "" {


Minor minor nit, but store the value of the filter field instead of getting it twice.

… for `docker images`. This deprecates the `filter` param for the `/images` endpoint and make a new filter called `reference` to replace it. It does change the CLI side (still possible to do `docker images busybox:musl`) but changes the cli code to use the filter instead (so that `docker images --filter busybox:musl` and `docker images busybox:musl` act the same). Signed-off-by: Vincent Demeester <vincent@sbr.pm>

stevvooe · 2016-11-11T16:07:30Z

LGTM

thaJeztah · 2016-11-11T16:11:57Z

ping @AkihiroSuda;

15:35:31 ----------------------------------------------------------------------
15:35:31 FAIL: docker_cli_prune_unix_test.go:26: DockerSwarmSuite.TestPruneNetwork
15:35:31 
15:35:31 [dfc709b8ed9a0] waiting for daemon to start
15:35:31 [dfc709b8ed9a0] daemon started
15:35:31 docker_cli_prune_unix_test.go:31:
15:35:31     c.Assert(err, checker.IsNil)
15:35:31 ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc42310d720), Stderr:[]uint8(nil)} ("exit status 1")
15:35:31 
15:35:31 [dfc709b8ed9a0] exiting daemon
15:35:34 
15:35:34 ----------------------------------------------------------------------

thaJeztah · 2016-11-11T16:14:46Z

docs/reference/api/docker_remote_api_v1.25.md

@@ -1733,7 +1733,7 @@ references on the command line.
  -   `label=key` or `label="key=value"` of an image label
  -   `before`=(`<image-name>[:<tag>]`,  `<image id>` or `<image@digest>`)
  -   `since`=(`<image-name>[:<tag>]`,  `<image id>` or `<image@digest>`)
-   **filter** - only return images with the specified name
+  -   `reference`=(`<image-name>[:<tag>]`)


@vdemeester can you do a follow up PR to;

add a line to the API changes; https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api.md#v125-api-changes

maybe add a note to the API reference about the deprecated option

Yep I will do a followup for the docs 😉

AkihiroSuda · 2016-11-11T16:32:38Z

@thaJeztah ah, it is discussed in #27938 I will try to find the solution ASAP..

thaJeztah · 2016-11-11T16:39:17Z

@AkihiroSuda ah! yes, no problem, just wanted to let you know 😄

Add reference filter and deprecated filter param…

GordonTheTurtle added the status/0-triage label Oct 29, 2016

vdemeester added status/1-design-review and removed status/0-triage labels Oct 29, 2016

thaJeztah added status/2-code-review and removed status/1-design-review labels Oct 29, 2016

vdemeester added this to the 1.13.0 milestone Oct 29, 2016

vdemeester force-pushed the images-filter-filters branch 3 times, most recently from dc77d9f to 3b052b2 Compare October 29, 2016 01:02

vdemeester added the status/failing-ci Indicates that the PR in its current state fails the test suite label Oct 29, 2016

vdemeester force-pushed the images-filter-filters branch from 3b052b2 to 3208c24 Compare November 3, 2016 13:01

vdemeester removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Nov 3, 2016

vdemeester force-pushed the images-filter-filters branch from 3208c24 to dd91673 Compare November 3, 2016 13:06

vdemeester commented Nov 3, 2016

View reviewed changes

stevvooe reviewed Nov 3, 2016

View reviewed changes

vdemeester mentioned this pull request Nov 4, 2016

Add a new Match method to the reference package distribution/distribution#2039

Merged

vdemeester force-pushed the images-filter-filters branch from dd91673 to 4a872fe Compare November 4, 2016 13:18

vdemeester added the status/needs-vendoring label Nov 4, 2016

vdemeester force-pushed the images-filter-filters branch 2 times, most recently from ee89d27 to d66e676 Compare November 10, 2016 13:46

vdemeester removed the status/needs-vendoring label Nov 10, 2016

vdemeester force-pushed the images-filter-filters branch from d66e676 to 9220755 Compare November 10, 2016 14:34

vdemeester added the status/failing-ci Indicates that the PR in its current state fails the test suite label Nov 10, 2016

vdemeester force-pushed the images-filter-filters branch from 9220755 to 7883d0c Compare November 10, 2016 17:51

vdemeester removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Nov 10, 2016

vdemeester force-pushed the images-filter-filters branch from 7883d0c to 6f3f4f6 Compare November 11, 2016 09:36

vdemeester removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Nov 11, 2016

cpuguy83 approved these changes Nov 11, 2016

View reviewed changes

vdemeester force-pushed the images-filter-filters branch from 6f3f4f6 to 820b809 Compare November 11, 2016 14:34

thaJeztah added impact/api impact/changelog impact/deprecation impact/documentation labels Nov 11, 2016

thaJeztah added status/4-merge and removed status/2-code-review labels Nov 11, 2016

thaJeztah reviewed Nov 11, 2016

View reviewed changes

cpuguy83 merged commit de32553 into moby:master Nov 11, 2016

vdemeester deleted the images-filter-filters branch November 11, 2016 16:28

thaJeztah mentioned this pull request Nov 11, 2016

add 1.13.0 CHANGELOG.md #28275

Merged

vdemeester mentioned this pull request Nov 16, 2016

Update images cli reference with reference filter #28477

Merged

albers mentioned this pull request Nov 19, 2016

Add bash completion for docker images --filter reference #28628

Merged

sdurrheimer mentioned this pull request Nov 22, 2016

Add zsh completion for 'docker image ls --filter reference' #28700

Merged

michelvocks mentioned this pull request Nov 22, 2016

#2488 Update vendoring to remove engine-api docker-archive/classicswarm#2492

Merged

2 tasks

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#27872 from vdemeester/images-filter-filters

356ff43

Add reference filter and deprecated filter param…

thaJeztah mentioned this pull request Nov 20, 2018

Add reference filter to the list of available filters #31969

Merged

thaJeztah mentioned this pull request Feb 10, 2020

API: remove deprecated "filter" parameter for API v1.41 and up #40491

Merged

thaJeztah mentioned this pull request Jun 29, 2021

Add support for shared-size parameter for images queries #42531

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add reference filter and deprecated filter param… #27872

Add reference filter and deprecated filter param… #27872

vdemeester commented Oct 29, 2016 •

edited

thaJeztah commented Oct 29, 2016

vdemeester commented Oct 29, 2016

stevvooe commented Nov 3, 2016

vdemeester left a comment

vdemeester Nov 3, 2016

stevvooe Nov 3, 2016

vdemeester Nov 3, 2016

stevvooe Nov 3, 2016

vdemeester Nov 3, 2016

stevvooe Nov 3, 2016

vdemeester commented Nov 4, 2016

vdemeester commented Nov 10, 2016

stevvooe commented Nov 11, 2016

vdemeester commented Nov 11, 2016

cpuguy83 left a comment

cpuguy83 Nov 11, 2016

stevvooe commented Nov 11, 2016

thaJeztah commented Nov 11, 2016

thaJeztah Nov 11, 2016

vdemeester Nov 11, 2016

AkihiroSuda commented Nov 11, 2016

thaJeztah commented Nov 11, 2016

Add reference filter and deprecated filter param… #27872

Add reference filter and deprecated filter param… #27872

Conversation

vdemeester commented Oct 29, 2016 • edited

thaJeztah commented Oct 29, 2016

vdemeester commented Oct 29, 2016

stevvooe commented Nov 3, 2016

vdemeester left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdemeester commented Nov 4, 2016

vdemeester commented Nov 10, 2016

stevvooe commented Nov 11, 2016

vdemeester commented Nov 11, 2016

cpuguy83 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevvooe commented Nov 11, 2016

thaJeztah commented Nov 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkihiroSuda commented Nov 11, 2016

thaJeztah commented Nov 11, 2016

vdemeester commented Oct 29, 2016 •

edited