Review corrections pass #2

Add API review comments to correct documentation and endpoints.  Also, add a libpode prune method to reduce code duplication.  Only used right now for the API but when the remote client is wired, we will switch over there too.

Signed-off-by: Brent Baude <bbaude@redhat.com>

11 files changed, 236 insertions, 114 deletions

diff --git a/pkg/api/handlers/containers.go b/pkg/api/handlers/containers.go
index 6b09321a0..b5c78ce53 100644
--- a/pkg/api/handlers/containers.go
+++ b/pkg/api/handlers/containers.go
@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"fmt"
+	"github.com/docker/docker/api/types"
 	"net/http"
 
 	"github.com/containers/libpod/libpod"
@@ -192,3 +193,55 @@ func RestartContainer(w http.ResponseWriter, r *http.Request) {
 	// Success
 	utils.WriteResponse(w, http.StatusNoContent, "")
 }
+
+func PruneContainers(w http.ResponseWriter, r *http.Request) {
+	var (
+		delContainers []string
+		space         int64
+	)
+	runtime := r.Context().Value("runtime").(*libpod.Runtime)
+	decoder := r.Context().Value("decoder").(*schema.Decoder)
+
+	query := struct {
+		Filters map[string][]string `schema:"filter"`
+	}{}
+	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
+	}
+
+	filterFuncs, err := utils.GenerateFilterFuncsFromMap(runtime, query.Filters)
+	if err != nil {
+		utils.InternalServerError(w, err)
+		return
+	}
+	prunedContainers, pruneErrors, err := runtime.PruneContainers(filterFuncs)
+	if err != nil {
+		utils.InternalServerError(w, err)
+		return
+	}
+
+	// Libpod response differs
+	if utils.IsLibpodRequest(r) {
+		var response []LibpodContainersPruneReport
+		for ctrID, size := range prunedContainers {
+			response = append(response, LibpodContainersPruneReport{ID: ctrID, SpaceReclaimed: size})
+		}
+		for ctrID, err := range pruneErrors {
+			response = append(response, LibpodContainersPruneReport{ID: ctrID, PruneError: err.Error()})
+		}
+		utils.WriteResponse(w, http.StatusOK, response)
+		return
+	}
+	for ctrID, size := range prunedContainers {
+		if pruneErrors[ctrID] == nil {
+			space += size
+			delContainers = append(delContainers, ctrID)
+		}
+	}
+	report := types.ContainersPruneReport{
+		ContainersDeleted: delContainers,
+		SpaceReclaimed:    uint64(space),
+	}
+	utils.WriteResponse(w, http.StatusOK, report)
+}
diff --git a/pkg/api/handlers/generic/containers.go b/pkg/api/handlers/generic/containers.go
index 5a0a51fd7..8dc73ae14 100644
--- a/pkg/api/handlers/generic/containers.go
+++ b/pkg/api/handlers/generic/containers.go
@@ -1,7 +1,6 @@
 package generic
 
 import (
-	"context"
 	"encoding/binary"
 	"fmt"
 	"net/http"
@@ -11,12 +10,10 @@ import (
 	"time"
 
 	"github.com/containers/libpod/libpod"
-	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/libpod/logs"
 	"github.com/containers/libpod/pkg/api/handlers"
 	"github.com/containers/libpod/pkg/api/handlers/utils"
 	"github.com/containers/libpod/pkg/util"
-	"github.com/docker/docker/api/types"
 	"github.com/gorilla/mux"
 	"github.com/gorilla/schema"
 	"github.com/pkg/errors"
@@ -47,14 +44,40 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
 }
 
 func ListContainers(w http.ResponseWriter, r *http.Request) {
+	var (
+		containers []*libpod.Container
+		err        error
+	)
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
-
-	containers, err := runtime.GetAllContainers()
+	decoder := r.Context().Value("decoder").(*schema.Decoder)
+	query := struct {
+		All     bool                `schema:"all"`
+		Limit   int                 `schema:"limit"`
+		Size    bool                `schema:"size"`
+		Filters map[string][]string `schema:"filters"`
+	}{
+		// override any golang type defaults
+	}
+	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
+	}
+	if query.All {
+		containers, err = runtime.GetAllContainers()
+	} else {
+		containers, err = runtime.GetRunningContainers()
+	}
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
-
+	if _, found := mux.Vars(r)["limit"]; found {
+		last := query.Limit
+		if len(containers) > last {
+			containers = containers[len(containers)-last:]
+		}
+	}
+	// TODO filters still need to be applied
 	infoData, err := runtime.Info()
 	if err != nil {
 		utils.InternalServerError(w, errors.Wrapf(err, "Failed to obtain system info"))
@@ -125,51 +148,6 @@ func WaitContainer(w http.ResponseWriter, r *http.Request) {
 	})
 }
 
-func PruneContainers(w http.ResponseWriter, r *http.Request) {
-	runtime := r.Context().Value("runtime").(*libpod.Runtime)
-
-	containers, err := runtime.GetAllContainers()
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
-	}
-
-	deletedContainers := []string{}
-	var spaceReclaimed uint64
-	for _, ctnr := range containers {
-		// Only remove stopped or exit'ed containers.
-		state, err := ctnr.State()
-		if err != nil {
-			utils.InternalServerError(w, err)
-			return
-		}
-		switch state {
-		case define.ContainerStateStopped, define.ContainerStateExited:
-		default:
-			continue
-		}
-
-		deletedContainers = append(deletedContainers, ctnr.ID())
-		cSize, err := ctnr.RootFsSize()
-		if err != nil {
-			utils.InternalServerError(w, err)
-			return
-		}
-		spaceReclaimed += uint64(cSize)
-
-		err = runtime.RemoveContainer(context.Background(), ctnr, false, false)
-		if err != nil && !(errors.Cause(err) == define.ErrCtrRemoved) {
-			utils.InternalServerError(w, err)
-			return
-		}
-	}
-	report := types.ContainersPruneReport{
-		ContainersDeleted: deletedContainers,
-		SpaceReclaimed:    spaceReclaimed,
-	}
-	utils.WriteResponse(w, http.StatusOK, report)
-}
-
 func LogsFromContainer(w http.ResponseWriter, r *http.Request) {
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
diff --git a/pkg/api/handlers/generic/containers_create.go b/pkg/api/handlers/generic/containers_create.go
index f98872690..edefd5757 100644
--- a/pkg/api/handlers/generic/containers_create.go
+++ b/pkg/api/handlers/generic/containers_create.go
@@ -40,7 +40,9 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()"))
 		return
 	}
-
+	if len(input.HostConfig.Links) > 0 {
+		utils.Error(w, utils.ErrLinkNotSupport.Error(), http.StatusBadRequest, errors.Wrapf(utils.ErrLinkNotSupport, "bad parameter"))
+	}
 	newImage, err := runtime.ImageRuntime().NewFromLocal(input.Image)
 	if err != nil {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "NewFromLocal()"))
diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go
index e16a4ea1f..df16843c7 100644
--- a/pkg/api/handlers/libpod/containers.go
+++ b/pkg/api/handlers/libpod/containers.go
@@ -1,7 +1,9 @@
 package libpod
 
 import (
+	"fmt"
 	"net/http"
+	"strconv"
 
 	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/libpod"
@@ -46,12 +48,18 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
 	utils.RemoveContainer(w, r, query.Force, query.Vols)
 }
 func ListContainers(w http.ResponseWriter, r *http.Request) {
+	var (
+		filters []string
+	)
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
 	query := struct {
-		Filter []string `schema:"filter"`
-		Last   int      `schema:"last"`
-		Size   bool     `schema:"size"`
-		Sync   bool     `schema:"sync"`
+		All       bool                `schema:"all"`
+		Filter    map[string][]string `schema:"filter"`
+		Last      int                 `schema:"last"`
+		Namespace bool                `schema:"namespace"`
+		Pod       bool                `schema:"pod"`
+		Size      bool                `schema:"size"`
+		Sync      bool                `schema:"sync"`
 	}{
 		// override any golang type defaults
 	}
@@ -63,15 +71,22 @@ func ListContainers(w http.ResponseWriter, r *http.Request) {
 	}
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
 	opts := shared.PsOptions{
-		All:       true,
+		All:       query.All,
 		Last:      query.Last,
 		Size:      query.Size,
 		Sort:      "",
-		Namespace: true,
+		Namespace: query.Namespace,
+		Pod:       query.Pod,
 		Sync:      query.Sync,
 	}
-
-	pss, err := shared.GetPsContainerOutput(runtime, opts, query.Filter, 2)
+	if len(query.Filter) > 0 {
+		for k, v := range query.Filter {
+			for _, val := range v {
+				filters = append(filters, fmt.Sprintf("%s=%s", k, val))
+			}
+		}
+	}
+	pss, err := shared.GetPsContainerOutput(runtime, opts, filters, 2)
 	if err != nil {
 		utils.InternalServerError(w, err)
 	}
@@ -117,19 +132,12 @@ func KillContainer(w http.ResponseWriter, r *http.Request) {
 }
 
 func WaitContainer(w http.ResponseWriter, r *http.Request) {
-	_, err := utils.WaitContainer(w, r)
+	exitCode, err := utils.WaitContainer(w, r)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
-	utils.WriteResponse(w, http.StatusNoContent, "")
-}
-
-func PruneContainers(w http.ResponseWriter, r *http.Request) {
-	// TODO Needs rebase to get  filers; Also would be handy to define
-	// an actual libpod container prune method.
-	// force
-	// filters
+	utils.WriteResponse(w, http.StatusOK, strconv.Itoa(int(exitCode)))
 }
 
 func LogsFromContainer(w http.ResponseWriter, r *http.Request) {
diff --git a/pkg/api/handlers/swagger.go b/pkg/api/handlers/swagger.go
index 5509c1d46..faae98798 100644
--- a/pkg/api/handlers/swagger.go
+++ b/pkg/api/handlers/swagger.go
@@ -58,6 +58,13 @@ type swagContainerPruneReport struct {
 	Body []ContainersPruneReport
 }
 
+// Prune containers
+// swagger:response DocsLibpodPruneResponse
+type swagLibpodContainerPruneReport struct {
+	// in: body
+	Body []LibpodContainersPruneReport
+}
+
 // Inspect container
 // swagger:response DocsContainerInspectResponse
 type swagContainerInspectResponse struct {
diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go
index 2526a3317..33cd51164 100644
--- a/pkg/api/handlers/types.go
+++ b/pkg/api/handlers/types.go
@@ -43,6 +43,12 @@ type ContainersPruneReport struct {
 	docker.ContainersPruneReport
 }
 
+type LibpodContainersPruneReport struct {
+	ID             string `json:"id"`
+	SpaceReclaimed int64  `json:"space"`
+	PruneError     string `json:"error"`
+}
+
 type Info struct {
 	docker.Info
 	BuildahVersion     string
diff --git a/pkg/api/handlers/utils/containers.go b/pkg/api/handlers/utils/containers.go
index 64d3d378a..2c986db3a 100644
--- a/pkg/api/handlers/utils/containers.go
+++ b/pkg/api/handlers/utils/containers.go
@@ -6,6 +6,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/define"
 	"github.com/gorilla/mux"
@@ -83,7 +84,7 @@ func WaitContainer(w http.ResponseWriter, r *http.Request) (int32, error) {
 	}
 
 	if len(query.Condition) > 0 {
-		return 0, errors.Errorf("the condition parameter is not supported")
+		UnSupportedParameter("condition")
 	}
 
 	name := mux.Vars(r)["name"]
@@ -101,3 +102,21 @@ func WaitContainer(w http.ResponseWriter, r *http.Request) (int32, error) {
 	}
 	return con.Wait()
 }
+
+// GenerateFilterFuncsFromMap is used to generate un-executed functions that can be used to filter
+// containers.  It is specifically designed for the RESTFUL API input.
+func GenerateFilterFuncsFromMap(r *libpod.Runtime, filters map[string][]string) ([]libpod.ContainerFilter, error) {
+	var (
+		filterFuncs []libpod.ContainerFilter
+	)
+	for k, v := range filters {
+		for _, val := range v {
+			f, err := shared.GenerateContainerFilterFuncs(k, val, r)
+			if err != nil {
+				return filterFuncs, err
+			}
+			filterFuncs = append(filterFuncs, f)
+		}
+	}
+	return filterFuncs, nil
+}
diff --git a/pkg/api/handlers/utils/handler.go b/pkg/api/handlers/utils/handler.go
index 2fd9bffba..f2ce26f1a 100644
--- a/pkg/api/handlers/utils/handler.go
+++ b/pkg/api/handlers/utils/handler.go
@@ -51,3 +51,11 @@ func WriteJSON(w http.ResponseWriter, code int, value interface{}) {
 		logrus.Errorf("unable to write json: %q", err)
 	}
 }
+
+func FilterMapToString(filters map[string][]string) (string, error) {
+	f, err := json.Marshal(filters)
+	if err != nil {
+		return "", err
+	}
+	return string(f), nil
+}
diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go
index 58920d106..b2d2ab388 100644
--- a/pkg/api/server/register_containers.go
+++ b/pkg/api/server/register_containers.go
@@ -42,6 +42,20 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	// description: Returns a list of containers
 	// parameters:
 	//  - in: query
+	//    name: all
+	//    type: boolean
+	//    default: false
+	//    description: Return all containers. By default, only running containers are shown
+	//  - in: query
+	//    name: limit
+	//    description: Return this number of most recently created containers, including non-running ones.
+	//    type: integer
+	//  - in: query
+	//    name: size
+	//    type: boolean
+	//    default: false
+	//    description: Return the size of container as fields SizeRw and SizeRootFs.
+	//  - in: query
 	//    name: filters
 	//    type: string
 	//    description: |
@@ -91,7 +105,7 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	//     $ref: "#/responses/DocsContainerPruneReport"
 	//   500:
 	//     $ref: "#/responses/InternalError"
-	r.HandleFunc(VersionedPath("/containers/prune"), APIHandler(s.Context, generic.PruneContainers)).Methods(http.MethodPost)
+	r.HandleFunc(VersionedPath("/containers/prune"), APIHandler(s.Context, handlers.PruneContainers)).Methods(http.MethodPost)
 	// swagger:operation DELETE /containers/{name} compat removeContainer
 	// ---
 	// tags:
@@ -175,6 +189,7 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	//    type: string
 	//    default: TERM
 	//    description: signal to be sent to container
+	//    default: SIGKILL
 	// produces:
 	// - application/json
 	// responses:
@@ -301,7 +316,8 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	//  - in: query
 	//    name: detachKeys
 	//    type: string
-	//    description: needs description
+	//    description: "Override the key sequence for detaching a container. Format is a single character [a-Z] or ctrl-<value> where <value> is one of: a-z, @, ^, [, , or _."
+	//    default: ctrl-p,ctrl-q
 	// produces:
 	// - application/json
 	// responses:
@@ -431,7 +447,7 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	//  - in: query
 	//    name: condition
 	//    type: string
-	//    description: Wait until the container reaches the given condition
+	//    description: not supported
 	// produces:
 	// - application/json
 	// responses:
@@ -541,6 +557,55 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	//  - containers
 	// summary: List containers
 	// description: Returns a list of containers
+	// parameters:
+	//  - in: query
+	//    name: all
+	//    type: boolean
+	//    default: false
+	//    description: Return all containers. By default, only running containers are shown
+	//  - in: query
+	//    name: limit
+	//    description: Return this number of most recently created containers, including non-running ones.
+	//    type: integer
+	//  - in: query
+	//    name: namespace
+	//    type: boolean
+	//    description: Include namespace information
+	//    default: false
+	//  - in: query
+	//    name: pod
+	//    type: boolean
+	//    default: false
+	//    description: Include Pod ID and Name if applicable
+	//  - in: query
+	//    name: size
+	//    type: boolean
+	//    default: false
+	//    description: Return the size of container as fields SizeRw and SizeRootFs.
+	//  - in: query
+	//    name: sync
+	//    type: boolean
+	//    default: false
+	//    description: Sync container state with OCI runtime
+	//  - in: query
+	//    name: filters
+	//    type: string
+	//    description: |
+	//       Returns a list of containers.
+	//        - ancestor=(<image-name>[:<tag>], <image id>, or <image@digest>)
+	//        - before=(<container id> or <container name>)
+	//        - expose=(<port>[/<proto>]|<startport-endport>/[<proto>])
+	//        - exited=<int> containers with exit code of <int>
+	//        - health=(starting|healthy|unhealthy|none)
+	//        - id=<ID> a container's ID
+	//        - is-task=(true|false)
+	//        - label=key or label="key=value" of a container label
+	//        - name=<name> a container's name
+	//        - network=(<network id> or <network name>)
+	//        - publish=(<port>[/<proto>]|<startport-endport>/[<proto>])
+	//        - since=(<container id> or <container name>)
+	//        - status=(created|restarting|running|removing|paused|exited|dead)
+	//        - volume=(<volume name> or <mount point destination>)
 	// produces:
 	// - application/json
 	// responses:
@@ -551,18 +616,14 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	//   500:
 	//     $ref: "#/responses/InternalError"
 	r.HandleFunc(VersionedPath("/libpod/containers/json"), APIHandler(s.Context, libpod.ListContainers)).Methods(http.MethodGet)
-	// swagger:operation POST /libpod/containers/prune libpod libpodPruneContainers
+	// swagger:operation POST  /libpod/containers/prune libpod libpodPruneContainers
 	// ---
 	// tags:
-	//  - containers
-	// summary: Prune unused containers
-	// description: Remove stopped and exited containers
+	//   - containers
+	// summary: Delete stopped containers
+	// description: Remove containers not in use
 	// parameters:
 	//  - in: query
-	//    name: force
-	//    type: boolean
-	//    description: TODO This should be removed from API.  Will always be true...
-	//  - in: query
 	//    name: filters
 	//    type: string
 	//    description:  |
@@ -573,12 +634,10 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	// - application/json
 	// responses:
 	//   200:
-	//     type: string
-	//     description: success
-	//     example: OK
+	//     $ref: "#/responses/DocsLibpodPruneResponse"
 	//   500:
 	//     $ref: "#/responses/InternalError"
-	r.HandleFunc(VersionedPath("/libpod/containers/prune"), APIHandler(s.Context, libpod.PruneContainers)).Methods(http.MethodPost)
+	r.HandleFunc(VersionedPath("/libpod/containers/prune"), APIHandler(s.Context, handlers.PruneContainers)).Methods(http.MethodPost)
 	// swagger:operation GET /libpod/containers/showmounted libpod showMounterContainers
 	// ---
 	// tags:
@@ -796,7 +855,8 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	//  - in: query
 	//    name: detachKeys
 	//    type: string
-	//    description: needs description
+	//    description: "Override the key sequence for detaching a container. Format is a single character [a-Z] or ctrl-<value> where <value> is one of: a-z, @, ^, [, , or _."
+	//    default: ctrl-p,ctrl-q
 	// produces:
 	// - application/json
 	// responses:
@@ -902,10 +962,6 @@ func (s *APIServer) RegisterContainersHandlers(r *mux.Router) error {
 	//    type: string
 	//    required: true
 	//    description: the name or ID of the container
-	//  - in: query
-	//    name: condition
-	//    type: string
-	//    description: Wait until the container reaches the given condition
 	// produces:
 	// - application/json
 	// responses:
diff --git a/pkg/api/server/swagger.go b/pkg/api/server/swagger.go
index f6643600a..5098390bc 100644
--- a/pkg/api/server/swagger.go
+++ b/pkg/api/server/swagger.go
@@ -50,15 +50,6 @@ type swagInternalError struct {
 	}
 }
 
-// Generic error
-// swagger:response GenericError
-type swagGenericError struct {
-	// in:body
-	Body struct {
-		utils.ErrorModel
-	}
-}
-
 // Conflict error in operation
 // swagger:response ConflictError
 type swagConflictError struct {
@@ -130,21 +121,6 @@ type swagListContainers struct {
 	}
 }
 
-// To be determined
-// swagger:response tbd
-type swagTBD struct {
-}
-
-// Success
-// swagger:response
-type swag struct {
-	// in:body
-	Body struct {
-		// example: OK
-		ok string
-	}
-}
-
 // Success
 // swagger:response
 type ok struct {
diff --git a/pkg/bindings/bindings.go b/pkg/bindings/bindings.go
new file mode 100644
index 000000000..e83c4a5e1
--- /dev/null
+++ b/pkg/bindings/bindings.go
@@ -0,0 +1,9 @@
+// Package bindings provides golang-based access
+// to the Podman REST API.  Users can then interact with API endpoints
+// to manage containers, images, pods, etc.
+//
+// This package exposes a series of methods that allow users to firstly
+// create their connection with the API endpoints.  Once the connection
+// is established, users can then manage the Podman container runtime.
+
+package bindings