From b5c0d15683de4c4d1f604cc34378c733295982fb Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 18 Feb 2019 11:29:14 +0100 Subject: podman-search: run in parallel Spin up a goroutine for each registry to make podman-search run in parallel. This has considerable speed improvements. For instance, a `podman search ruby` drops from 11 to 2 seconds when using the following search registries: ```toml [registries.search] registries = ['docker.io', 'registry.fedoraproject.org', 'quay.io', 'registry.access.redhat.com', 'registry.centos.org'] ``` The number of parallel goroutines is limited to 6 to play nice with local resources and the registries. Signed-off-by: Valentin Rothberg --- cmd/podman/search.go | 146 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 89 insertions(+), 57 deletions(-) diff --git a/cmd/podman/search.go b/cmd/podman/search.go index c8b64039a..02f171a68 100644 --- a/cmd/podman/search.go +++ b/cmd/podman/search.go @@ -5,6 +5,7 @@ import ( "reflect" "strconv" "strings" + "sync" "github.com/containers/image/docker" "github.com/containers/image/types" @@ -15,6 +16,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "golang.org/x/sync/semaphore" ) const ( @@ -164,7 +166,7 @@ func getRegistries(registry string) ([]string, error) { return registries, nil } -func getSearchOutput(term string, registries []string, opts searchOpts, filter searchFilterParams) ([]searchParams, error) { +func getSearchOutput(term string, registry string, opts searchOpts, filter searchFilterParams) ([]searchParams, error) { // Max number of queries by default is 25 limit := maxQueries if opts.limit != 0 { @@ -173,72 +175,102 @@ func getSearchOutput(term string, registries []string, opts searchOpts, filter s sc := common.GetSystemContext("", opts.authfile, false) sc.DockerInsecureSkipTLSVerify = opts.insecureSkipTLSVerify - sc.SystemRegistriesConfPath = sysreg.SystemRegistriesConfPath() // FIXME: Set this more globally. Probably no reason not to have it in every types.SystemContext, and to compute the value just once in one place. - var paramsArr []searchParams - for _, reg := range registries { - results, err := docker.SearchRegistry(context.TODO(), sc, reg, term, limit) - if err != nil { - logrus.Errorf("error searching registry %q: %v", reg, err) - continue + // FIXME: Set this more globally. Probably no reason not to have it in + // every types.SystemContext, and to compute the value just once in one + // place. + sc.SystemRegistriesConfPath = sysreg.SystemRegistriesConfPath() + results, err := docker.SearchRegistry(context.TODO(), sc, registry, term, limit) + if err != nil { + logrus.Errorf("error searching registry %q: %v", registry, err) + return []searchParams{}, nil + } + index := registry + arr := strings.Split(registry, ".") + if len(arr) > 2 { + index = strings.Join(arr[len(arr)-2:], ".") + } + + // limit is the number of results to output + // if the total number of results is less than the limit, output all + // if the limit has been set by the user, output those number of queries + limit = maxQueries + if len(results) < limit { + limit = len(results) + } + if opts.limit != 0 && opts.limit < len(results) { + limit = opts.limit + } + + paramsArr := []searchParams{} + for i := 0; i < limit; i++ { + if len(opts.filter) > 0 { + // Check whether query matches filters + if !(matchesAutomatedFilter(filter, results[i]) && matchesOfficialFilter(filter, results[i]) && matchesStarFilter(filter, results[i])) { + continue + } } - index := reg - arr := strings.Split(reg, ".") - if len(arr) > 2 { - index = strings.Join(arr[len(arr)-2:], ".") + official := "" + if results[i].IsOfficial { + official = "[OK]" } - - // limit is the number of results to output - // if the total number of results is less than the limit, output all - // if the limit has been set by the user, output those number of queries - limit := maxQueries - if len(results) < limit { - limit = len(results) + automated := "" + if results[i].IsAutomated { + automated = "[OK]" } - if opts.limit != 0 && opts.limit < len(results) { - limit = opts.limit + description := strings.Replace(results[i].Description, "\n", " ", -1) + if len(description) > 44 && !opts.noTrunc { + description = description[:descriptionTruncLength] + "..." } - - for i := 0; i < limit; i++ { - if len(opts.filter) > 0 { - // Check whether query matches filters - if !(matchesAutomatedFilter(filter, results[i]) && matchesOfficialFilter(filter, results[i]) && matchesStarFilter(filter, results[i])) { - continue - } - } - official := "" - if results[i].IsOfficial { - official = "[OK]" - } - automated := "" - if results[i].IsAutomated { - automated = "[OK]" - } - description := strings.Replace(results[i].Description, "\n", " ", -1) - if len(description) > 44 && !opts.noTrunc { - description = description[:descriptionTruncLength] + "..." - } - name := reg + "/" + results[i].Name - if index == "docker.io" && !strings.Contains(results[i].Name, "/") { - name = index + "/library/" + results[i].Name - } - params := searchParams{ - Index: index, - Name: name, - Description: description, - Official: official, - Automated: automated, - Stars: results[i].StarCount, - } - paramsArr = append(paramsArr, params) + name := registry + "/" + results[i].Name + if index == "docker.io" && !strings.Contains(results[i].Name, "/") { + name = index + "/library/" + results[i].Name + } + params := searchParams{ + Index: index, + Name: name, + Description: description, + Official: official, + Automated: automated, + Stars: results[i].StarCount, } + paramsArr = append(paramsArr, params) } return paramsArr, nil } func generateSearchOutput(term string, registries []string, opts searchOpts, filter searchFilterParams) error { - searchOutput, err := getSearchOutput(term, registries, opts, filter) - if err != nil { - return err + // searchOutputData is used as a return value for searching in parallel. + type searchOutputData struct { + data []searchParams + err error + } + + // Let's follow Firefox by limiting parallel downloads to 6. + sem := semaphore.NewWeighted(6) + wg := sync.WaitGroup{} + wg.Add(len(registries)) + data := make([]searchOutputData, len(registries)) + + getSearchOutputHelper := func(index int, registry string) { + defer sem.Release(1) + defer wg.Done() + searchOutput, err := getSearchOutput(term, registry, opts, filter) + data[index] = searchOutputData{data: searchOutput, err: err} + } + + ctx := context.Background() + for i := range registries { + sem.Acquire(ctx, 1) + go getSearchOutputHelper(i, registries[i]) + } + + wg.Wait() + searchOutput := []searchParams{} + for _, d := range data { + if d.err != nil { + return d.err + } + searchOutput = append(searchOutput, d.data...) } if len(searchOutput) == 0 { return nil -- cgit v1.2.3-54-g00ecf From 87c9f4cc226a8b709d39de473f09dd341b1a4fc3 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 18 Feb 2019 16:14:34 +0100 Subject: podman-search: refactor code to libpod/image/search.go Refactor the image-search logic from cmd/podman/search.go to libpod/image/search.go and update podman-search and the Varlink API to use it. Signed-off-by: Valentin Rothberg --- cmd/podman/search.go | 267 +++----------------------------------------- libpod/image/search.go | 280 +++++++++++++++++++++++++++++++++++++++++++++++ pkg/varlinkapi/images.go | 59 +++------- 3 files changed, 308 insertions(+), 298 deletions(-) create mode 100644 libpod/image/search.go diff --git a/cmd/podman/search.go b/cmd/podman/search.go index 02f171a68..a772827a6 100644 --- a/cmd/podman/search.go +++ b/cmd/podman/search.go @@ -1,22 +1,14 @@ package main import ( - "context" - "reflect" - "strconv" "strings" - "sync" - "github.com/containers/image/docker" "github.com/containers/image/types" "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/cmd/podman/formats" - "github.com/containers/libpod/libpod/common" - sysreg "github.com/containers/libpod/pkg/registries" + "github.com/containers/libpod/libpod/image" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "golang.org/x/sync/semaphore" ) const ( @@ -56,30 +48,6 @@ func init() { flags.BoolVar(&searchCommand.TlsVerify, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries (default: true)") } -type searchParams struct { - Index string - Name string - Description string - Stars int - Official string - Automated string -} - -type searchOpts struct { - filter []string - limit int - noTrunc bool - format string - authfile string - insecureSkipTLSVerify types.OptionalBool -} - -type searchFilterParams struct { - stars int - isAutomated *bool - isOfficial *bool -} - func searchCmd(c *cliconfig.SearchValues) error { args := c.InputArgs if len(args) > 1 { @@ -90,37 +58,24 @@ func searchCmd(c *cliconfig.SearchValues) error { } term := args[0] - // Check if search term has a registry in it - registry, err := sysreg.GetRegistry(term) - if err != nil { - return errors.Wrapf(err, "error getting registry from %q", term) - } - if registry != "" { - term = term[len(registry)+1:] - } - - format := genSearchFormat(c.Format) - opts := searchOpts{ - format: format, - noTrunc: c.NoTrunc, - limit: c.Limit, - filter: c.Filter, - authfile: getAuthFile(c.Authfile), + searchOptions := image.SearchOptions{ + NoTrunc: c.NoTrunc, + Limit: c.Limit, + Filter: c.Filter, + Authfile: getAuthFile(c.Authfile), } if c.Flag("tls-verify").Changed { - opts.insecureSkipTLSVerify = types.NewOptionalBool(!c.TlsVerify) - } - registries, err := getRegistries(registry) - if err != nil { - return err + searchOptions.InsecureSkipTLSVerify = types.NewOptionalBool(!c.TlsVerify) } - filter, err := parseSearchFilter(&opts) + results, err := image.SearchImages(term, searchOptions) if err != nil { return err } - - return generateSearchOutput(term, registries, opts, *filter) + format := genSearchFormat(c.Format) + out := formats.StdoutTemplateArray{Output: searchToGeneric(results), Template: format, Fields: results[0].HeaderMap()} + formats.Writer(out).Out() + return nil } func genSearchFormat(format string) string { @@ -132,205 +87,9 @@ func genSearchFormat(format string) string { return "table {{.Index}}\t{{.Name}}\t{{.Description}}\t{{.Stars}}\t{{.Official}}\t{{.Automated}}\t" } -func searchToGeneric(params []searchParams) (genericParams []interface{}) { +func searchToGeneric(params []image.SearchResult) (genericParams []interface{}) { for _, v := range params { genericParams = append(genericParams, interface{}(v)) } return genericParams } - -func (s *searchParams) headerMap() map[string]string { - v := reflect.Indirect(reflect.ValueOf(s)) - values := make(map[string]string, v.NumField()) - - for i := 0; i < v.NumField(); i++ { - key := v.Type().Field(i).Name - value := key - values[key] = strings.ToUpper(splitCamelCase(value)) - } - return values -} - -// getRegistries returns the list of registries to search, depending on an optional registry specification -func getRegistries(registry string) ([]string, error) { - var registries []string - if registry != "" { - registries = append(registries, registry) - } else { - var err error - registries, err = sysreg.GetRegistries() - if err != nil { - return nil, errors.Wrapf(err, "error getting registries to search") - } - } - return registries, nil -} - -func getSearchOutput(term string, registry string, opts searchOpts, filter searchFilterParams) ([]searchParams, error) { - // Max number of queries by default is 25 - limit := maxQueries - if opts.limit != 0 { - limit = opts.limit - } - - sc := common.GetSystemContext("", opts.authfile, false) - sc.DockerInsecureSkipTLSVerify = opts.insecureSkipTLSVerify - // FIXME: Set this more globally. Probably no reason not to have it in - // every types.SystemContext, and to compute the value just once in one - // place. - sc.SystemRegistriesConfPath = sysreg.SystemRegistriesConfPath() - results, err := docker.SearchRegistry(context.TODO(), sc, registry, term, limit) - if err != nil { - logrus.Errorf("error searching registry %q: %v", registry, err) - return []searchParams{}, nil - } - index := registry - arr := strings.Split(registry, ".") - if len(arr) > 2 { - index = strings.Join(arr[len(arr)-2:], ".") - } - - // limit is the number of results to output - // if the total number of results is less than the limit, output all - // if the limit has been set by the user, output those number of queries - limit = maxQueries - if len(results) < limit { - limit = len(results) - } - if opts.limit != 0 && opts.limit < len(results) { - limit = opts.limit - } - - paramsArr := []searchParams{} - for i := 0; i < limit; i++ { - if len(opts.filter) > 0 { - // Check whether query matches filters - if !(matchesAutomatedFilter(filter, results[i]) && matchesOfficialFilter(filter, results[i]) && matchesStarFilter(filter, results[i])) { - continue - } - } - official := "" - if results[i].IsOfficial { - official = "[OK]" - } - automated := "" - if results[i].IsAutomated { - automated = "[OK]" - } - description := strings.Replace(results[i].Description, "\n", " ", -1) - if len(description) > 44 && !opts.noTrunc { - description = description[:descriptionTruncLength] + "..." - } - name := registry + "/" + results[i].Name - if index == "docker.io" && !strings.Contains(results[i].Name, "/") { - name = index + "/library/" + results[i].Name - } - params := searchParams{ - Index: index, - Name: name, - Description: description, - Official: official, - Automated: automated, - Stars: results[i].StarCount, - } - paramsArr = append(paramsArr, params) - } - return paramsArr, nil -} - -func generateSearchOutput(term string, registries []string, opts searchOpts, filter searchFilterParams) error { - // searchOutputData is used as a return value for searching in parallel. - type searchOutputData struct { - data []searchParams - err error - } - - // Let's follow Firefox by limiting parallel downloads to 6. - sem := semaphore.NewWeighted(6) - wg := sync.WaitGroup{} - wg.Add(len(registries)) - data := make([]searchOutputData, len(registries)) - - getSearchOutputHelper := func(index int, registry string) { - defer sem.Release(1) - defer wg.Done() - searchOutput, err := getSearchOutput(term, registry, opts, filter) - data[index] = searchOutputData{data: searchOutput, err: err} - } - - ctx := context.Background() - for i := range registries { - sem.Acquire(ctx, 1) - go getSearchOutputHelper(i, registries[i]) - } - - wg.Wait() - searchOutput := []searchParams{} - for _, d := range data { - if d.err != nil { - return d.err - } - searchOutput = append(searchOutput, d.data...) - } - if len(searchOutput) == 0 { - return nil - } - out := formats.StdoutTemplateArray{Output: searchToGeneric(searchOutput), Template: opts.format, Fields: searchOutput[0].headerMap()} - return formats.Writer(out).Out() -} - -func parseSearchFilter(opts *searchOpts) (*searchFilterParams, error) { - filterParams := &searchFilterParams{} - ptrTrue := true - ptrFalse := false - for _, filter := range opts.filter { - arr := strings.Split(filter, "=") - switch arr[0] { - case "stars": - if len(arr) < 2 { - return nil, errors.Errorf("invalid `stars` filter %q, should be stars=", filter) - } - stars, err := strconv.Atoi(arr[1]) - if err != nil { - return nil, errors.Wrapf(err, "incorrect value type for stars filter") - } - filterParams.stars = stars - break - case "is-automated": - if len(arr) == 2 && arr[1] == "false" { - filterParams.isAutomated = &ptrFalse - } else { - filterParams.isAutomated = &ptrTrue - } - break - case "is-official": - if len(arr) == 2 && arr[1] == "false" { - filterParams.isOfficial = &ptrFalse - } else { - filterParams.isOfficial = &ptrTrue - } - break - default: - return nil, errors.Errorf("invalid filter type %q", filter) - } - } - return filterParams, nil -} - -func matchesStarFilter(filter searchFilterParams, result docker.SearchResult) bool { - return result.StarCount >= filter.stars -} - -func matchesAutomatedFilter(filter searchFilterParams, result docker.SearchResult) bool { - if filter.isAutomated != nil { - return result.IsAutomated == *filter.isAutomated - } - return true -} - -func matchesOfficialFilter(filter searchFilterParams, result docker.SearchResult) bool { - if filter.isOfficial != nil { - return result.IsOfficial == *filter.isOfficial - } - return true -} diff --git a/libpod/image/search.go b/libpod/image/search.go new file mode 100644 index 000000000..e9612ef47 --- /dev/null +++ b/libpod/image/search.go @@ -0,0 +1,280 @@ +package image + +import ( + "context" + "reflect" + "strconv" + "strings" + "sync" + + "github.com/containers/image/docker" + "github.com/containers/image/types" + "github.com/containers/libpod/libpod/common" + sysreg "github.com/containers/libpod/pkg/registries" + "github.com/fatih/camelcase" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "golang.org/x/sync/semaphore" +) + +const ( + descriptionTruncLength = 44 + maxQueries = 25 + maxParallelSearches = int64(6) +) + +// SearchResult is holding image-search related data. +type SearchResult struct { + // Index is the image index (e.g., "docker.io" or "quay.io") + Index string + // Name is the canoncical name of the image (e.g., "docker.io/library/alpine"). + Name string + // Description of the image. + Description string + // Stars is the number of stars of the image. + Stars int + // Official indicates if it's an official image. + Official string + // Automated indicates if the image was created by an automated build. + Automated string +} + +// SearchOptions are used to control the behaviour of SearchImages. +type SearchOptions struct { + // Filter allows to filter the results. + Filter []string + // Limit limits the number of queries per index (default: 25). Must be + // greater than 0 to overwrite the default value. + Limit int + // NoTrunc avoids the output to be truncated. + NoTrunc bool + // Authfile is the path to the authentication file. + Authfile string + // InsecureSkipTLSVerify allows to skip TLS verification. + InsecureSkipTLSVerify types.OptionalBool +} + +type searchFilterParams struct { + stars int + isAutomated *bool + isOfficial *bool +} + +func splitCamelCase(src string) string { + entries := camelcase.Split(src) + return strings.Join(entries, " ") +} + +// HeaderMap returns the headers of a SearchResult. +func (s *SearchResult) HeaderMap() map[string]string { + v := reflect.Indirect(reflect.ValueOf(s)) + values := make(map[string]string, v.NumField()) + + for i := 0; i < v.NumField(); i++ { + key := v.Type().Field(i).Name + value := key + values[key] = strings.ToUpper(splitCamelCase(value)) + } + return values +} + +// SearchImages searches images based on term and the specified SearchOptions +// in all registries. +func SearchImages(term string, options SearchOptions) ([]SearchResult, error) { + filter, err := parseSearchFilter(&options) + if err != nil { + return nil, err + } + + // Check if search term has a registry in it + registry, err := sysreg.GetRegistry(term) + if err != nil { + return nil, errors.Wrapf(err, "error getting registry from %q", term) + } + if registry != "" { + term = term[len(registry)+1:] + } + + registries, err := getRegistries(registry) + if err != nil { + return nil, err + } + + // searchOutputData is used as a return value for searching in parallel. + type searchOutputData struct { + data []SearchResult + err error + } + + // Let's follow Firefox by limiting parallel downloads to 6. + sem := semaphore.NewWeighted(maxParallelSearches) + wg := sync.WaitGroup{} + wg.Add(len(registries)) + data := make([]searchOutputData, len(registries)) + + searchImageInRegistryHelper := func(index int, registry string) { + defer sem.Release(1) + defer wg.Done() + searchOutput, err := searchImageInRegistry(term, registry, options, filter) + data[index] = searchOutputData{data: searchOutput, err: err} + } + + ctx := context.Background() + for i := range registries { + sem.Acquire(ctx, 1) + go searchImageInRegistryHelper(i, registries[i]) + } + + wg.Wait() + results := []SearchResult{} + for _, d := range data { + if d.err != nil { + return nil, d.err + } + results = append(results, d.data...) + } + return results, nil +} + +// getRegistries returns the list of registries to search, depending on an optional registry specification +func getRegistries(registry string) ([]string, error) { + var registries []string + if registry != "" { + registries = append(registries, registry) + } else { + var err error + registries, err = sysreg.GetRegistries() + if err != nil { + return nil, errors.Wrapf(err, "error getting registries to search") + } + } + return registries, nil +} + +func searchImageInRegistry(term string, registry string, options SearchOptions, filter *searchFilterParams) ([]SearchResult, error) { + // Max number of queries by default is 25 + limit := maxQueries + if options.Limit > 0 { + limit = options.Limit + } + + sc := common.GetSystemContext("", options.Authfile, false) + sc.DockerInsecureSkipTLSVerify = options.InsecureSkipTLSVerify + // FIXME: Set this more globally. Probably no reason not to have it in + // every types.SystemContext, and to compute the value just once in one + // place. + sc.SystemRegistriesConfPath = sysreg.SystemRegistriesConfPath() + results, err := docker.SearchRegistry(context.TODO(), sc, registry, term, limit) + if err != nil { + logrus.Errorf("error searching registry %q: %v", registry, err) + return []SearchResult{}, nil + } + index := registry + arr := strings.Split(registry, ".") + if len(arr) > 2 { + index = strings.Join(arr[len(arr)-2:], ".") + } + + // limit is the number of results to output + // if the total number of results is less than the limit, output all + // if the limit has been set by the user, output those number of queries + limit = maxQueries + if len(results) < limit { + limit = len(results) + } + if options.Limit != 0 && options.Limit < len(results) { + limit = options.Limit + } + + paramsArr := []SearchResult{} + for i := 0; i < limit; i++ { + if len(options.Filter) > 0 { + // Check whether query matches filters + if !(matchesAutomatedFilter(filter, results[i]) && matchesOfficialFilter(filter, results[i]) && matchesStarFilter(filter, results[i])) { + continue + } + } + official := "" + if results[i].IsOfficial { + official = "[OK]" + } + automated := "" + if results[i].IsAutomated { + automated = "[OK]" + } + description := strings.Replace(results[i].Description, "\n", " ", -1) + if len(description) > 44 && !options.NoTrunc { + description = description[:descriptionTruncLength] + "..." + } + name := registry + "/" + results[i].Name + if index == "docker.io" && !strings.Contains(results[i].Name, "/") { + name = index + "/library/" + results[i].Name + } + params := SearchResult{ + Index: index, + Name: name, + Description: description, + Official: official, + Automated: automated, + Stars: results[i].StarCount, + } + paramsArr = append(paramsArr, params) + } + return paramsArr, nil +} + +func parseSearchFilter(options *SearchOptions) (*searchFilterParams, error) { + filterParams := &searchFilterParams{} + ptrTrue := true + ptrFalse := false + for _, filter := range options.Filter { + arr := strings.Split(filter, "=") + switch arr[0] { + case "stars": + if len(arr) < 2 { + return nil, errors.Errorf("invalid `stars` filter %q, should be stars=", filter) + } + stars, err := strconv.Atoi(arr[1]) + if err != nil { + return nil, errors.Wrapf(err, "incorrect value type for stars filter") + } + filterParams.stars = stars + break + case "is-automated": + if len(arr) == 2 && arr[1] == "false" { + filterParams.isAutomated = &ptrFalse + } else { + filterParams.isAutomated = &ptrTrue + } + break + case "is-official": + if len(arr) == 2 && arr[1] == "false" { + filterParams.isOfficial = &ptrFalse + } else { + filterParams.isOfficial = &ptrTrue + } + break + default: + return nil, errors.Errorf("invalid filter type %q", filter) + } + } + return filterParams, nil +} + +func matchesStarFilter(filter *searchFilterParams, result docker.SearchResult) bool { + return result.StarCount >= filter.stars +} + +func matchesAutomatedFilter(filter *searchFilterParams, result docker.SearchResult) bool { + if filter.isAutomated != nil { + return result.IsAutomated == *filter.isAutomated + } + return true +} + +func matchesOfficialFilter(filter *searchFilterParams, result docker.SearchResult) bool { + if filter.isOfficial != nil { + return result.IsOfficial == *filter.isOfficial + } + return true +} diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go index 6e4974dc4..e80a47684 100644 --- a/pkg/varlinkapi/images.go +++ b/pkg/varlinkapi/images.go @@ -13,7 +13,6 @@ import ( "github.com/containers/buildah" "github.com/containers/buildah/imagebuildah" - "github.com/containers/image/docker" dockerarchive "github.com/containers/image/docker/archive" "github.com/containers/image/manifest" "github.com/containers/image/transports/alltransports" @@ -22,7 +21,6 @@ import ( "github.com/containers/libpod/cmd/podman/varlink" "github.com/containers/libpod/libpod" "github.com/containers/libpod/libpod/image" - sysreg "github.com/containers/libpod/pkg/registries" "github.com/containers/libpod/pkg/util" "github.com/containers/libpod/utils" "github.com/containers/storage/pkg/archive" @@ -437,53 +435,26 @@ func (i *LibpodAPI) RemoveImage(call iopodman.VarlinkCall, name string, force bo // SearchImages searches all registries configured in /etc/containers/registries.conf for an image // Requires an image name and a search limit as int func (i *LibpodAPI) SearchImages(call iopodman.VarlinkCall, query string, limit *int64, tlsVerify *bool) error { - sc := image.GetSystemContext("", "", false) - if tlsVerify != nil { - sc.DockerInsecureSkipTLSVerify = types.NewOptionalBool(!*tlsVerify) + searchOptions := image.SearchOptions{ + Limit: 1000, + InsecureSkipTLSVerify: types.NewOptionalBool(!*tlsVerify), } - var registries []string - - // Check if search query has a registry in it - registry, err := sysreg.GetRegistry(query) + results, err := image.SearchImages(query, searchOptions) if err != nil { - return call.ReplyErrorOccurred(fmt.Sprintf("error getting registry from %q: %q", query, err)) - } - if registry != "" { - registries = append(registries, registry) - query = query[len(registry)+1:] - } else { - registries, err = sysreg.GetRegistries() - if err != nil { - return call.ReplyErrorOccurred(fmt.Sprintf("unable to get system registries: %q", err)) - } + return call.ReplyErrorOccurred(err.Error()) } + var imageResults []iopodman.ImageSearchResult - for _, reg := range registries { - var lim = 1000 - if limit != nil { - lim = int(*limit) - } - results, err := docker.SearchRegistry(getContext(), sc, reg, query, lim) - if err != nil { - // If we are searching multiple registries, don't make something like an - // auth error fatal. Unfortunately we cannot differentiate between auth - // errors and other possibles errors - if len(registries) > 1 { - continue - } - return call.ReplyErrorOccurred(err.Error()) - } - for _, result := range results { - i := iopodman.ImageSearchResult{ - Registry: reg, - Description: result.Description, - Is_official: result.IsOfficial, - Is_automated: result.IsAutomated, - Name: result.Name, - Star_count: int64(result.StarCount), - } - imageResults = append(imageResults, i) + for _, result := range results { + i := iopodman.ImageSearchResult{ + Registry: result.Index, + Description: result.Description, + Is_official: result.Official == "[OK]", + Is_automated: result.Automated == "[OK]", + Name: result.Name, + Star_count: int64(result.Stars), } + imageResults = append(imageResults, i) } return call.ReplySearchImages(imageResults) } -- cgit v1.2.3-54-g00ecf From 0d3eaca28a5485540feee02918abcd6f294e9a1d Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 18 Feb 2019 17:28:26 +0100 Subject: SearchImages: extend API with filter parameter Signed-off-by: Valentin Rothberg --- API.md | 2 +- cmd/podman/varlink/io.podman.varlink | 2 +- pkg/varlinkapi/images.go | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/API.md b/API.md index 1a75e1492..f61afbee4 100755 --- a/API.md +++ b/API.md @@ -107,7 +107,7 @@ in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in [func RestartPod(name: string) string](#RestartPod) -[func SearchImages(query: string, limit: , tlsVerify: ) ImageSearchResult](#SearchImages) +[func SearchImages(query: string, limit: int, tlsVerify: ?bool, filter: []string) ImageSearchResult](#SearchImages) [func SendFile(type: string, length: int) string](#SendFile) diff --git a/cmd/podman/varlink/io.podman.varlink b/cmd/podman/varlink/io.podman.varlink index 20c8a0de5..3bb3aa20a 100644 --- a/cmd/podman/varlink/io.podman.varlink +++ b/cmd/podman/varlink/io.podman.varlink @@ -681,7 +681,7 @@ method RemoveImage(name: string, force: bool) -> (image: string) # SearchImages searches available registries for images that contain the # contents of "query" in their name. If "limit" is given, limits the amount of # search results per registry. -method SearchImages(query: string, limit: ?int, tlsVerify: ?bool) -> (results: []ImageSearchResult) +method SearchImages(query: string, limit: ?int, tlsVerify: ?bool, filter: []string) -> (results: []ImageSearchResult) # DeleteUnusedImages deletes any images not associated with a container. The IDs of the deleted images are returned # in a string array. diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go index e80a47684..f4b8a7e18 100644 --- a/pkg/varlinkapi/images.go +++ b/pkg/varlinkapi/images.go @@ -434,9 +434,10 @@ func (i *LibpodAPI) RemoveImage(call iopodman.VarlinkCall, name string, force bo // SearchImages searches all registries configured in /etc/containers/registries.conf for an image // Requires an image name and a search limit as int -func (i *LibpodAPI) SearchImages(call iopodman.VarlinkCall, query string, limit *int64, tlsVerify *bool) error { +func (i *LibpodAPI) SearchImages(call iopodman.VarlinkCall, query string, limit *int64, tlsVerify *bool, filter []string) error { searchOptions := image.SearchOptions{ Limit: 1000, + Filter: filter, InsecureSkipTLSVerify: types.NewOptionalBool(!*tlsVerify), } results, err := image.SearchImages(query, searchOptions) -- cgit v1.2.3-54-g00ecf From 8a49b59ed403b25340f315b3cb9ea90482ad6cd9 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 19 Feb 2019 15:09:19 +0100 Subject: image.SearchImages: use SearchFilter type Use an `image.SearchFilter` instead of a `[]string` in the SearchImages API. Signed-off-by: Valentin Rothberg --- cmd/podman/search.go | 7 ++++- libpod/image/search.go | 73 +++++++++++++++++++++++------------------------- pkg/varlinkapi/images.go | 7 ++++- 3 files changed, 47 insertions(+), 40 deletions(-) diff --git a/cmd/podman/search.go b/cmd/podman/search.go index a772827a6..f63131c84 100644 --- a/cmd/podman/search.go +++ b/cmd/podman/search.go @@ -58,10 +58,15 @@ func searchCmd(c *cliconfig.SearchValues) error { } term := args[0] + filter, err := image.ParseSearchFilter(c.Filter) + if err != nil { + return err + } + searchOptions := image.SearchOptions{ NoTrunc: c.NoTrunc, Limit: c.Limit, - Filter: c.Filter, + Filter: *filter, Authfile: getAuthFile(c.Authfile), } if c.Flag("tls-verify").Changed { diff --git a/libpod/image/search.go b/libpod/image/search.go index e9612ef47..212eff00b 100644 --- a/libpod/image/search.go +++ b/libpod/image/search.go @@ -42,7 +42,7 @@ type SearchResult struct { // SearchOptions are used to control the behaviour of SearchImages. type SearchOptions struct { // Filter allows to filter the results. - Filter []string + Filter SearchFilter // Limit limits the number of queries per index (default: 25). Must be // greater than 0 to overwrite the default value. Limit int @@ -54,10 +54,14 @@ type SearchOptions struct { InsecureSkipTLSVerify types.OptionalBool } -type searchFilterParams struct { - stars int - isAutomated *bool - isOfficial *bool +// SearchFilter allows filtering the results of SearchImages. +type SearchFilter struct { + // Stars describes the minimal amount of starts of an image. + Stars int + // IsAutomated decides if only images from automated builds are displayed. + IsAutomated types.OptionalBool + // IsOfficial decides if only official images are displayed. + IsOfficial types.OptionalBool } func splitCamelCase(src string) string { @@ -81,11 +85,6 @@ func (s *SearchResult) HeaderMap() map[string]string { // SearchImages searches images based on term and the specified SearchOptions // in all registries. func SearchImages(term string, options SearchOptions) ([]SearchResult, error) { - filter, err := parseSearchFilter(&options) - if err != nil { - return nil, err - } - // Check if search term has a registry in it registry, err := sysreg.GetRegistry(term) if err != nil { @@ -115,7 +114,7 @@ func SearchImages(term string, options SearchOptions) ([]SearchResult, error) { searchImageInRegistryHelper := func(index int, registry string) { defer sem.Release(1) defer wg.Done() - searchOutput, err := searchImageInRegistry(term, registry, options, filter) + searchOutput, err := searchImageInRegistry(term, registry, options) data[index] = searchOutputData{data: searchOutput, err: err} } @@ -151,7 +150,7 @@ func getRegistries(registry string) ([]string, error) { return registries, nil } -func searchImageInRegistry(term string, registry string, options SearchOptions, filter *searchFilterParams) ([]SearchResult, error) { +func searchImageInRegistry(term string, registry string, options SearchOptions) ([]SearchResult, error) { // Max number of queries by default is 25 limit := maxQueries if options.Limit > 0 { @@ -188,11 +187,9 @@ func searchImageInRegistry(term string, registry string, options SearchOptions, paramsArr := []SearchResult{} for i := 0; i < limit; i++ { - if len(options.Filter) > 0 { - // Check whether query matches filters - if !(matchesAutomatedFilter(filter, results[i]) && matchesOfficialFilter(filter, results[i]) && matchesStarFilter(filter, results[i])) { - continue - } + // Check whether query matches filters + if !(options.Filter.matchesAutomatedFilter(results[i]) && options.Filter.matchesOfficialFilter(results[i]) && options.Filter.matchesStarFilter(results[i])) { + continue } official := "" if results[i].IsOfficial { @@ -223,12 +220,12 @@ func searchImageInRegistry(term string, registry string, options SearchOptions, return paramsArr, nil } -func parseSearchFilter(options *SearchOptions) (*searchFilterParams, error) { - filterParams := &searchFilterParams{} - ptrTrue := true - ptrFalse := false - for _, filter := range options.Filter { - arr := strings.Split(filter, "=") +// ParseSearchFilter turns the filter into a SearchFilter that can be used for +// searching images. +func ParseSearchFilter(filter []string) (*SearchFilter, error) { + sFilter := new(SearchFilter) + for _, f := range filter { + arr := strings.Split(f, "=") switch arr[0] { case "stars": if len(arr) < 2 { @@ -238,43 +235,43 @@ func parseSearchFilter(options *SearchOptions) (*searchFilterParams, error) { if err != nil { return nil, errors.Wrapf(err, "incorrect value type for stars filter") } - filterParams.stars = stars + sFilter.Stars = stars break case "is-automated": if len(arr) == 2 && arr[1] == "false" { - filterParams.isAutomated = &ptrFalse + sFilter.IsAutomated = types.OptionalBoolFalse } else { - filterParams.isAutomated = &ptrTrue + sFilter.IsAutomated = types.OptionalBoolTrue } break case "is-official": if len(arr) == 2 && arr[1] == "false" { - filterParams.isOfficial = &ptrFalse + sFilter.IsOfficial = types.OptionalBoolFalse } else { - filterParams.isOfficial = &ptrTrue + sFilter.IsOfficial = types.OptionalBoolTrue } break default: - return nil, errors.Errorf("invalid filter type %q", filter) + return nil, errors.Errorf("invalid filter type %q", f) } } - return filterParams, nil + return sFilter, nil } -func matchesStarFilter(filter *searchFilterParams, result docker.SearchResult) bool { - return result.StarCount >= filter.stars +func (f *SearchFilter) matchesStarFilter(result docker.SearchResult) bool { + return result.StarCount >= f.Stars } -func matchesAutomatedFilter(filter *searchFilterParams, result docker.SearchResult) bool { - if filter.isAutomated != nil { - return result.IsAutomated == *filter.isAutomated +func (f *SearchFilter) matchesAutomatedFilter(result docker.SearchResult) bool { + if f.IsAutomated != types.OptionalBoolUndefined { + return result.IsAutomated == (f.IsAutomated == types.OptionalBoolTrue) } return true } -func matchesOfficialFilter(filter *searchFilterParams, result docker.SearchResult) bool { - if filter.isOfficial != nil { - return result.IsOfficial == *filter.isOfficial +func (f *SearchFilter) matchesOfficialFilter(result docker.SearchResult) bool { + if f.IsOfficial != types.OptionalBoolUndefined { + return result.IsOfficial == (f.IsOfficial == types.OptionalBoolTrue) } return true } diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go index f4b8a7e18..032160b1b 100644 --- a/pkg/varlinkapi/images.go +++ b/pkg/varlinkapi/images.go @@ -435,9 +435,14 @@ func (i *LibpodAPI) RemoveImage(call iopodman.VarlinkCall, name string, force bo // SearchImages searches all registries configured in /etc/containers/registries.conf for an image // Requires an image name and a search limit as int func (i *LibpodAPI) SearchImages(call iopodman.VarlinkCall, query string, limit *int64, tlsVerify *bool, filter []string) error { + sFilter, err := image.ParseSearchFilter(filter) + if err != nil { + return call.ReplyErrorOccurred(err.Error()) + } + searchOptions := image.SearchOptions{ Limit: 1000, - Filter: filter, + Filter: *sFilter, InsecureSkipTLSVerify: types.NewOptionalBool(!*tlsVerify), } results, err := image.SearchImages(query, searchOptions) -- cgit v1.2.3-54-g00ecf From 6ae4401bd1c7988b84c6e1f715ca85ad74ee4048 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 19 Feb 2019 16:29:14 +0100 Subject: iopodman.SearchImages: add ImageSearchFilter to Varlink API Also add some argument checks to the Varlink function to avoid referencing nil pointers, and complement the API.md descriptions. The varlink endpoint can be tested via varlink CLI: $ varlink call -m unix:/run/podman/io.podman/io.podman.SearchImages \ '{"query": "ruby", "limit": 0, "tlsVerify": false, "filter": {}}' Signed-off-by: Valentin Rothberg --- API.md | 13 ++++++++++++- cmd/podman/varlink/io.podman.varlink | 8 +++++++- pkg/varlinkapi/images.go | 34 +++++++++++++++++++++++++++------- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/API.md b/API.md index f61afbee4..c1d8c6a36 100755 --- a/API.md +++ b/API.md @@ -107,7 +107,7 @@ in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in [func RestartPod(name: string) string](#RestartPod) -[func SearchImages(query: string, limit: int, tlsVerify: ?bool, filter: []string) ImageSearchResult](#SearchImages) +[func SearchImages(query: string, limit: int, tlsVerify: ?bool, filter: ImageSearchFilter) ImageSearchResult](#SearchImages) [func SendFile(type: string, length: int) string](#SendFile) @@ -163,6 +163,8 @@ in the [API.md](https://github.com/containers/libpod/blob/master/API.md) file in [type ImageHistory](#ImageHistory) +[type ImageSearchFilter](#ImageSearchFilter) + [type ImageSearchResult](#ImageSearchResult) [type InfoDistribution](#InfoDistribution) @@ -1408,6 +1410,15 @@ tags [[]string](#[]string) size [int](https://godoc.org/builtin#int) comment [string](https://godoc.org/builtin#string) +### type ImageSearchFilter + +Represents a filter for SearchImages + +is_official [bool](https://godoc.org/builtin#bool) + +is_automated [bool](https://godoc.org/builtin#bool) + +star_count [int](https://godoc.org/builtin#int) ### type ImageSearchResult Represents a single search result from SearchImages diff --git a/cmd/podman/varlink/io.podman.varlink b/cmd/podman/varlink/io.podman.varlink index 3bb3aa20a..c53a5454a 100644 --- a/cmd/podman/varlink/io.podman.varlink +++ b/cmd/podman/varlink/io.podman.varlink @@ -72,6 +72,12 @@ type ImageSearchResult ( star_count: int ) +type ImageSearchFilter ( + is_official: ?bool, + is_automated: ?bool, + star_count: int +) + type Container ( id: string, image: string, @@ -681,7 +687,7 @@ method RemoveImage(name: string, force: bool) -> (image: string) # SearchImages searches available registries for images that contain the # contents of "query" in their name. If "limit" is given, limits the amount of # search results per registry. -method SearchImages(query: string, limit: ?int, tlsVerify: ?bool, filter: []string) -> (results: []ImageSearchResult) +method SearchImages(query: string, limit: ?int, tlsVerify: ?bool, filter: ImageSearchFilter) -> (results: []ImageSearchResult) # DeleteUnusedImages deletes any images not associated with a container. The IDs of the deleted images are returned # in a string array. diff --git a/pkg/varlinkapi/images.go b/pkg/varlinkapi/images.go index 032160b1b..d12ab97ab 100644 --- a/pkg/varlinkapi/images.go +++ b/pkg/varlinkapi/images.go @@ -434,16 +434,36 @@ func (i *LibpodAPI) RemoveImage(call iopodman.VarlinkCall, name string, force bo // SearchImages searches all registries configured in /etc/containers/registries.conf for an image // Requires an image name and a search limit as int -func (i *LibpodAPI) SearchImages(call iopodman.VarlinkCall, query string, limit *int64, tlsVerify *bool, filter []string) error { - sFilter, err := image.ParseSearchFilter(filter) - if err != nil { - return call.ReplyErrorOccurred(err.Error()) +func (i *LibpodAPI) SearchImages(call iopodman.VarlinkCall, query string, limit *int64, tlsVerify *bool, filter iopodman.ImageSearchFilter) error { + // Transform all arguments to proper types first + argLimit := 0 + argTLSVerify := types.OptionalBoolUndefined + argIsOfficial := types.OptionalBoolUndefined + argIsAutomated := types.OptionalBoolUndefined + if limit != nil { + argLimit = int(*limit) + } + if tlsVerify != nil { + argTLSVerify = types.NewOptionalBool(!*tlsVerify) + } + if filter.Is_official != nil { + argIsOfficial = types.NewOptionalBool(*filter.Is_official) + } + if filter.Is_automated != nil { + argIsAutomated = types.NewOptionalBool(*filter.Is_automated) + } + + // Transform a SearchFilter the backend can deal with + sFilter := image.SearchFilter{ + IsOfficial: argIsOfficial, + IsAutomated: argIsAutomated, + Stars: int(filter.Star_count), } searchOptions := image.SearchOptions{ - Limit: 1000, - Filter: *sFilter, - InsecureSkipTLSVerify: types.NewOptionalBool(!*tlsVerify), + Limit: argLimit, + Filter: sFilter, + InsecureSkipTLSVerify: argTLSVerify, } results, err := image.SearchImages(query, searchOptions) if err != nil { -- cgit v1.2.3-54-g00ecf