From 9d9493e41a2d9fb79221986507d1789265355d2d Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 16 Oct 2018 11:50:10 +0000 Subject: [PATCH 1/5] Add checkAllAndLatest() function The check about the --all and --latest option is used and repeated and some commands. Factor it out and put it into common. Signed-off-by: Adrian Reber --- cmd/podman/common.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cmd/podman/common.go b/cmd/podman/common.go index 8ae1c9e0fd..3401b1a935 100644 --- a/cmd/podman/common.go +++ b/cmd/podman/common.go @@ -89,6 +89,21 @@ func validateFlags(c *cli.Context, flags []cli.Flag) error { return nil } +// checkAllAndLatest checks that --all and --latest are used correctly +func checkAllAndLatest(c *cli.Context) error { + argLen := len(c.Args()) + if (c.Bool("all") || c.Bool("latest")) && argLen > 0 { + return errors.Errorf("no arguments are needed with --all or --latest") + } + if c.Bool("all") && c.Bool("latest") { + return errors.Errorf("--all and --latest cannot be used together") + } + if argLen < 1 && !c.Bool("all") && !c.Bool("latest") { + return errors.Errorf("you must provide at least one pod name or id") + } + return nil +} + // getContext returns a non-nil, empty context func getContext() context.Context { return context.TODO() From 215cf7b8984f687a79ce6055e350ee3e75d81b79 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 16 Oct 2018 13:36:08 +0000 Subject: [PATCH 2/5] Also factor out getAllOrLatestContainers() function Just as the checkAllAndLatest() function the new code in getAllOrLatestContainers() is used in some commands and duplicated. This factors out this code to be used in other places without duplicating it. Signed-off-by: Adrian Reber --- cmd/podman/common.go | 52 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/cmd/podman/common.go b/cmd/podman/common.go index 3401b1a935..f9e746b28e 100644 --- a/cmd/podman/common.go +++ b/cmd/podman/common.go @@ -104,6 +104,58 @@ func checkAllAndLatest(c *cli.Context) error { return nil } +// getAllOrLatestContainers tries to return the correct list of containers +// depending if --all, --latest or is used. +// It requires the Context (c) and the Runtime (runtime). As different +// commands are using different container state for the --all option +// the desired state has to be specified in filterState. If no filter +// is desired a -1 can be used to get all containers. For a better +// error message, if the filter fails, a corresponding verb can be +// specified which will then appear in the error message. +func getAllOrLatestContainers(c *cli.Context, runtime *libpod.Runtime, filterState libpod.ContainerStatus, verb string) ([]*libpod.Container, error) { + var containers []*libpod.Container + var lastError error + var err error + if c.Bool("all") { + if filterState != -1 { + var filterFuncs []libpod.ContainerFilter + filterFuncs = append(filterFuncs, func(c *libpod.Container) bool { + state, _ := c.State() + return state == filterState + }) + containers, err = runtime.GetContainers(filterFuncs...) + } else { + containers, err = runtime.GetContainers() + } + if err != nil { + return nil, errors.Wrapf(err, "unable to get %s containers", verb) + } + } else if c.Bool("latest") { + lastCtr, err := runtime.GetLatestContainer() + if err != nil { + return nil, errors.Wrapf(err, "unable to get latest container") + } + containers = append(containers, lastCtr) + } else { + args := c.Args() + for _, i := range args { + container, err := runtime.LookupContainer(i) + if err != nil { + if lastError != nil { + fmt.Fprintln(os.Stderr, lastError) + } + lastError = errors.Wrapf(err, "unable to find container %s", i) + } + if container != nil { + // This is here to make sure this does not return [] but only nil + containers = append(containers, container) + } + } + } + + return containers, lastError +} + // getContext returns a non-nil, empty context func getContext() context.Context { return context.TODO() From fea37b387c746471177f90f15b04d7735a88e621 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 16 Oct 2018 12:04:45 +0000 Subject: [PATCH 3/5] Use the new checkAllAndLatest() function Instead of duplicating the same code in multiple commands this uses the newly added function checkAllAndLatest() instead. Signed-off-by: Adrian Reber --- cmd/podman/cleanup.go | 19 +++++++------------ cmd/podman/kill.go | 16 ++++------------ cmd/podman/rm.go | 10 +++------- cmd/podman/stop.go | 14 +++++--------- cmd/podman/utils.go | 11 ++--------- 5 files changed, 21 insertions(+), 49 deletions(-) diff --git a/cmd/podman/cleanup.go b/cmd/podman/cleanup.go index 3fd1507836..1d8b2fbec1 100644 --- a/cmd/podman/cleanup.go +++ b/cmd/podman/cleanup.go @@ -44,33 +44,25 @@ func cleanupCmd(c *cli.Context) error { } defer runtime.Shutdown(false) - args := c.Args() - - ctx := getContext() + if err := checkAllAndLatest(c); err != nil { + return err + } var lastError error var cleanupContainers []*libpod.Container if c.Bool("all") { - if c.Bool("lastest") { - return errors.New("--all and --latest cannot be used together") - } - if len(args) != 0 { - return errors.New("--all and explicit container IDs cannot be used together") - } cleanupContainers, err = runtime.GetContainers() if err != nil { return errors.Wrapf(err, "unable to get container list") } } else if c.Bool("latest") { - if len(args) != 0 { - return errors.New("--latest and explicit container IDs cannot be used together") - } lastCtr, err := runtime.GetLatestContainer() if err != nil { return errors.Wrapf(err, "unable to get latest container") } cleanupContainers = append(cleanupContainers, lastCtr) } else { + args := c.Args() for _, i := range args { container, err := runtime.LookupContainer(i) if err != nil { @@ -81,6 +73,9 @@ func cleanupCmd(c *cli.Context) error { cleanupContainers = append(cleanupContainers, container) } } + + ctx := getContext() + for _, ctr := range cleanupContainers { if err = ctr.Cleanup(ctx); err != nil { if lastError != nil { diff --git a/cmd/podman/kill.go b/cmd/podman/kill.go index 56dd170b5e..37b24a9238 100644 --- a/cmd/podman/kill.go +++ b/cmd/podman/kill.go @@ -41,19 +41,10 @@ var ( // killCmd kills one or more containers with a signal func killCmd(c *cli.Context) error { - args := c.Args() - if (!c.Bool("all") && !c.Bool("latest")) && len(args) == 0 { - return errors.Errorf("you must specify one or more containers to kill") - } - if (c.Bool("all") || c.Bool("latest")) && len(args) > 0 { - return errors.Errorf("you cannot specify any containers to kill with --latest or --all") - } - if c.Bool("all") && c.Bool("latest") { - return errors.Errorf("--all and --latest cannot be used together") - } - if len(args) < 1 && !c.Bool("all") && !c.Bool("latest") { - return errors.Errorf("you must provide at least one container name or id") + if err := checkAllAndLatest(c); err != nil { + return err } + if err := validateFlags(c, killFlags); err != nil { return err } @@ -96,6 +87,7 @@ func killCmd(c *cli.Context) error { } containers = append(containers, lastCtr) } else { + args := c.Args() for _, i := range args { container, err := runtime.LookupContainer(i) if err != nil { diff --git a/cmd/podman/rm.go b/cmd/podman/rm.go index 38b1546ff1..847ea642d8 100644 --- a/cmd/podman/rm.go +++ b/cmd/podman/rm.go @@ -63,13 +63,8 @@ func rmCmd(c *cli.Context) error { } defer runtime.Shutdown(false) - args := c.Args() - if c.Bool("latest") && c.Bool("all") { - return errors.Errorf("--all and --latest cannot be used together") - } - - if len(args) == 0 && !c.Bool("all") && !c.Bool("latest") { - return errors.Errorf("specify one or more containers to remove") + if err := checkAllAndLatest(c); err != nil { + return err } if c.Bool("all") { @@ -84,6 +79,7 @@ func rmCmd(c *cli.Context) error { } delContainers = append(delContainers, lastCtr) } else { + args := c.Args() for _, i := range args { container, err := runtime.LookupContainer(i) if err != nil { diff --git a/cmd/podman/stop.go b/cmd/podman/stop.go index ff0b36bf1d..f5c0fca854 100644 --- a/cmd/podman/stop.go +++ b/cmd/podman/stop.go @@ -44,16 +44,11 @@ var ( ) func stopCmd(c *cli.Context) error { - args := c.Args() - if (c.Bool("all") || c.Bool("latest")) && len(args) > 0 { - return errors.Errorf("no arguments are needed with --all or --latest") - } - if c.Bool("all") && c.Bool("latest") { - return errors.Errorf("--all and --latest cannot be used together") - } - if len(args) < 1 && !c.Bool("all") && !c.Bool("latest") { - return errors.Errorf("you must provide at least one container name or id") + + if err := checkAllAndLatest(c); err != nil { + return err } + if err := validateFlags(c, stopFlags); err != nil { return err } @@ -86,6 +81,7 @@ func stopCmd(c *cli.Context) error { } containers = append(containers, lastCtr) } else { + args := c.Args() for _, i := range args { container, err := runtime.LookupContainer(i) if err != nil { diff --git a/cmd/podman/utils.go b/cmd/podman/utils.go index b193cf8896..f9971fd88f 100644 --- a/cmd/podman/utils.go +++ b/cmd/podman/utils.go @@ -160,15 +160,8 @@ func (f *RawTtyFormatter) Format(entry *logrus.Entry) ([]byte, error) { } func checkMutuallyExclusiveFlags(c *cli.Context) error { - argLen := len(c.Args()) - if (c.Bool("all") || c.Bool("latest")) && argLen > 0 { - return errors.Errorf("no arguments are needed with --all or --latest") - } - if c.Bool("all") && c.Bool("latest") { - return errors.Errorf("--all and --latest cannot be used together") - } - if argLen < 1 && !c.Bool("all") && !c.Bool("latest") { - return errors.Errorf("you must provide at least one pod name or id") + if err := checkAllAndLatest(c); err != nil { + return err } if err := validateFlags(c, startFlags); err != nil { return err From c10ac01395066ddeb321a83ab64bc02b2e765f9a Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 16 Oct 2018 16:39:11 +0000 Subject: [PATCH 4/5] Use the newly added getAllOrLatestContainers() function This removes duplicate code paths which has been previously factored out as getAllOrLatestContainers(). Signed-off-by: Adrian Reber --- cmd/podman/cleanup.go | 27 +-------------------------- cmd/podman/kill.go | 34 +--------------------------------- cmd/podman/rm.go | 25 +------------------------ cmd/podman/stop.go | 36 +----------------------------------- 4 files changed, 4 insertions(+), 118 deletions(-) diff --git a/cmd/podman/cleanup.go b/cmd/podman/cleanup.go index 1d8b2fbec1..bc4af9f502 100644 --- a/cmd/podman/cleanup.go +++ b/cmd/podman/cleanup.go @@ -5,7 +5,6 @@ import ( "os" "github.com/containers/libpod/cmd/podman/libpodruntime" - "github.com/containers/libpod/libpod" "github.com/pkg/errors" "github.com/urfave/cli" ) @@ -48,31 +47,7 @@ func cleanupCmd(c *cli.Context) error { return err } - var lastError error - var cleanupContainers []*libpod.Container - if c.Bool("all") { - cleanupContainers, err = runtime.GetContainers() - if err != nil { - return errors.Wrapf(err, "unable to get container list") - } - } else if c.Bool("latest") { - lastCtr, err := runtime.GetLatestContainer() - if err != nil { - return errors.Wrapf(err, "unable to get latest container") - } - cleanupContainers = append(cleanupContainers, lastCtr) - } else { - args := c.Args() - for _, i := range args { - container, err := runtime.LookupContainer(i) - if err != nil { - fmt.Fprintln(os.Stderr, err) - lastError = errors.Wrapf(err, "unable to find container %s", i) - continue - } - cleanupContainers = append(cleanupContainers, container) - } - } + cleanupContainers, lastError := getAllOrLatestContainers(c, runtime, -1, "all") ctx := getContext() diff --git a/cmd/podman/kill.go b/cmd/podman/kill.go index 37b24a9238..7ca5bd7c54 100644 --- a/cmd/podman/kill.go +++ b/cmd/podman/kill.go @@ -67,39 +67,7 @@ func killCmd(c *cli.Context) error { killSignal = uint(sysSignal) } - var filterFuncs []libpod.ContainerFilter - var containers []*libpod.Container - var lastError error - if c.Bool("all") { - // only get running containers - filterFuncs = append(filterFuncs, func(c *libpod.Container) bool { - state, _ := c.State() - return state == libpod.ContainerStateRunning - }) - containers, err = runtime.GetContainers(filterFuncs...) - if err != nil { - return errors.Wrapf(err, "unable to get running containers") - } - } else if c.Bool("latest") { - lastCtr, err := runtime.GetLatestContainer() - if err != nil { - return errors.Wrapf(err, "unable to get last created container") - } - containers = append(containers, lastCtr) - } else { - args := c.Args() - for _, i := range args { - container, err := runtime.LookupContainer(i) - if err != nil { - if lastError != nil { - fmt.Fprintln(os.Stderr, lastError) - } - lastError = errors.Wrapf(err, "unable to find container %s", i) - continue - } - containers = append(containers, container) - } - } + containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") for _, ctr := range containers { if err := ctr.Kill(killSignal); err != nil { diff --git a/cmd/podman/rm.go b/cmd/podman/rm.go index 847ea642d8..c6641e8796 100644 --- a/cmd/podman/rm.go +++ b/cmd/podman/rm.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "os" rt "runtime" "github.com/containers/libpod/cmd/podman/libpodruntime" @@ -67,29 +66,7 @@ func rmCmd(c *cli.Context) error { return err } - if c.Bool("all") { - delContainers, err = runtime.GetContainers() - if err != nil { - return errors.Wrapf(err, "unable to get container list") - } - } else if c.Bool("latest") { - lastCtr, err := runtime.GetLatestContainer() - if err != nil { - return errors.Wrapf(err, "unable to get latest container") - } - delContainers = append(delContainers, lastCtr) - } else { - args := c.Args() - for _, i := range args { - container, err := runtime.LookupContainer(i) - if err != nil { - fmt.Fprintln(os.Stderr, err) - lastError = errors.Wrapf(err, "unable to find container %s", i) - continue - } - delContainers = append(delContainers, container) - } - } + delContainers, lastError = getAllOrLatestContainers(c, runtime, -1, "all") for _, container := range delContainers { f := func() error { diff --git a/cmd/podman/stop.go b/cmd/podman/stop.go index f5c0fca854..edadbda893 100644 --- a/cmd/podman/stop.go +++ b/cmd/podman/stop.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "os" rt "runtime" "github.com/containers/libpod/cmd/podman/libpodruntime" @@ -60,40 +59,7 @@ func stopCmd(c *cli.Context) error { } defer runtime.Shutdown(false) - var filterFuncs []libpod.ContainerFilter - var containers []*libpod.Container - var lastError error - - if c.Bool("all") { - // only get running containers - filterFuncs = append(filterFuncs, func(c *libpod.Container) bool { - state, _ := c.State() - return state == libpod.ContainerStateRunning - }) - containers, err = runtime.GetContainers(filterFuncs...) - if err != nil { - return errors.Wrapf(err, "unable to get running containers") - } - } else if c.Bool("latest") { - lastCtr, err := runtime.GetLatestContainer() - if err != nil { - return errors.Wrapf(err, "unable to get last created container") - } - containers = append(containers, lastCtr) - } else { - args := c.Args() - for _, i := range args { - container, err := runtime.LookupContainer(i) - if err != nil { - if lastError != nil { - fmt.Fprintln(os.Stderr, lastError) - } - lastError = errors.Wrapf(err, "unable to find container %s", i) - continue - } - containers = append(containers, container) - } - } + containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") var stopFuncs []workerInput for _, ctr := range containers { From e8d69030b621874159176eb67292ce06632ea2fd Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 16 Oct 2018 12:07:32 +0000 Subject: [PATCH 5/5] Add --all and --latest to checkpoint/restore This add the convenience options --all and --latest to the subcommands checkpoint and restore. Signed-off-by: Adrian Reber --- cmd/podman/checkpoint.go | 25 ++++++++++++------------- cmd/podman/restore.go | 28 +++++++++++++++------------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/cmd/podman/checkpoint.go b/cmd/podman/checkpoint.go index 8582ce1389..bf280920d7 100644 --- a/cmd/podman/checkpoint.go +++ b/cmd/podman/checkpoint.go @@ -6,6 +6,7 @@ import ( "os" "github.com/containers/libpod/cmd/podman/libpodruntime" + "github.com/containers/libpod/libpod" "github.com/containers/libpod/pkg/rootless" "github.com/pkg/errors" "github.com/urfave/cli" @@ -22,6 +23,11 @@ var ( Name: "keep, k", Usage: "keep all temporary checkpoint files", }, + cli.BoolFlag{ + Name: "all, a", + Usage: "checkpoint all running containers", + }, + LatestFlag, } checkpointCommand = cli.Command{ Name: "checkpoint", @@ -45,21 +51,14 @@ func checkpointCmd(c *cli.Context) error { defer runtime.Shutdown(false) keep := c.Bool("keep") - args := c.Args() - if len(args) < 1 { - return errors.Errorf("you must provide at least one container name or id") + + if err := checkAllAndLatest(c); err != nil { + return err } - var lastError error - for _, arg := range args { - ctr, err := runtime.LookupContainer(arg) - if err != nil { - if lastError != nil { - fmt.Fprintln(os.Stderr, lastError) - } - lastError = errors.Wrapf(err, "error looking up container %q", arg) - continue - } + containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") + + for _, ctr := range containers { if err = ctr.Checkpoint(context.TODO(), keep); err != nil { if lastError != nil { fmt.Fprintln(os.Stderr, lastError) diff --git a/cmd/podman/restore.go b/cmd/podman/restore.go index 623c4936e6..067a2b5d41 100644 --- a/cmd/podman/restore.go +++ b/cmd/podman/restore.go @@ -6,6 +6,7 @@ import ( "os" "github.com/containers/libpod/cmd/podman/libpodruntime" + "github.com/containers/libpod/libpod" "github.com/containers/libpod/pkg/rootless" "github.com/pkg/errors" "github.com/urfave/cli" @@ -22,6 +23,14 @@ var ( Name: "keep, k", Usage: "keep all temporary checkpoint files", }, + // restore --all would make more sense if there would be + // dedicated state for container which are checkpointed. + // TODO: add ContainerStateCheckpointed + cli.BoolFlag{ + Name: "all, a", + Usage: "restore all checkpointed containers", + }, + LatestFlag, } restoreCommand = cli.Command{ Name: "restore", @@ -45,21 +54,14 @@ func restoreCmd(c *cli.Context) error { defer runtime.Shutdown(false) keep := c.Bool("keep") - args := c.Args() - if len(args) < 1 { - return errors.Errorf("you must provide at least one container name or id") + + if err := checkAllAndLatest(c); err != nil { + return err } - var lastError error - for _, arg := range args { - ctr, err := runtime.LookupContainer(arg) - if err != nil { - if lastError != nil { - fmt.Fprintln(os.Stderr, lastError) - } - lastError = errors.Wrapf(err, "error looking up container %q", arg) - continue - } + containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "checkpointed") + + for _, ctr := range containers { if err = ctr.Restore(context.TODO(), keep); err != nil { if lastError != nil { fmt.Fprintln(os.Stderr, lastError)