Add StartAndAttach() API endpoint for containers

This solves our prior problems with attach races by ensuring the
order is correct.

Also contains substantial cleanups to the attach code.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #482
Approved by: baude

7 files changed, 235 insertions, 227 deletions

diff --git a/cmd/podman/attach.go b/cmd/podman/attach.go
index e468964f6..7f7e4d192 100644
--- a/cmd/podman/attach.go
+++ b/cmd/podman/attach.go
@@ -1,11 +1,8 @@
 package main
 
 import (
-	"sync"
-
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/libpod"
-	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
 
@@ -65,27 +62,10 @@ func attachCmd(c *cli.Context) error {
 	if conState != libpod.ContainerStateRunning {
 		return errors.Errorf("you can only attach to running containers")
 	}
-	// Create a bool channel to track that the console socket attach
-	// is successful.
-	attached := make(chan bool)
-	// Create a waitgroup so we can sync and wait for all goroutines
-	// to finish before exiting main
-	var wg sync.WaitGroup
 
-	// We increment the wg counter because we need to do the attach
-	wg.Add(1)
-	// Attach to the running container
-	go func() {
-		logrus.Debugf("trying to attach to the container %s", ctr.ID())
-		defer wg.Done()
-		if err := ctr.Attach(c.Bool("no-stdin"), c.String("detach-keys"), attached); err != nil {
-			logrus.Errorf("unable to attach to container %s: %q", ctr.ID(), err)
-		}
-	}()
-	if !<-attached {
-		return errors.Errorf("unable to attach to container %s", ctr.ID())
+	if err := ctr.Attach(c.Bool("no-stdin"), c.String("detach-keys")); err != nil {
+		return errors.Wrapf(err, "error attaching to container %s", ctr.ID())
 	}
-	wg.Wait()
 
 	return nil
 }
diff --git a/cmd/podman/run.go b/cmd/podman/run.go
index 3d6175cef..f68db9036 100644
--- a/cmd/podman/run.go
+++ b/cmd/podman/run.go
@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
-	"sync"
 
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/libpod"
@@ -116,38 +115,30 @@ func runCmd(c *cli.Context) error {
 		}
 	}
 
-	// Create a bool channel to track that the console socket attach
-	// is successful.
-	attached := make(chan bool)
-	// Create a waitgroup so we can sync and wait for all goroutines
-	// to finish before exiting main
-	var wg sync.WaitGroup
-
-	if !createConfig.Detach {
-		// We increment the wg counter because we need to do the attach
-		wg.Add(1)
-		// Attach to the running container
-		go func() {
-			logrus.Debugf("trying to attach to the container %s", ctr.ID())
-			defer wg.Done()
-			if err := ctr.Attach(false, c.String("detach-keys"), attached); err != nil {
-				logrus.Errorf("unable to attach to container %s: %q", ctr.ID(), err)
-			}
-		}()
-		if !<-attached {
-			return errors.Errorf("unable to attach to container %s", ctr.ID())
-		}
-	}
-	// Start the container
-	if err := ctr.Start(); err != nil {
-		return errors.Wrapf(err, "unable to start container %q", ctr.ID())
-	}
+	// Handle detached start
 	if createConfig.Detach {
+		if err := ctr.Start(); err != nil {
+			return errors.Wrapf(err, "unable to start container %q", ctr.ID())
+		}
+
 		fmt.Printf("%s\n", ctr.ID())
 		exitCode = 0
 		return nil
 	}
-	wg.Wait()
+
+	// TODO: that "false" should probably be linked to -i
+	// Handle this when we split streams to allow attaching just stdin/out/err
+	attachChan, err := ctr.StartAndAttach(false, c.String("detach-keys"))
+	if err != nil {
+		return errors.Wrapf(err, "unable to start container %q", ctr.ID())
+	}
+
+	// Wait for attach to complete
+	err = <-attachChan
+	if err != nil {
+		return errors.Wrapf(err, "error attaching to container %s", ctr.ID())
+	}
+
 	if ecode, err := ctr.ExitCode(); err != nil {
 		logrus.Errorf("unable to get exit code of container %s: %q", ctr.ID(), err)
 	} else {
diff --git a/cmd/podman/start.go b/cmd/podman/start.go
index 5053abc07..c90fab645 100644
--- a/cmd/podman/start.go
+++ b/cmd/podman/start.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"os"
 	"strconv"
-	"sync"
 
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/libpod"
@@ -75,13 +74,6 @@ func startCmd(c *cli.Context) error {
 	}
 	var lastError error
 	for _, container := range args {
-		// Create a bool channel to track that the console socket attach
-		// is successful.
-		attached := make(chan bool)
-		// Create a waitgroup so we can sync and wait for all goroutines
-		// to finish before exiting main
-		var wg sync.WaitGroup
-
 		ctr, err := runtime.LookupContainer(container)
 		if err != nil {
 			if lastError != nil {
@@ -104,50 +96,41 @@ func startCmd(c *cli.Context) error {
 		if err != nil {
 			return errors.Wrapf(err, "unable to parse annotations in %s", ctr.ID())
 		}
+
+		// Handle start --attach
 		// We only get a terminal session if both a tty was specified in the spec and
 		// -a on the command-line was given.
 		if attach && tty {
-			// We increment the wg counter because we need to do the attach
-			wg.Add(1)
-			// Attach to the running container
-			go func() {
-				logrus.Debugf("trying to attach to the container %s", ctr.ID())
-				defer wg.Done()
-				if err := ctr.Attach(noStdIn, c.String("detach-keys"), attached); err != nil {
-					logrus.Errorf("unable to attach to container %s: %q", ctr.ID(), err)
-				}
-			}()
-			if !<-attached {
-				return errors.Errorf("unable to attach to container %s", ctr.ID())
+			attachChan, err := ctr.StartAndAttach(noStdIn, c.String("detach-keys"))
+			if err != nil {
+				return errors.Wrapf(err, "unable to start container %s", ctr.ID())
+			}
+
+			// Wait for attach to complete
+			err = <-attachChan
+			if err != nil {
+				return errors.Wrapf(err, "error attaching to container %s", ctr.ID())
 			}
+
+			if ecode, err := ctr.ExitCode(); err != nil {
+				logrus.Errorf("unable to get exit code of container %s: %q", ctr.ID(), err)
+			} else {
+				exitCode = int(ecode)
+			}
+
+			return ctr.Cleanup()
 		}
-		err = ctr.Start()
-		if err != nil {
+
+		// Handle non-attach start
+		if err := ctr.Start(); err != nil {
 			if lastError != nil {
 				fmt.Fprintln(os.Stderr, lastError)
 			}
-			lastError = errors.Wrapf(err, "unable to start %s", container)
+			lastError = errors.Wrapf(err, "unable to start container %q", container)
 			continue
 		}
-		if !attach {
-			fmt.Println(ctr.ID())
-		}
-		wg.Wait()
-		if ecode, err := ctr.ExitCode(); err != nil {
-			logrus.Errorf("unable to get exit code of container %s: %q", ctr.ID(), err)
-		} else {
-			exitCode = int(ecode)
-		}
-		if lastError != nil {
-			fmt.Fprintln(os.Stderr, lastError)
-		}
-		// We can only do this if we attached
-		// Otherwise the container is probably still running
-		if attach && tty {
-			lastError = ctr.Cleanup()
-			// No need for LastError as we can only have one ctr
-			// with attach
-		}
+		fmt.Println(ctr.ID())
 	}
+
 	return lastError
 }
diff --git a/libpod/container_api.go b/libpod/container_api.go
index 499cf826f..d3afe82b7 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -9,14 +9,11 @@ import (
 	"github.com/containers/storage"
 	"github.com/docker/docker/daemon/caps"
 	"github.com/docker/docker/pkg/stringid"
-	"github.com/docker/docker/pkg/term"
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/libpod/driver"
 	"github.com/projectatomic/libpod/pkg/inspect"
 	"github.com/sirupsen/logrus"
-	"golang.org/x/crypto/ssh/terminal"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/tools/remotecommand"
 )
 
 // Init creates a container in the OCI runtime
@@ -34,27 +31,13 @@ func (c *Container) Init() (err error) {
 		return errors.Wrapf(ErrCtrExists, "container %s has already been created in runtime", c.ID())
 	}
 
-	if err := c.mountStorage(); err != nil {
+	if err := c.prepare(); err != nil {
 		return err
 	}
 	defer func() {
 		if err != nil {
-			if err2 := c.cleanupStorage(); err2 != nil {
-				logrus.Errorf("Error cleaning up storage for container %s: %v", c.ID(), err2)
-			}
-		}
-	}()
-
-	// Make a network namespace for the container
-	if c.config.CreateNetNS && c.state.NetNS == nil {
-		if err := c.runtime.createNetNS(c); err != nil {
-			return err
-		}
-	}
-	defer func() {
-		if err != nil {
-			if err2 := c.runtime.teardownNetNS(c); err2 != nil {
-				logrus.Errorf("Error tearing down network namespace for container %s: %v", c.ID(), err2)
+			if err2 := c.cleanup(); err2 != nil {
+				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2)
 			}
 		}
 	}()
@@ -81,61 +64,86 @@ func (c *Container) Start() (err error) {
 		return errors.Wrapf(ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID())
 	}
 
-	// Mount storage for the container if necessary
-	if err := c.mountStorage(); err != nil {
+	if err := c.prepare(); err != nil {
 		return err
 	}
 	defer func() {
 		if err != nil {
-			if err2 := c.cleanupStorage(); err2 != nil {
-				logrus.Errorf("Error cleaning up storage for container %s: %v", c.ID(), err2)
+			if err2 := c.cleanup(); err2 != nil {
+				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2)
 			}
 		}
 	}()
 
-	// Create a network namespace if necessary
-	if c.config.CreateNetNS && c.state.NetNS == nil {
-		if err := c.runtime.createNetNS(c); err != nil {
+	// Reinitialize the container if we need to
+	if c.state.State == ContainerStateStopped {
+		if err := c.reinit(); err != nil {
 			return err
 		}
 	}
+
+	// Start the container
+	return c.start()
+}
+
+// StartAndAttach starts a container and attaches to it
+// StartAndAttach can start created or stopped containers
+// Stopped containers will be deleted and re-created in runc, undergoing a fresh
+// Init()
+// If successful, an error channel will be returned containing the result of the
+// attach call.
+// The channel will be closed automatically after the result of attach has been
+// sent
+func (c *Container) StartAndAttach(noStdin bool, keys string) (attachResChan <-chan error, err error) {
+	if !c.locked {
+		c.lock.Lock()
+		defer c.lock.Unlock()
+
+		if err := c.syncContainer(); err != nil {
+			return nil, err
+		}
+	}
+
+	// Container must be created or stopped to be started
+	if !(c.state.State == ContainerStateCreated || c.state.State == ContainerStateStopped) {
+		return nil, errors.Wrapf(ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID())
+	}
+
+	if err := c.prepare(); err != nil {
+		return nil, err
+	}
 	defer func() {
 		if err != nil {
-			if err2 := c.runtime.teardownNetNS(c); err2 != nil {
-				logrus.Errorf("Error tearing down network namespace for container %s: %v", c.ID(), err2)
+			if err2 := c.cleanup(); err2 != nil {
+				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2)
 			}
 		}
 	}()
 
 	// Reinitialize the container if we need to
 	if c.state.State == ContainerStateStopped {
-		// If necessary, delete attach and ctl files
-		if err := c.removeConmonFiles(); err != nil {
-			return err
+		if err := c.reinit(); err != nil {
+			return nil, err
 		}
+	}
 
-		// Delete the container in the runtime
-		if err := c.runtime.ociRuntime.deleteContainer(c); err != nil {
-			return errors.Wrapf(err, "error removing container %s from runtime", c.ID())
-		}
+	attachChan := make(chan error)
 
-		// Our state is now Configured, as we've removed ourself from
-		// the runtime
-		// Set and save now to make sure that, if the init() below fails
-		// we still have a valid state
-		c.state.State = ContainerStateConfigured
-		if err := c.save(); err != nil {
-			return err
+	// Attach to the container before starting it
+	go func() {
+		if err := c.attach(noStdin, keys); err != nil {
+			attachChan <- err
 		}
+		close(attachChan)
+	}()
 
-		// Reinitialize the container
-		if err := c.init(); err != nil {
-			return err
-		}
+	// Start the container
+	if err := c.start(); err != nil {
+		// TODO: interrupt the attach here if we error
+		return nil, err
 	}
 
-	// Start the container
-	return c.start()
+	return attachChan, nil
 }
 
 // Stop uses the container's stop signal (or SIGTERM if no signal was specified)
@@ -336,7 +344,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e
 
 // Attach attaches to a container
 // Returns fully qualified URL of streaming server for the container
-func (c *Container) Attach(noStdin bool, keys string, attached chan<- bool) error {
+func (c *Container) Attach(noStdin bool, keys string) error {
 	if !c.locked {
 		c.lock.Lock()
 		if err := c.syncContainer(); err != nil {
@@ -351,24 +359,7 @@ func (c *Container) Attach(noStdin bool, keys string, attached chan<- bool) erro
 		return errors.Wrapf(ErrCtrStateInvalid, "can only attach to created or running containers")
 	}
 
-	// Check the validity of the provided keys first
-	var err error
-	detachKeys := []byte{}
-	if len(keys) > 0 {
-		detachKeys, err = term.ToBytes(keys)
-		if err != nil {
-			return errors.Wrapf(err, "invalid detach keys")
-		}
-	}
-
-	resize := make(chan remotecommand.TerminalSize)
-	if terminal.IsTerminal(int(os.Stdin.Fd())) {
-		resizeTty(resize)
-	} else {
-		defer close(resize)
-	}
-	err = c.attachContainerSocket(resize, noStdin, detachKeys, attached)
-	return err
+	return c.attach(noStdin, keys)
 }
 
 // Mount mounts a container's filesystem on the host
@@ -398,7 +389,6 @@ func (c *Container) Mount(label string) (string, error) {
 	}
 	c.state.Mountpoint = mountPoint
 	c.state.Mounted = true
-	c.config.MountLabel = mountLabel
 
 	if err := c.save(); err != nil {
 		return "", err
@@ -629,12 +619,7 @@ func (c *Container) Cleanup() error {
 		return errors.Wrapf(ErrCtrStateInvalid, "container %s has active exec sessions, refusing to clean up", c.ID())
 	}
 
-	// Stop the container's network namespace (if it has one)
-	if err := c.cleanupNetwork(); err != nil {
-		logrus.Errorf("unable cleanup network for container %s: %q", c.ID(), err)
-	}
-
-	return c.cleanupStorage()
+	return c.cleanup()
 }
 
 // Batch starts a batch operation on the given container
diff --git a/libpod/container_attach.go b/libpod/container_attach.go
index 25e8bcdac..c1579d731 100644
--- a/libpod/container_attach.go
+++ b/libpod/container_attach.go
@@ -5,8 +5,10 @@ import (
 	"io"
 	"net"
 	"os"
+	gosignal "os/signal"
 	"path/filepath"
 
+	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/term"
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/utils"
@@ -24,8 +26,59 @@ const (
 	AttachPipeStderr = 3
 )
 
+// resizeTty handles TTY resizing for Attach()
+func resizeTty(resize chan remotecommand.TerminalSize) {
+	sigchan := make(chan os.Signal, 1)
+	gosignal.Notify(sigchan, signal.SIGWINCH)
+	sendUpdate := func() {
+		winsize, err := term.GetWinsize(os.Stdin.Fd())
+		if err != nil {
+			logrus.Warnf("Could not get terminal size %v", err)
+			return
+		}
+		resize <- remotecommand.TerminalSize{
+			Width:  winsize.Width,
+			Height: winsize.Height,
+		}
+	}
+	go func() {
+		defer close(resize)
+		// Update the terminal size immediately without waiting
+		// for a SIGWINCH to get the correct initial size.
+		sendUpdate()
+		for range sigchan {
+			sendUpdate()
+		}
+	}()
+}
+
+func (c *Container) attach(noStdin bool, keys string) error {
+	// Check the validity of the provided keys first
+	var err error
+	detachKeys := []byte{}
+	if len(keys) > 0 {
+		detachKeys, err = term.ToBytes(keys)
+		if err != nil {
+			return errors.Wrapf(err, "invalid detach keys")
+		}
+	}
+
+	// TODO: allow resize channel to be passed in for CRI-O use
+	resize := make(chan remotecommand.TerminalSize)
+	if terminal.IsTerminal(int(os.Stdin.Fd())) {
+		resizeTty(resize)
+	} else {
+		defer close(resize)
+	}
+
+	logrus.Debugf("Attaching to container %s", c.ID())
+
+	return c.attachContainerSocket(resize, noStdin, detachKeys)
+}
+
 // attachContainerSocket connects to the container's attach socket and deals with the IO
-func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, noStdIn bool, detachKeys []byte, attached chan<- bool) error {
+// TODO add a channel to allow interruptiong
+func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, noStdIn bool, detachKeys []byte) error {
 	inputStream := os.Stdin
 	outputStream := os.Stdout
 	errorStream := os.Stderr
@@ -78,9 +131,6 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi
 	}
 	defer conn.Close()
 
-	// signal back that the connection was made
-	attached <- true
-
 	receiveStdoutError := make(chan error)
 	if outputStream != nil || errorStream != nil {
 		go func() {
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 6b375ef6f..4d1bce787 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -6,7 +6,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	gosignal "os/signal"
 	"path/filepath"
 	"strings"
 	"syscall"
@@ -17,9 +16,7 @@ import (
 	"github.com/containers/storage/pkg/chrootarchive"
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/namesgenerator"
-	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/stringid"
-	"github.com/docker/docker/pkg/term"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
 	"github.com/opencontainers/selinux/go-selinux/label"
@@ -30,7 +27,6 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/ulule/deepcopier"
 	"golang.org/x/sys/unix"
-	"k8s.io/client-go/tools/remotecommand"
 )
 
 const (
@@ -375,6 +371,33 @@ func (c *Container) init() error {
 	return c.save()
 }
 
+// Reinitialize a container
+// Deletes and recreates a container in the runtime
+// Should only be done on ContainerStateStopped containers
+func (c *Container) reinit() error {
+	// If necessary, delete attach and ctl files
+	if err := c.removeConmonFiles(); err != nil {
+		return err
+	}
+
+	// Delete the container in the runtime
+	if err := c.runtime.ociRuntime.deleteContainer(c); err != nil {
+		return errors.Wrapf(err, "error removing container %s from runtime", c.ID())
+	}
+
+	// Our state is now Configured, as we've removed ourself from
+	// the runtime
+	// Set and save now to make sure that, if the init() below fails
+	// we still have a valid state
+	c.state.State = ContainerStateConfigured
+	if err := c.save(); err != nil {
+		return err
+	}
+
+	// Initialize the container again
+	return c.init()
+}
+
 // Initialize (if necessary) and start a container
 // Performs all necessary steps to start a container that is not running
 // Does not lock or check validity
@@ -393,28 +416,13 @@ func (c *Container) initAndStart() (err error) {
 		return errors.Wrapf(ErrCtrStateInvalid, "cannot start paused container %s", c.ID())
 	}
 
-	// Mount if necessary
-	if err := c.mountStorage(); err != nil {
+	if err := c.prepare(); err != nil {
 		return err
 	}
 	defer func() {
 		if err != nil {
-			if err2 := c.cleanupStorage(); err2 != nil {
-				logrus.Errorf("Error cleaning up storage for container %s: %v", c.ID(), err2)
-			}
-		}
-	}()
-
-	// Create a network namespace if necessary
-	if c.config.CreateNetNS && c.state.NetNS == nil {
-		if err := c.runtime.createNetNS(c); err != nil {
-			return err
-		}
-	}
-	defer func() {
-		if err != nil {
-			if err2 := c.runtime.teardownNetNS(c); err2 != nil {
-				logrus.Errorf("Error tearing down network namespace for container %s: %v", c.ID(), err2)
+			if err2 := c.cleanup(); err2 != nil {
+				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2)
 			}
 		}
 	}()
@@ -482,32 +490,6 @@ func (c *Container) stop(timeout uint) error {
 	return c.cleanupStorage()
 }
 
-// resizeTty handles TTY resizing for Attach()
-func resizeTty(resize chan remotecommand.TerminalSize) {
-	sigchan := make(chan os.Signal, 1)
-	gosignal.Notify(sigchan, signal.SIGWINCH)
-	sendUpdate := func() {
-		winsize, err := term.GetWinsize(os.Stdin.Fd())
-		if err != nil {
-			logrus.Warnf("Could not get terminal size %v", err)
-			return
-		}
-		resize <- remotecommand.TerminalSize{
-			Width:  winsize.Width,
-			Height: winsize.Height,
-		}
-	}
-	go func() {
-		defer close(resize)
-		// Update the terminal size immediately without waiting
-		// for a SIGWINCH to get the correct initial size.
-		sendUpdate()
-		for range sigchan {
-			sendUpdate()
-		}
-	}()
-}
-
 // mountStorage sets up the container's root filesystem
 // It mounts the image and any other requested mounts
 // TODO: Add ability to override mount label so we can use this for Mount() too
@@ -554,6 +536,29 @@ func (c *Container) mountStorage() (err error) {
 	return c.save()
 }
 
+// prepare mounts the container and sets up other required resources like net
+// namespaces
+func (c *Container) prepare() (err error) {
+	// Mount storage if not mounted
+	if err := c.mountStorage(); err != nil {
+		return err
+	}
+
+	// Set up network namespace if not already set up
+	if c.config.CreateNetNS && c.state.NetNS == nil {
+		if err := c.runtime.createNetNS(c); err != nil {
+			// Tear down storage before exiting to make sure we
+			// don't leak mounts
+			if err2 := c.cleanupStorage(); err2 != nil {
+				logrus.Errorf("Error cleaning up storage for container %s: %v", c.ID(), err2)
+			}
+			return err
+		}
+	}
+
+	return nil
+}
+
 // cleanupNetwork unmounts and cleans up the container's network
 func (c *Container) cleanupNetwork() error {
 	// Stop the container's network namespace (if it has one)
@@ -593,6 +598,27 @@ func (c *Container) cleanupStorage() error {
 	return c.save()
 }
 
+// Unmount the a container and free its resources
+func (c *Container) cleanup() error {
+	var lastError error
+
+	// Clean up network namespace, if present
+	if err := c.cleanupNetwork(); err != nil {
+		lastError = nil
+	}
+
+	// Unmount storage
+	if err := c.cleanupStorage(); err != nil {
+		if lastError != nil {
+			logrus.Errorf("Error unmounting container %s storage: %v", c.ID(), err)
+		} else {
+			lastError = err
+		}
+	}
+
+	return lastError
+}
+
 // Make standard bind mounts to include in the container
 func (c *Container) makeBindMounts() error {
 	if c.state.BindMounts == nil {
diff --git a/libpod/pod.go b/libpod/pod.go
index bd02efdae..16b590a1c 100644
--- a/libpod/pod.go
+++ b/libpod/pod.go
@@ -240,14 +240,7 @@ func (p *Pod) Stop(cleanup bool) (map[string]error, error) {
 		}
 
 		if cleanup {
-			// Clean up storage to ensure we don't leave dangling mounts
-			if err := ctr.cleanupStorage(); err != nil {
-				ctrErrors[ctr.ID()] = err
-				continue
-			}
-
-			// Clean up network namespace
-			if err := ctr.cleanupNetwork(); err != nil {
+			if err := ctr.cleanup(); err != nil {
 				ctrErrors[ctr.ID()] = err
 			}
 		}