Skip to content

Commit

Permalink
Drop container does not exist on removal to debugf
Browse files Browse the repository at this point in the history
We have race conditions where a container can be removed
by two different processes when running podman --remove rm.

It can be cleaned up in the API or by the conmon executing
podman container cleanup.

When we fail to remove a container that does not exists we should
not be printing errors or warnings, we should just debug the fact.

[NO TESTS NEEDED] Since this is a race condition it is difficult to
test.

Signed-off-by: Daniel J Walsh <[email protected]>
  • Loading branch information
rhatdan authored and mheon committed May 25, 2021
1 parent 18e917c commit ac8b7dd
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
2 changes: 1 addition & 1 deletion pkg/domain/infra/abi/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string,
case nil:
// remove container names that we successfully deleted
reports = append(reports, &report)
case define.ErrNoSuchCtr:
case define.ErrNoSuchCtr, define.ErrCtrExists:
// There is still a potential this is a libpod container
tmpNames = append(tmpNames, ctr)
default:
Expand Down
13 changes: 8 additions & 5 deletions pkg/domain/infra/tunnel/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/containers/podman/v3/pkg/errorhandling"
"github.com/containers/podman/v3/pkg/specgen"
"github.com/containers/podman/v3/pkg/util"
"github.com/containers/storage/types"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -580,7 +581,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri
if err := containers.Remove(ic.ClientCtx, ctr.ID, removeOptions); err != nil {
if errorhandling.Contains(err, define.ErrNoSuchCtr) ||
errorhandling.Contains(err, define.ErrCtrRemoved) {
logrus.Warnf("Container %s does not exist: %v", ctr.ID, err)
logrus.Debugf("Container %s does not exist: %v", ctr.ID, err)
} else {
logrus.Errorf("Error removing container %s: %v", ctr.ID, err)
}
Expand Down Expand Up @@ -613,8 +614,9 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri
rmOptions := new(containers.RemoveOptions).WithForce(false).WithVolumes(true)
if err := containers.Remove(ic.ClientCtx, ctr.ID, rmOptions); err != nil {
if errorhandling.Contains(err, define.ErrNoSuchCtr) ||
errorhandling.Contains(err, define.ErrCtrRemoved) {
logrus.Warnf("Container %s does not exist: %v", ctr.ID, err)
errorhandling.Contains(err, define.ErrCtrRemoved) ||
errorhandling.Contains(err, types.ErrLayerUnknown) {
logrus.Debugf("Container %s does not exist: %v", ctr.ID, err)
} else {
logrus.Errorf("Error removing container %s: %v", ctr.ID, err)
}
Expand Down Expand Up @@ -691,8 +693,9 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta
if !shouldRestart {
if err := containers.Remove(ic.ClientCtx, con.ID, new(containers.RemoveOptions).WithForce(false).WithVolumes(true)); err != nil {
if errorhandling.Contains(err, define.ErrNoSuchCtr) ||
errorhandling.Contains(err, define.ErrCtrRemoved) {
logrus.Warnf("Container %s does not exist: %v", con.ID, err)
errorhandling.Contains(err, define.ErrCtrRemoved) ||
errorhandling.Contains(err, types.ErrLayerUnknown) {
logrus.Debugf("Container %s does not exist: %v", con.ID, err)
} else {
logrus.Errorf("Error removing container %s: %v", con.ID, err)
}
Expand Down

0 comments on commit ac8b7dd

Please sign in to comment.