Merge pull request #5319 from baude/apiv2volumestests

binding tests for volumes

7 files changed, 391 insertions, 47 deletions

diff --git a/pkg/api/handlers/libpod/volumes.go b/pkg/api/handlers/libpod/volumes.go
index 7e7e46718..9b10ee890 100644
--- a/pkg/api/handlers/libpod/volumes.go
+++ b/pkg/api/handlers/libpod/volumes.go
@@ -3,9 +3,11 @@ package libpod
 import (
 	"encoding/json"
 	"net/http"
+	"strings"
 
 	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/pkg/api/handlers"
 	"github.com/containers/libpod/pkg/api/handlers/utils"
 	"github.com/gorilla/schema"
@@ -29,7 +31,6 @@ func CreateVolume(w http.ResponseWriter, r *http.Request) {
 			errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String()))
 		return
 	}
-
 	// decode params from body
 	if err := json.NewDecoder(r.Body).Decode(&input); err != nil {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()"))
@@ -49,14 +50,21 @@ func CreateVolume(w http.ResponseWriter, r *http.Request) {
 		parsedOptions, err := shared.ParseVolumeOptions(input.Opts)
 		if err != nil {
 			utils.InternalServerError(w, err)
+			return
 		}
 		volumeOptions = append(volumeOptions, parsedOptions...)
 	}
 	vol, err := runtime.NewVolume(r.Context(), volumeOptions...)
 	if err != nil {
 		utils.InternalServerError(w, err)
+		return
+	}
+	config, err := vol.Config()
+	if err != nil {
+		utils.InternalServerError(w, err)
+		return
 	}
-	utils.WriteResponse(w, http.StatusOK, vol.Name())
+	utils.WriteResponse(w, http.StatusOK, config)
 }
 
 func InspectVolume(w http.ResponseWriter, r *http.Request) {
@@ -76,25 +84,46 @@ func InspectVolume(w http.ResponseWriter, r *http.Request) {
 }
 
 func ListVolumes(w http.ResponseWriter, r *http.Request) {
-	//var (
-	//	runtime = r.Context().Value("runtime").(*libpod.Runtime)
-	//	decoder = r.Context().Value("decoder").(*schema.Decoder)
-	//)
-	//query := struct {
-	//	Filter string `json:"filter"`
-	//}{
-	//	// override any golang type defaults
-	//}
-	//
-	//if err := decoder.Decode(&query, r.URL.Query()); err != nil {
-	//	utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
-	//		errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String()))
-	//	return
-	//}
-	/*
-		This is all in main in cmd and needs to be extracted from there first.
-	*/
+	var (
+		decoder       = r.Context().Value("decoder").(*schema.Decoder)
+		err           error
+		runtime       = r.Context().Value("runtime").(*libpod.Runtime)
+		volumeConfigs []*libpod.VolumeConfig
+		volumeFilters []libpod.VolumeFilter
+	)
+	query := struct {
+		Filters map[string][]string `schema:"filters"`
+	}{
+		// override any golang type defaults
+	}
+
+	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
+		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
+			errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String()))
+		return
+	}
 
+	if len(query.Filters) > 0 {
+		volumeFilters, err = generateVolumeFilters(query.Filters)
+		if err != nil {
+			utils.InternalServerError(w, err)
+			return
+		}
+	}
+	vols, err := runtime.Volumes(volumeFilters...)
+	if err != nil {
+		utils.InternalServerError(w, err)
+		return
+	}
+	for _, v := range vols {
+		config, err := v.Config()
+		if err != nil {
+			utils.InternalServerError(w, err)
+			return
+		}
+		volumeConfigs = append(volumeConfigs, config)
+	}
+	utils.WriteResponse(w, http.StatusOK, volumeConfigs)
 }
 
 func PruneVolumes(w http.ResponseWriter, r *http.Request) {
@@ -133,9 +162,77 @@ func RemoveVolume(w http.ResponseWriter, r *http.Request) {
 	vol, err := runtime.LookupVolume(name)
 	if err != nil {
 		utils.VolumeNotFound(w, name, err)
+		return
 	}
 	if err := runtime.RemoveVolume(r.Context(), vol, query.Force); err != nil {
+		if errors.Cause(err) == define.ErrVolumeBeingUsed {
+			utils.Error(w, "volumes being used", http.StatusConflict, err)
+			return
+		}
 		utils.InternalServerError(w, err)
+		return
 	}
 	utils.WriteResponse(w, http.StatusNoContent, "")
 }
+
+func generateVolumeFilters(filters map[string][]string) ([]libpod.VolumeFilter, error) {
+	var vf []libpod.VolumeFilter
+	for filter, v := range filters {
+		for _, val := range v {
+			switch filter {
+			case "name":
+				nameVal := val
+				vf = append(vf, func(v *libpod.Volume) bool {
+					return nameVal == v.Name()
+				})
+			case "driver":
+				driverVal := val
+				vf = append(vf, func(v *libpod.Volume) bool {
+					return v.Driver() == driverVal
+				})
+			case "scope":
+				scopeVal := val
+				vf = append(vf, func(v *libpod.Volume) bool {
+					return v.Scope() == scopeVal
+				})
+			case "label":
+				filterArray := strings.SplitN(val, "=", 2)
+				filterKey := filterArray[0]
+				var filterVal string
+				if len(filterArray) > 1 {
+					filterVal = filterArray[1]
+				} else {
+					filterVal = ""
+				}
+				vf = append(vf, func(v *libpod.Volume) bool {
+					for labelKey, labelValue := range v.Labels() {
+						if labelKey == filterKey && ("" == filterVal || labelValue == filterVal) {
+							return true
+						}
+					}
+					return false
+				})
+			case "opt":
+				filterArray := strings.SplitN(val, "=", 2)
+				filterKey := filterArray[0]
+				var filterVal string
+				if len(filterArray) > 1 {
+					filterVal = filterArray[1]
+				} else {
+					filterVal = ""
+				}
+				vf = append(vf, func(v *libpod.Volume) bool {
+					for labelKey, labelValue := range v.Options() {
+						if labelKey == filterKey && ("" == filterVal || labelValue == filterVal) {
+							return true
+						}
+					}
+					return false
+				})
+			default:
+				return nil, errors.Errorf("%q is in an invalid volume filter", filter)
+			}
+		}
+	}
+	return vf, nil
+}
diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go
index c72b0f817..2930a9567 100644
--- a/pkg/api/handlers/types.go
+++ b/pkg/api/handlers/types.go
@@ -128,11 +128,16 @@ type CreateContainerConfig struct {
 	NetworkingConfig dockerNetwork.NetworkingConfig
 }
 
+// swagger:model VolumeCreate
 type VolumeCreateConfig struct {
-	Name   string            `json:"name"`
-	Driver string            `schema:"driver"`
-	Label  map[string]string `schema:"label"`
-	Opts   map[string]string `schema:"opts"`
+	// New volume's name. Can be left blank
+	Name string `schema:"name"`
+	// Volume driver to use
+	Driver string `schema:"driver"`
+	// User-defined key/value metadata.
+	Label map[string]string `schema:"label"`
+	// Mapping of driver options and values.
+	Opts map[string]string `schema:"opts"`
 }
 
 type IDResponse struct {
diff --git a/pkg/api/server/register_volumes.go b/pkg/api/server/register_volumes.go
index efe56a3ad..d1317904b 100644
--- a/pkg/api/server/register_volumes.go
+++ b/pkg/api/server/register_volumes.go
@@ -11,15 +11,42 @@ func (s *APIServer) registerVolumeHandlers(r *mux.Router) error {
 	// swagger:operation POST /libpod/volumes/create volumes createVolume
 	// ---
 	// summary: Create a volume
+	// parameters:
+	//  - in: body
+	//    name: create
+	//    description: attributes for creating a container
+	//    schema:
+	//      $ref: "#/definitions/VolumeCreate"
+	// produces:
+	// - application/json
+	// responses:
+	//   '201':
+	//     $ref: "#/responses/VolumeCreateResponse"
+	//   '500':
+	//      "$ref": "#/responses/InternalError"
+	r.Handle(VersionedPath("/libpod/volumes/create"), s.APIHandler(libpod.CreateVolume)).Methods(http.MethodPost)
+	// swagger:operation POST /libpod/volumes/json volumes listVolumes
+	// ---
+	// summary: List volumes
+	// description: Returns a list of networks
 	// produces:
 	// - application/json
+	// parameters:
+	//  - in: query
+	//    name: filters
+	//    type: string
+	//    description: |
+	//      JSON encoded value of the filters (a map[string][]string) to process on the networks list. Available filters:
+	//        - driver=<volume-driver-name> Matches volumes based on their driver.
+	//        - label=<key> or label=<key>:<value> Matches volumes based on the presence of a label alone or a label and a value.
+	//        - name=<volume-name> Matches all of volume name.
+	//        - opt=<driver-option> Matches a storage driver options
 	// responses:
 	//   '200':
-	//     description: tbd
+	//     "$ref": "#/responses/VolumeList"
 	//   '500':
 	//      "$ref": "#/responses/InternalError"
-	r.Handle("/libpod/volumes/create", s.APIHandler(libpod.CreateVolume)).Methods(http.MethodPost)
-	r.Handle("/libpod/volumes/json", s.APIHandler(libpod.ListVolumes)).Methods(http.MethodGet)
+	r.Handle(VersionedPath("/libpod/volumes/json"), s.APIHandler(libpod.ListVolumes)).Methods(http.MethodGet)
 	// swagger:operation POST /libpod/volumes/prune volumes pruneVolumes
 	// ---
 	// summary: Prune volumes
@@ -30,7 +57,7 @@ func (s *APIServer) registerVolumeHandlers(r *mux.Router) error {
 	//     description: no error
 	//   '500':
 	//      "$ref": "#/responses/InternalError"
-	r.Handle("/libpod/volumes/prune", s.APIHandler(libpod.PruneVolumes)).Methods(http.MethodPost)
+	r.Handle(VersionedPath("/libpod/volumes/prune"), s.APIHandler(libpod.PruneVolumes)).Methods(http.MethodPost)
 	// swagger:operation GET /libpod/volumes/{name}/json volumes inspectVolume
 	// ---
 	// summary: Inspect volume
@@ -49,7 +76,7 @@ func (s *APIServer) registerVolumeHandlers(r *mux.Router) error {
 	//       "$ref": "#/responses/NoSuchVolume"
 	//   '500':
 	//      "$ref": "#/responses/InternalError"
-	r.Handle("/libpod/volumes/{name}/json", s.APIHandler(libpod.InspectVolume)).Methods(http.MethodGet)
+	r.Handle(VersionedPath("/libpod/volumes/{name}/json"), s.APIHandler(libpod.InspectVolume)).Methods(http.MethodGet)
 	// swagger:operation DELETE /libpod/volumes/{name} volumes removeVolume
 	// ---
 	// summary: Remove volume
@@ -68,12 +95,12 @@ func (s *APIServer) registerVolumeHandlers(r *mux.Router) error {
 	// responses:
 	//   204:
 	//     description: no error
-	//   400:
-	//     $ref: "#/responses/BadParamError"
 	//   404:
 	//     $ref: "#/responses/NoSuchVolume"
+	//   409:
+	//     description: Volume is in use and cannot be removed
 	//   500:
 	//     $ref: "#/responses/InternalError"
-	r.Handle("/libpod/volumes/{name}", s.APIHandler(libpod.RemoveVolume)).Methods(http.MethodDelete)
+	r.Handle(VersionedPath("/libpod/volumes/{name}"), s.APIHandler(libpod.RemoveVolume)).Methods(http.MethodDelete)
 	return nil
 }
diff --git a/pkg/api/server/swagger.go b/pkg/api/server/swagger.go
index fc409d816..011196e5a 100644
--- a/pkg/api/server/swagger.go
+++ b/pkg/api/server/swagger.go
@@ -1,6 +1,7 @@
 package server
 
 import (
+	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/api/handlers"
 	"github.com/containers/libpod/pkg/api/handlers/utils"
 )
@@ -139,3 +140,19 @@ type ok struct {
 		ok string
 	}
 }
+
+// Volume create response
+// swagger:response VolumeCreateResponse
+type swagVolumeCreateResponse struct {
+	// in:body
+	Body struct {
+		libpod.VolumeConfig
+	}
+}
+
+// Volume list
+// swagger:response VolumeList
+type swagVolumeListResponse struct {
+	// in:body
+	Body []libpod.Volume
+}
diff --git a/pkg/bindings/containers/create.go b/pkg/bindings/containers/create.go
index 2943cb522..43a3ef02d 100644
--- a/pkg/bindings/containers/create.go
+++ b/pkg/bindings/containers/create.go
@@ -19,7 +19,7 @@ func CreateWithSpec(ctx context.Context, s specgen.SpecGenerator) (utils.Contain
 	}
 	specgenString, err := jsoniter.MarshalToString(s)
 	if err != nil {
-		return ccr, nil
+		return ccr, err
 	}
 	stringReader := strings.NewReader(specgenString)
 	response, err := conn.DoRequest(stringReader, http.MethodPost, "/containers/create", nil)
diff --git a/pkg/bindings/test/volumes_test.go b/pkg/bindings/test/volumes_test.go
new file mode 100644
index 000000000..c8940d46e
--- /dev/null
+++ b/pkg/bindings/test/volumes_test.go
@@ -0,0 +1,174 @@
+package test_bindings
+
+import (
+	"context"
+	"fmt"
+	"github.com/containers/libpod/pkg/api/handlers"
+	"github.com/containers/libpod/pkg/bindings/containers"
+	"github.com/containers/libpod/pkg/bindings/volumes"
+	"net/http"
+	"time"
+
+	"github.com/containers/libpod/pkg/bindings"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gexec"
+)
+
+var _ = Describe("Podman volumes", func() {
+	var (
+		//tempdir    string
+		//err        error
+		//podmanTest *PodmanTestIntegration
+		bt       *bindingTest
+		s        *gexec.Session
+		connText context.Context
+		err      error
+		trueFlag = true
+	)
+
+	BeforeEach(func() {
+		//tempdir, err = CreateTempDirInTempDir()
+		//if err != nil {
+		//	os.Exit(1)
+		//}
+		//podmanTest = PodmanTestCreate(tempdir)
+		//podmanTest.Setup()
+		//podmanTest.SeedImages()
+		bt = newBindingTest()
+		bt.RestoreImagesFromCache()
+		s = bt.startAPIService()
+		time.Sleep(1 * time.Second)
+		connText, err = bindings.NewConnection(context.Background(), bt.sock)
+		Expect(err).To(BeNil())
+	})
+
+	AfterEach(func() {
+		//podmanTest.Cleanup()
+		//f := CurrentGinkgoTestDescription()
+		//processTestResult(f)
+		s.Kill()
+		bt.cleanup()
+	})
+
+	It("create volume", func() {
+		// create a volume with blank config should work
+		_, err := volumes.Create(connText, handlers.VolumeCreateConfig{})
+		Expect(err).To(BeNil())
+
+		vcc := handlers.VolumeCreateConfig{
+			Name:  "foobar",
+			Label: nil,
+			Opts:  nil,
+		}
+		vol, err := volumes.Create(connText, vcc)
+		Expect(err).To(BeNil())
+		Expect(vol.Name).To(Equal("foobar"))
+
+		// create volume with same name should 500
+		_, err = volumes.Create(connText, vcc)
+		Expect(err).ToNot(BeNil())
+		code, _ := bindings.CheckResponseCode(err)
+		Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
+	})
+
+	It("inspect volume", func() {
+		vol, err := volumes.Create(connText, handlers.VolumeCreateConfig{})
+		Expect(err).To(BeNil())
+		data, err := volumes.Inspect(connText, vol.Name)
+		Expect(err).To(BeNil())
+		Expect(data.Name).To(Equal(vol.Name))
+	})
+
+	It("remove volume", func() {
+		// removing a bogus volume should result in 404
+		err := volumes.Remove(connText, "foobar", nil)
+		code, _ := bindings.CheckResponseCode(err)
+		Expect(code).To(BeNumerically("==", http.StatusNotFound))
+
+		// Removing an unused volume should work
+		vol, err := volumes.Create(connText, handlers.VolumeCreateConfig{})
+		Expect(err).To(BeNil())
+		err = volumes.Remove(connText, vol.Name, nil)
+		Expect(err).To(BeNil())
+
+		// Removing a volume that is being used without force should be 409
+		vol, err = volumes.Create(connText, handlers.VolumeCreateConfig{})
+		Expect(err).To(BeNil())
+		session := bt.runPodman([]string{"run", "-dt", "-v", fmt.Sprintf("%s:/foobar", vol.Name), "--name", "vtest", alpine.name, "top"})
+		session.Wait(45)
+		err = volumes.Remove(connText, vol.Name, nil)
+		Expect(err).ToNot(BeNil())
+		code, _ = bindings.CheckResponseCode(err)
+		Expect(code).To(BeNumerically("==", http.StatusConflict))
+
+		// Removing with a volume in use with force should work with a stopped container
+		zero := 0
+		err = containers.Stop(connText, "vtest", &zero)
+		Expect(err).To(BeNil())
+		err = volumes.Remove(connText, vol.Name, &trueFlag)
+		Expect(err).To(BeNil())
+	})
+
+	It("list volumes", func() {
+		// no volumes should be ok
+		vols, err := volumes.List(connText, nil)
+		Expect(err).To(BeNil())
+		Expect(len(vols)).To(BeZero())
+
+		// create a bunch of named volumes and make verify with list
+		volNames := []string{"homer", "bart", "lisa", "maggie", "marge"}
+		for i := 0; i < 5; i++ {
+			_, err = volumes.Create(connText, handlers.VolumeCreateConfig{Name: volNames[i]})
+			Expect(err).To(BeNil())
+		}
+		vols, err = volumes.List(connText, nil)
+		Expect(err).To(BeNil())
+		Expect(len(vols)).To(BeNumerically("==", 5))
+		for _, v := range vols {
+			Expect(StringInSlice(v.Name, volNames)).To(BeTrue())
+		}
+
+		// list with bad filter should be 500
+		filters := make(map[string][]string)
+		filters["foobar"] = []string{"1234"}
+		_, err = volumes.List(connText, filters)
+		Expect(err).ToNot(BeNil())
+		code, _ := bindings.CheckResponseCode(err)
+		Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
+
+		filters = make(map[string][]string)
+		filters["name"] = []string{"homer"}
+		vols, err = volumes.List(connText, filters)
+		Expect(err).To(BeNil())
+		Expect(len(vols)).To(BeNumerically("==", 1))
+		Expect(vols[0].Name).To(Equal("homer"))
+	})
+
+	// TODO we need to add filtering to tests
+	It("prune unused volume", func() {
+		// Pruning when no volumes present should be ok
+		_, err := volumes.Prune(connText)
+		Expect(err).To(BeNil())
+
+		// Removing an unused volume should work
+		_, err = volumes.Create(connText, handlers.VolumeCreateConfig{})
+		Expect(err).To(BeNil())
+		vols, err := volumes.Prune(connText)
+		Expect(err).To(BeNil())
+		Expect(len(vols)).To(BeNumerically("==", 1))
+
+		_, err = volumes.Create(connText, handlers.VolumeCreateConfig{Name: "homer"})
+		Expect(err).To(BeNil())
+		_, err = volumes.Create(connText, handlers.VolumeCreateConfig{})
+		Expect(err).To(BeNil())
+		session := bt.runPodman([]string{"run", "-dt", "-v", fmt.Sprintf("%s:/homer", "homer"), "--name", "vtest", alpine.name, "top"})
+		session.Wait(45)
+		vols, err = volumes.Prune(connText)
+		Expect(err).To(BeNil())
+		Expect(len(vols)).To(BeNumerically("==", 1))
+		_, err = volumes.Inspect(connText, "homer")
+		Expect(err).To(BeNil())
+	})
+
+})
diff --git a/pkg/bindings/volumes/volumes.go b/pkg/bindings/volumes/volumes.go
index 7f6a9cc9b..0bc818605 100644
--- a/pkg/bindings/volumes/volumes.go
+++ b/pkg/bindings/volumes/volumes.go
@@ -5,27 +5,33 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/api/handlers"
 	"github.com/containers/libpod/pkg/bindings"
+	jsoniter "github.com/json-iterator/go"
 )
 
 // Create creates a volume given its configuration.
-func Create(ctx context.Context, config handlers.VolumeCreateConfig) (string, error) {
-	// TODO This is incomplete.  The config needs to be sent via the body
+func Create(ctx context.Context, config handlers.VolumeCreateConfig) (*libpod.VolumeConfig, error) {
 	var (
-		volumeID string
+		v libpod.VolumeConfig
 	)
 	conn, err := bindings.GetClient(ctx)
 	if err != nil {
-		return "", err
+		return nil, err
+	}
+	createString, err := jsoniter.MarshalToString(config)
+	if err != nil {
+		return nil, err
 	}
-	response, err := conn.DoRequest(nil, http.MethodPost, "/volumes/create", nil)
+	stringReader := strings.NewReader(createString)
+	response, err := conn.DoRequest(stringReader, http.MethodPost, "/volumes/create", nil)
 	if err != nil {
-		return volumeID, err
+		return nil, err
 	}
-	return volumeID, response.Process(&volumeID)
+	return &v, response.Process(&v)
 }
 
 // Inspect returns low-level information about a volume.
@@ -37,18 +43,36 @@ func Inspect(ctx context.Context, nameOrID string) (*libpod.InspectVolumeData, e
 	if err != nil {
 		return nil, err
 	}
-	response, err := conn.DoRequest(nil, http.MethodPost, "/volumes/%s/json", nil, nameOrID)
+	response, err := conn.DoRequest(nil, http.MethodGet, "/volumes/%s/json", nil, nameOrID)
 	if err != nil {
 		return &inspect, err
 	}
 	return &inspect, response.Process(&inspect)
 }
 
-func List() error {
-	// TODO
-	// The API side of things for this one does a lot in main and therefore
-	// is not implemented yet.
-	return bindings.ErrNotImplemented // nolint:typecheck
+// List returns the configurations for existing volumes in the form of a slice.  Optionally, filters
+// can be used to refine the list of volumes.
+func List(ctx context.Context, filters map[string][]string) ([]*libpod.VolumeConfig, error) {
+	var (
+		vols []*libpod.VolumeConfig
+	)
+	conn, err := bindings.GetClient(ctx)
+	if err != nil {
+		return nil, err
+	}
+	params := url.Values{}
+	if len(filters) > 0 {
+		strFilters, err := bindings.FiltersToString(filters)
+		if err != nil {
+			return nil, err
+		}
+		params.Set("filters", strFilters)
+	}
+	response, err := conn.DoRequest(nil, http.MethodGet, "/volumes/json", params)
+	if err != nil {
+		return vols, err
+	}
+	return vols, response.Process(&vols)
 }
 
 // Prune removes unused volumes from the local filesystem.
@@ -78,7 +102,7 @@ func Remove(ctx context.Context, nameOrID string, force *bool) error {
 	if force != nil {
 		params.Set("force", strconv.FormatBool(*force))
 	}
-	response, err := conn.DoRequest(nil, http.MethodPost, "/volumes/%s/prune", params, nameOrID)
+	response, err := conn.DoRequest(nil, http.MethodDelete, "/volumes/%s", params, nameOrID)
 	if err != nil {
 		return err
 	}