From 140f87c474f5a075d5833335593895491b83dab6 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 19 Oct 2018 16:02:14 -0400 Subject: EXPERIMENTAL: Do not call out to runc for sync When syncing container state, we normally call out to runc to see the container's status. This does have significant performance implications, though, and we've seen issues with large amounts of runc processes being spawned. This patch attempts to use stat calls on the container exit file created by Conmon instead to sync state. This massively decreases the cost of calling updateContainer (it has gone from an almost-unconditional fork/exec of runc to a single stat call that can be avoided in most states). Signed-off-by: Matthew Heon --- libpod/runtime_ctr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod/runtime_ctr.go') diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index b63726f29..09dc7c48b 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -256,7 +256,7 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) } // Need to update container state to make sure we know it's stopped - if err := c.syncContainer(); err != nil { + if err := c.waitForExitFileAndSync(); err != nil { return err } } else if !(c.state.State == ContainerStateConfigured || -- cgit v1.2.3-54-g00ecf From 2dd9cae37cb076418393ba61c0fb7b8cf97148f3 Mon Sep 17 00:00:00 2001 From: baude Date: Wed, 7 Nov 2018 13:20:43 -0600 Subject: rm -f now removes a paused container We now can remove a paused container by sending it a kill signal while it is paused. We then unpause the container and it is immediately killed. Also, reworked how the parallelWorker results are handled to provide a more consistent approach to how each subcommand implements it. It also fixes a bug where if one container errors, the error message is duplicated when printed out. Signed-off-by: baude --- cmd/podman/kill.go | 23 +++++++---------------- cmd/podman/pause.go | 17 ++--------------- cmd/podman/restart.go | 16 ++-------------- cmd/podman/rm.go | 26 +++++++++++--------------- cmd/podman/shared/parallel.go | 10 +++++++--- cmd/podman/stop.go | 23 +++++++++-------------- cmd/podman/unpause.go | 18 ++---------------- cmd/podman/utils.go | 17 +++++++++++++++++ docs/podman-rm.1.md | 19 ++++++++++++++++--- libpod/runtime_ctr.go | 14 +++++++++++++- test/e2e/pause_test.go | 19 ++++++++++--------- 11 files changed, 96 insertions(+), 106 deletions(-) (limited to 'libpod/runtime_ctr.go') diff --git a/cmd/podman/kill.go b/cmd/podman/kill.go index 27882aeee..cfe4b4218 100644 --- a/cmd/podman/kill.go +++ b/cmd/podman/kill.go @@ -43,7 +43,6 @@ var ( // killCmd kills one or more containers with a signal func killCmd(c *cli.Context) error { var ( - lastError error killFuncs []shared.ParallelWorkerInput killSignal uint = uint(syscall.SIGTERM) ) @@ -75,8 +74,12 @@ func killCmd(c *cli.Context) error { containers, err := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") if err != nil { - return err + if len(containers) == 0 { + return err + } + fmt.Println(err.Error()) } + for _, ctr := range containers { con := ctr f := func() error { @@ -95,18 +98,6 @@ func killCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - killErrors := shared.ParallelExecuteWorkerPool(maxWorkers, killFuncs) - - for cid, result := range killErrors { - if result != nil { - if len(killErrors) > 1 { - fmt.Println(result.Error()) - } - lastError = result - continue - } - fmt.Println(cid) - } - - return lastError + killErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, killFuncs) + return printParallelOutput(killErrors, errCount) } diff --git a/cmd/podman/pause.go b/cmd/podman/pause.go index 1e1585216..fcb2f3cb8 100644 --- a/cmd/podman/pause.go +++ b/cmd/podman/pause.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" "github.com/containers/libpod/cmd/podman/libpodruntime" @@ -37,7 +36,6 @@ var ( func pauseCmd(c *cli.Context) error { var ( - lastError error pauseContainers []*libpod.Container pauseFuncs []shared.ParallelWorkerInput ) @@ -90,17 +88,6 @@ func pauseCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - pauseErrors := shared.ParallelExecuteWorkerPool(maxWorkers, pauseFuncs) - - for cid, result := range pauseErrors { - if result != nil { - if len(pauseErrors) > 1 { - fmt.Println(result.Error()) - } - lastError = result - continue - } - fmt.Println(cid) - } - return lastError + pauseErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, pauseFuncs) + return printParallelOutput(pauseErrors, errCount) } diff --git a/cmd/podman/restart.go b/cmd/podman/restart.go index 2e264db79..630493ef4 100644 --- a/cmd/podman/restart.go +++ b/cmd/podman/restart.go @@ -1,8 +1,6 @@ package main import ( - "fmt" - "github.com/containers/libpod/cmd/podman/libpodruntime" "github.com/containers/libpod/cmd/podman/shared" "github.com/containers/libpod/libpod" @@ -46,7 +44,6 @@ func restartCmd(c *cli.Context) error { var ( restartFuncs []shared.ParallelWorkerInput containers []*libpod.Container - lastError error restartContainers []*libpod.Container ) @@ -124,15 +121,6 @@ func restartCmd(c *cli.Context) error { logrus.Debugf("Setting maximum workers to %d", maxWorkers) - restartErrors := shared.ParallelExecuteWorkerPool(maxWorkers, restartFuncs) - - for cid, result := range restartErrors { - if result != nil { - fmt.Println(result.Error()) - lastError = result - continue - } - fmt.Println(cid) - } - return lastError + restartErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, restartFuncs) + return printParallelOutput(restartErrors, errCount) } diff --git a/cmd/podman/rm.go b/cmd/podman/rm.go index 0fb5345ee..7c0569b78 100644 --- a/cmd/podman/rm.go +++ b/cmd/podman/rm.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/containers/libpod/cmd/podman/libpodruntime" "github.com/containers/libpod/cmd/podman/shared" - "github.com/containers/libpod/libpod" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/urfave/cli" @@ -46,9 +45,7 @@ Running containers will not be removed without the -f option. // saveCmd saves the image to either docker-archive or oci func rmCmd(c *cli.Context) error { var ( - delContainers []*libpod.Container - lastError error - deleteFuncs []shared.ParallelWorkerInput + deleteFuncs []shared.ParallelWorkerInput ) ctx := getContext() @@ -65,7 +62,13 @@ func rmCmd(c *cli.Context) error { return err } - delContainers, lastError = getAllOrLatestContainers(c, runtime, -1, "all") + delContainers, err := getAllOrLatestContainers(c, runtime, -1, "all") + if err != nil { + if len(delContainers) == 0 { + return err + } + fmt.Println(err.Error()) + } for _, container := range delContainers { con := container @@ -84,14 +87,7 @@ func rmCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - deleteErrors := shared.ParallelExecuteWorkerPool(maxWorkers, deleteFuncs) - for cid, result := range deleteErrors { - if result != nil { - fmt.Println(result.Error()) - lastError = result - continue - } - fmt.Println(cid) - } - return lastError + // Run the parallel funcs + deleteErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, deleteFuncs) + return printParallelOutput(deleteErrors, errCount) } diff --git a/cmd/podman/shared/parallel.go b/cmd/podman/shared/parallel.go index 633781a45..e6ce50f95 100644 --- a/cmd/podman/shared/parallel.go +++ b/cmd/podman/shared/parallel.go @@ -30,9 +30,10 @@ func ParallelWorker(wg *sync.WaitGroup, jobs <-chan ParallelWorkerInput, results // ParallelExecuteWorkerPool takes container jobs and performs them in parallel. The worker // int determines how many workers/threads should be premade. -func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) map[string]error { +func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) (map[string]error, int) { var ( - wg sync.WaitGroup + wg sync.WaitGroup + errorCount int ) resultChan := make(chan containerError, len(functions)) @@ -62,9 +63,12 @@ func ParallelExecuteWorkerPool(workers int, functions []ParallelWorkerInput) map close(resultChan) for ctrError := range resultChan { results[ctrError.ContainerID] = ctrError.Err + if ctrError.Err != nil { + errorCount += 1 + } } - return results + return results, errorCount } // Parallelize provides the maximum number of parallel workers (int) as calculated by a basic diff --git a/cmd/podman/stop.go b/cmd/podman/stop.go index cb36fd5cd..04022839a 100644 --- a/cmd/podman/stop.go +++ b/cmd/podman/stop.go @@ -59,7 +59,13 @@ func stopCmd(c *cli.Context) error { } defer runtime.Shutdown(false) - containers, lastError := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") + containers, err := getAllOrLatestContainers(c, runtime, libpod.ContainerStateRunning, "running") + if err != nil { + if len(containers) == 0 { + return err + } + fmt.Println(err.Error()) + } var stopFuncs []shared.ParallelWorkerInput for _, ctr := range containers { @@ -85,17 +91,6 @@ func stopCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - stopErrors := shared.ParallelExecuteWorkerPool(maxWorkers, stopFuncs) - - for cid, result := range stopErrors { - if result != nil && result != libpod.ErrCtrStopped { - if len(stopErrors) > 1 { - fmt.Println(result.Error()) - } - lastError = result - continue - } - fmt.Println(cid) - } - return lastError + stopErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, stopFuncs) + return printParallelOutput(stopErrors, errCount) } diff --git a/cmd/podman/unpause.go b/cmd/podman/unpause.go index 648fc9d3d..d77e056f8 100644 --- a/cmd/podman/unpause.go +++ b/cmd/podman/unpause.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" "github.com/containers/libpod/cmd/podman/libpodruntime" @@ -37,7 +36,6 @@ var ( func unpauseCmd(c *cli.Context) error { var ( - lastError error unpauseContainers []*libpod.Container unpauseFuncs []shared.ParallelWorkerInput ) @@ -90,18 +88,6 @@ func unpauseCmd(c *cli.Context) error { } logrus.Debugf("Setting maximum workers to %d", maxWorkers) - unpauseErrors := shared.ParallelExecuteWorkerPool(maxWorkers, unpauseFuncs) - - for cid, result := range unpauseErrors { - if result != nil && result != libpod.ErrCtrStopped { - if len(unpauseErrors) > 1 { - fmt.Println(result.Error()) - } - lastError = result - continue - } - fmt.Println(cid) - } - - return lastError + unpauseErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, unpauseFuncs) + return printParallelOutput(unpauseErrors, errCount) } diff --git a/cmd/podman/utils.go b/cmd/podman/utils.go index afeccb668..5735156c2 100644 --- a/cmd/podman/utils.go +++ b/cmd/podman/utils.go @@ -207,3 +207,20 @@ func getPodsFromContext(c *cli.Context, r *libpod.Runtime) ([]*libpod.Pod, error } return pods, lastError } + +//printParallelOutput takes the map of parallel worker results and outputs them +// to stdout +func printParallelOutput(m map[string]error, errCount int) error { + var lastError error + for cid, result := range m { + if result != nil { + if errCount > 1 { + fmt.Println(result.Error()) + } + lastError = result + continue + } + fmt.Println(cid) + } + return lastError +} diff --git a/docs/podman-rm.1.md b/docs/podman-rm.1.md index 7474a0d1f..56664a8c1 100644 --- a/docs/podman-rm.1.md +++ b/docs/podman-rm.1.md @@ -13,7 +13,7 @@ podman\-rm - Remove one or more containers **--force, f** -Force the removal of a running container +Force the removal of a running and paused containers **--all, a** @@ -29,16 +29,29 @@ to run containers such as CRI-O, the last started container could be from either Remove the volumes associated with the container. (Not yet implemented) ## EXAMPLE - +Remove a container by its name *mywebserver* +``` podman rm mywebserver - +``` +Remove several containers by name and container id. +``` podman rm mywebserver myflaskserver 860a4b23 +``` +Forcibly remove a container by container ID. +``` podman rm -f 860a4b23 +``` +Remove all containers regardless of its run state. +``` podman rm -f -a +``` +Forcibly remove the latest container created. +``` podman rm -f --latest +``` ## SEE ALSO podman(1), podman-rmi(1) diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 09dc7c48b..09d0ec042 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -246,7 +246,19 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool) } if c.state.State == ContainerStatePaused { - return errors.Wrapf(ErrCtrStateInvalid, "container %s is paused, cannot remove until unpaused", c.ID()) + if !force { + return errors.Wrapf(ErrCtrStateInvalid, "container %s is paused, cannot remove until unpaused", c.ID()) + } + if err := c.runtime.ociRuntime.killContainer(c, 9); err != nil { + return err + } + if err := c.unpause(); err != nil { + return err + } + // Need to update container state to make sure we know it's stopped + if err := c.waitForExitFileAndSync(); err != nil { + return err + } } // Check that the container's in a good state to be removed diff --git a/test/e2e/pause_test.go b/test/e2e/pause_test.go index 1a2eb1a09..e80915670 100644 --- a/test/e2e/pause_test.go +++ b/test/e2e/pause_test.go @@ -91,7 +91,7 @@ var _ = Describe("Podman pause", func() { }) - It("podman remove a paused container by id", func() { + It("podman remove a paused container by id without force", func() { session := podmanTest.RunTopContainer("") session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -111,25 +111,26 @@ var _ = Describe("Podman pause", func() { Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState)) - result = podmanTest.Podman([]string{"rm", "--force", cid}) - result.WaitWithDefaultTimeout() + }) - Expect(result.ExitCode()).To(Equal(125)) - Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) - Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState)) + It("podman remove a paused container by id with force", func() { + session := podmanTest.RunTopContainer("") + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + cid := session.OutputToString() - result = podmanTest.Podman([]string{"unpause", cid}) + result := podmanTest.Podman([]string{"pause", cid}) result.WaitWithDefaultTimeout() Expect(result.ExitCode()).To(Equal(0)) - Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) + Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState)) result = podmanTest.Podman([]string{"rm", "--force", cid}) result.WaitWithDefaultTimeout() Expect(result.ExitCode()).To(Equal(0)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) - }) It("podman stop a paused container by id", func() { -- cgit v1.2.3-54-g00ecf