From 1cc9731dfad9ec06ab162ce432c82ec4d675e60e Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 5 Jun 2020 16:09:26 -0400 Subject: [PATCH] Add parallel operation to `podman stop` This is the other command that benefits greatly from being run in parallel, due to the potential 15-second timeout for containers that ignore SIGTERM. While we're at it, also clean up how stop timeout is set. This needs to be an optional parameter, so that the value set when the container is created with `--stop-timeout` will be respected. Signed-off-by: Matthew Heon --- cmd/podman/containers/stop.go | 3 +- pkg/domain/entities/containers.go | 2 +- pkg/domain/infra/abi/containers.go | 51 ++++++++++++++------------- pkg/domain/infra/tunnel/containers.go | 2 +- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/cmd/podman/containers/stop.go b/cmd/podman/containers/stop.go index 22c4879611..0f2a91af04 100644 --- a/cmd/podman/containers/stop.go +++ b/cmd/podman/containers/stop.go @@ -85,9 +85,8 @@ func stop(cmd *cobra.Command, args []string) error { var ( errs utils.OutputErrors ) - stopOptions.Timeout = containerConfig.Engine.StopTimeout if cmd.Flag("time").Changed { - stopOptions.Timeout = stopTimeout + stopOptions.Timeout = &stopTimeout } responses, err := registry.ContainerEngine().ContainerStop(context.Background(), args, stopOptions) diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 8d85a9b237..2363e6677d 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -84,7 +84,7 @@ type StopOptions struct { CIDFiles []string Ignore bool Latest bool - Timeout uint + Timeout *uint } type StopReport struct { diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index eb45d4630c..043fcfe7e9 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -162,32 +162,33 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr) { return nil, err } - for _, con := range ctrs { - report := entities.StopReport{Id: con.ID()} - err = con.StopWithTimeout(options.Timeout) - if err != nil { - // These first two are considered non-fatal under the right conditions - if errors.Cause(err) == define.ErrCtrStopped { - logrus.Debugf("Container %s is already stopped", con.ID()) - reports = append(reports, &report) - continue - - } else if options.All && errors.Cause(err) == define.ErrCtrStateInvalid { - logrus.Debugf("Container %s is not running, could not stop", con.ID()) - reports = append(reports, &report) - continue - } - report.Err = err - reports = append(reports, &report) - continue - } else if err := con.Cleanup(ctx); err != nil { - // Only if no error, proceed to cleanup to ensure all - // mounts are removed before we exit. - report.Err = err - reports = append(reports, &report) - continue + errMap, err := parallel.ParallelContainerOp(ctx, ctrs, func(c *libpod.Container) error { + var err error + if options.Timeout != nil { + err = c.StopWithTimeout(*options.Timeout) + } else { + err = c.Stop() } - reports = append(reports, &report) + if err != nil { + switch { + case errors.Cause(err) == define.ErrCtrStopped: + logrus.Debugf("Container %s is already stopped", c.ID()) + case options.All && errors.Cause(err) == define.ErrCtrStateInvalid: + logrus.Debugf("Container %s is not running, could not stop", c.ID()) + default: + return err + } + } + return c.Cleanup(ctx) + }) + if err != nil { + return nil, err + } + for ctr, err := range errMap { + report := new(entities.StopReport) + report.Id = ctr.ID() + report.Err = err + reports = append(reports, report) } return reports, nil } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 36b7bf535b..1981055e24 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -100,7 +100,7 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin } for _, c := range ctrs { report := entities.StopReport{Id: c.ID} - if err = containers.Stop(ic.ClientCxt, c.ID, &options.Timeout); err != nil { + if err = containers.Stop(ic.ClientCxt, c.ID, options.Timeout); err != nil { // These first two are considered non-fatal under the right conditions if errors.Cause(err).Error() == define.ErrCtrStopped.Error() { logrus.Debugf("Container %s is already stopped", c.ID)