Teardown pod network even if the network namespace is closed

Random-Liu · Random-Liu · commit 986f75408338 · 2018-10-12T01:20:17.000-07:00
Signed-off-by: Lantao Liu &lt;lantaol@google.com&gt;
diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go
@@ -17,7 +17,6 @@ limitations under the License.
 package server
 
 import (
-	"os"
 	"time"
 
 	"github.com/containerd/containerd"
@@ -60,23 +59,21 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb
 	}
 
 	// Teardown network for sandbox.
-	if sandbox.NetNSPath != "" && sandbox.NetNS != nil {
-		if _, err := os.Stat(sandbox.NetNSPath); err != nil {
-			if !os.IsNotExist(err) {
-				return nil, errors.Wrapf(err, "failed to stat network namespace path %s", sandbox.NetNSPath)
-			}
-		} else {
-			if teardownErr := c.teardownPod(id, sandbox.NetNSPath, sandbox.Config); teardownErr != nil {
-				return nil, errors.Wrapf(teardownErr, "failed to destroy network for sandbox %q", id)
-			}
+	if sandbox.NetNSPath != "" {
+		netNSPath := sandbox.NetNSPath
+		if sandbox.NetNS == nil || sandbox.NetNS.Closed() {
+			// Use empty netns path if netns is not available. This is defined in:
+			// https://github.com/containernetworking/cni/blob/v0.7.0-alpha1/SPEC.md
+			netNSPath = ""
 		}
-		/*TODO:It is still possible that containerd crashes after we teardown the network, but before we remove the network namespace.
-		In that case, we'll not be able to remove the sandbox anymore. The chance is slim, but we should be aware of that.
-		In the future, once TearDownPod is idempotent, this will be fixed.*/
-
-		//Close the sandbox network namespace if it was created
-		if err = sandbox.NetNS.Remove(); err != nil {
-			return nil, errors.Wrapf(err, "failed to remove network namespace for sandbox %q", id)
+		if err := c.teardownPod(id, netNSPath, sandbox.Config); err != nil {
+			return nil, errors.Wrapf(err, "failed to destroy network for sandbox %q", id)
+		}
+		// Close the sandbox network namespace if it was created
+		if sandbox.NetNS != nil {
+			if err = sandbox.NetNS.Remove(); err != nil {
+				return nil, errors.Wrapf(err, "failed to remove network namespace for sandbox %q", id)
+			}
 		}
 	}
 
diff --git a/pkg/store/sandbox/netns.go b/pkg/store/sandbox/netns.go
@@ -27,6 +27,13 @@ import (
 	osinterface "github.com/containerd/cri/pkg/os"
 )
 
+// The NetNS library assumes only containerd manages the lifecycle of the
+// network namespace mount. The only case that netns will be unmounted by
+// someone else is node reboot.
+// If this assumption is broken, NetNS won't be aware of the external
+// unmount, and there will be a state mismatch.
+// TODO(random-liu): Don't cache state, always load from the system.
+
 // ErrClosedNetNS is the error returned when network namespace is closed.
 var ErrClosedNetNS = errors.New("network namespace is closed")
 
