From 4a9bd7a18f2db7db103a94287ef34cbd1be1626b Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 16 Feb 2021 05:17:52 -0500 Subject: [PATCH] When stopping a container, print rawInput When we stop a container we are printing the full id, this does not match Docker behaviour or the start behavior. We should be printing the users rawInput when we successfully stop the container. Fixes: https://github.com/containers/podman/issues/9386 Signed-off-by: Daniel J Walsh --- cmd/podman/containers/stop.go | 2 +- pkg/domain/entities/containers.go | 5 +++-- pkg/domain/infra/abi/containers.go | 13 ++++++++++++- pkg/domain/infra/tunnel/containers.go | 11 +++++++++-- pkg/domain/infra/tunnel/helpers.go | 23 +++++++++++++++++------ test/e2e/start_test.go | 17 +++++++++++++++++ test/e2e/stop_test.go | 6 ++---- 7 files changed, 61 insertions(+), 16 deletions(-) diff --git a/cmd/podman/containers/stop.go b/cmd/podman/containers/stop.go index 7338c8d986..1fe41a1bd8 100644 --- a/cmd/podman/containers/stop.go +++ b/cmd/podman/containers/stop.go @@ -115,7 +115,7 @@ func stop(cmd *cobra.Command, args []string) error { } for _, r := range responses { if r.Err == nil { - fmt.Println(r.Id) + fmt.Println(r.RawInput) } else { errs = append(errs, r.Err) } diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index 2d50d68262..b89c42eab0 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -88,8 +88,9 @@ type StopOptions struct { } type StopReport struct { - Err error - Id string //nolint + Err error + Id string //nolint + RawInput string } type TopOptions struct { diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 7a672d8636..9bef97a5c8 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -138,10 +138,16 @@ 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 - ctrs, err := getContainersByContext(options.All, options.Latest, names, ic.Libpod) + ctrs, rawInputs, err := getContainersAndInputByContext(options.All, options.Latest, names, ic.Libpod) if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr) { return nil, err } + ctrMap := map[string]string{} + if len(rawInputs) == len(ctrs) { + for i := range ctrs { + ctrMap[ctrs[i].ID()] = rawInputs[i] + } + } errMap, err := parallelctr.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { var err error if options.Timeout != nil { @@ -174,6 +180,11 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin for ctr, err := range errMap { report := new(entities.StopReport) report.Id = ctr.ID() + if options.All { + report.RawInput = ctr.ID() + } else { + report.RawInput = ctrMap[ctr.ID()] + } report.Err = err reports = append(reports, report) } diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index c2c282ef96..5a83faaf0d 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -82,16 +82,23 @@ 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{} - ctrs, err := getContainersByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds) + ctrs, rawInputs, err := getContainersAndInputByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds) if err != nil { return nil, err } + ctrMap := map[string]string{} + for i := range ctrs { + ctrMap[ctrs[i].ID] = rawInputs[i] + } options := new(containers.StopOptions).WithIgnore(opts.Ignore) if to := opts.Timeout; to != nil { options.WithTimeout(*to) } for _, c := range ctrs { - report := entities.StopReport{Id: c.ID} + report := entities.StopReport{ + Id: c.ID, + RawInput: ctrMap[c.ID], + } if err = containers.Stop(ic.ClientCtx, c.ID, options); err != nil { // These first two are considered non-fatal under the right conditions if errors.Cause(err).Error() == define.ErrCtrStopped.Error() { diff --git a/pkg/domain/infra/tunnel/helpers.go b/pkg/domain/infra/tunnel/helpers.go index e40e275961..5fa70bffa8 100644 --- a/pkg/domain/infra/tunnel/helpers.go +++ b/pkg/domain/infra/tunnel/helpers.go @@ -14,16 +14,26 @@ import ( // FIXME: the `ignore` parameter is very likely wrong here as it should rather // be used on *errors* from operations such as remove. func getContainersByContext(contextWithConnection context.Context, all, ignore bool, namesOrIDs []string) ([]entities.ListContainer, error) { + ctrs, _, err := getContainersAndInputByContext(contextWithConnection, all, ignore, namesOrIDs) + return ctrs, err +} + +func getContainersAndInputByContext(contextWithConnection context.Context, all, ignore bool, namesOrIDs []string) ([]entities.ListContainer, []string, error) { if all && len(namesOrIDs) > 0 { - return nil, errors.New("cannot lookup containers and all") + return nil, nil, errors.New("cannot lookup containers and all") } options := new(containers.ListOptions).WithAll(true).WithSync(true) allContainers, err := containers.List(contextWithConnection, options) if err != nil { - return nil, err + return nil, nil, err } + rawInputs := []string{} if all { - return allContainers, err + for i := range allContainers { + rawInputs = append(rawInputs, allContainers[i].ID) + } + + return allContainers, rawInputs, err } // Note: it would be nicer if the lists endpoint would support that as @@ -42,7 +52,7 @@ func getContainersByContext(contextWithConnection context.Context, all, ignore b if ignore && errorhandling.Contains(err, define.ErrNoSuchCtr) { continue } - return nil, err + return nil, nil, err } // Now we can do a full match of the ID to find the right @@ -52,16 +62,17 @@ func getContainersByContext(contextWithConnection context.Context, all, ignore b for _, ctr := range allContainers { if ctr.ID == inspectData.ID { filtered = append(filtered, ctr) + rawInputs = append(rawInputs, nameOrID) found = true break } } if !found && !ignore { - return nil, errors.Wrapf(define.ErrNoSuchCtr, "unable to find container %q", nameOrID) + return nil, nil, errors.Wrapf(define.ErrNoSuchCtr, "unable to find container %q", nameOrID) } } - return filtered, nil + return filtered, rawInputs, nil } func getPodsByContext(contextWithConnection context.Context, all bool, namesOrIDs []string) ([]*entities.ListPodsReport, error) { diff --git a/test/e2e/start_test.go b/test/e2e/start_test.go index a6f22e0073..74be105d86 100644 --- a/test/e2e/start_test.go +++ b/test/e2e/start_test.go @@ -95,6 +95,23 @@ var _ = Describe("Podman start", func() { Expect(session.OutputToString()).To(Equal(shortID)) }) + It("podman container start single container by short id", func() { + session := podmanTest.Podman([]string{"container", "create", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + cid := session.OutputToString() + shortID := cid[0:10] + session = podmanTest.Podman([]string{"container", "start", shortID}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Equal(shortID)) + + session = podmanTest.Podman([]string{"stop", shortID}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Equal(shortID)) + }) + It("podman start single container by name", func() { name := "foobar99" session := podmanTest.Podman([]string{"create", "--name", name, ALPINE, "ls"}) diff --git a/test/e2e/stop_test.go b/test/e2e/stop_test.go index 750d38ffbb..337fd651dd 100644 --- a/test/e2e/stop_test.go +++ b/test/e2e/stop_test.go @@ -149,13 +149,12 @@ var _ = Describe("Podman stop", func() { session := podmanTest.RunTopContainer("test4") session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) - cid1 := session.OutputToString() session = podmanTest.Podman([]string{"stop", "--time", "1", "test4"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) output := session.OutputToString() - Expect(output).To(ContainSubstring(cid1)) + Expect(output).To(ContainSubstring("test4")) finalCtrs := podmanTest.Podman([]string{"ps", "-q"}) finalCtrs.WaitWithDefaultTimeout() @@ -167,14 +166,13 @@ var _ = Describe("Podman stop", func() { 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"}) // 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)) + Expect(output).To(ContainSubstring("test5")) finalCtrs := podmanTest.Podman([]string{"ps", "-q"}) finalCtrs.WaitWithDefaultTimeout()