From 7e15084d1987425c70850353462b0b80269300f9 Mon Sep 17 00:00:00 2001
From: Matthew Heon <mheon@redhat.com>
Date: Thu, 8 Nov 2018 16:06:59 -0500
Subject: Accurately update state if prepare() partially fails

We are seeing some issues where, when part of prepare() fails
(originally noticed due to a bad static IP), the other half does
not successfully clean up, and the state can be left in a bad
place (not knowing about an active SHM mount for example).

Signed-off-by: Matthew Heon <mheon@redhat.com>
---
 libpod/container_internal_linux.go | 58 ++++++++++++++++++++++++++------------
 libpod/networking_linux.go         |  5 +++-
 2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 163cd75e7..66c7e8a04 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -57,7 +57,7 @@ func (c *Container) prepare() (err error) {
 		networkStatus                   []*cnitypes.Result
 		createNetNSErr, mountStorageErr error
 		mountPoint                      string
-		saveNetworkStatus               bool
+		tmpStateLock                    sync.Mutex
 	)
 
 	wg.Add(2)
@@ -66,17 +66,55 @@ func (c *Container) prepare() (err error) {
 		defer wg.Done()
 		// Set up network namespace if not already set up
 		if c.config.CreateNetNS && c.state.NetNS == nil && !c.config.PostConfigureNetNS {
-			saveNetworkStatus = true
 			netNS, networkStatus, createNetNSErr = c.runtime.createNetNS(c)
+
+			tmpStateLock.Lock()
+			defer tmpStateLock.Unlock()
+
+			// Assign NetNS attributes to container
+			if createNetNSErr == nil {
+				c.state.NetNS = netNS
+				c.state.NetworkStatus = networkStatus
+			}
 		}
 	}()
 	// Mount storage if not mounted
 	go func() {
 		defer wg.Done()
 		mountPoint, mountStorageErr = c.mountStorage()
+
+		if mountStorageErr != nil {
+			return
+		}
+
+		tmpStateLock.Lock()
+		defer tmpStateLock.Unlock()
+
+		// Finish up mountStorage
+		c.state.Mounted = true
+		c.state.Mountpoint = mountPoint
+		if c.state.UserNSRoot == "" {
+			c.state.RealMountpoint = c.state.Mountpoint
+		} else {
+			c.state.RealMountpoint = filepath.Join(c.state.UserNSRoot, "mountpoint")
+		}
+
+		logrus.Debugf("Created root filesystem for container %s at %s", c.ID(), c.state.Mountpoint)
+	}()
+
+	defer func() {
+		if err != nil {
+			if err2 := c.cleanupNetwork(); err2 != nil {
+				logrus.Errorf("Error cleaning up container %s network: %v", c.ID(), err2)
+			}
+			if err2 := c.cleanupStorage(); err2 != nil {
+				logrus.Errorf("Error cleaning up container %s storage: %v", c.ID(), err2)
+			}
+		}
 	}()
 
 	wg.Wait()
+
 	if createNetNSErr != nil {
 		if mountStorageErr != nil {
 			logrus.Error(createNetNSErr)
@@ -88,22 +126,6 @@ func (c *Container) prepare() (err error) {
 		return mountStorageErr
 	}
 
-	// Assign NetNS attributes to container
-	if saveNetworkStatus {
-		c.state.NetNS = netNS
-		c.state.NetworkStatus = networkStatus
-	}
-
-	// Finish up mountStorage
-	c.state.Mounted = true
-	c.state.Mountpoint = mountPoint
-	if c.state.UserNSRoot == "" {
-		c.state.RealMountpoint = c.state.Mountpoint
-	} else {
-		c.state.RealMountpoint = filepath.Join(c.state.UserNSRoot, "mountpoint")
-	}
-
-	logrus.Debugf("Created root filesystem for container %s at %s", c.ID(), c.state.Mountpoint)
 	// Save the container
 	return c.save()
 }
diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index 863a764e2..952ad83a5 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -90,13 +90,16 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS ns.NetNS) ([]*cnitypes.Re
 }
 
 // Create and configure a new network namespace for a container
-func (r *Runtime) createNetNS(ctr *Container) (ns.NetNS, []*cnitypes.Result, error) {
+func (r *Runtime) createNetNS(ctr *Container) (n ns.NetNS, q []*cnitypes.Result, err error) {
 	ctrNS, err := netns.NewNS()
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, "error creating network namespace for container %s", ctr.ID())
 	}
 	defer func() {
 		if err != nil {
+			if err2 := netns.UnmountNS(ctrNS); err2 != nil {
+				logrus.Errorf("Error unmounting partially created network namespace for container %s: %v", ctr.ID(), err2)
+			}
 			if err2 := ctrNS.Close(); err2 != nil {
 				logrus.Errorf("Error closing partially created network namespace for container %s: %v", ctr.ID(), err2)
 			}
-- 
cgit v1.2.3-54-g00ecf