Skip to content

Commit

Permalink
pkg/netns: make UnmountNS() idempotent
Browse files Browse the repository at this point in the history
Podman might call us more than once on the same path. If the path is not
mounted or does not exists simply return no error.

Second, retry the unmount/remove until the unmount succeeded. For some
reason we must use MNT_DETACH as otherwise the unmount call will fail
all time the time. However MNT_DETACH means it unmounts async in the
background. Now if we call remove on the file and the unmount was not
done yet it will fail with EBUSY. In this case we try again until it
works or we get another error.

This should help containers/podman#19721

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
  • Loading branch information
Luap99 committed Aug 8, 2024
1 parent e32811a commit a095edd
Showing 1 changed file with 25 additions and 14 deletions.
39 changes: 25 additions & 14 deletions pkg/netns/netns_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ import (
"runtime"
"strings"
"sync"
"time"

"github.com/containernetworking/plugins/pkg/ns"
"github.com/containers/storage/pkg/homedir"
"github.com/containers/storage/pkg/unshare"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
)

Expand Down Expand Up @@ -177,26 +179,35 @@ func newNSPath(nsPath string) (ns.NetNS, error) {

// UnmountNS unmounts the given netns path
func UnmountNS(nsPath string) error {
var rErr error
// Only unmount if it's been bind-mounted (don't touch namespaces in /proc...)
if !strings.HasPrefix(nsPath, "/proc/") {
if err := unix.Unmount(nsPath, unix.MNT_DETACH); err != nil {
// Do not return here, always try to remove below.
// This is important in case podman now is in a new userns compared to
// when the netns was created. The umount will fail EINVAL but removing
// the file will work and the kernel will destroy the bind mount in the
// other ns because of this. We also need it so pasta doesn't leak.
rErr = fmt.Errorf("failed to unmount NS: at %s: %w", nsPath, err)
// EINVAL means the path exists but is not mounted, just try to remove the path below
if err := unix.Unmount(nsPath, unix.MNT_DETACH); err != nil && !errors.Is(err, unix.EINVAL) {
// If path does not exists we can return without error as we have nothing to do.
if errors.Is(err, unix.ENOENT) {
return nil
}

return fmt.Errorf("failed to unmount NS: at %s: %w", nsPath, err)
}

if err := os.Remove(nsPath); err != nil {
err := fmt.Errorf("failed to remove ns path: %w", err)
if rErr != nil {
err = fmt.Errorf("%v, %w", err, rErr)
for {
if err := os.Remove(nsPath); err != nil {
if errors.Is(err, unix.EBUSY) {
// mount is still busy, sleep a moment and try again to remove
logrus.Debugf("Netns %s still busy, try removing it again in 10ms", nsPath)
time.Sleep(10 * time.Millisecond)
continue
}
// If path does not exists we can return without error.
if errors.Is(err, unix.ENOENT) {
break
}
return fmt.Errorf("failed to remove ns path: %w", err)
}
rErr = err
break
}
}

return rErr
return nil
}

0 comments on commit a095edd

Please sign in to comment.