From 21396819c1fef756796ffd93d8abb800c9655992 Mon Sep 17 00:00:00 2001 From: zhangyue Date: Fri, 2 Nov 2018 10:13:59 +0800 Subject: [PATCH] feature: support images filter flag Signed-off-by: zhangyue --- apis/filters/parse.go | 37 ++++++++++++++++ apis/filters/parse_test.go | 27 ++++++++++++ apis/server/image_bridge.go | 10 +++-- apis/swagger.yml | 2 - cli/events.go | 12 ++---- cli/image_list.go | 13 ++++-- client/image_list.go | 18 ++++++-- client/image_list_test.go | 5 ++- client/interface.go | 2 +- cri/v1alpha1/cri.go | 3 +- cri/v1alpha2/cri.go | 3 +- daemon/mgr/image.go | 85 +++++++++++++++++++++++++++++++++++-- daemon/mgr/system.go | 2 +- test/api_image_list_test.go | 35 ++++++++++++++- test/cli_images_test.go | 35 ++++++++++++++- test/cli_run_test.go | 2 +- test/environment/cleanup.go | 3 +- 17 files changed, 259 insertions(+), 35 deletions(-) diff --git a/apis/filters/parse.go b/apis/filters/parse.go index c1c83ccbbf..49108587d8 100644 --- a/apis/filters/parse.go +++ b/apis/filters/parse.go @@ -3,6 +3,7 @@ package filters import ( "encoding/json" "errors" + "path" "strings" ) @@ -36,6 +37,12 @@ func NewArgs(initialArgs ...KeyValuePair) Args { return args } +// Contains returns true if the key exists in the mapping +func (args Args) Contains(field string) bool { + _, ok := args.fields[field] + return ok +} + // Get returns the list of values associated with the key func (args Args) Get(key string) []string { values := args.fields[key] @@ -150,3 +157,33 @@ func FromParam(p string) (Args, error) { } return args, nil } + +// FromFilterOpts parse key=value to Args string from cli opts +func FromFilterOpts(filter []string) (Args, error) { + filterArgs := NewArgs() + + for _, f := range filter { + var err error + filterArgs, err = ParseFlag(f, filterArgs) + if err != nil { + return filterArgs, err + } + } + return filterArgs, nil +} + +// Validate compared the set of accepted keys against the keys in the mapping. +// An error is returned if any mapping keys are not in the accepted set. +func (args Args) Validate(accepted map[string]bool) error { + for name := range args.fields { + if !accepted[name] { + return errors.New("invalid filter " + name) + } + } + return nil +} + +// FamiliarMatch decide the ref match the pattern or not +func FamiliarMatch(pattern string, ref string) (bool, error) { + return path.Match(pattern, ref) +} diff --git a/apis/filters/parse_test.go b/apis/filters/parse_test.go index 0447921539..8d2e9c95bc 100644 --- a/apis/filters/parse_test.go +++ b/apis/filters/parse_test.go @@ -149,3 +149,30 @@ func TestFromParam(t *testing.T) { } } } + +func TestFromFilterOpts(t *testing.T) { + filterOpts := []string{ + "reference=img1", + "since=img2", + "before=img3", + "reference=img3", + } + + args, err := FromFilterOpts(filterOpts) + if err != nil { + t.Fatal(err) + } + + images := args.Get("reference") + if len(images) != 2 { + t.Fatal("Expected two values of reference key, but got one.") + } + + if !args.Contains("since") { + t.Fatal("Excepted get since key, but got none.") + } + + if !args.Contains("before") { + t.Fatal("Excepted get before key, but got none.") + } +} diff --git a/apis/server/image_bridge.go b/apis/server/image_bridge.go index 2e4146e330..053f9ec4c7 100644 --- a/apis/server/image_bridge.go +++ b/apis/server/image_bridge.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/alibaba/pouch/apis/filters" "github.com/alibaba/pouch/apis/metrics" "github.com/alibaba/pouch/apis/types" "github.com/alibaba/pouch/daemon/mgr" @@ -75,9 +76,12 @@ func (s *Server) getImage(ctx context.Context, rw http.ResponseWriter, req *http } func (s *Server) listImages(ctx context.Context, rw http.ResponseWriter, req *http.Request) error { - filters := req.FormValue("filters") + filter, err := filters.FromParam(req.FormValue("filters")) + if err != nil { + return err + } - imageList, err := s.ImageMgr.ListImages(ctx, filters) + imageList, err := s.ImageMgr.ListImages(ctx, filter) if err != nil { logrus.Errorf("failed to list images: %v", err) return err @@ -91,7 +95,7 @@ func (s *Server) searchImages(ctx context.Context, rw http.ResponseWriter, req * searchResultItem, err := s.ImageMgr.SearchImages(ctx, searchPattern, registry) if err != nil { - logrus.Errorf("failed to search images from resgitry: %v", err) + logrus.Errorf("failed to search images from registry: %v", err) return err } return EncodeResponse(rw, http.StatusOK, searchResultItem) diff --git a/apis/swagger.yml b/apis/swagger.yml index 1c61f5cd15..5899f93cd9 100644 --- a/apis/swagger.yml +++ b/apis/swagger.yml @@ -348,8 +348,6 @@ paths: A JSON encoded value of the filters (a `map[string][]string`) to process on the images list. Available filters: - `before`=(`[:]`, `` or ``) - - `dangling=true` - - `label=key` or `label="key=value"` of an image label - `reference`=(`[:]`) - `since`=(`[:]`, `` or ``) type: "string" diff --git a/cli/events.go b/cli/events.go index 0006cb4f13..a69adad76d 100644 --- a/cli/events.go +++ b/cli/events.go @@ -59,15 +59,9 @@ func (e *EventsCommand) runEvents() error { ctx := context.Background() apiClient := e.cli.Client() - eventFilterArgs := filters.NewArgs() - - // TODO: parse params - for _, f := range e.filter { - var err error - eventFilterArgs, err = filters.ParseFlag(f, eventFilterArgs) - if err != nil { - return err - } + eventFilterArgs, err := filters.FromFilterOpts(e.filter) + if err != nil { + return err } responseBody, err := apiClient.Events(ctx, e.since, e.until, eventFilterArgs) diff --git a/cli/image_list.go b/cli/image_list.go index 305422980e..32b37e376f 100644 --- a/cli/image_list.go +++ b/cli/image_list.go @@ -4,11 +4,12 @@ import ( "context" "fmt" + "github.com/alibaba/pouch/apis/filters" "github.com/alibaba/pouch/apis/types" "github.com/alibaba/pouch/pkg/reference" "github.com/alibaba/pouch/pkg/utils" - digest "github.com/opencontainers/go-digest" + "github.com/opencontainers/go-digest" "github.com/spf13/cobra" ) @@ -38,6 +39,7 @@ type ImagesCommand struct { flagQuiet bool flagDigest bool flagNoTrunc bool + flagFilter []string } // Init initialize images command. @@ -63,6 +65,7 @@ func (i *ImagesCommand) addFlags() { flagSet.BoolVarP(&i.flagQuiet, "quiet", "q", false, "Only show image numeric ID") flagSet.BoolVar(&i.flagDigest, "digest", false, "Show images with digest") flagSet.BoolVar(&i.flagNoTrunc, "no-trunc", false, "Do not truncate output") + flagSet.StringSliceVarP(&i.flagFilter, "filter", "f", []string{}, "Filter output based on conditions provided") } // runImages is the entry of images container command. @@ -70,10 +73,14 @@ func (i *ImagesCommand) runImages(args []string) error { ctx := context.Background() apiClient := i.cli.Client() - imageList, err := apiClient.ImageList(ctx) + imageFilterArgs, err := filters.FromFilterOpts(i.flagFilter) if err != nil { - return fmt.Errorf("failed to get image list: %v", err) + return err + } + imageList, err := apiClient.ImageList(ctx, imageFilterArgs) + if err != nil { + return fmt.Errorf("failed to get image list: %v", err) } if i.flagQuiet { diff --git a/client/image_list.go b/client/image_list.go index 87bddf78c5..d5f531e2a2 100644 --- a/client/image_list.go +++ b/client/image_list.go @@ -2,13 +2,26 @@ package client import ( "context" + "net/url" + "github.com/alibaba/pouch/apis/filters" "github.com/alibaba/pouch/apis/types" ) // ImageList requests daemon to list all images -func (client *APIClient) ImageList(ctx context.Context) ([]types.ImageInfo, error) { - resp, err := client.get(ctx, "/images/json", nil, nil) +func (client *APIClient) ImageList(ctx context.Context, filter filters.Args) ([]types.ImageInfo, error) { + query := url.Values{} + + if filter.Len() > 0 { + filtersJSON, err := filters.ToParam(filter) + if err != nil { + return nil, err + } + + query.Set("filters", filtersJSON) + } + + resp, err := client.get(ctx, "/images/json", query, nil) if err != nil { return nil, err } @@ -19,5 +32,4 @@ func (client *APIClient) ImageList(ctx context.Context) ([]types.ImageInfo, erro ensureCloseReader(resp) return imageList, err - } diff --git a/client/image_list_test.go b/client/image_list_test.go index 04544921a8..482e4061b9 100644 --- a/client/image_list_test.go +++ b/client/image_list_test.go @@ -12,6 +12,7 @@ import ( "github.com/alibaba/pouch/apis/types" + "github.com/alibaba/pouch/apis/filters" "github.com/stretchr/testify/assert" ) @@ -19,7 +20,7 @@ func TestImageListServerError(t *testing.T) { client := &APIClient{ HTTPCli: newMockClient(errorMockResponse(http.StatusInternalServerError, "Server error")), } - _, err := client.ImageList(context.Background()) + _, err := client.ImageList(context.Background(), filters.NewArgs()) if err == nil || !strings.Contains(err.Error(), "Server error") { t.Fatalf("expected a Server Error, got %v", err) } @@ -62,7 +63,7 @@ func TestImageList(t *testing.T) { HTTPCli: httpClient, } - image, err := client.ImageList(context.Background()) + image, err := client.ImageList(context.Background(), filters.NewArgs()) if err != nil { t.Fatal(err) } diff --git a/client/interface.go b/client/interface.go index b0a8a50eae..a740747b8a 100644 --- a/client/interface.go +++ b/client/interface.go @@ -49,7 +49,7 @@ type ContainerAPIClient interface { // ImageAPIClient defines methods of Image client. type ImageAPIClient interface { - ImageList(ctx context.Context) ([]types.ImageInfo, error) + ImageList(ctx context.Context, filters filters.Args) ([]types.ImageInfo, error) ImageInspect(ctx context.Context, name string) (types.ImageInfo, error) ImagePull(ctx context.Context, name, tag, encodedAuth string) (io.ReadCloser, error) ImageRemove(ctx context.Context, name string, force bool) error diff --git a/cri/v1alpha1/cri.go b/cri/v1alpha1/cri.go index c088395980..3e605d44aa 100644 --- a/cri/v1alpha1/cri.go +++ b/cri/v1alpha1/cri.go @@ -13,6 +13,7 @@ import ( goruntime "runtime" "time" + "github.com/alibaba/pouch/apis/filters" apitypes "github.com/alibaba/pouch/apis/types" anno "github.com/alibaba/pouch/cri/annotations" cni "github.com/alibaba/pouch/cri/ocicni" @@ -1023,7 +1024,7 @@ func (c *CriManager) Status(ctx context.Context, r *runtime.StatusRequest) (*run // ListImages lists existing images. func (c *CriManager) ListImages(ctx context.Context, r *runtime.ListImagesRequest) (*runtime.ListImagesResponse, error) { // TODO: handle image list filters. - imageList, err := c.ImageMgr.ListImages(ctx, "") + imageList, err := c.ImageMgr.ListImages(ctx, filters.NewArgs()) if err != nil { return nil, err } diff --git a/cri/v1alpha2/cri.go b/cri/v1alpha2/cri.go index cd4bfea3d6..121122ce9d 100644 --- a/cri/v1alpha2/cri.go +++ b/cri/v1alpha2/cri.go @@ -14,6 +14,7 @@ import ( "strconv" "time" + "github.com/alibaba/pouch/apis/filters" apitypes "github.com/alibaba/pouch/apis/types" anno "github.com/alibaba/pouch/cri/annotations" runtime "github.com/alibaba/pouch/cri/apis/v1alpha2" @@ -1307,7 +1308,7 @@ func (c *CriManager) ListImages(ctx context.Context, r *runtime.ListImagesReques }(time.Now()) // TODO: handle image list filters. - imageList, err := c.ImageMgr.ListImages(ctx, "") + imageList, err := c.ImageMgr.ListImages(ctx, filters.NewArgs()) if err != nil { return nil, err } diff --git a/daemon/mgr/image.go b/daemon/mgr/image.go index 5a4e548b6f..8d615d8af6 100644 --- a/daemon/mgr/image.go +++ b/daemon/mgr/image.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/alibaba/pouch/apis/filters" "github.com/alibaba/pouch/apis/types" "github.com/alibaba/pouch/ctrd" "github.com/alibaba/pouch/daemon/config" @@ -21,7 +22,7 @@ import ( "github.com/containerd/containerd/content" ctrdmetaimages "github.com/containerd/containerd/images" "github.com/containerd/containerd/platforms" - digest "github.com/opencontainers/go-digest" + "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" pkgerrors "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -29,6 +30,13 @@ import ( var deadlineLoadImagesAtBootup = time.Second * 10 +// the filter tags set allowed when pouch images -f +var acceptedImageFilterTags = map[string]bool{ + "before": true, + "since": true, + "reference": true, +} + // ImageMgr as an interface defines all operations against images. type ImageMgr interface { // PullImage pulls images from specified registry. @@ -38,7 +46,7 @@ type ImageMgr interface { GetImage(ctx context.Context, idOrRef string) (*types.ImageInfo, error) // ListImages lists images stored by containerd. - ListImages(ctx context.Context, filter ...string) ([]types.ImageInfo, error) + ListImages(ctx context.Context, filter filters.Args) ([]types.ImageInfo, error) // Search Images from specified registry. SearchImages(ctx context.Context, name string, registry string) ([]types.SearchResultItem, error) @@ -161,12 +169,81 @@ func (mgr *ImageManager) GetImage(ctx context.Context, idOrRef string) (*types.I } // ListImages lists images stored by containerd. -func (mgr *ImageManager) ListImages(ctx context.Context, filter ...string) ([]types.ImageInfo, error) { - // TODO: support filter functionality +func (mgr *ImageManager) ListImages(ctx context.Context, filter filters.Args) ([]types.ImageInfo, error) { + if err := filter.Validate(acceptedImageFilterTags); err != nil { + return nil, err + } + ctrdImageInfos := mgr.localStore.ListCtrdImageInfo() imgInfos := make([]types.ImageInfo, 0, len(ctrdImageInfos)) + var ( + beforeFilter, sinceFilter *types.ImageInfo + beforeTime, sinceTime time.Time + err error + ) + + beforeImages := filter.Get("before") + if len(beforeImages) > 0 { + // use the last one if multi before images was assigned + beforeFilter, err = mgr.GetImage(ctx, beforeImages[len(beforeImages)-1]) + if err != nil { + return nil, err + } + beforeTime, err = time.Parse(utils.TimeLayout, beforeFilter.CreatedAt) + if err != nil { + return nil, err + } + } + + sinceImages := filter.Get("since") + if len(sinceImages) > 0 { + // use the last one if multi since images was assigned + sinceFilter, err = mgr.GetImage(ctx, sinceImages[len(sinceImages)-1]) + if err != nil { + return nil, err + } + sinceTime, err = time.Parse(utils.TimeLayout, sinceFilter.CreatedAt) + if err != nil { + return nil, err + } + } + for _, img := range ctrdImageInfos { + if beforeFilter != nil { + if img.OCISpec.Created.Equal(beforeTime) || img.OCISpec.Created.After(beforeTime) { + continue + } + } + if sinceFilter != nil { + if img.OCISpec.Created.Equal(sinceTime) || img.OCISpec.Created.Before(sinceTime) { + continue + } + } + + if filter.Contains("reference") { + var found bool + referenceFilters := filter.Get("reference") + for _, ref := range mgr.localStore.GetReferences(img.ID) { + for _, pattern := range referenceFilters { + matched, err := filters.FamiliarMatch(pattern, ref.String()) + if err != nil { + return nil, err + } + if matched { + found = true + break + } + } + if found { + break + } + } + if !found { + continue + } + } + imgInfo, err := mgr.containerdImageToImageInfo(ctx, img.ID) if err != nil { logrus.Warnf("failed to convert containerd image(%v) to ImageInfo during list images: %v", img.ID, err) diff --git a/daemon/mgr/system.go b/daemon/mgr/system.go index 11636e7478..91fb097def 100644 --- a/daemon/mgr/system.go +++ b/daemon/mgr/system.go @@ -106,7 +106,7 @@ func (mgr *SystemManager) Info() (types.SystemInfo, error) { OSName = osName } - images, err := mgr.imageMgr.ListImages(context.Background(), "") + images, err := mgr.imageMgr.ListImages(context.Background(), filters.NewArgs()) if err != nil { logrus.Warnf("failed to get image info: %v", err) } diff --git a/test/api_image_list_test.go b/test/api_image_list_test.go index 02dd4070ab..047f36e984 100644 --- a/test/api_image_list_test.go +++ b/test/api_image_list_test.go @@ -2,7 +2,10 @@ package main import ( "net/url" + "reflect" + "github.com/alibaba/pouch/apis/filters" + "github.com/alibaba/pouch/apis/types" "github.com/alibaba/pouch/test/environment" "github.com/alibaba/pouch/test/request" @@ -56,6 +59,34 @@ func (suite *APIImageListSuite) TestImageListDigest(c *check.C) { // TestImageListFilter tests listing images with filter. func (suite *APIImageListSuite) TestImageListFilter(c *check.C) { - // TODO: missing case - helpwantedForMissingCase(c, "iamge api list filter cases") + q := url.Values{} + + repoDigest := "registry.hub.docker.com/library/busybox@sha256:141c253bc4c3fd0a201d32dc1f493bcf3fff003b6df416dea4f41046e0f37d47" + repoTag := "registry.hub.docker.com/library/busybox:1.28" + + f := filters.NewArgs() + f.Add("reference", repoTag) + filterJSON, err := filters.ToParam(f) + c.Assert(err, check.IsNil) + + q.Add("filters", filterJSON) + query := request.WithQuery(q) + resp, err := request.Get("/images/json", query) + c.Assert(err, check.IsNil) + c.Assert(resp.StatusCode, check.Equals, 200) + + got := []types.ImageInfo{} + err = request.DecodeBody(&got, resp.Body) + c.Assert(err, check.IsNil) + + c.Assert(got, check.NotNil) + c.Assert(len(got), check.Equals, 1) + c.Assert(got[0].ID, check.NotNil) + c.Assert(got[0].CreatedAt, check.NotNil) + c.Assert(got[0].Config, check.NotNil) + c.Assert(got[0].Architecture, check.NotNil) + c.Assert(got[0].Size, check.NotNil) + c.Assert(got[0].Os, check.NotNil) + c.Assert(reflect.DeepEqual(got[0].RepoTags, []string{repoTag}), check.Equals, true) + c.Assert(reflect.DeepEqual(got[0].RepoDigests, []string{repoDigest}), check.Equals, true) } diff --git a/test/cli_images_test.go b/test/cli_images_test.go index f73aeab39d..e78a610791 100644 --- a/test/cli_images_test.go +++ b/test/cli_images_test.go @@ -7,6 +7,7 @@ import ( "sort" "strings" + "github.com/alibaba/pouch/apis/filters" "github.com/alibaba/pouch/apis/types" "github.com/alibaba/pouch/client" "github.com/alibaba/pouch/pkg/utils" @@ -31,8 +32,10 @@ func (suite *PouchImagesSuite) SetUpSuite(c *check.C) { SkipIfFalse(c, environment.IsLinux) environment.PruneAllContainers(apiClient) + environment.PruneAllImages(apiClient) PullImage(c, busyboxImage) + PullImage(c, helloworldImage) } // TestImagesWorks tests "pouch images" work. @@ -81,6 +84,36 @@ func (suite *PouchImagesSuite) TestImagesWorks(c *check.C) { } } +//TestImageListFilter test the filter flag works right +func (suite *PouchImagesSuite) TestImageListFilter(c *check.C) { + busyBoxImageInfo, err := getImageInfo(apiClient, busyboxImage) + c.Assert(err, check.IsNil) + + //Test Reference filter + referenceRes := command.PouchRun("images", "-f", "reference="+busyboxImage) + items := imagesListToKV(referenceRes.Combined()) + c.Assert(len(items), check.Equals, 1) + c.Assert(items[busyboxImage][1], check.Equals, busyBoxImageInfo.RepoTags[0]) + + //Test before filter + beforeRes1 := command.PouchRun("images", "-f", "before="+busyboxImage) + items1 := imagesListToKV(beforeRes1.Combined()) + beforeRes2 := command.PouchRun("images", "-f", "before="+helloworldImage) + items2 := imagesListToKV(beforeRes2.Combined()) + c.Assert(len(items1)+len(items2), check.Equals, 1) + + //Test since filter + sinceRes1 := command.PouchRun("images", "-f", "since="+busyboxImage) + items1 = imagesListToKV(sinceRes1.Combined()) + sinceRes2 := command.PouchRun("images", "-f", "since="+helloworldImage) + items2 = imagesListToKV(sinceRes2.Combined()) + c.Assert(len(items1)+len(items2), check.Equals, 1) + + //Test invalid filter + invalidRes := command.PouchRun("images", "-f", "after="+busyboxImage) + c.Assert(invalidRes.Stderr(), check.Equals, "Error: failed to get image list: {\"message\":\"Invalid filter after\"}\n\n") +} + // imagesListToKV parse "pouch images" into key-value mapping. func imagesListToKV(list string) map[string][]string { // skip header @@ -101,7 +134,7 @@ func imagesListToKV(list string) map[string][]string { // getImageInfo is used to retrieve the information about image. func getImageInfo(apiClient client.ImageAPIClient, name string) (types.ImageInfo, error) { ctx := context.Background() - images, err := apiClient.ImageList(ctx) + images, err := apiClient.ImageList(ctx, filters.NewArgs()) if err != nil { return types.ImageInfo{}, errors.Wrap(err, "fail to list images") } diff --git a/test/cli_run_test.go b/test/cli_run_test.go index cea7dfb66a..3a15d16f51 100644 --- a/test/cli_run_test.go +++ b/test/cli_run_test.go @@ -402,7 +402,7 @@ func (suite *PouchRunSuite) TestRunWithEnv(c *check.C) { res := command.PouchRun("run", "--rm", "--env", "A=a,b,c", // should not split args by comma "--env", "B=b1", - "docker.io/library/alpine", + busyboxImage, "sh", "-c", "echo ${A}-${B}", ) res.Assert(c, icmd.Success) diff --git a/test/environment/cleanup.go b/test/environment/cleanup.go index 614bc7f6bf..dce8388f34 100644 --- a/test/environment/cleanup.go +++ b/test/environment/cleanup.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/alibaba/pouch/apis/filters" "github.com/alibaba/pouch/apis/types" "github.com/alibaba/pouch/client" @@ -13,7 +14,7 @@ import ( // PruneAllImages deletes all images from pouchd. func PruneAllImages(apiClient client.ImageAPIClient) error { ctx := context.Background() - images, err := apiClient.ImageList(ctx) + images, err := apiClient.ImageList(ctx, filters.NewArgs()) if err != nil { return errors.Wrap(err, "fail to list images") }