Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reference filter and deprecated filter param… #27872

Merged
merged 1 commit into from
Nov 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion api/server/router/image/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/backend"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/api/types/registry"
"golang.org/x/net/context"
)
Expand All @@ -25,7 +26,7 @@ type containerBackend interface {
type imageBackend interface {
ImageDelete(imageRef string, force, prune bool) ([]types.ImageDelete, error)
ImageHistory(imageName string) ([]*types.ImageHistory, error)
Images(filterArgs string, filter string, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error)
Images(imageFilters filters.Args, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error)
LookupImage(name string) (*types.ImageInspect, error)
TagImage(imageName, repository, tag string) error
ImagesPrune(config *types.ImagesPruneConfig) (*types.ImagesPruneReport, error)
Expand Down
15 changes: 13 additions & 2 deletions api/server/router/image/image_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/backend"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/api/types/versions"
"github.com/docker/docker/pkg/ioutils"
"github.com/docker/docker/pkg/streamformatter"
Expand Down Expand Up @@ -247,8 +248,18 @@ func (s *imageRouter) getImagesJSON(ctx context.Context, w http.ResponseWriter,
return err
}

// FIXME: The filter parameter could just be a match filter
images, err := s.backend.Images(r.Form.Get("filters"), r.Form.Get("filter"), httputils.BoolValue(r, "all"), false)
imageFilters, err := filters.FromParam(r.Form.Get("filters"))
if err != nil {
return err
}

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

images, err := s.backend.Images(imageFilters, httputils.BoolValue(r, "all"), false)
if err != nil {
return err
}
Expand Down
5 changes: 2 additions & 3 deletions api/types/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,8 @@ type ImageImportOptions struct {

// ImageListOptions holds parameters to filter the list of images with.
type ImageListOptions struct {
MatchName string
All bool
Filters filters.Args
All bool
Filters filters.Args
}

// ImageLoadResponse returns information to the client about a load process.
Expand Down
10 changes: 7 additions & 3 deletions cli/command/image/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,14 @@ func newListCommand(dockerCli *command.DockerCli) *cobra.Command {
func runImages(dockerCli *command.DockerCli, opts imagesOptions) error {
ctx := context.Background()

filters := opts.filter.Value()
if opts.matchName != "" {
filters.Add("reference", opts.matchName)
}

options := types.ImageListOptions{
MatchName: opts.matchName,
All: opts.all,
Filters: opts.filter.Value(),
All: opts.all,
Filters: filters,
}

images, err := dockerCli.Client().ImageList(ctx, options)
Expand Down
4 changes: 0 additions & 4 deletions client/image_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ func (cli *Client) ImageList(ctx context.Context, options types.ImageListOptions
}
query.Set("filters", filterJSON)
}
if options.MatchName != "" {
// FIXME rename this parameter, to not be confused with the filters flag
query.Set("filter", options.MatchName)
}
if options.All {
query.Set("all", "1")
}
Expand Down
11 changes: 0 additions & 11 deletions client/image_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,6 @@ func TestImageList(t *testing.T) {
"filters": "",
},
},
{
options: types.ImageListOptions{
All: true,
MatchName: "image_name",
},
expectedQueryParams: map[string]string{
"all": "1",
"filter": "image_name",
"filters": "",
},
},
{
options: types.ImageListOptions{
Filters: filters,
Expand Down
3 changes: 2 additions & 1 deletion daemon/disk_usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/Sirupsen/logrus"
"github.com/docker/distribution/digest"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/layer"
"github.com/docker/docker/pkg/directory"
"github.com/docker/docker/volume"
Expand Down Expand Up @@ -44,7 +45,7 @@ func (daemon *Daemon) SystemDiskUsage() (*types.DiskUsage, error) {
}

// Get all top images with extra attributes
allImages, err := daemon.Images("", "", false, true)
allImages, err := daemon.Images(filters.NewArgs(), false, true)
if err != nil {
return nil, fmt.Errorf("failed to retrieve image list: %v", err)
}
Expand Down
44 changes: 17 additions & 27 deletions daemon/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,25 @@ package daemon
import (
"encoding/json"
"fmt"
"path"
"sort"
"time"

"github.com/pkg/errors"

"github.com/docker/distribution/reference"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/container"
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
"github.com/docker/docker/reference"
)

var acceptedImageFilterTags = map[string]bool{
"dangling": true,
"label": true,
"before": true,
"since": true,
"dangling": true,
"label": true,
"before": true,
"since": true,
"reference": true,
}

// byCreated is a temporary type used to sort a list of images by creation
Expand All @@ -42,17 +42,13 @@ func (daemon *Daemon) Map() map[image.ID]*image.Image {
// filter is a shell glob string applied to repository names. The argument
// named all controls whether all images in the graph are filtered, or just
// the heads.
func (daemon *Daemon) Images(filterArgs, filter string, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error) {
func (daemon *Daemon) Images(imageFilters filters.Args, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error) {
var (
allImages map[image.ID]*image.Image
err error
danglingOnly = false
)

imageFilters, err := filters.FromParam(filterArgs)
if err != nil {
return nil, err
}
if err := imageFilters.Validate(acceptedImageFilterTags); err != nil {
return nil, err
}
Expand Down Expand Up @@ -93,16 +89,6 @@ func (daemon *Daemon) Images(filterArgs, filter string, all bool, withExtraAttrs
var allLayers map[layer.ChainID]layer.Layer
var allContainers []*container.Container

var filterTagged bool
if filter != "" {
filterRef, err := reference.ParseNamed(filter)
if err == nil { // parse error means wildcard repo
if _, ok := filterRef.(reference.NamedTagged); ok {
filterTagged = true
}
}
}

for id, img := range allImages {
if beforeFilter != nil {
if img.Created.Equal(beforeFilter.Created) || img.Created.After(beforeFilter.Created) {
Expand Down Expand Up @@ -145,12 +131,16 @@ func (daemon *Daemon) Images(filterArgs, filter string, all bool, withExtraAttrs
newImage := newImage(img, size)

for _, ref := range daemon.referenceStore.References(id.Digest()) {
if filter != "" { // filter by tag/repo name
if filterTagged { // filter by tag, require full ref match
if ref.String() != filter {
continue
if imageFilters.Include("reference") {
var found bool
var matchErr error
for _, pattern := range imageFilters.Get("reference") {
found, matchErr = reference.Match(pattern, ref)
if matchErr != nil {
return nil, matchErr
}
} else if matched, err := path.Match(filter, ref.Name()); !matched || err != nil { // name only match, FIXME: docs say exact
}
if !found {
continue
}
}
Expand All @@ -168,7 +158,7 @@ func (daemon *Daemon) Images(filterArgs, filter string, all bool, withExtraAttrs
//dangling=false case, so dangling image is not needed
continue
}
if filter != "" { // skip images with no references if filtering by tag
if imageFilters.Include("reference") { // skip images with no references if filtering by reference
continue
}
newImage.RepoDigests = []string{"<none>@<none>"}
Expand Down
12 changes: 9 additions & 3 deletions docs/deprecated.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,29 @@ The following list of features are deprecated in Engine.
To learn more about Docker Engine's deprecation policy,
see [Feature Deprecation Policy](https://docs.docker.com/engine/#feature-deprecation-policy).

## `filter` param for `/images/json` endpoint
**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/tag/v1.13.0)**

**Target For Removal In Release: v1.16**

The `filter` param to filter the list of image by reference (name or name:tag) is now implemented as a regular filter, named `reference`.

### `repository:shortid` image references
**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/)**
**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/tag/v1.13.0)**

**Target For Removal In Release: v1.16**

`repository:shortid` syntax for referencing images is very little used, collides with with tag references can be confused with digest references.

### `docker daemon` subcommand
**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/)**
**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/tag/v1.13.0)**

**Target For Removal In Release: v1.16**

The daemon is moved to a separate binary (`dockerd`), and should be used instead.

### Duplicate keys with conflicting values in engine labels
**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/)**
**Deprecated In Release: [v1.13](https://github.com/docker/docker/releases/tag/v1.13.0)**

**Target For Removal In Release: v1.16**

Expand Down
2 changes: 1 addition & 1 deletion docs/reference/api/docker_remote_api_v1.25.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>]`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vdemeester can you do a follow up PR to;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I will do a followup for the docs 😉


### Build image from a Dockerfile

Expand Down