From b5c0d15683de4c4d1f604cc34378c733295982fb Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Mon, 18 Feb 2019 11:29:14 +0100
Subject: [PATCH 1/5] 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 <rothberg@redhat.com>
---
cmd/podman/search.go | 148 ++++++++++++++++++++++++++-----------------
1 file changed, 90 insertions(+), 58 deletions(-)
diff --git a/cmd/podman/search.go b/cmd/podman/search.go
index c8b64039a7..02f171a686 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
- }
- index := reg
- arr := strings.Split(reg, ".")
- if len(arr) > 2 {
- index = strings.Join(arr[len(arr)-2:], ".")
- }
+ // 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
- }
+ // 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
+ }
- 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
- }
+ 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 := 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)
}
+ 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 {
- 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
From 87c9f4cc226a8b709d39de473f09dd341b1a4fc3 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Mon, 18 Feb 2019 16:14:34 +0100
Subject: [PATCH 2/5] 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 <rothberg@redhat.com>
---
cmd/podman/search.go | 269 ++-----------------------------------
libpod/image/search.go | 280 +++++++++++++++++++++++++++++++++++++++
pkg/varlinkapi/images.go | 59 +++------
3 files changed, 309 insertions(+), 299 deletions(-)
create mode 100644 libpod/image/search.go
diff --git a/cmd/podman/search.go b/cmd/podman/search.go
index 02f171a686..a772827a62 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)
+ searchOptions.InsecureSkipTLSVerify = types.NewOptionalBool(!c.TlsVerify)
}
- registries, err := getRegistries(registry)
+
+ results, err := image.SearchImages(term, searchOptions)
if err != nil {
return err
}
-
- filter, err := parseSearchFilter(&opts)
- 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=<value>", 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 0000000000..e9612ef479
--- /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=<value>", 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 6e4974dc42..e80a47684a 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)
}
From 0d3eaca28a5485540feee02918abcd6f294e9a1d Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Mon, 18 Feb 2019 17:28:26 +0100
Subject: [PATCH 3/5] SearchImages: extend API with filter parameter
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
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 1a75e14925..f61afbee40 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 20c8a0de55..3bb3aa20ad 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 e80a47684a..f4b8a7e181 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)
From 8a49b59ed403b25340f315b3cb9ea90482ad6cd9 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Tue, 19 Feb 2019 15:09:19 +0100
Subject: [PATCH 4/5] image.SearchImages: use SearchFilter type
Use an `image.SearchFilter` instead of a `[]string` in the SearchImages
API.
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
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 a772827a62..f63131c84c 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 e9612ef479..212eff00b9 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 f4b8a7e181..032160b1b2 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)
From 6ae4401bd1c7988b84c6e1f715ca85ad74ee4048 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Tue, 19 Feb 2019 16:29:14 +0100
Subject: [PATCH 5/5] 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 <rothberg@redhat.com>
---
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 f61afbee40..c1d8c6a362 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)
+### <a name="ImageSearchFilter"></a>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)
### <a name="ImageSearchResult"></a>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 3bb3aa20ad..c53a5454af 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 032160b1b2..d12ab97ab1 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 {