podman cp: support copying on tmpfs mounts

Traditionally, the path resolution for containers has been resolved on
the *host*; relative to the container's mount point or relative to
specified bind mounts or volumes.

While this works nicely for non-running containers, it poses a problem
for running ones.  In that case, certain kinds of mounts (e.g., tmpfs)
will not resolve correctly.  A tmpfs is held in memory and hence cannot
be resolved relatively to the container's mount point.  A copy operation
will succeed but the data will not show up inside the container.

To support these kinds of mounts, we need to join the *running*
container's mount namespace (and PID namespace) when copying.

Note that this change implies moving the copy and stat logic into
`libpod` since we need to keep the container locked to avoid race
conditions.  The immediate benefit is that all logic is now inside
`libpod`; the code isn't scattered anymore.

Further note that Docker does not support copying to tmpfs mounts.

Tests have been extended to cover *both* path resolutions for running
and created containers.  New tests have been added to exercise the
tmpfs-mount case.

For the record: Some tests could be improved by using `start -a` instead
of a start-exec sequence.  Unfortunately, `start -a` is flaky in the CI
which forced me to use the more expensive start-exec option.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

cmd/podman/containers/cp.go

@@ -191,6 +191,7 @@ func copyFromContainer(container string, containerPath string, hostPath string)
 		putOptions := buildahCopiah.PutOptions{
 			ChownDirs:     &idPair,
 			ChownFiles:    &idPair,
+			IgnoreDevices: true,
 		}
 		if !containerInfo.IsDir && (!hostInfo.IsDir || hostInfoErr != nil) {
 			// If we're having a file-to-file copy, make sure to

libpod/container.go

@@ -904,6 +904,12 @@ func (c *Container) NamespacePath(linuxNS LinuxNS) (string, error) { //nolint:in
 		}
 	}
 
+	return c.namespacePath(linuxNS)
+}
+
+// namespacePath returns the path of one of the container's namespaces
+// If the container is not running, an error will be returned
+func (c *Container) namespacePath(linuxNS LinuxNS) (string, error) { //nolint:interfacer
 	if c.state.State != define.ContainerStateRunning && c.state.State != define.ContainerStatePaused {
 		return "", errors.Wrapf(define.ErrCtrStopped, "cannot get namespace path unless container %s is running", c.ID())
 	}

libpod/container_api.go

@@ -2,6 +2,7 @@ package libpod
 
 import (
 	"context"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"os"
@@ -349,10 +350,6 @@ func (c *Container) Mount() (string, error) {
 		}
 	}
 
-	if c.state.State == define.ContainerStateRemoving {
-		return "", errors.Wrapf(define.ErrCtrStateInvalid, "cannot mount container %s as it is being removed", c.ID())
-	}
-
 	defer c.newContainerEvent(events.Mount)
 	return c.mount()
 }
@@ -367,7 +364,6 @@ func (c *Container) Unmount(force bool) error {
 			return err
 		}
 	}
-
 	if c.state.Mounted {
 		mounted, err := c.runtime.storageService.MountedContainerImage(c.ID())
 		if err != nil {
@@ -847,31 +843,59 @@ func (c *Container) ShouldRestart(ctx context.Context) bool {
 	return c.shouldRestart()
 }
 
-// ResolvePath resolves the specified path on the root for the container.  The
+// CopyFromArchive copies the contents from the specified tarStream to path
-// root must either be the mounted image of the container or the already
+// *inside* the container.
-// mounted container storage.
+func (c *Container) CopyFromArchive(ctx context.Context, containerPath string, tarStream io.Reader) (func() error, error) {
-//
-// It returns the resolved root and the resolved path.  Note that the path may
-// resolve to the container's mount point or to a volume or bind mount.
-func (c *Container) ResolvePath(ctx context.Context, root string, path string) (string, string, error) {
-	logrus.Debugf("Resolving path %q (root %q) on container %s", path, root, c.ID())
-
-	// Minimal sanity checks.
-	if len(root)*len(path) == 0 {
-		return "", "", errors.Wrapf(define.ErrInternal, "ResolvePath: root (%q) and path (%q) must be non empty", root, path)
-	}
-	if _, err := os.Stat(root); err != nil {
-		return "", "", errors.Wrapf(err, "cannot locate root to resolve path on container %s", c.ID())
-	}
-
 	if !c.batched {
 		c.lock.Lock()
 		defer c.lock.Unlock()
 
 		if err := c.syncContainer(); err != nil {
-			return "", "", err
+			return nil, err
 		}
 	}
 
-	return c.resolvePath(root, path)
+	return c.copyFromArchive(ctx, containerPath, tarStream)
+}
+
+// CopyToArchive copies the contents from the specified path *inside* the
+// container to the tarStream.
+func (c *Container) CopyToArchive(ctx context.Context, containerPath string, tarStream io.Writer) (func() error, error) {
+	if !c.batched {
+		c.lock.Lock()
+		defer c.lock.Unlock()
+
+		if err := c.syncContainer(); err != nil {
+			return nil, err
+		}
+	}
+
+	return c.copyToArchive(ctx, containerPath, tarStream)
+}
+
+// Stat the specified path *inside* the container and return a file info.
+func (c *Container) Stat(ctx context.Context, containerPath string) (*define.FileInfo, error) {
+	if !c.batched {
+		c.lock.Lock()
+		defer c.lock.Unlock()
+
+		if err := c.syncContainer(); err != nil {
+			return nil, err
+		}
+	}
+
+	var mountPoint string
+	var err error
+	if c.state.Mounted {
+		mountPoint = c.state.Mountpoint
+	} else {
+		mountPoint, err = c.mount()
+		if err != nil {
+			return nil, err
+		}
+		defer c.unmount(false)
+	}
+
+	info, _, _, err := c.stat(ctx, mountPoint, containerPath)
+	return info, err
 }

libpod/container_copy_linux.go

@@ -0,0 +1,266 @@
+// +build linux
+
+package libpod
+
+import (
+	"context"
+	"io"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+
+	buildahCopiah "github.com/containers/buildah/copier"
+	"github.com/containers/buildah/pkg/chrootuser"
+	"github.com/containers/buildah/util"
+	"github.com/containers/podman/v3/libpod/define"
+	"github.com/containers/storage"
+	"github.com/containers/storage/pkg/idtools"
+	"github.com/docker/docker/pkg/archive"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
+)
+
+func (c *Container) copyFromArchive(ctx context.Context, path string, reader io.Reader) (func() error, error) {
+	var (
+		mountPoint   string
+		resolvedRoot string
+		resolvedPath string
+		unmount      func()
+		err          error
+	)
+
+	// Make sure that "/" copies the *contents* of the mount point and not
+	// the directory.
+	if path == "/" {
+		path = "/."
+	}
+
+	// Optimization: only mount if the container is not already.
+	if c.state.Mounted {
+		mountPoint = c.state.Mountpoint
+		unmount = func() {}
+	} else {
+		// NOTE: make sure to unmount in error paths.
+		mountPoint, err = c.mount()
+		if err != nil {
+			return nil, err
+		}
+		unmount = func() { c.unmount(false) }
+	}
+
+	if c.state.State == define.ContainerStateRunning {
+		resolvedRoot = "/"
+		resolvedPath = c.pathAbs(path)
+	} else {
+		resolvedRoot, resolvedPath, err = c.resolvePath(mountPoint, path)
+		if err != nil {
+			unmount()
+			return nil, err
+		}
+	}
+
+	decompressed, err := archive.DecompressStream(reader)
+	if err != nil {
+		unmount()
+		return nil, err
+	}
+
+	idMappings, idPair, err := getIDMappingsAndPair(c, mountPoint)
+	if err != nil {
+		decompressed.Close()
+		unmount()
+		return nil, err
+	}
+
+	logrus.Debugf("Container copy *to* %q (resolved: %q) on container %q (ID: %s)", path, resolvedPath, c.Name(), c.ID())
+
+	return func() error {
+		defer unmount()
+		defer decompressed.Close()
+		putOptions := buildahCopiah.PutOptions{
+			UIDMap:     idMappings.UIDMap,
+			GIDMap:     idMappings.GIDMap,
+			ChownDirs:  idPair,
+			ChownFiles: idPair,
+		}
+
+		return c.joinMountAndExec(ctx,
+			func() error {
+				return buildahCopiah.Put(resolvedRoot, resolvedPath, putOptions, decompressed)
+			},
+		)
+	}, nil
+}
+
+func (c *Container) copyToArchive(ctx context.Context, path string, writer io.Writer) (func() error, error) {
+	var (
+		mountPoint string
+		unmount    func()
+		err        error
+	)
+
+	// Optimization: only mount if the container is not already.
+	if c.state.Mounted {
+		mountPoint = c.state.Mountpoint
+		unmount = func() {}
+	} else {
+		// NOTE: make sure to unmount in error paths.
+		mountPoint, err = c.mount()
+		if err != nil {
+			return nil, err
+		}
+		unmount = func() { c.unmount(false) }
+	}
+
+	statInfo, resolvedRoot, resolvedPath, err := c.stat(ctx, mountPoint, path)
+	if err != nil {
+		unmount()
+		return nil, err
+	}
+
+	idMappings, idPair, err := getIDMappingsAndPair(c, mountPoint)
+	if err != nil {
+		unmount()
+		return nil, err
+	}
+
+	logrus.Debugf("Container copy *from* %q (resolved: %q) on container %q (ID: %s)", path, resolvedPath, c.Name(), c.ID())
+
+	return func() error {
+		defer unmount()
+		getOptions := buildahCopiah.GetOptions{
+			// Unless the specified points to ".", we want to copy the base directory.
+			KeepDirectoryNames: statInfo.IsDir && filepath.Base(path) != ".",
+			UIDMap:             idMappings.UIDMap,
+			GIDMap:             idMappings.GIDMap,
+			ChownDirs:          idPair,
+			ChownFiles:         idPair,
+			Excludes:           []string{"dev", "proc", "sys"},
+		}
+		return c.joinMountAndExec(ctx,
+			func() error {
+				return buildahCopiah.Get(resolvedRoot, "", getOptions, []string{resolvedPath}, writer)
+			},
+		)
+	}, nil
+}
+
+// getIDMappingsAndPair returns the ID mappings for the container and the host
+// ID pair.
+func getIDMappingsAndPair(container *Container, containerMount string) (*storage.IDMappingOptions, *idtools.IDPair, error) {
+	user, err := getContainerUser(container, containerMount)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	idMappingOpts, err := container.IDMappings()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	hostUID, hostGID, err := util.GetHostIDs(idtoolsToRuntimeSpec(idMappingOpts.UIDMap), idtoolsToRuntimeSpec(idMappingOpts.GIDMap), user.UID, user.GID)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	idPair := idtools.IDPair{UID: int(hostUID), GID: int(hostGID)}
+	return &idMappingOpts, &idPair, nil
+}
+
+// getContainerUser returns the specs.User of the container.
+func getContainerUser(container *Container, mountPoint string) (specs.User, error) {
+	userspec := container.Config().User
+
+	uid, gid, _, err := chrootuser.GetUser(mountPoint, userspec)
+	u := specs.User{
+		UID:      uid,
+		GID:      gid,
+		Username: userspec,
+	}
+
+	if !strings.Contains(userspec, ":") {
+		groups, err2 := chrootuser.GetAdditionalGroupsForUser(mountPoint, uint64(u.UID))
+		if err2 != nil {
+			if errors.Cause(err2) != chrootuser.ErrNoSuchUser && err == nil {
+				err = err2
+			}
+		} else {
+			u.AdditionalGids = groups
+		}
+	}
+
+	return u, err
+}
+
+// idtoolsToRuntimeSpec converts idtools ID mapping to the one of the runtime spec.
+func idtoolsToRuntimeSpec(idMaps []idtools.IDMap) (convertedIDMap []specs.LinuxIDMapping) {
+	for _, idmap := range idMaps {
+		tempIDMap := specs.LinuxIDMapping{
+			ContainerID: uint32(idmap.ContainerID),
+			HostID:      uint32(idmap.HostID),
+			Size:        uint32(idmap.Size),
+		}
+		convertedIDMap = append(convertedIDMap, tempIDMap)
+	}
+	return convertedIDMap
+}
+
+// joinMountAndExec executes the specified function `f` inside the container's
+// mount and PID namespace.  That allows for having the exact view on the
+// container's file system.
+//
+// Note, if the container is not running `f()` will be executed as is.
+func (c *Container) joinMountAndExec(ctx context.Context, f func() error) error {
+	if c.state.State != define.ContainerStateRunning {
+		return f()
+	}
+
+	// Container's running, so we need to execute `f()` inside its mount NS.
+	errChan := make(chan error)
+	go func() {
+		runtime.LockOSThread()
+
+		// Join the mount and PID NS of the container.
+		getFD := func(ns LinuxNS) (*os.File, error) {
+			nsPath, err := c.namespacePath(ns)
+			if err != nil {
+				return nil, err
+			}
+			return os.Open(nsPath)
+		}
+
+		mountFD, err := getFD(MountNS)
+		if err != nil {
+			errChan <- err
+			return
+		}
+		defer mountFD.Close()
+
+		pidFD, err := getFD(PIDNS)
+		if err != nil {
+			errChan <- err
+			return
+		}
+		defer pidFD.Close()
+		if err := unix.Unshare(unix.CLONE_NEWNS); err != nil {
+			errChan <- err
+			return
+		}
+		if err := unix.Setns(int(pidFD.Fd()), unix.CLONE_NEWPID); err != nil {
+			errChan <- err
+			return
+		}
+
+		if err := unix.Setns(int(mountFD.Fd()), unix.CLONE_NEWNS); err != nil {
+			errChan <- err
+			return
+		}
+
+		// Last but not least, execute the workload.
+		errChan <- f()
+	}()
+	return <-errChan
+}

libpod/container_copy_unsupported.go

@@ -0,0 +1,16 @@
+// +build !linux
+
+package libpod
+
+import (
+	"context"
+	"io"
+)
+
+func (c *Container) copyFromArchive(ctx context.Context, path string, reader io.Reader) (func() error, error) {
+	return nil, nil
+}
+
+func (c *Container) copyToArchive(ctx context.Context, path string, writer io.Writer) (func() error, error) {
+	return nil, nil
+}

libpod/container_internal.go

@@ -2086,6 +2086,10 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[s
 
 // mount mounts the container's root filesystem
 func (c *Container) mount() (string, error) {
+	if c.state.State == define.ContainerStateRemoving {
+		return "", errors.Wrapf(define.ErrCtrStateInvalid, "cannot mount container %s as it is being removed", c.ID())
+	}
+
 	mountPoint, err := c.runtime.storageService.MountContainerImage(c.ID())
 	if err != nil {
 		return "", errors.Wrapf(err, "error mounting storage for container %s", c.ID())

libpod/container_path_resolution.go

@@ -1,3 +1,4 @@
+// +linux
 package libpod
 
 import (
@@ -10,6 +11,19 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// pathAbs returns an absolute path.  If the specified path is
+// relative, it will be resolved relative to the container's working dir.
+func (c *Container) pathAbs(path string) string {
+	if !filepath.IsAbs(path) {
+		// If the containerPath is not absolute, it's relative to the
+		// container's working dir.  To be extra careful, let's first
+		// join the working dir with "/", and the add the containerPath
+		// to it.
+		path = filepath.Join(filepath.Join("/", c.WorkingDir()), path)
+	}
+	return path
+}
+
 // resolveContainerPaths resolves the container's mount point and the container
 // path as specified by the user.  Both may resolve to paths outside of the
 // container's mount point when the container path hits a volume or bind mount.
@@ -20,14 +34,7 @@ import (
 // the host).
 func (c *Container) resolvePath(mountPoint string, containerPath string) (string, string, error) {
 	// Let's first make sure we have a path relative to the mount point.
-	pathRelativeToContainerMountPoint := containerPath
+	pathRelativeToContainerMountPoint := c.pathAbs(containerPath)
-	if !filepath.IsAbs(containerPath) {
-		// If the containerPath is not absolute, it's relative to the
-		// container's working dir.  To be extra careful, let's first
-		// join the working dir with "/", and the add the containerPath
-		// to it.
-		pathRelativeToContainerMountPoint = filepath.Join(filepath.Join("/", c.WorkingDir()), containerPath)
-	}
 	resolvedPathOnTheContainerMountPoint := filepath.Join(mountPoint, pathRelativeToContainerMountPoint)
 	pathRelativeToContainerMountPoint = strings.TrimPrefix(pathRelativeToContainerMountPoint, mountPoint)
 	pathRelativeToContainerMountPoint = filepath.Join("/", pathRelativeToContainerMountPoint)

libpod/container_stat_linux.go

@@ -0,0 +1,157 @@
+// +build linux
+
+package libpod
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/containers/buildah/copier"
+	"github.com/containers/podman/v3/libpod/define"
+	"github.com/containers/podman/v3/pkg/copy"
+	"github.com/pkg/errors"
+)
+
+// statInsideMount stats the specified path *inside* the container's mount and PID
+// namespace.  It returns the file info along with the resolved root ("/") and
+// the resolved path (relative to the root).
+func (c *Container) statInsideMount(ctx context.Context, containerPath string) (*copier.StatForItem, string, string, error) {
+	resolvedRoot := "/"
+	resolvedPath := c.pathAbs(containerPath)
+	var statInfo *copier.StatForItem
+
+	err := c.joinMountAndExec(ctx,
+		func() error {
+			var statErr error
+			statInfo, statErr = secureStat(resolvedRoot, resolvedPath)
+			return statErr
+		},
+	)
+
+	return statInfo, resolvedRoot, resolvedPath, err
+}
+
+// statOnHost stats the specified path *on the host*.  It returns the file info
+// along with the resolved root and the resolved path.  Both paths are absolute
+// to the host's root.  Note that the paths may resolved outside the
+// container's mount point (e.g., to a volume or bind mount).
+func (c *Container) statOnHost(ctx context.Context, mountPoint string, containerPath string) (*copier.StatForItem, string, string, error) {
+	// Now resolve the container's path.  It may hit a volume, it may hit a
+	// bind mount, it may be relative.
+	resolvedRoot, resolvedPath, err := c.resolvePath(mountPoint, containerPath)
+	if err != nil {
+		return nil, "", "", err
+	}
+
+	statInfo, err := secureStat(resolvedRoot, resolvedPath)
+	return statInfo, resolvedRoot, resolvedPath, err
+}
+
+func (c *Container) stat(ctx context.Context, containerMountPoint string, containerPath string) (*define.FileInfo, string, string, error) {
+	var (
+		resolvedRoot     string
+		resolvedPath     string
+		absContainerPath string
+		statInfo         *copier.StatForItem
+		statErr          error
+	)
+
+	// Make sure that "/" copies the *contents* of the mount point and not
+	// the directory.
+	if containerPath == "/" {
+		containerPath = "/."
+	}
+
+	if c.state.State == define.ContainerStateRunning {
+		// If the container is running, we need to join it's mount namespace
+		// and stat there.
+		statInfo, resolvedRoot, resolvedPath, statErr = c.statInsideMount(ctx, containerPath)
+	} else {
+		// If the container is NOT running, we need to resolve the path
+		// on the host.
+		statInfo, resolvedRoot, resolvedPath, statErr = c.statOnHost(ctx, containerMountPoint, containerPath)
+	}
+
+	if statErr != nil {
+		if statInfo == nil {
+			return nil, "", "", statErr
+		}
+		// Not all errors from secureStat map to ErrNotExist, so we
+		// have to look into the error string.  Turning it into an
+		// ENOENT let's the API handlers return the correct status code
+		// which is crucial for the remote client.
+		if os.IsNotExist(statErr) || strings.Contains(statErr.Error(), "o such file or directory") {
+			statErr = copy.ErrENOENT
+		}
+	}
+
+	if statInfo.IsSymlink {
+		// Evaluated symlinks are always relative to the container's mount point.
+		absContainerPath = statInfo.ImmediateTarget
+	} else if strings.HasPrefix(resolvedPath, containerMountPoint) {
+		// If the path is on the container's mount point, strip it off.
+		absContainerPath = strings.TrimPrefix(resolvedPath, containerMountPoint)
+		absContainerPath = filepath.Join("/", absContainerPath)
+	} else {
+		// No symlink and not on the container's mount point, so let's
+		// move it back to the original input.  It must have evaluated
+		// to a volume or bind mount but we cannot return host paths.
+		absContainerPath = containerPath
+	}
+
+	// Preserve the base path as specified by the user.  The `filepath`
+	// packages likes to remove trailing slashes and dots that are crucial
+	// to the copy logic.
+	absContainerPath = copy.PreserveBasePath(containerPath, absContainerPath)
+	resolvedPath = copy.PreserveBasePath(containerPath, resolvedPath)
+
+	info := &define.FileInfo{
+		IsDir:      statInfo.IsDir,
+		Name:       filepath.Base(absContainerPath),
+		Size:       statInfo.Size,
+		Mode:       statInfo.Mode,
+		ModTime:    statInfo.ModTime,
+		LinkTarget: absContainerPath,
+	}
+
+	return info, resolvedRoot, resolvedPath, statErr
+}
+
+// secureStat extracts file info for path in a chroot'ed environment in root.
+func secureStat(root string, path string) (*copier.StatForItem, error) {
+	var glob string
+	var err error
+
+	// If root and path are equal, then dir must be empty and the glob must
+	// be ".".
+	if filepath.Clean(root) == filepath.Clean(path) {
+		glob = "."
+	} else {
+		glob, err = filepath.Rel(root, path)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	globStats, err := copier.Stat(root, "", copier.StatOptions{}, []string{glob})
+	if err != nil {
+		return nil, err
+	}
+
+	if len(globStats) != 1 {
+		return nil, errors.Errorf("internal error: secureStat: expected 1 item but got %d", len(globStats))
+	}
+
+	stat, exists := globStats[0].Results[glob] // only one glob passed, so that's okay
+	if !exists {
+		return nil, copy.ErrENOENT
+	}
+
+	var statErr error
+	if stat.Error != "" {
+		statErr = errors.New(stat.Error)
+	}
+	return stat, statErr
+}

libpod/container_stat_unsupported.go

@@ -0,0 +1,13 @@
+// +build !linux
+
+package libpod
+
+import (
+	"context"
+
+	"github.com/containers/podman/v3/libpod/define"
+)
+
+func (c *Container) stat(ctx context.Context, containerMountPoint string, containerPath string) (*define.FileInfo, string, string, error) {
+	return nil, "", "", nil
+}

libpod/define/fileinfo.go

@@ -0,0 +1,16 @@
+package define
+
+import (
+	"os"
+	"time"
+)
+
+// FileInfo describes the attributes of a file or diretory.
+type FileInfo struct {
+	Name       string      `json:"name"`
+	Size       int64       `json:"size"`
+	Mode       os.FileMode `json:"mode"`
+	ModTime    time.Time   `json:"mtime"`
+	IsDir      bool        `json:"isDir"`
+	LinkTarget string      `json:"linkTarget"`
+}

pkg/copy/fileinfo.go

@@ -7,8 +7,8 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"time"
 
+	"github.com/containers/podman/v3/libpod/define"
 	"github.com/pkg/errors"
 )
 
@@ -22,14 +22,7 @@ var ErrENOENT = errors.New("No such file or directory")
 
 // FileInfo describes a file or directory and is returned by
 // (*CopyItem).Stat().
-type FileInfo struct {
+type FileInfo = define.FileInfo
-	Name       string      `json:"name"`
-	Size       int64       `json:"size"`
-	Mode       os.FileMode `json:"mode"`
-	ModTime    time.Time   `json:"mtime"`
-	IsDir      bool        `json:"isDir"`
-	LinkTarget string      `json:"linkTarget"`
-}
 
 // EncodeFileInfo serializes the specified FileInfo as a base64 encoded JSON
 // payload.  Intended for Docker compat.

pkg/domain/entities/containers.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/containers/image/v5/types"
 	"github.com/containers/podman/v3/libpod/define"
-	"github.com/containers/podman/v3/pkg/copy"
 	"github.com/containers/podman/v3/pkg/specgen"
 	"github.com/cri-o/ocicni/pkg/ocicni"
 )
@@ -145,7 +144,7 @@ type ContainerInspectReport struct {
 }
 
 type ContainerStatReport struct {
-	copy.FileInfo
+	define.FileInfo
 }
 
 type CommitOptions struct {

pkg/domain/infra/abi/archive.go

@@ -3,72 +3,16 @@ package abi
 import (
 	"context"
 	"io"
-	"path/filepath"
-	"strings"
 
-	buildahCopiah "github.com/containers/buildah/copier"
-	"github.com/containers/buildah/pkg/chrootuser"
-	"github.com/containers/buildah/util"
-	"github.com/containers/podman/v3/libpod"
 	"github.com/containers/podman/v3/pkg/domain/entities"
-	"github.com/containers/storage"
-	"github.com/containers/storage/pkg/archive"
-	"github.com/containers/storage/pkg/idtools"
-	"github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
-// NOTE: Only the parent directory of the container path must exist.  The path
-// itself may be created while copying.
 func (ic *ContainerEngine) ContainerCopyFromArchive(ctx context.Context, nameOrID string, containerPath string, reader io.Reader) (entities.ContainerCopyFunc, error) {
 	container, err := ic.Libpod.LookupContainer(nameOrID)
 	if err != nil {
 		return nil, err
 	}
-
+	return container.CopyFromArchive(ctx, containerPath, reader)
-	containerMountPoint, err := container.Mount()
-	if err != nil {
-		return nil, err
-	}
-
-	unmount := func() {
-		if err := container.Unmount(false); err != nil {
-			logrus.Errorf("Error unmounting container: %v", err)
-		}
-	}
-
-	_, resolvedRoot, resolvedContainerPath, err := ic.containerStat(container, containerMountPoint, containerPath)
-	if err != nil {
-		unmount()
-		return nil, err
-	}
-
-	decompressed, err := archive.DecompressStream(reader)
-	if err != nil {
-		unmount()
-		return nil, err
-	}
-
-	idMappings, idPair, err := getIDMappingsAndPair(container, resolvedRoot)
-	if err != nil {
-		unmount()
-		return nil, err
-	}
-
-	logrus.Debugf("Container copy *to* %q (resolved: %q) on container %q (ID: %s)", containerPath, resolvedContainerPath, container.Name(), container.ID())
-
-	return func() error {
-		defer unmount()
-		defer decompressed.Close()
-		putOptions := buildahCopiah.PutOptions{
-			UIDMap:     idMappings.UIDMap,
-			GIDMap:     idMappings.GIDMap,
-			ChownDirs:  idPair,
-			ChownFiles: idPair,
-		}
-		return buildahCopiah.Put(resolvedRoot, resolvedContainerPath, putOptions, decompressed)
-	}, nil
 }
 
 func (ic *ContainerEngine) ContainerCopyToArchive(ctx context.Context, nameOrID string, containerPath string, writer io.Writer) (entities.ContainerCopyFunc, error) {
@@ -76,108 +20,5 @@ func (ic *ContainerEngine) ContainerCopyToArchive(ctx context.Context, nameOrID
 	if err != nil {
 		return nil, err
 	}
-
+	return container.CopyToArchive(ctx, containerPath, writer)
-	containerMountPoint, err := container.Mount()
-	if err != nil {
-		return nil, err
-	}
-
-	unmount := func() {
-		if err := container.Unmount(false); err != nil {
-			logrus.Errorf("Error unmounting container: %v", err)
-		}
-	}
-
-	// Make sure that "/" copies the *contents* of the mount point and not
-	// the directory.
-	if containerPath == "/" {
-		containerPath = "/."
-	}
-
-	statInfo, resolvedRoot, resolvedContainerPath, err := ic.containerStat(container, containerMountPoint, containerPath)
-	if err != nil {
-		unmount()
-		return nil, err
-	}
-
-	idMappings, idPair, err := getIDMappingsAndPair(container, resolvedRoot)
-	if err != nil {
-		unmount()
-		return nil, err
-	}
-
-	logrus.Debugf("Container copy *from* %q (resolved: %q) on container %q (ID: %s)", containerPath, resolvedContainerPath, container.Name(), container.ID())
-
-	return func() error {
-		defer container.Unmount(false)
-		getOptions := buildahCopiah.GetOptions{
-			// Unless the specified points to ".", we want to copy the base directory.
-			KeepDirectoryNames: statInfo.IsDir && filepath.Base(containerPath) != ".",
-			UIDMap:             idMappings.UIDMap,
-			GIDMap:             idMappings.GIDMap,
-			ChownDirs:          idPair,
-			ChownFiles:         idPair,
-		}
-		return buildahCopiah.Get(resolvedRoot, "", getOptions, []string{resolvedContainerPath}, writer)
-	}, nil
-}
-
-// getIDMappingsAndPair returns the ID mappings for the container and the host
-// ID pair.
-func getIDMappingsAndPair(container *libpod.Container, containerMount string) (*storage.IDMappingOptions, *idtools.IDPair, error) {
-	user, err := getContainerUser(container, containerMount)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	idMappingOpts, err := container.IDMappings()
-	if err != nil {
-		return nil, nil, err
-	}
-
-	hostUID, hostGID, err := util.GetHostIDs(idtoolsToRuntimeSpec(idMappingOpts.UIDMap), idtoolsToRuntimeSpec(idMappingOpts.GIDMap), user.UID, user.GID)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	idPair := idtools.IDPair{UID: int(hostUID), GID: int(hostGID)}
-	return &idMappingOpts, &idPair, nil
-}
-
-// getContainerUser returns the specs.User of the container.
-func getContainerUser(container *libpod.Container, mountPoint string) (specs.User, error) {
-	userspec := container.Config().User
-
-	uid, gid, _, err := chrootuser.GetUser(mountPoint, userspec)
-	u := specs.User{
-		UID:      uid,
-		GID:      gid,
-		Username: userspec,
-	}
-
-	if !strings.Contains(userspec, ":") {
-		groups, err2 := chrootuser.GetAdditionalGroupsForUser(mountPoint, uint64(u.UID))
-		if err2 != nil {
-			if errors.Cause(err2) != chrootuser.ErrNoSuchUser && err == nil {
-				err = err2
-			}
-		} else {
-			u.AdditionalGids = groups
-		}
-	}
-
-	return u, err
-}
-
-// idtoolsToRuntimeSpec converts idtools ID mapping to the one of the runtime spec.
-func idtoolsToRuntimeSpec(idMaps []idtools.IDMap) (convertedIDMap []specs.LinuxIDMapping) {
-	for _, idmap := range idMaps {
-		tempIDMap := specs.LinuxIDMapping{
-			ContainerID: uint32(idmap.ContainerID),
-			HostID:      uint32(idmap.HostID),
-			Size:        uint32(idmap.Size),
-		}
-		convertedIDMap = append(convertedIDMap, tempIDMap)
-	}
-	return convertedIDMap
 }

pkg/domain/infra/abi/containers_stat.go

@@ -2,139 +2,20 @@ package abi
 
 import (
 	"context"
-	"os"
-	"path/filepath"
-	"strings"
 
-	buildahCopiah "github.com/containers/buildah/copier"
-	"github.com/containers/podman/v3/libpod"
-	"github.com/containers/podman/v3/pkg/copy"
 	"github.com/containers/podman/v3/pkg/domain/entities"
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
-func (ic *ContainerEngine) containerStat(container *libpod.Container, containerMountPoint string, containerPath string) (*entities.ContainerStatReport, string, string, error) {
-	// Make sure that "/" copies the *contents* of the mount point and not
-	// the directory.
-	if containerPath == "/" {
-		containerPath += "/."
-	}
-
-	// Now resolve the container's path.  It may hit a volume, it may hit a
-	// bind mount, it may be relative.
-	resolvedRoot, resolvedContainerPath, err := container.ResolvePath(context.Background(), containerMountPoint, containerPath)
-	if err != nil {
-		return nil, "", "", err
-	}
-
-	statInfo, statInfoErr := secureStat(resolvedRoot, resolvedContainerPath)
-	if statInfoErr != nil {
-		// Not all errors from secureStat map to ErrNotExist, so we
-		// have to look into the error string.  Turning it into an
-		// ENOENT let's the API handlers return the correct status code
-		// which is crucial for the remote client.
-		if os.IsNotExist(err) || strings.Contains(statInfoErr.Error(), "o such file or directory") {
-			statInfoErr = copy.ErrENOENT
-		}
-		//  If statInfo is nil, there's nothing we can do anymore.  A
-		//  non-nil statInfo may indicate a symlink where we must have
-		//  a closer look.
-		if statInfo == nil {
-			return nil, "", "", statInfoErr
-		}
-	}
-
-	// Now make sure that the info's LinkTarget is relative to the
-	// container's mount.
-	var absContainerPath string
-
-	if statInfo.IsSymlink {
-		// Evaluated symlinks are always relative to the container's mount point.
-		absContainerPath = statInfo.ImmediateTarget
-	} else if strings.HasPrefix(resolvedContainerPath, containerMountPoint) {
-		// If the path is on the container's mount point, strip it off.
-		absContainerPath = strings.TrimPrefix(resolvedContainerPath, containerMountPoint)
-		absContainerPath = filepath.Join("/", absContainerPath)
-	} else {
-		// No symlink and not on the container's mount point, so let's
-		// move it back to the original input.  It must have evaluated
-		// to a volume or bind mount but we cannot return host paths.
-		absContainerPath = containerPath
-	}
-
-	// Now we need to make sure to preserve the base path as specified by
-	// the user.  The `filepath` packages likes to remove trailing slashes
-	// and dots that are crucial to the copy logic.
-	absContainerPath = copy.PreserveBasePath(containerPath, absContainerPath)
-	resolvedContainerPath = copy.PreserveBasePath(containerPath, resolvedContainerPath)
-
-	info := copy.FileInfo{
-		IsDir:      statInfo.IsDir,
-		Name:       filepath.Base(absContainerPath),
-		Size:       statInfo.Size,
-		Mode:       statInfo.Mode,
-		ModTime:    statInfo.ModTime,
-		LinkTarget: absContainerPath,
-	}
-
-	return &entities.ContainerStatReport{FileInfo: info}, resolvedRoot, resolvedContainerPath, statInfoErr
-}
-
 func (ic *ContainerEngine) ContainerStat(ctx context.Context, nameOrID string, containerPath string) (*entities.ContainerStatReport, error) {
 	container, err := ic.Libpod.LookupContainer(nameOrID)
 	if err != nil {
 		return nil, err
 	}
 
-	containerMountPoint, err := container.Mount()
+	info, err := container.Stat(ctx, containerPath)
-	if err != nil {
+
+	if info != nil {
+		return &entities.ContainerStatReport{FileInfo: *info}, err
+	}
 	return nil, err
-	}
-
-	defer func() {
-		if err := container.Unmount(false); err != nil {
-			logrus.Errorf("Error unmounting container: %v", err)
-		}
-	}()
-
-	statReport, _, _, err := ic.containerStat(container, containerMountPoint, containerPath)
-	return statReport, err
-}
-
-// secureStat extracts file info for path in a chroot'ed environment in root.
-func secureStat(root string, path string) (*buildahCopiah.StatForItem, error) {
-	var glob string
-	var err error
-
-	// If root and path are equal, then dir must be empty and the glob must
-	// be ".".
-	if filepath.Clean(root) == filepath.Clean(path) {
-		glob = "."
-	} else {
-		glob, err = filepath.Rel(root, path)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	globStats, err := buildahCopiah.Stat(root, "", buildahCopiah.StatOptions{}, []string{glob})
-	if err != nil {
-		return nil, err
-	}
-
-	if len(globStats) != 1 {
-		return nil, errors.Errorf("internal error: secureStat: expected 1 item but got %d", len(globStats))
-	}
-
-	stat, exists := globStats[0].Results[glob] // only one glob passed, so that's okay
-	if !exists {
-		return nil, copy.ErrENOENT
-	}
-
-	var statErr error
-	if stat.Error != "" {
-		statErr = errors.New(stat.Error)
-	}
-	return stat, statErr
 }

test/e2e/cp_test.go

@@ -212,6 +212,7 @@ var _ = Describe("Podman cp", func() {
 
 	// Copy the root dir "/" of a container to the host.
 	It("podman cp the root directory from the ctr to an existing directory on the host ", func() {
+		SkipIfRootless("cannot copy tty devices in rootless mode")
 		container := "copyroottohost"
 		session := podmanTest.RunTopContainer(container)
 		session.WaitWithDefaultTimeout()

test/system/065-cp.bats

@@ -15,6 +15,7 @@ load helpers
         random-1-$(random_string 15)
         random-2-$(random_string 20)
     )
+
     echo "${randomcontent[0]}" > $srcdir/hostfile0
     echo "${randomcontent[1]}" > $srcdir/hostfile1
     echo "${randomcontent[2]}" > $srcdir/hostfile2
@@ -24,6 +25,10 @@ load helpers
     run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity
     run_podman exec cpcontainer mkdir /srv/subdir
 
+    # Commit the image for testing non-running containers
+    run_podman commit -q cpcontainer
+    cpimage="$output"
+
     # format is: <id> | <destination arg to cp> | <full dest path> | <test name>
     # where:
     #    id        is 0-2, one of the random strings/files
@@ -44,8 +49,7 @@ load helpers
 0 | subdir               | /srv/subdir/hostfile0 | copy to workdir/subdir
 "
 
-    # Copy one of the files into container, exec+cat, confirm the file
+    # RUNNING container
-    # is there and matches what we expect
     while read id dest dest_fullname description; do
         run_podman cp $srcdir/hostfile$id cpcontainer:$dest
         run_podman exec cpcontainer cat $dest_fullname
@@ -67,6 +71,44 @@ load helpers
     is "$output" 'Error: "/IdoNotExist/" could not be found on container cpcontainer: No such file or directory' \
        "copy into nonexistent path in container"
 
+    run_podman kill cpcontainer
+    run_podman rm -f cpcontainer
+
+    # CREATED container
+    while read id dest dest_fullname description; do
+        run_podman create --name cpcontainer --workdir=/srv $cpimage sleep infinity
+        run_podman cp $srcdir/hostfile$id cpcontainer:$dest
+        run_podman start cpcontainer
+        run_podman exec cpcontainer cat $dest_fullname
+        is "$output" "${randomcontent[$id]}" "$description (cp -> ctr:$dest)"
+        run_podman kill cpcontainer
+        run_podman rm -f cpcontainer
+    done < <(parse_table "$tests")
+
+    run_podman rmi -f $cpimage
+}
+
+@test "podman cp file from host to container tmpfs mount" {
+    srcdir=$PODMAN_TMPDIR/cp-test-file-host-to-ctr
+    mkdir -p $srcdir
+    content=tmpfile-content$(random_string 20)
+    echo $content > $srcdir/file
+
+    # RUNNING container
+    run_podman run -d --mount type=tmpfs,dst=/tmp --name cpcontainer $IMAGE sleep infinity
+    run_podman cp $srcdir/file cpcontainer:/tmp
+    run_podman exec cpcontainer cat /tmp/file
+    is "$output" "${content}" "cp to running container's tmpfs"
+    run_podman kill cpcontainer
+    run_podman rm -f cpcontainer
+
+    # CREATED container (with copy up)
+    run_podman create --mount type=tmpfs,dst=/tmp --name cpcontainer $IMAGE sleep infinity
+    run_podman cp $srcdir/file cpcontainer:/tmp
+    run_podman start cpcontainer
+    run_podman exec cpcontainer cat /tmp/file
+    is "$output" "${content}" "cp to created container's tmpfs"
+    run_podman kill cpcontainer
     run_podman rm -f cpcontainer
 }
 
@@ -87,6 +129,10 @@ load helpers
     run_podman exec cpcontainer sh -c "echo ${randomcontent[1]} > /srv/containerfile1"
     run_podman exec cpcontainer sh -c "mkdir /srv/subdir; echo ${randomcontent[2]} > /srv/subdir/containerfile2"
 
+    # Commit the image for testing non-running containers
+    run_podman commit -q cpcontainer
+    cpimage="$output"
+
     # format is: <id> | <source arg to cp> | <destination arg (appended to $srcdir) to cp> | <full dest path (appended to $srcdir)> | <test name>
     tests="
 0 | /tmp/containerfile    |          | /containerfile  | copy to srcdir/
@@ -98,20 +144,33 @@ load helpers
 2 | subdir/containerfile2 | /        | /containerfile2 | copy from workdir/subdir (rel path) to srcdir
 "
 
-    # Copy one of the files to the host, cat, confirm the file
+    # RUNNING container
-    # is there and matches what we expect
     while read id src dest dest_fullname description; do
         # dest may be "''" for empty table cells
         if [[ $dest == "''" ]];then
             unset dest
         fi
         run_podman cp cpcontainer:$src "$srcdir$dest"
-        run cat $srcdir$dest_fullname
+        is "$(< $srcdir$dest_fullname)" "${randomcontent[$id]}" "$description (cp ctr:$src to \$srcdir$dest)"
-        is "$output" "${randomcontent[$id]}" "$description (cp ctr:$src to \$srcdir$dest)"
+        rm $srcdir$dest_fullname
-        rm $srcdir/$dest_fullname
     done < <(parse_table "$tests")
-
+    run_podman kill cpcontainer
     run_podman rm -f cpcontainer
+
+    # Created container
+    run_podman create --name cpcontainer --workdir=/srv $cpimage
+    while read id src dest dest_fullname description; do
+        # dest may be "''" for empty table cells
+        if [[ $dest == "''" ]];then
+            unset dest
+        fi
+        run_podman cp cpcontainer:$src "$srcdir$dest"
+        is "$(< $srcdir$dest_fullname)" "${randomcontent[$id]}" "$description (cp ctr:$src to \$srcdir$dest)"
+        rm $srcdir$dest_fullname
+    done < <(parse_table "$tests")
+    run_podman rm -f cpcontainer
+
+    run_podman rmi -f $cpimage
 }
 
 
@@ -134,6 +193,10 @@ load helpers
     run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity
     run_podman exec cpcontainer mkdir /srv/subdir
 
+    # Commit the image for testing non-running containers
+    run_podman commit -q cpcontainer
+    cpimage="$output"
+
     # format is: <source arg to cp (appended to srcdir)> | <destination arg to cp> | <full dest path> | <test name>
     tests="
     | /        | /dir-test             | copy to root
@@ -144,6 +207,7 @@ load helpers
     | subdir/. | /srv/subdir/dir-test  | copy to workdir subdir (rel path)
 "
 
+    # RUNNING container
     while read src dest dest_fullname description; do
         # src may be "''" for empty table cells
         if [[ $src == "''" ]];then
@@ -156,52 +220,97 @@ load helpers
         run_podman exec cpcontainer cat $dest_fullname/hostfile1
         is "$output" "${randomcontent[1]}" "$description (cp -> ctr:$dest)"
     done < <(parse_table "$tests")
-
+    run_podman kill cpcontainer
     run_podman rm -f cpcontainer
+
+    # CREATED container
+    while read src dest dest_fullname description; do
+        # src may be "''" for empty table cells
+        if [[ $src == "''" ]];then
+            unset src
+        fi
+        run_podman create --name cpcontainer --workdir=/srv $cpimage sleep infinity
+        run_podman cp $srcdir$src cpcontainer:$dest
+        run_podman start cpcontainer
+        run_podman exec cpcontainer cat $dest_fullname/hostfile0 $dest_fullname/hostfile1
+        is "${lines[0]}" "${randomcontent[0]}" "$description (cp -> ctr:$dest)"
+        is "${lines[1]}" "${randomcontent[1]}" "$description (cp -> ctr:$dest)"
+        run_podman kill cpcontainer
+        run_podman rm -f cpcontainer
+    done < <(parse_table "$tests")
+
+    run_podman rmi -f $cpimage
 }
 
 
 @test "podman cp dir from container to host" {
-    srcdir=$PODMAN_TMPDIR/dir-test
+    destdir=$PODMAN_TMPDIR/cp-test-dir-ctr-to-host
-    mkdir -p $srcdir
+    mkdir -p $destdir
 
+    # Create 2 files with random content in the container.
+    local -a randomcontent=(
+        random-0-$(random_string 10)
+        random-1-$(random_string 15)
+    )
     run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity
-    run_podman exec cpcontainer sh -c 'mkdir /srv/subdir; echo "This first file is on the container" > /srv/subdir/containerfile1'
+    run_podman exec cpcontainer sh -c "mkdir /srv/subdir; echo ${randomcontent[0]} > /srv/subdir/containerfile0"
-    run_podman exec cpcontainer sh -c 'echo "This second file is on the container as well" > /srv/subdir/containerfile2'
+    run_podman exec cpcontainer sh -c "echo ${randomcontent[1]} > /srv/subdir/containerfile1"
     # "." and "dir/." will copy the contents, so make sure that a dir ending
     # with dot is treated correctly.
     run_podman exec cpcontainer sh -c 'mkdir /tmp/subdir.; cp /srv/subdir/* /tmp/subdir./'
 
-    run_podman cp cpcontainer:/srv $srcdir
+    # Commit the image for testing non-running containers
-    run cat $srcdir/srv/subdir/containerfile1
+    run_podman commit -q cpcontainer
-    is "$output" "This first file is on the container"
+    cpimage="$output"
-    run cat $srcdir/srv/subdir/containerfile2
-    is "$output" "This second file is on the container as well"
-    rm -rf $srcdir/srv/subdir
 
-    run_podman cp cpcontainer:/srv/. $srcdir
+    # format is: <source arg to cp (appended to /srv)> | <full dest path> | <test name>
-    run ls $srcdir/subdir
+    tests="
-    run cat $srcdir/subdir/containerfile1
+ /srv           | /srv/subdir | copy /srv
-    is "$output" "This first file is on the container"
+ /srv/          | /srv/subdir | copy /srv/
-    run cat $srcdir/subdir/containerfile2
+ /srv/.         | /subdir     | copy /srv/.
-    is "$output" "This second file is on the container as well"
+ /srv/subdir/.  |             | copy /srv/subdir/.
-    rm -rf $srcdir/subdir
+ /tmp/subdir.   | /subdir.    | copy /tmp/subdir.
-
+"
-    run_podman cp cpcontainer:/srv/subdir/. $srcdir
-    run cat $srcdir/containerfile1
-    is "$output" "This first file is on the container"
-    run cat $srcdir/containerfile2
-    is "$output" "This second file is on the container as well"
-    rm -rf $srcdir/subdir
-
-    run_podman cp cpcontainer:/tmp/subdir. $srcdir
-    run cat $srcdir/subdir./containerfile1
-    is "$output" "This first file is on the container"
-    run cat $srcdir/subdir./containerfile2
-    is "$output" "This second file is on the container as well"
-    rm -rf $srcdir/subdir.
 
+    # RUNNING container
+    while read src dest_fullname description; do
+        if [[ $src == "''" ]];then
+            unset src
+        fi
+        if [[ $dest == "''" ]];then
+            unset dest
+        fi
+        if [[ $dest_fullname == "''" ]];then
+            unset dest_fullname
+        fi
+        run_podman cp cpcontainer:$src $destdir
+        is "$(< $destdir$dest_fullname/containerfile0)" "${randomcontent[0]}" "$description"
+        is "$(< $destdir$dest_fullname/containerfile1)" "${randomcontent[1]}" "$description"
+        rm -rf $destdir/*
+    done < <(parse_table "$tests")
+    run_podman kill cpcontainer
     run_podman rm -f cpcontainer
+
+    # CREATED container
+    run_podman create --name cpcontainer --workdir=/srv $cpimage
+    while read src dest_fullname description; do
+        if [[ $src == "''" ]];then
+            unset src
+        fi
+        if [[ $dest == "''" ]];then
+            unset dest
+        fi
+        if [[ $dest_fullname == "''" ]];then
+            unset dest_fullname
+        fi
+        run_podman cp cpcontainer:$src $destdir
+        is "$(< $destdir$dest_fullname/containerfile0)" "${randomcontent[0]}" "$description"
+        is "$(< $destdir$dest_fullname/containerfile1)" "${randomcontent[1]}" "$description"
+        rm -rf $destdir/*
+    done < <(parse_table "$tests")
+    run_podman rm -f cpcontainer
+
+    run_podman rmi -f $cpimage
 }
 
 
@@ -228,9 +337,7 @@ load helpers
     run_podman create --name cpcontainer -v $volume1:/tmp/volume -v $volume2:/tmp/volume/sub-volume $IMAGE
 
     run_podman cp $srcdir/hostfile cpcontainer:/tmp/volume/sub-volume
-
+    is "$(< $volume2_mount/hostfile)" "This file should be in volume2"
-    run cat $volume2_mount/hostfile
-    is "$output" "This file should be in volume2"
 
     # Volume 1 must be empty.
     run ls $volume1_mount
@@ -254,9 +361,7 @@ load helpers
     run_podman create --name cpcontainer -v $volume:/tmp/volume -v $mountdir:/tmp/volume/mount $IMAGE
 
     run_podman cp $srcdir/hostfile cpcontainer:/tmp/volume/mount
-
+    is "$(< $mountdir/hostfile)" "This file should be in the mount"
-    run cat $mountdir/hostfile
-    is "$output" "This file should be in the mount"
 
     run_podman rm -f cpcontainer
     run_podman volume rm $volume
@@ -284,7 +389,7 @@ load helpers
     # cp no longer supports wildcarding
     run_podman 125 cp 'cpcontainer:/tmp/*' $dstdir
 
-    run_podman rm cpcontainer
+    run_podman rm -f cpcontainer
 }
 
 
@@ -308,7 +413,7 @@ load helpers
     # make sure there are no files in dstdir
     is "$(/bin/ls -1 $dstdir)" "" "incorrectly copied symlink from host"
 
-    run_podman rm cpcontainer
+    run_podman rm -f cpcontainer
 }
 
 
@@ -332,7 +437,7 @@ load helpers
     # make sure there are no files in dstdir
     is "$(/bin/ls -1 $dstdir)" "" "incorrectly copied symlink from host"
 
-    run_podman rm cpcontainer
+    run_podman rm -f cpcontainer
 }
 
 
@@ -352,7 +457,7 @@ load helpers
     # dstdir must be empty
     is "$(/bin/ls -1 $dstdir)" "" "incorrectly copied symlink from host"
 
-    run_podman rm cpcontainer
+    run_podman rm -f cpcontainer
 }
 
 
@@ -409,6 +514,7 @@ load helpers
     run_podman exec cpcontainer cat /tmp/d3/x
     is "$output" "$rand_content3" "cp creates file named x"
 
+    run_podman kill cpcontainer
     run_podman rm -f cpcontainer
 }
 
@@ -446,6 +552,7 @@ load helpers
     run_podman exec cpcontainer cat $graphroot/$rand_filename
     is "$output" "$rand_content" "Contents of file copied into container"
 
+    run_podman kill cpcontainer
     run_podman rm -f cpcontainer
 }
 
@@ -494,6 +601,7 @@ load helpers
     run_podman 125 cp - cpcontainer:/tmp/IdoNotExist < $tar_file
     is "$output" 'Error: destination must be a directory when copying from stdin'
 
+    run_podman kill cpcontainer
     run_podman rm -f cpcontainer
 }
 
@@ -527,8 +635,7 @@ load helpers
     fi
 
     tar xvf $srcdir/stdout.tar -C $srcdir
-    run cat $srcdir/file.txt
+    is "$(< $srcdir/file.txt)" "$rand_content"
-    is "$output" "$rand_content"
     run 1 ls $srcdir/empty.txt
     rm -f $srcdir/*
 
@@ -539,11 +646,10 @@ load helpers
     fi
 
     tar xvf $srcdir/stdout.tar -C $srcdir
-    run cat $srcdir/tmp/file.txt
+    is "$(< $srcdir/tmp/file.txt)" "$rand_content"
-    is "$output" "$rand_content"
+    is "$(< $srcdir/tmp/empty.txt)" ""
-    run cat $srcdir/tmp/empty.txt
-    is "$output" ""
 
+    run_podman kill cpcontainer
     run_podman rm -f cpcontainer
 }