Switch podman stop/kill/wait handlers to use abi

Change API Handlers to use the same functions that the
local podman uses.

At the same time:

 implement remote API for --all and --ignore flags for podman stop
 implement remote API for --all flags for podman stop

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

22 files changed, 344 insertions, 174 deletions

diff --git a/cmd/podman/containers/kill.go b/cmd/podman/containers/kill.go
index 28040e08a..36e3e5f59 100644
--- a/cmd/podman/containers/kill.go
+++ b/cmd/podman/containers/kill.go
@@ -2,8 +2,9 @@ package containers
 
 import (
 	"context"
-	"errors"
 	"fmt"
+	"io/ioutil"
+	"strings"
 
 	"github.com/containers/common/pkg/completion"
 	"github.com/containers/podman/v2/cmd/podman/common"
@@ -12,6 +13,7 @@ import (
 	"github.com/containers/podman/v2/cmd/podman/validate"
 	"github.com/containers/podman/v2/pkg/domain/entities"
 	"github.com/containers/podman/v2/pkg/signal"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -59,7 +61,7 @@ func killFlags(cmd *cobra.Command) {
 	flags.StringVarP(&killOptions.Signal, signalFlagName, "s", "KILL", "Signal to send to the container")
 	_ = cmd.RegisterFlagCompletionFunc(signalFlagName, common.AutocompleteStopSignal)
 	cidfileFlagName := "cidfile"
-	flags.StringArrayVar(&killOptions.CIDFiles, cidfileFlagName, []string{}, "Read the container ID from the file")
+	flags.StringArrayVar(&cidFiles, cidfileFlagName, []string{}, "Read the container ID from the file")
 	_ = cmd.RegisterFlagCompletionFunc(cidfileFlagName, completion.AutocompleteDefault)
 }
 
@@ -94,6 +96,15 @@ func kill(_ *cobra.Command, args []string) error {
 	if sig < 1 || sig > 64 {
 		return errors.New("valid signals are 1 through 64")
 	}
+	for _, cidFile := range cidFiles {
+		content, err := ioutil.ReadFile(string(cidFile))
+		if err != nil {
+			return errors.Wrap(err, "error reading CIDFile")
+		}
+		id := strings.Split(string(content), "\n")[0]
+		args = append(args, id)
+	}
+
 	responses, err := registry.ContainerEngine().ContainerKill(context.Background(), args, killOptions)
 	if err != nil {
 		return err
diff --git a/cmd/podman/containers/stop.go b/cmd/podman/containers/stop.go
index 3a4211357..7338c8d98 100644
--- a/cmd/podman/containers/stop.go
+++ b/cmd/podman/containers/stop.go
@@ -3,6 +3,8 @@ package containers
 import (
 	"context"
 	"fmt"
+	"io/ioutil"
+	"strings"
 
 	"github.com/containers/common/pkg/completion"
 	"github.com/containers/podman/v2/cmd/podman/common"
@@ -10,6 +12,7 @@ import (
 	"github.com/containers/podman/v2/cmd/podman/utils"
 	"github.com/containers/podman/v2/cmd/podman/validate"
 	"github.com/containers/podman/v2/pkg/domain/entities"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -58,7 +61,7 @@ func stopFlags(cmd *cobra.Command) {
 	flags.BoolVarP(&stopOptions.Ignore, "ignore", "i", false, "Ignore errors when a specified container is missing")
 
 	cidfileFlagName := "cidfile"
-	flags.StringArrayVarP(&stopOptions.CIDFiles, cidfileFlagName, "", nil, "Read the container ID from the file")
+	flags.StringArrayVar(&cidFiles, cidfileFlagName, nil, "Read the container ID from the file")
 	_ = cmd.RegisterFlagCompletionFunc(cidfileFlagName, completion.AutocompleteDefault)
 
 	timeFlagName := "time"
@@ -97,6 +100,15 @@ func stop(cmd *cobra.Command, args []string) error {
 		stopOptions.Timeout = &stopTimeout
 	}
 
+	for _, cidFile := range cidFiles {
+		content, err := ioutil.ReadFile(string(cidFile))
+		if err != nil {
+			return errors.Wrap(err, "error reading CIDFile")
+		}
+		id := strings.Split(string(content), "\n")[0]
+		args = append(args, id)
+	}
+
 	responses, err := registry.ContainerEngine().ContainerStop(context.Background(), args, stopOptions)
 	if err != nil {
 		return err
diff --git a/cmd/podman/containers/wait.go b/cmd/podman/containers/wait.go
index 2bbfbccc9..14d660678 100644
--- a/cmd/podman/containers/wait.go
+++ b/cmd/podman/containers/wait.go
@@ -50,7 +50,7 @@ func waitFlags(cmd *cobra.Command) {
 	flags := cmd.Flags()
 
 	intervalFlagName := "interval"
-	flags.StringVarP(&waitInterval, intervalFlagName, "i", "250ns", "Time Interval to wait before polling for completion")
+	flags.StringVarP(&waitInterval, intervalFlagName, "i", "250ms", "Time Interval to wait before polling for completion")
 	_ = cmd.RegisterFlagCompletionFunc(intervalFlagName, completion.AutocompleteNone)
 
 	conditionFlagName := "condition"
diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go
index 0f91a4362..a79486466 100644
--- a/pkg/api/handlers/compat/containers.go
+++ b/pkg/api/handlers/compat/containers.go
@@ -31,11 +31,11 @@ import (
 func RemoveContainer(w http.ResponseWriter, r *http.Request) {
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
 	query := struct {
-		All     bool `schema:"all"`
-		Force   bool `schema:"force"`
-		Ignore  bool `schema:"ignore"`
-		Link    bool `schema:"link"`
-		Volumes bool `schema:"v"`
+		Force         bool `schema:"force"`
+		Ignore        bool `schema:"ignore"`
+		Link          bool `schema:"link"`
+		DockerVolumes bool `schema:"v"`
+		LibpodVolumes bool `schema:"volumes"`
 	}{
 		// override any golang type defaults
 	}
@@ -46,10 +46,19 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if query.Link && !utils.IsLibpodRequest(r) {
-		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
-			utils.ErrLinkNotSupport)
-		return
+	options := entities.RmOptions{
+		Force:  query.Force,
+		Ignore: query.Ignore,
+	}
+	if utils.IsLibpodRequest(r) {
+		options.Volumes = query.LibpodVolumes
+	} else {
+		if query.Link {
+			utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
+				utils.ErrLinkNotSupport)
+			return
+		}
+		options.Volumes = query.DockerVolumes
 	}
 
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
@@ -57,12 +66,6 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) {
 	// code.
 	containerEngine := abi.ContainerEngine{Libpod: runtime}
 	name := utils.GetName(r)
-	options := entities.RmOptions{
-		All:     query.All,
-		Force:   query.Force,
-		Volumes: query.Volumes,
-		Ignore:  query.Ignore,
-	}
 	report, err := containerEngine.ContainerRm(r.Context(), []string{name}, options)
 	if err != nil {
 		if errors.Cause(err) == define.ErrNoSuchCtr {
@@ -193,45 +196,48 @@ func KillContainer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	sig, err := signal.ParseSignalNameOrNumber(query.Signal)
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
-	}
+	// Now use the ABI implementation to prevent us from having duplicate
+	// code.
+	containerEngine := abi.ContainerEngine{Libpod: runtime}
 	name := utils.GetName(r)
-	con, err := runtime.LookupContainer(name)
-	if err != nil {
-		utils.ContainerNotFound(w, name, err)
-		return
+	options := entities.KillOptions{
+		Signal: query.Signal,
 	}
-
-	state, err := con.State()
+	report, err := containerEngine.ContainerKill(r.Context(), []string{name}, options)
 	if err != nil {
-		utils.InternalServerError(w, err)
-		return
-	}
+		if errors.Cause(err) == define.ErrCtrStateInvalid ||
+			errors.Cause(err) == define.ErrCtrStopped {
+			utils.Error(w, fmt.Sprintf("Container %s is not running", name), http.StatusConflict, err)
+			return
+		}
+		if errors.Cause(err) == define.ErrNoSuchCtr {
+			utils.ContainerNotFound(w, name, err)
+			return
+		}
 
-	// If the Container is stopped already, send a 409
-	if state == define.ContainerStateStopped || state == define.ContainerStateExited {
-		utils.Error(w, fmt.Sprintf("Container %s is not running", name), http.StatusConflict, errors.New(fmt.Sprintf("Cannot kill Container %s, it is not running", name)))
+		utils.InternalServerError(w, err)
 		return
 	}
 
-	signal := uint(sig)
-
-	err = con.Kill(signal)
-	if err != nil {
-		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrapf(err, "unable to kill Container %s", name))
+	if len(report) > 0 && report[0].Err != nil {
+		utils.InternalServerError(w, report[0].Err)
 		return
 	}
-
 	// Docker waits for the container to stop if the signal is 0 or
 	// SIGKILL.
-	if !utils.IsLibpodRequest(r) && (signal == 0 || syscall.Signal(signal) == syscall.SIGKILL) {
-		if _, err = con.Wait(); err != nil {
-			utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrapf(err, "failed to wait for Container %s", con.ID()))
+	if !utils.IsLibpodRequest(r) {
+		sig, err := signal.ParseSignalNameOrNumber(query.Signal)
+		if err != nil {
+			utils.InternalServerError(w, err)
 			return
 		}
+		if sig == 0 || syscall.Signal(sig) == syscall.SIGKILL {
+			var opts entities.WaitOptions
+			if _, err := containerEngine.ContainerWait(r.Context(), []string{name}, opts); err != nil {
+				utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
+				return
+			}
+		}
 	}
 	// Success
 	utils.WriteResponse(w, http.StatusNoContent, nil)
@@ -242,6 +248,10 @@ func WaitContainer(w http.ResponseWriter, r *http.Request) {
 	// /{version}/containers/(name)/wait
 	exitCode, err := utils.WaitContainer(w, r)
 	if err != nil {
+		if errors.Cause(err) == define.ErrNoSuchCtr {
+			logrus.Warnf("container not found %q: %v", utils.GetName(r), err)
+			return
+		}
 		logrus.Warnf("failed to wait on container %q: %v", mux.Vars(r)["name"], err)
 		return
 	}
diff --git a/pkg/api/handlers/compat/containers_restart.go b/pkg/api/handlers/compat/containers_restart.go
index e8928596a..70edfcbb3 100644
--- a/pkg/api/handlers/compat/containers_restart.go
+++ b/pkg/api/handlers/compat/containers_restart.go
@@ -4,7 +4,10 @@ import (
 	"net/http"
 
 	"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/domain/entities"
+	"github.com/containers/podman/v2/pkg/domain/infra/abi"
 	"github.com/gorilla/schema"
 	"github.com/pkg/errors"
 )
@@ -12,34 +15,49 @@ import (
 func RestartContainer(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
+	// Now use the ABI implementation to prevent us from having duplicate
+	// code.
+	containerEngine := abi.ContainerEngine{Libpod: runtime}
+
 	// /{version}/containers/(name)/restart
 	query := struct {
-		Timeout int `schema:"t"`
+		All           bool `schema:"all"`
+		DockerTimeout uint `schema:"t"`
+		LibpodTimeout uint `schema:"timeout"`
 	}{
-		// Override golang default values for types
+		// override any golang type defaults
 	}
 	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
-		utils.BadRequest(w, "url", r.URL.String(), errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
+		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
+			errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
 		return
 	}
 
 	name := utils.GetName(r)
-	con, err := runtime.LookupContainer(name)
-	if err != nil {
-		utils.ContainerNotFound(w, name, err)
-		return
-	}
 
-	timeout := con.StopTimeout()
-	if _, found := r.URL.Query()["t"]; found {
-		timeout = uint(query.Timeout)
+	options := entities.RestartOptions{
+		All:     query.All,
+		Timeout: &query.DockerTimeout,
+	}
+	if utils.IsLibpodRequest(r) {
+		options.Timeout = &query.LibpodTimeout
 	}
+	report, err := containerEngine.ContainerRestart(r.Context(), []string{name}, options)
+	if err != nil {
+		if errors.Cause(err) == define.ErrNoSuchCtr {
+			utils.ContainerNotFound(w, name, err)
+			return
+		}
 
-	if err := con.RestartWithTimeout(r.Context(), timeout); err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
 
+	if len(report) > 0 && report[0].Err != nil {
+		utils.InternalServerError(w, report[0].Err)
+		return
+	}
+
 	// Success
 	utils.WriteResponse(w, http.StatusNoContent, nil)
 }
diff --git a/pkg/api/handlers/compat/containers_stop.go b/pkg/api/handlers/compat/containers_stop.go
index 8bc58cf59..000685aa0 100644
--- a/pkg/api/handlers/compat/containers_stop.go
+++ b/pkg/api/handlers/compat/containers_stop.go
@@ -6,6 +6,8 @@ import (
 	"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/domain/entities"
+	"github.com/containers/podman/v2/pkg/domain/infra/abi"
 	"github.com/gorilla/schema"
 	"github.com/pkg/errors"
 )
@@ -13,10 +15,15 @@ import (
 func StopContainer(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
+	// Now use the ABI implementation to prevent us from having duplicate
+	// code.
+	containerEngine := abi.ContainerEngine{Libpod: runtime}
 
 	// /{version}/containers/(name)/stop
 	query := struct {
-		Timeout int `schema:"t"`
+		Ignore        bool `schema:"ignore"`
+		DockerTimeout uint `schema:"t"`
+		LibpodTimeout uint `schema:"timeout"`
 	}{
 		// override any golang type defaults
 	}
@@ -27,31 +34,46 @@ func StopContainer(w http.ResponseWriter, r *http.Request) {
 	}
 
 	name := utils.GetName(r)
+
+	options := entities.StopOptions{
+		Ignore: query.Ignore,
+	}
+	if utils.IsLibpodRequest(r) {
+		if query.LibpodTimeout > 0 {
+			options.Timeout = &query.LibpodTimeout
+		}
+	} else {
+		if query.DockerTimeout > 0 {
+			options.Timeout = &query.DockerTimeout
+		}
+	}
 	con, err := runtime.LookupContainer(name)
 	if err != nil {
 		utils.ContainerNotFound(w, name, err)
 		return
 	}
-
 	state, err := con.State()
 	if err != nil {
-		utils.InternalServerError(w, errors.Wrapf(err, "unable to get state for Container %s", name))
+		utils.InternalServerError(w, err)
 		return
 	}
-	// If the Container is stopped already, send a 304
 	if state == define.ContainerStateStopped || state == define.ContainerStateExited {
 		utils.WriteResponse(w, http.StatusNotModified, nil)
 		return
 	}
+	report, err := containerEngine.ContainerStop(r.Context(), []string{name}, options)
+	if err != nil {
+		if errors.Cause(err) == define.ErrNoSuchCtr {
+			utils.ContainerNotFound(w, name, err)
+			return
+		}
 
-	var stopError error
-	if query.Timeout > 0 {
-		stopError = con.StopWithTimeout(uint(query.Timeout))
-	} else {
-		stopError = con.Stop()
+		utils.InternalServerError(w, err)
+		return
 	}
-	if stopError != nil {
-		utils.InternalServerError(w, errors.Wrapf(stopError, "failed to stop %s", name))
+
+	if len(report) > 0 && report[0].Err != nil {
+		utils.InternalServerError(w, report[0].Err)
 		return
 	}
 
diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go
index a0cb1d49e..f6e348cef 100644
--- a/pkg/api/handlers/libpod/containers.go
+++ b/pkg/api/handlers/libpod/containers.go
@@ -148,6 +148,12 @@ func GetContainer(w http.ResponseWriter, r *http.Request) {
 func WaitContainer(w http.ResponseWriter, r *http.Request) {
 	exitCode, err := utils.WaitContainer(w, r)
 	if err != nil {
+		name := utils.GetName(r)
+		if errors.Cause(err) == define.ErrNoSuchCtr {
+			utils.ContainerNotFound(w, name, err)
+			return
+		}
+		logrus.Warnf("failed to wait on container %q: %v", name, err)
 		return
 	}
 	utils.WriteResponse(w, http.StatusOK, strconv.Itoa(int(exitCode)))
diff --git a/pkg/api/handlers/utils/containers.go b/pkg/api/handlers/utils/containers.go
index 1439a3a75..fac237f87 100644
--- a/pkg/api/handlers/utils/containers.go
+++ b/pkg/api/handlers/utils/containers.go
@@ -6,6 +6,8 @@ import (
 
 	"github.com/containers/podman/v2/libpod"
 	"github.com/containers/podman/v2/libpod/define"
+	"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"
 )
@@ -16,10 +18,13 @@ func WaitContainer(w http.ResponseWriter, r *http.Request) (int32, error) {
 		interval time.Duration
 	)
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
+	// Now use the ABI implementation to prevent us from having duplicate
+	// code.
+	containerEngine := abi.ContainerEngine{Libpod: runtime}
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
 	query := struct {
-		Interval  string `schema:"interval"`
-		Condition string `schema:"condition"`
+		Interval  string                 `schema:"interval"`
+		Condition define.ContainerStatus `schema:"condition"`
 	}{
 		// Override golang default values for types
 	}
@@ -27,6 +32,10 @@ func WaitContainer(w http.ResponseWriter, r *http.Request) (int32, error) {
 		Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
 		return 0, err
 	}
+	options := entities.WaitOptions{
+		Condition: define.ContainerStateStopped,
+	}
+	name := GetName(r)
 	if _, found := r.URL.Query()["interval"]; found {
 		interval, err = time.ParseDuration(query.Interval)
 		if err != nil {
@@ -40,19 +49,19 @@ func WaitContainer(w http.ResponseWriter, r *http.Request) (int32, error) {
 			return 0, err
 		}
 	}
-	condition := define.ContainerStateStopped
+	options.Interval = interval
+
 	if _, found := r.URL.Query()["condition"]; found {
-		condition, err = define.StringToContainerStatus(query.Condition)
-		if err != nil {
-			InternalServerError(w, err)
-			return 0, err
-		}
+		options.Condition = query.Condition
 	}
-	name := GetName(r)
-	con, err := runtime.LookupContainer(name)
+
+	report, err := containerEngine.ContainerWait(r.Context(), []string{name}, options)
 	if err != nil {
-		ContainerNotFound(w, name, err)
 		return 0, err
 	}
-	return con.WaitForConditionWithInterval(interval, condition)
+	if len(report) == 0 {
+		InternalServerError(w, errors.New("No reports returned"))
+		return 0, err
+	}
+	return report[0].ExitCode, report[0].Error
 }
diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go
index e30747800..ff1781d1e 100644
--- a/pkg/api/server/register_containers.go
+++ b/pkg/api/server/register_containers.go
@@ -199,6 +199,11 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//    required: true
 	//    description: the name or ID of the container
 	//  - in: query
+	//    name: all
+	//    type: boolean
+	//    default: false
+	//    description: Send kill signal to all containers
+	//  - in: query
 	//    name: signal
 	//    type: string
 	//    default: TERM
@@ -486,6 +491,11 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//        - paused
 	//        - running
 	//        - stopped
+	//  - in: query
+	//    name: interval
+	//    type: string
+	//    default: "250ms"
+	//    description: Time Interval to wait before polling for completion.
 	// produces:
 	// - application/json
 	// responses:
@@ -1219,9 +1229,20 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//    required: true
 	//    description: the name or ID of the container
 	//  - in: query
-	//    name: t
+	//    name: all
+	//    type: boolean
+	//    default: false
+	//    description: Stop all containers
+	//  - in: query
+	//    name: timeout
 	//    type: integer
+	//    default: 10
 	//    description: number of seconds to wait before killing container
+	//  - in: query
+	//    name: Ignore
+	//    type: boolean
+	//    default: false
+	//    description: do not return error if container is already stopped
 	// produces:
 	// - application/json
 	// responses:
diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go
index 40fcfbded..8e644b712 100644
--- a/pkg/bindings/containers/containers.go
+++ b/pkg/bindings/containers/containers.go
@@ -5,7 +5,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"strconv"
 	"strings"
 
 	"github.com/containers/podman/v2/libpod/define"
@@ -83,18 +82,9 @@ func Remove(ctx context.Context, nameOrID string, options *RemoveOptions) error
 	if err != nil {
 		return err
 	}
-	params := url.Values{}
-	if v := options.GetVolumes(); options.Changed("Volumes") {
-		params.Set("v", strconv.FormatBool(v))
-	}
-	if all := options.GetAll(); options.Changed("All") {
-		params.Set("all", strconv.FormatBool(all))
-	}
-	if force := options.GetForce(); options.Changed("Force") {
-		params.Set("force", strconv.FormatBool(force))
-	}
-	if ignore := options.GetIgnore(); options.Changed("Ignore") {
-		params.Set("ignore", strconv.FormatBool(ignore))
+	params, err := options.ToParams()
+	if err != nil {
+		return err
 	}
 	response, err := conn.DoRequest(nil, http.MethodDelete, "/containers/%s", params, nil, nameOrID)
 	if err != nil {
@@ -130,7 +120,7 @@ func Inspect(ctx context.Context, nameOrID string, options *InspectOptions) (*de
 // Kill sends a given signal to a given container.  The signal should be the string
 // representation of a signal like 'SIGKILL'. The nameOrID can be a container name
 // or a partial/full ID
-func Kill(ctx context.Context, nameOrID string, sig string, options *KillOptions) error {
+func Kill(ctx context.Context, nameOrID string, options *KillOptions) error {
 	if options == nil {
 		options = new(KillOptions)
 	}
@@ -142,7 +132,6 @@ func Kill(ctx context.Context, nameOrID string, sig string, options *KillOptions
 	if err != nil {
 		return err
 	}
-	params.Set("signal", sig)
 	response, err := conn.DoRequest(nil, http.MethodPost, "/containers/%s/kill", params, nil, nameOrID)
 	if err != nil {
 		return err
@@ -180,9 +169,9 @@ func Restart(ctx context.Context, nameOrID string, options *RestartOptions) erro
 	if err != nil {
 		return err
 	}
-	params := url.Values{}
-	if options.Changed("Timeout") {
-		params.Set("t", strconv.Itoa(options.GetTimeout()))
+	params, err := options.ToParams()
+	if err != nil {
+		return err
 	}
 	response, err := conn.DoRequest(nil, http.MethodPost, "/containers/%s/restart", params, nil, nameOrID)
 	if err != nil {
@@ -335,9 +324,9 @@ func Wait(ctx context.Context, nameOrID string, options *WaitOptions) (int32, er
 	if err != nil {
 		return exitCode, err
 	}
-	params := url.Values{}
-	if options.Changed("Condition") {
-		params.Set("condition", options.GetCondition().String())
+	params, err := options.ToParams()
+	if err != nil {
+		return exitCode, err
 	}
 	response, err := conn.DoRequest(nil, http.MethodPost, "/containers/%s/wait", params, nil, nameOrID)
 	if err != nil {
diff --git a/pkg/bindings/containers/types.go b/pkg/bindings/containers/types.go
index 3fb1ab733..771cde72c 100644
--- a/pkg/bindings/containers/types.go
+++ b/pkg/bindings/containers/types.go
@@ -123,7 +123,6 @@ type PruneOptions struct {
 //go:generate go run ../generator/generator.go RemoveOptions
 // RemoveOptions are optional options for removing containers
 type RemoveOptions struct {
-	All     *bool
 	Ignore  *bool
 	Force   *bool
 	Volumes *bool
@@ -138,6 +137,7 @@ type InspectOptions struct {
 //go:generate go run ../generator/generator.go KillOptions
 // KillOptions are optional options for killing containers
 type KillOptions struct {
+	Signal *string
 }
 
 //go:generate go run ../generator/generator.go PauseOptions
@@ -177,11 +177,13 @@ type UnpauseOptions struct{}
 // WaitOptions are optional options for waiting on containers
 type WaitOptions struct {
 	Condition *define.ContainerStatus
+	Interval  *string
 }
 
 //go:generate go run ../generator/generator.go StopOptions
 // StopOptions are optional options for stopping containers
 type StopOptions struct {
+	Ignore  *bool
 	Timeout *uint
 }
 
diff --git a/pkg/bindings/containers/types_kill_options.go b/pkg/bindings/containers/types_kill_options.go
index dd84f0d9f..c5d5a3c6a 100644
--- a/pkg/bindings/containers/types_kill_options.go
+++ b/pkg/bindings/containers/types_kill_options.go
@@ -86,3 +86,19 @@ func (o *KillOptions) ToParams() (url.Values, error) {
 	}
 	return params, nil
 }
+
+// WithSignal
+func (o *KillOptions) WithSignal(value string) *KillOptions {
+	v := &value
+	o.Signal = v
+	return o
+}
+
+// GetSignal
+func (o *KillOptions) GetSignal() string {
+	var signal string
+	if o.Signal == nil {
+		return signal
+	}
+	return *o.Signal
+}
diff --git a/pkg/bindings/containers/types_remove_options.go b/pkg/bindings/containers/types_remove_options.go
index 3ef32fa03..ffe1488c1 100644
--- a/pkg/bindings/containers/types_remove_options.go
+++ b/pkg/bindings/containers/types_remove_options.go
@@ -87,22 +87,6 @@ func (o *RemoveOptions) ToParams() (url.Values, error) {
 	return params, nil
 }
 
-// WithAll
-func (o *RemoveOptions) WithAll(value bool) *RemoveOptions {
-	v := &value
-	o.All = v
-	return o
-}
-
-// GetAll
-func (o *RemoveOptions) GetAll() bool {
-	var all bool
-	if o.All == nil {
-		return all
-	}
-	return *o.All
-}
-
 // WithIgnore
 func (o *RemoveOptions) WithIgnore(value bool) *RemoveOptions {
 	v := &value
diff --git a/pkg/bindings/containers/types_stop_options.go b/pkg/bindings/containers/types_stop_options.go
index db692dbf0..940ec5832 100644
--- a/pkg/bindings/containers/types_stop_options.go
+++ b/pkg/bindings/containers/types_stop_options.go
@@ -87,6 +87,22 @@ func (o *StopOptions) ToParams() (url.Values, error) {
 	return params, nil
 }
 
+// WithIgnore
+func (o *StopOptions) WithIgnore(value bool) *StopOptions {
+	v := &value
+	o.Ignore = v
+	return o
+}
+
+// GetIgnore
+func (o *StopOptions) GetIgnore() bool {
+	var ignore bool
+	if o.Ignore == nil {
+		return ignore
+	}
+	return *o.Ignore
+}
+
 // WithTimeout
 func (o *StopOptions) WithTimeout(value uint) *StopOptions {
 	v := &value
diff --git a/pkg/bindings/containers/types_wait_options.go b/pkg/bindings/containers/types_wait_options.go
index 470d67611..2f5aa983e 100644
--- a/pkg/bindings/containers/types_wait_options.go
+++ b/pkg/bindings/containers/types_wait_options.go
@@ -103,3 +103,19 @@ func (o *WaitOptions) GetCondition() define.ContainerStatus {
 	}
 	return *o.Condition
 }
+
+// WithInterval
+func (o *WaitOptions) WithInterval(value string) *WaitOptions {
+	v := &value
+	o.Interval = v
+	return o
+}
+
+// GetInterval
+func (o *WaitOptions) GetInterval() string {
+	var interval string
+	if o.Interval == nil {
+		return interval
+	}
+	return *o.Interval
+}
diff --git a/pkg/bindings/test/containers_test.go b/pkg/bindings/test/containers_test.go
index 3d7526cb8..9b9f98047 100644
--- a/pkg/bindings/test/containers_test.go
+++ b/pkg/bindings/test/containers_test.go
@@ -443,7 +443,7 @@ var _ = Describe("Podman containers ", func() {
 
 	It("podman kill bogus container", func() {
 		// Killing bogus container should return 404
-		err := containers.Kill(bt.conn, "foobar", "SIGTERM", nil)
+		err := containers.Kill(bt.conn, "foobar", new(containers.KillOptions).WithSignal("SIGTERM"))
 		Expect(err).ToNot(BeNil())
 		code, _ := bindings.CheckResponseCode(err)
 		Expect(code).To(BeNumerically("==", http.StatusNotFound))
@@ -454,7 +454,7 @@ var _ = Describe("Podman containers ", func() {
 		var name = "top"
 		_, err := bt.RunTopContainer(&name, bindings.PFalse, nil)
 		Expect(err).To(BeNil())
-		err = containers.Kill(bt.conn, name, "SIGINT", nil)
+		err = containers.Kill(bt.conn, name, new(containers.KillOptions).WithSignal("SIGINT"))
 		Expect(err).To(BeNil())
 		_, err = containers.Exists(bt.conn, name, nil)
 		Expect(err).To(BeNil())
@@ -465,7 +465,7 @@ var _ = Describe("Podman containers ", func() {
 		var name = "top"
 		cid, err := bt.RunTopContainer(&name, bindings.PFalse, nil)
 		Expect(err).To(BeNil())
-		err = containers.Kill(bt.conn, cid, "SIGTERM", nil)
+		err = containers.Kill(bt.conn, cid, new(containers.KillOptions).WithSignal("SIGTERM"))
 		Expect(err).To(BeNil())
 		_, err = containers.Exists(bt.conn, cid, nil)
 		Expect(err).To(BeNil())
@@ -476,7 +476,7 @@ var _ = Describe("Podman containers ", func() {
 		var name = "top"
 		cid, err := bt.RunTopContainer(&name, bindings.PFalse, nil)
 		Expect(err).To(BeNil())
-		err = containers.Kill(bt.conn, cid, "SIGKILL", nil)
+		err = containers.Kill(bt.conn, cid, new(containers.KillOptions).WithSignal("SIGKILL"))
 		Expect(err).To(BeNil())
 	})
 
@@ -485,7 +485,7 @@ var _ = Describe("Podman containers ", func() {
 		var name = "top"
 		cid, err := bt.RunTopContainer(&name, bindings.PFalse, nil)
 		Expect(err).To(BeNil())
-		err = containers.Kill(bt.conn, cid, "foobar", nil)
+		err = containers.Kill(bt.conn, cid, new(containers.KillOptions).WithSignal("foobar"))
 		Expect(err).ToNot(BeNil())
 		code, _ := bindings.CheckResponseCode(err)
 		Expect(code).To(BeNumerically("==", http.StatusInternalServerError))
@@ -501,7 +501,7 @@ var _ = Describe("Podman containers ", func() {
 		Expect(err).To(BeNil())
 		containerLatestList, err := containers.List(bt.conn, new(containers.ListOptions).WithLast(1))
 		Expect(err).To(BeNil())
-		err = containers.Kill(bt.conn, containerLatestList[0].Names[0], "SIGTERM", nil)
+		err = containers.Kill(bt.conn, containerLatestList[0].Names[0], new(containers.KillOptions).WithSignal("SIGTERM"))
 		Expect(err).To(BeNil())
 	})
 
diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go
index 2c32f792f..63be5578f 100644
--- a/pkg/domain/entities/containers.go
+++ b/pkg/domain/entities/containers.go
@@ -81,11 +81,10 @@ type PauseUnpauseReport struct {
 }
 
 type StopOptions struct {
-	All      bool
-	CIDFiles []string
-	Ignore   bool
-	Latest   bool
-	Timeout  *uint
+	All     bool
+	Ignore  bool
+	Latest  bool
+	Timeout *uint
 }
 
 type StopReport struct {
@@ -104,10 +103,9 @@ type TopOptions struct {
 }
 
 type KillOptions struct {
-	All      bool
-	Latest   bool
-	Signal   string
-	CIDFiles []string
+	All    bool
+	Latest bool
+	Signal string
 }
 
 type KillReport struct {
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index 48a32817d..d0599a595 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"os"
 	"strconv"
-	"strings"
 	"sync"
 	"time"
 
@@ -139,14 +138,6 @@ func (ic *ContainerEngine) ContainerUnpause(ctx context.Context, namesOrIds []st
 }
 func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []string, options entities.StopOptions) ([]*entities.StopReport, error) {
 	names := namesOrIds
-	for _, cidFile := range options.CIDFiles {
-		content, err := ioutil.ReadFile(cidFile)
-		if err != nil {
-			return nil, errors.Wrap(err, "error reading CIDFile")
-		}
-		id := strings.Split(string(content), "\n")[0]
-		names = append(names, id)
-	}
 	ctrs, err := getContainersByContext(options.All, options.Latest, names, ic.Libpod)
 	if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr) {
 		return nil, err
@@ -202,14 +193,6 @@ func (ic *ContainerEngine) ContainerPrune(ctx context.Context, options entities.
 }
 
 func (ic *ContainerEngine) ContainerKill(ctx context.Context, namesOrIds []string, options entities.KillOptions) ([]*entities.KillReport, error) {
-	for _, cidFile := range options.CIDFiles {
-		content, err := ioutil.ReadFile(cidFile)
-		if err != nil {
-			return nil, errors.Wrap(err, "error reading CIDFile")
-		}
-		id := strings.Split(string(content), "\n")[0]
-		namesOrIds = append(namesOrIds, id)
-	}
 	sig, err := signal.ParseSignalNameOrNumber(options.Signal)
 	if err != nil {
 		return nil, err
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index 0c61714c3..e9c513f8e 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/pkg/domain/infra/tunnel/containers.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"strconv"
 	"strings"
@@ -41,7 +40,7 @@ func (ic *ContainerEngine) ContainerWait(ctx context.Context, namesOrIds []strin
 		return nil, err
 	}
 	responses := make([]entities.WaitReport, 0, len(cons))
-	options := new(containers.WaitOptions).WithCondition(opts.Condition)
+	options := new(containers.WaitOptions).WithCondition(opts.Condition).WithInterval(opts.Interval.String())
 	for _, c := range cons {
 		response := entities.WaitReport{Id: c.ID}
 		exitCode, err := containers.Wait(ic.ClientCtx, c.ID, options)
@@ -83,19 +82,11 @@ func (ic *ContainerEngine) ContainerUnpause(ctx context.Context, namesOrIds []st
 
 func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []string, opts entities.StopOptions) ([]*entities.StopReport, error) {
 	reports := []*entities.StopReport{}
-	for _, cidFile := range opts.CIDFiles {
-		content, err := ioutil.ReadFile(cidFile)
-		if err != nil {
-			return nil, errors.Wrap(err, "error reading CIDFile")
-		}
-		id := strings.Split(string(content), "\n")[0]
-		namesOrIds = append(namesOrIds, id)
-	}
 	ctrs, err := getContainersByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds)
 	if err != nil {
 		return nil, err
 	}
-	options := new(containers.StopOptions)
+	options := new(containers.StopOptions).WithIgnore(opts.Ignore)
 	if to := opts.Timeout; to != nil {
 		options.WithTimeout(*to)
 	}
@@ -126,23 +117,16 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin
 }
 
 func (ic *ContainerEngine) ContainerKill(ctx context.Context, namesOrIds []string, opts entities.KillOptions) ([]*entities.KillReport, error) {
-	for _, cidFile := range opts.CIDFiles {
-		content, err := ioutil.ReadFile(cidFile)
-		if err != nil {
-			return nil, errors.Wrap(err, "error reading CIDFile")
-		}
-		id := strings.Split(string(content), "\n")[0]
-		namesOrIds = append(namesOrIds, id)
-	}
 	ctrs, err := getContainersByContext(ic.ClientCtx, opts.All, false, namesOrIds)
 	if err != nil {
 		return nil, err
 	}
+	options := new(containers.KillOptions).WithSignal(opts.Signal)
 	reports := make([]*entities.KillReport, 0, len(ctrs))
 	for _, c := range ctrs {
 		reports = append(reports, &entities.KillReport{
 			Id:  c.ID,
-			Err: containers.Kill(ic.ClientCtx, c.ID, opts.Signal, nil),
+			Err: containers.Kill(ic.ClientCtx, c.ID, options),
 		})
 	}
 	return reports, nil
diff --git a/test/e2e/kill_test.go b/test/e2e/kill_test.go
index 8b31cae72..c1c1b003e 100644
--- a/test/e2e/kill_test.go
+++ b/test/e2e/kill_test.go
@@ -167,4 +167,20 @@ var _ = Describe("Podman kill", func() {
 		Expect(wait.ExitCode()).To(BeZero())
 	})
 
+	It("podman stop --all", func() {
+		session := podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
+
+		session = podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(2))
+
+		session = podmanTest.Podman([]string{"kill", "--all"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
+	})
 })
diff --git a/test/e2e/restart_test.go b/test/e2e/restart_test.go
index 584ccd22b..bcaab8947 100644
--- a/test/e2e/restart_test.go
+++ b/test/e2e/restart_test.go
@@ -225,4 +225,26 @@ var _ = Describe("Podman restart", func() {
 		// line count should be equal
 		Expect(beforeRestart.OutputToString()).To(Equal(afterRestart.OutputToString()))
 	})
+
+	It("podman restart --all", func() {
+		session := podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
+
+		session = podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(2))
+
+		session = podmanTest.Podman([]string{"stop", "--all"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"restart", "--all"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(2))
+	})
 })
diff --git a/test/e2e/stop_test.go b/test/e2e/stop_test.go
index c25709a63..750d38ffb 100644
--- a/test/e2e/stop_test.go
+++ b/test/e2e/stop_test.go
@@ -164,13 +164,14 @@ var _ = Describe("Podman stop", func() {
 	})
 
 	It("podman stop container --timeout", func() {
-		session := podmanTest.RunTopContainer("test5")
+		session := podmanTest.Podman([]string{"run", "-d", "--name", "test5", ALPINE, "sleep", "100"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		cid1 := session.OutputToString()
-
 		session = podmanTest.Podman([]string{"stop", "--timeout", "1", "test5"})
-		session.WaitWithDefaultTimeout()
+		// Without timeout container stops in 10 seconds
+		// If not stopped in 5 seconds, then --timeout did not work
+		session.Wait(5)
 		Expect(session.ExitCode()).To(Equal(0))
 		output := session.OutputToString()
 		Expect(output).To(ContainSubstring(cid1))
@@ -307,4 +308,38 @@ var _ = Describe("Podman stop", func() {
 		result.WaitWithDefaultTimeout()
 		Expect(result.ExitCode()).To(Equal(125))
 	})
+
+	It("podman stop --all", func() {
+		session := podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
+
+		session = podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(2))
+
+		session = podmanTest.Podman([]string{"stop", "--all"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
+	})
+
+	It("podman stop --ignore", func() {
+		session := podmanTest.RunTopContainer("")
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		cid := session.OutputToString()
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
+
+		session = podmanTest.Podman([]string{"stop", "bogus", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(125))
+
+		session = podmanTest.Podman([]string{"stop", "--ignore", "bogus", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
+	})
 })