From 60a955d414955cdede16d911d9562e43925697c7 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 2 Feb 2018 22:52:31 -0600 Subject: [PATCH] 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 | 38 +++++++++++-------- pkg/kubelet/dockershim/docker_sandbox_test.go | 3 -- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index b314969a6c5..e4d26b47c03 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/pkg/kubelet/dockershim/docker_sandbox.go @@ -178,22 +178,18 @@ func (ds *dockerService) RunPodSandbox(ctx context.Context, r *runtimeapi.RunPod func (ds *dockerService) StopPodSandbox(ctx context.Context, r *runtimeapi.StopPodSandboxRequest) (*runtimeapi.StopPodSandboxResponse, error) { var namespace, name string var hostNetwork bool - var checkpointErr, statusErr error podSandboxID := r.PodSandboxId resp := &runtimeapi.StopPodSandboxResponse{} - // Try to retrieve sandbox information from docker daemon or sandbox checkpoint - statusResp, statusErr := ds.PodSandboxStatus(ctx, &runtimeapi.PodSandboxStatusRequest{PodSandboxId: podSandboxID}) - status := statusResp.GetStatus() + // Try to retrieve minimal sandbox information from docker daemon or sandbox checkpoint. + inspectResult, metadata, statusErr := ds.getPodSandboxDetails(podSandboxID) if statusErr == nil { - hostNetwork = status.GetLinux().GetNamespaces().GetOptions().GetNetwork() == runtimeapi.NamespaceMode_NODE - m := status.GetMetadata() - namespace = m.Namespace - name = m.Name + namespace = metadata.Namespace + name = metadata.Name + hostNetwork = (networkNamespaceMode(inspectResult) == runtimeapi.NamespaceMode_NODE) } else { - var checkpoint *PodSandboxCheckpoint - checkpoint, checkpointErr = ds.checkpointHandler.GetCheckpoint(podSandboxID) + checkpoint, checkpointErr := ds.checkpointHandler.GetCheckpoint(podSandboxID) // Proceed if both sandbox container and checkpoint could not be found. This means that following // actions will only have sandbox ID and not have pod namespace and name information. @@ -358,12 +354,26 @@ func (ds *dockerService) getIP(podSandboxID string, sandbox *dockertypes.Contain return "" } +// Returns the inspect container response, the sandbox metadata, and network namespace mode +func (ds *dockerService) getPodSandboxDetails(podSandboxID string) (*dockertypes.ContainerJSON, *runtimeapi.PodSandboxMetadata, error) { + resp, err := ds.client.InspectContainer(podSandboxID) + if err != nil { + return nil, nil, err + } + + metadata, err := parseSandboxName(resp.Name) + if err != nil { + return nil, nil, err + } + + return resp, metadata, nil +} + // PodSandboxStatus returns the status of the PodSandbox. func (ds *dockerService) PodSandboxStatus(ctx context.Context, req *runtimeapi.PodSandboxStatusRequest) (*runtimeapi.PodSandboxStatusResponse, error) { podSandboxID := req.PodSandboxId - // Inspect the container. - r, err := ds.client.InspectContainer(podSandboxID) + r, metadata, err := ds.getPodSandboxDetails(podSandboxID) if err != nil { return nil, err } @@ -388,10 +398,6 @@ func (ds *dockerService) PodSandboxStatus(ctx context.Context, req *runtimeapi.P IP = ds.getIP(podSandboxID, r) } - metadata, err := parseSandboxName(r.Name) - if err != nil { - return nil, err - } labels, annotations := extractLabels(r.Config.Labels) status := &runtimeapi.PodSandboxStatus{ Id: r.ID, diff --git a/pkg/kubelet/dockershim/docker_sandbox_test.go b/pkg/kubelet/dockershim/docker_sandbox_test.go index a9c55a1104b..05fce81c1c1 100644 --- a/pkg/kubelet/dockershim/docker_sandbox_test.go +++ b/pkg/kubelet/dockershim/docker_sandbox_test.go @@ -222,9 +222,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(getTestCTX(), &runtimeapi.RunPodSandboxRequest{Config: c})