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

@@ -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,33 +66,29 @@ 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()
-	}()
 
-	wg.Wait()
-	if createNetNSErr != nil {
 		if mountStorageErr != nil {
-			logrus.Error(createNetNSErr)
+			return
-			return mountStorageErr
-		}
-		return createNetNSErr
-	}
-	if mountStorageErr != nil {
-		return mountStorageErr
 		}
 
-	// Assign NetNS attributes to container
+		tmpStateLock.Lock()
-	if saveNetworkStatus {
+		defer tmpStateLock.Unlock()
-		c.state.NetNS = netNS
-		c.state.NetworkStatus = networkStatus
-	}
 
 		// Finish up mountStorage
 		c.state.Mounted = true
@@ -104,6 +100,32 @@ func (c *Container) prepare() (err error) {
 		}
 
 		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)
+			return mountStorageErr
+		}
+		return createNetNSErr
+	}
+	if mountStorageErr != nil {
+		return mountStorageErr
+	}
+
 	// Save the container
 	return c.save()
 }

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)
 			}