From c39f63eaf49c5ec1161749907f4f81ebb9a796e0 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Tue, 9 Oct 2018 23:13:25 -0700 Subject: [PATCH] Teardown pod network even if the network namespace is closed Signed-off-by: Lantao Liu --- pkg/server/sandbox_stop.go | 31 ++++++++++++++----------------- pkg/store/sandbox/netns.go | 7 +++++++ 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go index 2d5b3ca33..e0f203309 100644 --- 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 index a96eb6aad..8a08194cb 100644 --- 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")