opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2026-03-13 08:01:19 +08:00
Code Issues Packages Projects Releases Wiki Activity

exec: retry rm -rf on ENOTEMPTY and EBUSY

Browse Source 
when running on NFS, a RemoveAll could cause EBUSY because of some
unlinked files that are still kept open and "silly renamed" to
.nfs$ID.

This is only half of the fix, as conmon needs to be fixed too.

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2040379
Related: https://github.com/containers/conmon/pull/319

[NO NEW TESTS NEEDED] as it requires NFS as the underlying storage.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This commit is contained in:
Giuseppe Scrivano
2022-01-14 13:15:08 +01:00
parent 75e6994d4e
commit e252b3b4f2
2 changed files with 56 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
libpod/container_exec.go
View File

@@ -14,6 +14,7 @@ import (
"github.com/containers/storage/pkg/stringid"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
)
// ExecConfig contains the configuration of an exec session
@@ -774,13 +775,40 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi
return exitCode, nil
}
// cleanup an exec session after its done
func (c *Container) cleanupExecBundle(sessionID string) error {
if err := os.RemoveAll(c.execBundlePath(sessionID)); err != nil && !os.IsNotExist(err) {
return err
// cleanupExecBundle cleanups an exec session after its done
// Please be careful when using this function since it might temporarily unlock
// the container when os.RemoveAll($bundlePath) fails with ENOTEMPTY or EBUSY
// errors.
func (c *Container) cleanupExecBundle(sessionID string) (Err error) {
path := c.execBundlePath(sessionID)
for attempts := 0; attempts < 50; attempts++ {
Err = os.RemoveAll(path)
if Err == nil || os.IsNotExist(Err) {
return nil
}
if pathErr, ok := Err.(*os.PathError); ok {
Err = pathErr.Err
if errors.Cause(Err) == unix.ENOTEMPTY || errors.Cause(Err) == unix.EBUSY {
// give other processes a chance to use the container
if !c.batched {
if err := c.save(); err != nil {
return err
}
c.lock.Unlock()
}
time.Sleep(time.Millisecond * 100)
if !c.batched {
c.lock.Lock()
if err := c.syncContainer(); err != nil {
return err
}
}
continue
}
}
return
}
return nil
return
}
// the path to a containers exec session bundle

41
libpod/runtime_ctr.go
View File

@@ -653,6 +653,20 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo
}
}
// Check that no other containers depend on the container.
// Only used if not removing a pod - pods guarantee that all
// deps will be evicted at the same time.
if !removePod {
deps, err := r.state.ContainerInUse(c)
if err != nil {
return err
}
if len(deps) != 0 {
depsStr := strings.Join(deps, ", ")
return errors.Wrapf(define.ErrCtrExists, "container %s has dependent containers which must be removed before it: %s", c.ID(), depsStr)
}
}
// Check that the container's in a good state to be removed.
if c.state.State == define.ContainerStateRunning {
time := c.StopTimeout()
@@ -675,25 +689,6 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo
}
}
// Remove all active exec sessions
if err := c.removeAllExecSessions(); err != nil {
return err
}
// Check that no other containers depend on the container.
// Only used if not removing a pod - pods guarantee that all
// deps will be evicted at the same time.
if !removePod {
deps, err := r.state.ContainerInUse(c)
if err != nil {
return err
}
if len(deps) != 0 {
depsStr := strings.Join(deps, ", ")
return errors.Wrapf(define.ErrCtrExists, "container %s has dependent containers which must be removed before it: %s", c.ID(), depsStr)
}
}
var cleanupErr error
// Clean up network namespace, cgroups, mounts.
@@ -713,6 +708,14 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo
return errors.Wrapf(err, "unable to set container %s removing state in database", c.ID())
}
// Remove all active exec sessions
// removing the exec sessions might temporarily unlock the container's lock. Using it
// after setting the state to ContainerStateRemoving will prevent that the container is
// restarted
if err := c.removeAllExecSessions(); err != nil {
return err
}
// Stop the container's storage
if err := c.teardownStorage(); err != nil {
if cleanupErr == nil {