From 45dffed8ac13be81833e7c6240ef1c05e45f15f2 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 30 Mar 2017 17:33:58 -0500 Subject: [PATCH] kubelet/network: return but tolerate errors returned from GetNetNS() Runtimes should never return "" and nil errors, since network plugin drivers need to treat netns differently in different cases. So return errors when we can't get the netns, and fix up the plugins to do the right thing. Namely, we don't need a NetNS on pod network teardown. We do need a netns for pod Status checks and for network setup. --- pkg/kubelet/dockershim/docker_service.go | 2 +- pkg/kubelet/dockershim/helpers.go | 8 ++++++-- pkg/kubelet/network/cni/cni.go | 4 +++- pkg/kubelet/network/kubenet/kubenet_linux.go | 8 ++++---- pkg/kubelet/network/plugins.go | 2 ++ 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go index d7aadb4f668..caed4427255 100644 --- a/pkg/kubelet/dockershim/docker_service.go +++ b/pkg/kubelet/dockershim/docker_service.go @@ -315,7 +315,7 @@ func (ds *dockerService) GetNetNS(podSandboxID string) (string, error) { if err != nil { return "", err } - return getNetworkNamespace(r), nil + return getNetworkNamespace(r) } // GetPodPortMappings returns the port mappings of the given podSandbox ID. diff --git a/pkg/kubelet/dockershim/helpers.go b/pkg/kubelet/dockershim/helpers.go index 1fb5e8f0aa5..22ab777e6af 100644 --- a/pkg/kubelet/dockershim/helpers.go +++ b/pkg/kubelet/dockershim/helpers.go @@ -266,8 +266,12 @@ func getApparmorSecurityOpts(sc *runtimeapi.LinuxContainerSecurityContext, separ return fmtOpts, nil } -func getNetworkNamespace(c *dockertypes.ContainerJSON) string { - return fmt.Sprintf(dockerNetNSFmt, c.State.Pid) +func getNetworkNamespace(c *dockertypes.ContainerJSON) (string, error) { + if c.State.Pid == 0 { + // Docker reports pid 0 for an exited container. + return "", fmt.Errorf("Cannot find network namespace for the terminated container %q", c.ID) + } + return fmt.Sprintf(dockerNetNSFmt, c.State.Pid), nil } // dockerFilter wraps around dockerfilters.Args and provides methods to modify diff --git a/pkg/kubelet/network/cni/cni.go b/pkg/kubelet/network/cni/cni.go index 6060c63ceb3..27275f4bfe2 100644 --- a/pkg/kubelet/network/cni/cni.go +++ b/pkg/kubelet/network/cni/cni.go @@ -251,9 +251,11 @@ func (plugin *cniNetworkPlugin) TearDownPod(namespace string, name string, id ku if err := plugin.checkInitialized(); err != nil { return err } + + // Lack of namespace should not be fatal on teardown netnsPath, err := plugin.host.GetNetNS(id.ID) if err != nil { - return fmt.Errorf("CNI failed to retrieve network namespace path: %v", err) + glog.Warningf("CNI failed to retrieve network namespace path: %v", err) } return plugin.deleteFromNetwork(plugin.getDefaultNetwork(), name, namespace, id, netnsPath) diff --git a/pkg/kubelet/network/kubenet/kubenet_linux.go b/pkg/kubelet/network/kubenet/kubenet_linux.go index 2b9ab97bb01..a6997e0f02a 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux.go @@ -741,9 +741,9 @@ func podIsExited(p *kubecontainer.Pod) bool { return true } -func (plugin *kubenetNetworkPlugin) buildCNIRuntimeConf(ifName string, id kubecontainer.ContainerID) (*libcni.RuntimeConf, error) { +func (plugin *kubenetNetworkPlugin) buildCNIRuntimeConf(ifName string, id kubecontainer.ContainerID, needNetNs bool) (*libcni.RuntimeConf, error) { netnsPath, err := plugin.host.GetNetNS(id.ID) - if err != nil { + if needNetNs && err != nil { glog.Errorf("Kubenet failed to retrieve network namespace path: %v", err) } @@ -755,7 +755,7 @@ func (plugin *kubenetNetworkPlugin) buildCNIRuntimeConf(ifName string, id kubeco } func (plugin *kubenetNetworkPlugin) addContainerToNetwork(config *libcni.NetworkConfig, ifName, namespace, name string, id kubecontainer.ContainerID) (cnitypes.Result, error) { - rt, err := plugin.buildCNIRuntimeConf(ifName, id) + rt, err := plugin.buildCNIRuntimeConf(ifName, id, true) if err != nil { return nil, fmt.Errorf("Error building CNI config: %v", err) } @@ -769,7 +769,7 @@ func (plugin *kubenetNetworkPlugin) addContainerToNetwork(config *libcni.Network } func (plugin *kubenetNetworkPlugin) delContainerFromNetwork(config *libcni.NetworkConfig, ifName, namespace, name string, id kubecontainer.ContainerID) error { - rt, err := plugin.buildCNIRuntimeConf(ifName, id) + rt, err := plugin.buildCNIRuntimeConf(ifName, id, false) if err != nil { return fmt.Errorf("Error building CNI config: %v", err) } diff --git a/pkg/kubelet/network/plugins.go b/pkg/kubelet/network/plugins.go index 859ff46e567..a71f7ef837f 100644 --- a/pkg/kubelet/network/plugins.go +++ b/pkg/kubelet/network/plugins.go @@ -144,6 +144,8 @@ type Host interface { // CNI plugin wrappers like kubenet. type NamespaceGetter interface { // GetNetNS returns network namespace information for the given containerID. + // Runtimes should *never* return an empty namespace and nil error for + // a container; if error is nil then the namespace string must be valid. GetNetNS(containerID string) (string, error) }