diff options
author | Valentin Rothberg <rothberg@redhat.com> | 2020-09-09 16:35:38 +0200 |
---|---|---|
committer | Valentin Rothberg <rothberg@redhat.com> | 2020-09-10 10:06:48 +0200 |
commit | 5c44ebf96e0a90a83cb115fa6fb7013b53dcebd8 (patch) | |
tree | b4919c9c776ece0b8fcbf7d20201fca379c9e623 | |
parent | 6b1a1fcc5cb92a9fd5800b0d1af44f26093a8153 (diff) | |
download | podman-5c44ebf96e0a90a83cb115fa6fb7013b53dcebd8.tar.gz podman-5c44ebf96e0a90a83cb115fa6fb7013b53dcebd8.tar.bz2 podman-5c44ebf96e0a90a83cb115fa6fb7013b53dcebd8.zip |
remote run: consult events for exit code
After attaching to a container, we wait for the container to finish and
return its exit code. Waiting for the container may not always succeed,
for instance, when the container has been force removed by another
process. In such case, we have to look at the *last* container-exit
event.
Also refactor the `ContainerRun` method a bit to return early on errors
and de-spaghetti the code.
Enable the remote-disabled system test.
Fixes: #7117
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
-rw-r--r-- | pkg/domain/infra/tunnel/containers.go | 93 | ||||
-rw-r--r-- | test/system/055-rm.bats | 2 |
2 files changed, 76 insertions, 19 deletions
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index cc919561f..062b38a70 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -8,11 +8,13 @@ import ( "os" "strconv" "strings" + "sync" "time" "github.com/containers/common/pkg/config" "github.com/containers/image/v5/docker/reference" "github.com/containers/podman/v2/libpod/define" + "github.com/containers/podman/v2/libpod/events" "github.com/containers/podman/v2/pkg/api/handlers" "github.com/containers/podman/v2/pkg/bindings" "github.com/containers/podman/v2/pkg/bindings/containers" @@ -507,33 +509,90 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta for _, w := range con.Warnings { fmt.Fprintf(os.Stderr, "%s\n", w) } + report := entities.ContainerRunReport{Id: con.ID} - // Attach - if !opts.Detach { - err = startAndAttach(ic, con.ID, &opts.DetachKeys, opts.InputStream, opts.OutputStream, opts.ErrorStream) - if err == nil { - exitCode, err := containers.Wait(ic.ClientCxt, con.ID, nil) - if err == nil { - report.ExitCode = int(exitCode) - } + + if opts.Detach { + // Detach and return early + err := containers.Start(ic.ClientCxt, con.ID, nil) + if err != nil { + report.ExitCode = define.ExitCode(err) } - } else { - err = containers.Start(ic.ClientCxt, con.ID, nil) + return &report, err } - if err != nil { + + // Attach + if err := startAndAttach(ic, con.ID, &opts.DetachKeys, opts.InputStream, opts.OutputStream, opts.ErrorStream); err != nil { report.ExitCode = define.ExitCode(err) + if opts.Rm { + if rmErr := containers.Remove(ic.ClientCxt, con.ID, bindings.PFalse, bindings.PTrue); rmErr != nil { + logrus.Debugf("unable to remove container %s after failing to start and attach to it", con.ID) + } + } + return &report, err } + if opts.Rm { - if err := containers.Remove(ic.ClientCxt, con.ID, bindings.PFalse, bindings.PTrue); err != nil { - if errors.Cause(err) == define.ErrNoSuchCtr || - errors.Cause(err) == define.ErrCtrRemoved { - logrus.Warnf("Container %s does not exist: %v", con.ID, err) - } else { - logrus.Errorf("Error removing container %s: %v", con.ID, err) + // Defer the removal, so we can return early if needed and + // de-spaghetti the code. + defer func() { + if err := containers.Remove(ic.ClientCxt, con.ID, bindings.PFalse, bindings.PTrue); err != nil { + if errors.Cause(err) == define.ErrNoSuchCtr || + errors.Cause(err) == define.ErrCtrRemoved { + logrus.Warnf("Container %s does not exist: %v", con.ID, err) + } else { + logrus.Errorf("Error removing container %s: %v", con.ID, err) + } } + }() + } + + // Wait + exitCode, waitErr := containers.Wait(ic.ClientCxt, con.ID, nil) + if waitErr == nil { + report.ExitCode = int(exitCode) + return &report, nil + } + + // Determine why the wait failed. If the container doesn't exist, + // consult the events. + if !strings.Contains(waitErr.Error(), define.ErrNoSuchCtr.Error()) { + return &report, waitErr + } + + // Events + eventsChannel := make(chan *events.Event) + eventOptions := entities.EventsOptions{ + EventChan: eventsChannel, + Filter: []string{ + "type=container", + fmt.Sprintf("container=%s", con.ID), + fmt.Sprintf("event=%s", events.Exited), + }, + } + + var lastEvent *events.Event + var mutex sync.Mutex + mutex.Lock() + // Read the events. + go func() { + for e := range eventsChannel { + lastEvent = e } + mutex.Unlock() + }() + + eventsErr := ic.Events(ctx, eventOptions) + + // Wait for all events to be read + mutex.Lock() + if eventsErr != nil || lastEvent == nil { + logrus.Errorf("Cannot get exit code: %v", err) + report.ExitCode = define.ExecErrorCodeNotFound + return &report, nil // compat with local client } + report.ExitCode = lastEvent.ContainerExitCode return &report, err } diff --git a/test/system/055-rm.bats b/test/system/055-rm.bats index 478ba0f20..c8475c3e9 100644 --- a/test/system/055-rm.bats +++ b/test/system/055-rm.bats @@ -44,8 +44,6 @@ load helpers # # See https://github.com/containers/podman/issues/3795 @test "podman rm -f" { - skip_if_remote "FIXME: pending #7117" - rand=$(random_string 30) ( sleep 3; run_podman rm -f $rand ) & run_podman 137 run --name $rand $IMAGE sleep 30 |