Fixup search

podman-remote search had some FIXMEs in tests that were failing.
So I reworked the search handler to use the local abi.  This
means the podman search and podman-remote search will use the
same functions.

While doing this, I noticed we were just outputing errors via
logrus.Error rather then returning them, which works ok for
podman but the messages get lost on podman-remote.  Changed
the code to actually return the error messages to the caller.

This allows us to turn on the remaining podman-remote FIXME
tests.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

libpod/image/search.go

@@ -102,8 +102,8 @@ func SearchImages(term string, options SearchOptions) ([]SearchResult, error) {
 	searchImageInRegistryHelper := func(index int, registry string) {
 		defer sem.Release(1)
 		defer wg.Done()
-		searchOutput := searchImageInRegistry(term, registry, options)
-		data[index] = searchOutputData{data: searchOutput}
+		searchOutput, err := searchImageInRegistry(term, registry, options)
+		data[index] = searchOutputData{data: searchOutput, err: err}
 	}
 
 	ctx := context.Background()
@@ -116,14 +116,22 @@ func SearchImages(term string, options SearchOptions) ([]SearchResult, error) {
 
 	wg.Wait()
 	results := []SearchResult{}
+	var lastError error
 	for _, d := range data {
 		if d.err != nil {
-			return nil, d.err
+			if lastError != nil {
+				logrus.Errorf("%v", lastError)
+			}
+			lastError = d.err
+			continue
 		}
 		results = append(results, d.data...)
 	}
+	if len(results) > 0 {
 		return results, nil
 	}
+	return results, lastError
+}
 
 // getRegistries returns the list of registries to search, depending on an optional registry specification
 func getRegistries(registry string) ([]string, error) {
@@ -140,7 +148,7 @@ func getRegistries(registry string) ([]string, error) {
 	return registries, nil
 }
 
-func searchImageInRegistry(term string, registry string, options SearchOptions) []SearchResult {
+func searchImageInRegistry(term string, registry string, options SearchOptions) ([]SearchResult, error) {
 	// Max number of queries by default is 25
 	limit := maxQueries
 	if options.Limit > 0 {
@@ -156,16 +164,14 @@ func searchImageInRegistry(term string, registry string, options SearchOptions)
 	if options.ListTags {
 		results, err := searchRepositoryTags(registry, term, sc, options)
 		if err != nil {
-			logrus.Errorf("error listing registry tags %q: %v", registry, err)
-			return []SearchResult{}
+			return []SearchResult{}, err
 		}
-		return results
+		return results, nil
 	}
 
 	results, err := docker.SearchRegistry(context.TODO(), sc, registry, term, limit)
 	if err != nil {
-		logrus.Errorf("error searching registry %q: %v", registry, err)
-		return []SearchResult{}
+		return []SearchResult{}, err
 	}
 	index := registry
 	arr := strings.Split(registry, ".")
@@ -219,7 +225,7 @@ func searchImageInRegistry(term string, registry string, options SearchOptions)
 		}
 		paramsArr = append(paramsArr, params)
 	}
-	return paramsArr
+	return paramsArr, nil
 }
 
 func searchRepositoryTags(registry, term string, sc *types.SystemContext, options SearchOptions) ([]SearchResult, error) {

pkg/api/handlers/compat/images_search.go

@@ -1,25 +1,30 @@
 package compat
 
 import (
+	"fmt"
 	"net/http"
-	"strconv"
 
 	"github.com/containers/image/v5/types"
-	"github.com/containers/podman/v2/libpod/image"
+	"github.com/containers/podman/v2/libpod"
+	"github.com/containers/podman/v2/libpod/define"
 	"github.com/containers/podman/v2/pkg/api/handlers/utils"
 	"github.com/containers/podman/v2/pkg/auth"
-	"github.com/docker/docker/api/types/registry"
+	"github.com/containers/podman/v2/pkg/domain/entities"
+	"github.com/containers/podman/v2/pkg/domain/infra/abi"
 	"github.com/gorilla/schema"
 	"github.com/pkg/errors"
 )
 
 func SearchImages(w http.ResponseWriter, r *http.Request) {
+	runtime := r.Context().Value("runtime").(*libpod.Runtime)
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
 	query := struct {
 		Term      string              `json:"term"`
 		Limit     int                 `json:"limit"`
+		NoTrunc   bool                `json:"noTrunc"`
 		Filters   map[string][]string `json:"filters"`
 		TLSVerify bool                `json:"tlsVerify"`
+		ListTags  bool                `json:"listTags"`
 	}{
 		// This is where you can override the golang default value for one of fields
 	}
@@ -29,67 +34,40 @@ func SearchImages(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	filter := image.SearchFilter{}
-	if len(query.Filters) > 0 {
-		if len(query.Filters["stars"]) > 0 {
-			stars, err := strconv.Atoi(query.Filters["stars"][0])
-			if err != nil {
-				utils.InternalServerError(w, err)
-				return
-			}
-			filter.Stars = stars
-		}
-		if len(query.Filters["is-official"]) > 0 {
-			isOfficial, err := strconv.ParseBool(query.Filters["is-official"][0])
-			if err != nil {
-				utils.InternalServerError(w, err)
-				return
-			}
-			filter.IsOfficial = types.NewOptionalBool(isOfficial)
-		}
-		if len(query.Filters["is-automated"]) > 0 {
-			isAutomated, err := strconv.ParseBool(query.Filters["is-automated"][0])
-			if err != nil {
-				utils.InternalServerError(w, err)
-				return
-			}
-			filter.IsAutomated = types.NewOptionalBool(isAutomated)
-		}
-	}
-	options := image.SearchOptions{
-		Filter: filter,
-		Limit:  query.Limit,
-	}
-
-	if _, found := r.URL.Query()["tlsVerify"]; found {
-		options.InsecureSkipTLSVerify = types.NewOptionalBool(!query.TLSVerify)
-	}
-
 	_, authfile, key, err := auth.GetCredentials(r)
 	if err != nil {
 		utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String()))
 		return
 	}
 	defer auth.RemoveAuthfile(authfile)
-	options.Authfile = authfile
 
-	results, err := image.SearchImages(query.Term, options)
+	filters := []string{}
+	for key, val := range query.Filters {
+		filters = append(filters, fmt.Sprintf("%s=%s", key, val[0]))
+	}
+
+	options := entities.ImageSearchOptions{
+		Authfile: authfile,
+		Limit:    query.Limit,
+		NoTrunc:  query.NoTrunc,
+		ListTags: query.ListTags,
+		Filters:  filters,
+	}
+	if _, found := r.URL.Query()["tlsVerify"]; found {
+		options.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify)
+	}
+	ir := abi.ImageEngine{Libpod: runtime}
+	reports, err := ir.Search(r.Context(), query.Term, options)
 	if err != nil {
-		utils.BadRequest(w, "term", query.Term, err)
+		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
 		return
 	}
-
-	compatResults := make([]registry.SearchResult, 0, len(results))
-	for _, result := range results {
-		compatResult := registry.SearchResult{
-			Name:        result.Name,
-			Description: result.Description,
-			StarCount:   result.Stars,
-			IsAutomated: result.Automated == "[OK]",
-			IsOfficial:  result.Official == "[OK]",
+	if !utils.IsLibpodRequest(r) {
+		if len(reports) == 0 {
+			utils.ImageNotFound(w, query.Term, define.ErrNoSuchImage)
+			return
 		}
-		compatResults = append(compatResults, compatResult)
 	}
 
-	utils.WriteResponse(w, http.StatusOK, compatResults)
+	utils.WriteResponse(w, http.StatusOK, reports)
 }

pkg/api/handlers/libpod/images.go

@@ -589,92 +589,6 @@ func UntagImage(w http.ResponseWriter, r *http.Request) {
 	utils.WriteResponse(w, http.StatusCreated, "")
 }
 
-func SearchImages(w http.ResponseWriter, r *http.Request) {
-	decoder := r.Context().Value("decoder").(*schema.Decoder)
-	query := struct {
-		Term      string              `json:"term"`
-		Limit     int                 `json:"limit"`
-		NoTrunc   bool                `json:"noTrunc"`
-		Filters   map[string][]string `json:"filters"`
-		TLSVerify bool                `json:"tlsVerify"`
-		ListTags  bool                `json:"listTags"`
-	}{
-		// This is where you can override the golang default value for one of fields
-	}
-
-	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
-		utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
-		return
-	}
-
-	filter := image.SearchFilter{}
-	if len(query.Filters) > 0 {
-		if len(query.Filters["stars"]) > 0 {
-			stars, err := strconv.Atoi(query.Filters["stars"][0])
-			if err != nil {
-				utils.InternalServerError(w, err)
-				return
-			}
-			filter.Stars = stars
-		}
-		if len(query.Filters["is-official"]) > 0 {
-			isOfficial, err := strconv.ParseBool(query.Filters["is-official"][0])
-			if err != nil {
-				utils.InternalServerError(w, err)
-				return
-			}
-			filter.IsOfficial = types.NewOptionalBool(isOfficial)
-		}
-		if len(query.Filters["is-automated"]) > 0 {
-			isAutomated, err := strconv.ParseBool(query.Filters["is-automated"][0])
-			if err != nil {
-				utils.InternalServerError(w, err)
-				return
-			}
-			filter.IsAutomated = types.NewOptionalBool(isAutomated)
-		}
-	}
-	options := image.SearchOptions{
-		Limit:    query.Limit,
-		NoTrunc:  query.NoTrunc,
-		ListTags: query.ListTags,
-		Filter:   filter,
-	}
-
-	if _, found := r.URL.Query()["tlsVerify"]; found {
-		options.InsecureSkipTLSVerify = types.NewOptionalBool(!query.TLSVerify)
-	}
-
-	_, authfile, key, err := auth.GetCredentials(r)
-	if err != nil {
-		utils.Error(w, "failed to retrieve repository credentials", http.StatusBadRequest, errors.Wrapf(err, "failed to parse %q header for %s", key, r.URL.String()))
-		return
-	}
-	defer auth.RemoveAuthfile(authfile)
-	options.Authfile = authfile
-
-	searchResults, err := image.SearchImages(query.Term, options)
-	if err != nil {
-		utils.BadRequest(w, "term", query.Term, err)
-		return
-	}
-	// Convert from image.SearchResults to entities.ImageSearchReport. We don't
-	// want to leak any low-level packages into the remote client, which
-	// requires converting.
-	reports := make([]entities.ImageSearchReport, len(searchResults))
-	for i := range searchResults {
-		reports[i].Index = searchResults[i].Index
-		reports[i].Name = searchResults[i].Name
-		reports[i].Description = searchResults[i].Description
-		reports[i].Stars = searchResults[i].Stars
-		reports[i].Official = searchResults[i].Official
-		reports[i].Automated = searchResults[i].Automated
-		reports[i].Tag = searchResults[i].Tag
-	}
-
-	utils.WriteResponse(w, http.StatusOK, reports)
-}
-
 // ImagesBatchRemove is the endpoint for batch image removal.
 func ImagesBatchRemove(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)

pkg/api/server/register_images.go

@@ -1019,7 +1019,7 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
 	//      $ref: "#/responses/DocsSearchResponse"
 	//   500:
 	//      $ref: '#/responses/InternalError'
-	r.Handle(VersionedPath("/libpod/images/search"), s.APIHandler(libpod.SearchImages)).Methods(http.MethodGet)
+	r.Handle(VersionedPath("/libpod/images/search"), s.APIHandler(compat.SearchImages)).Methods(http.MethodGet)
 	// swagger:operation GET /libpod/images/{name:.*}/get libpod libpodExportImage
 	// ---
 	// tags:

test/e2e/search_test.go

@@ -299,7 +299,6 @@ registries = ['{{.Host}}:{{.Port}}']`
 	})
 
 	It("podman search doesn't attempt HTTP if force secure is true", func() {
-		SkipIfRemote("FIXME This should work on podman-remote")
 		if podmanTest.Host.Arch == "ppc64le" {
 			Skip("No registry image for ppc64le")
 		}
@@ -324,15 +323,11 @@ registries = ['{{.Host}}:{{.Port}}']`
 		registryFileTmpl.Execute(&buffer, registryEndpoints[5])
 		podmanTest.setRegistriesConfigEnv(buffer.Bytes())
 		ioutil.WriteFile(fmt.Sprintf("%s/registry5.conf", tempdir), buffer.Bytes(), 0644)
-		if IsRemote() {
-			podmanTest.RestartRemoteService()
-			defer podmanTest.RestartRemoteService()
-		}
 
 		search := podmanTest.Podman([]string{"search", image, "--tls-verify=true"})
 		search.WaitWithDefaultTimeout()
 
-		Expect(search.ExitCode()).To(Equal(0))
+		Expect(search.ExitCode()).To(Equal(125))
 		Expect(search.OutputToString()).Should(BeEmpty())
 		match, _ := search.ErrorGrepString("error")
 		Expect(match).Should(BeTrue())
@@ -342,7 +337,6 @@ registries = ['{{.Host}}:{{.Port}}']`
 	})
 
 	It("podman search doesn't attempt HTTP if registry is not listed as insecure", func() {
-		SkipIfRemote("FIXME This should work on podman-remote")
 		if podmanTest.Host.Arch == "ppc64le" {
 			Skip("No registry image for ppc64le")
 		}
@@ -376,7 +370,7 @@ registries = ['{{.Host}}:{{.Port}}']`
 		search := podmanTest.Podman([]string{"search", image})
 		search.WaitWithDefaultTimeout()
 
-		Expect(search.ExitCode()).To(Equal(0))
+		Expect(search.ExitCode()).To(Equal(125))
 		Expect(search.OutputToString()).Should(BeEmpty())
 		match, _ := search.ErrorGrepString("error")
 		Expect(match).Should(BeTrue())
@@ -386,7 +380,6 @@ registries = ['{{.Host}}:{{.Port}}']`
 	})
 
 	It("podman search doesn't attempt HTTP if one registry is not listed as insecure", func() {
-		SkipIfRemote("FIXME This should work on podman-remote")
 		if podmanTest.Host.Arch == "ppc64le" {
 			Skip("No registry image for ppc64le")
 		}
@@ -431,7 +424,7 @@ registries = ['{{.Host}}:{{.Port}}']`
 		search := podmanTest.Podman([]string{"search", "my-alpine"})
 		search.WaitWithDefaultTimeout()
 
-		Expect(search.ExitCode()).To(Equal(0))
+		Expect(search.ExitCode()).To(Equal(125))
 		Expect(search.OutputToString()).Should(BeEmpty())
 		match, _ := search.ErrorGrepString("error")
 		Expect(match).Should(BeTrue())

test/python/docker/test_images.py

@@ -82,8 +82,16 @@ class TestImages(unittest.TestCase):
 
     def test_search_image(self):
         """Search for image"""
-        for r in self.client.images.search("libpod/alpine"):
-            self.assertIn("quay.io/libpod/alpine", r["Name"])
+        for r in self.client.images.search("alpine"):
+            self.assertIn("alpine", r["Name"])
+
+    def test_search_bogus_image(self):
+        """Search for bogus image should throw exception"""
+        try:
+            r = self.client.images.search("bogus/bogus")
+        except:
+            return
+        self.assertTrue(len(r)==0)
 
     def test_remove_image(self):
         """Remove image"""