From 0e84b08ed9a3138465254dfa44b16f849a628519 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 2 Feb 2018 23:58:12 -0600 Subject: [PATCH] UPSTREAM: 59301: dockershim: don't check pod IP in StopPodSandbox We're about to tear the container down, there's no point. It also suppresses an annoying error message due to kubelet stupidity that causes multiple parallel calls to StopPodSandbox for the same sandbox. docker_sandbox.go:355] failed to read pod IP from plugin/docker: NetworkPlugin cni failed on the status hook for pod "docker-registry-1-deploy_default": Unexpected command output nsenter: cannot open /proc/22646/ns/net: No such file or directory 1) A first StopPodSandbox() request triggered by SyncLoop(PLEG) for a ContainerDied event calls into TearDownPod() and thus the network plugin. Until this completes, networkReady=true for the sandbox. 2) A second StopPodSandbox() request triggered by SyncLoop(REMOVE) calls PodSandboxStatus() and calls into the network plugin to read the IP address because networkReady=true 3) The first request exits the network plugin, sets networReady=false, and calls StopContainer() on the sandbox. This destroys the network namespace. 4) The second request finally gets around to running nsenter but the network namespace is already destroyed. It returns an error which is logged by getIP(). --- .../pkg/kubelet/dockershim/docker_sandbox.go | 20 ++++++++++++------- .../kubelet/dockershim/docker_sandbox_test.go | 3 --- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/vendor/k8s.io/kubernetes/pkg/kubelet/dockershim/docker_sandbox.go b/vendor/k8s.io/kubernetes/pkg/kubelet/dockershim/docker_sandbox.go index b595e310096b..cbea165f529f 100644 --- a/vendor/k8s.io/kubernetes/pkg/kubelet/dockershim/docker_sandbox.go +++ b/vendor/k8s.io/kubernetes/pkg/kubelet/dockershim/docker_sandbox.go @@ -177,7 +177,7 @@ func (ds *dockerService) StopPodSandbox(podSandboxID string) error { var checkpointErr, statusErr error // Try to retrieve sandbox information from docker daemon or sandbox checkpoint - status, statusErr := ds.PodSandboxStatus(podSandboxID) + status, statusErr := ds.internalPodSandboxStatus(podSandboxID, false) if statusErr == nil { nsOpts := status.GetLinux().GetNamespaces().GetOptions() hostNetwork = nsOpts != nil && nsOpts.HostNetwork @@ -341,8 +341,7 @@ func (ds *dockerService) getIP(podSandboxID string, sandbox *dockertypes.Contain return "" } -// PodSandboxStatus returns the status of the PodSandbox. -func (ds *dockerService) PodSandboxStatus(podSandboxID string) (*runtimeapi.PodSandboxStatus, error) { +func (ds *dockerService) internalPodSandboxStatus(podSandboxID string, checkIP bool) (*runtimeapi.PodSandboxStatus, error) { // Inspect the container. r, err := ds.client.InspectContainer(podSandboxID) if err != nil { @@ -363,10 +362,12 @@ func (ds *dockerService) PodSandboxStatus(podSandboxID string) (*runtimeapi.PodS } var IP string - // TODO: Remove this when sandbox is available on windows - // This is a workaround for windows, where sandbox is not in use, and pod IP is determined through containers belonging to the Pod. - if IP = ds.determinePodIPBySandboxID(podSandboxID); IP == "" { - IP = ds.getIP(podSandboxID, r) + if checkIP { + // TODO: Remove this when sandbox is available on windows + // This is a workaround for windows, where sandbox is not in use, and pod IP is determined through containers belonging to the Pod. + if IP = ds.determinePodIPBySandboxID(podSandboxID); IP == "" { + IP = ds.getIP(podSandboxID, r) + } } hostNetwork := sharesHostNetwork(r) @@ -397,6 +398,11 @@ func (ds *dockerService) PodSandboxStatus(podSandboxID string) (*runtimeapi.PodS }, nil } +// PodSandboxStatus returns the status of the PodSandbox. +func (ds *dockerService) PodSandboxStatus(podSandboxID string) (*runtimeapi.PodSandboxStatus, error) { + return ds.internalPodSandboxStatus(podSandboxID, true) +} + // ListPodSandbox returns a list of Sandbox. func (ds *dockerService) ListPodSandbox(filter *runtimeapi.PodSandboxFilter) ([]*runtimeapi.PodSandbox, error) { // By default, list all containers whether they are running or not. diff --git a/vendor/k8s.io/kubernetes/pkg/kubelet/dockershim/docker_sandbox_test.go b/vendor/k8s.io/kubernetes/pkg/kubelet/dockershim/docker_sandbox_test.go index 2c30e09fa1c9..6a3ef2316f26 100644 --- a/vendor/k8s.io/kubernetes/pkg/kubelet/dockershim/docker_sandbox_test.go +++ b/vendor/k8s.io/kubernetes/pkg/kubelet/dockershim/docker_sandbox_test.go @@ -207,9 +207,6 @@ func TestNetworkPluginInvocation(t *testing.T) { mockPlugin.EXPECT().Name().Return("mockNetworkPlugin").AnyTimes() setup := mockPlugin.EXPECT().SetUpPod(ns, name, cID) - // StopPodSandbox performs a lookup on status to figure out if the sandbox - // is running with hostnetworking, as all its given is the ID. - mockPlugin.EXPECT().GetPodNetworkStatus(ns, name, cID) mockPlugin.EXPECT().TearDownPod(ns, name, cID).After(setup) _, err := ds.RunPodSandbox(c)