From 31cdd70808f364972150975a668b6996f96f3c2c Mon Sep 17 00:00:00 2001 From: Michal Rostecki Date: Fri, 5 Aug 2016 14:25:03 +0200 Subject: [PATCH] Set pod state as "unknown" when CNI plugin fails Before this change, CNI plugin failure didn't change anything in the pod status, so pods having containers without requested network were "running". Fixes #29148 --- pkg/kubelet/dockertools/docker_manager.go | 19 ++- .../dockertools/docker_manager_test.go | 131 ++++++++++++------ 2 files changed, 107 insertions(+), 43 deletions(-) diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index c55d3e6cc46..bf195884264 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/pkg/kubelet/dockertools/docker_manager.go @@ -332,7 +332,7 @@ var ( // determineContainerIP determines the IP address of the given container. It is expected // that the container passed is the infrastructure container of a pod and the responsibility // of the caller to ensure that the correct container is passed. -func (dm *DockerManager) determineContainerIP(podNamespace, podName string, container *dockertypes.ContainerJSON) string { +func (dm *DockerManager) determineContainerIP(podNamespace, podName string, container *dockertypes.ContainerJSON) (string, error) { result := "" if container.NetworkSettings != nil { @@ -348,12 +348,13 @@ func (dm *DockerManager) determineContainerIP(podNamespace, podName string, cont netStatus, err := dm.networkPlugin.GetPodNetworkStatus(podNamespace, podName, kubecontainer.DockerID(container.ID).ContainerID()) if err != nil { glog.Errorf("NetworkPlugin %s failed on the status hook for pod '%s' - %v", dm.networkPlugin.Name(), podName, err) + return result, err } else if netStatus != nil { result = netStatus.IP.String() } } - return result + return result, nil } func (dm *DockerManager) inspectContainer(id string, podName, podNamespace string) (*kubecontainer.ContainerStatus, string, error) { @@ -404,7 +405,12 @@ func (dm *DockerManager) inspectContainer(id string, podName, podNamespace strin status.State = kubecontainer.ContainerStateRunning status.StartedAt = startedAt if containerName == PodInfraContainerName { - ip = dm.determineContainerIP(podNamespace, podName, iResult) + ip, err = dm.determineContainerIP(podNamespace, podName, iResult) + // Kubelet doesn't handle the network error scenario + if err != nil { + status.State = kubecontainer.ContainerStateUnknown + status.Message = fmt.Sprintf("Network error: %#v", err) + } } return &status, ip, nil } @@ -2049,7 +2055,12 @@ func (dm *DockerManager) SyncPod(pod *api.Pod, _ api.PodStatus, podStatus *kubec } // Overwrite the podIP passed in the pod status, since we just started the infra container. - podIP = dm.determineContainerIP(pod.Namespace, pod.Name, podInfraContainer) + podIP, err = dm.determineContainerIP(pod.Namespace, pod.Name, podInfraContainer) + if err != nil { + glog.Errorf("Network error: %v; Skipping pod %q", err, format.Pod(pod)) + result.Fail(err) + return + } glog.V(4).Infof("Determined pod ip after infra change: %q: %q", format.Pod(pod), podIP) } } diff --git a/pkg/kubelet/dockertools/docker_manager_test.go b/pkg/kubelet/dockertools/docker_manager_test.go index 8032019652e..86f9d97029f 100644 --- a/pkg/kubelet/dockertools/docker_manager_test.go +++ b/pkg/kubelet/dockertools/docker_manager_test.go @@ -2248,50 +2248,103 @@ func TestPruneInitContainers(t *testing.T) { } func TestGetPodStatusFromNetworkPlugin(t *testing.T) { - const ( - containerID = "123" - infraContainerID = "9876" - fakePodIP = "10.10.10.10" - ) - dm, fakeDocker := newTestDockerManager() - ctrl := gomock.NewController(t) - defer ctrl.Finish() - fnp := mock_network.NewMockNetworkPlugin(ctrl) - dm.networkPlugin = fnp - - pod := &api.Pod{ - ObjectMeta: api.ObjectMeta{ - UID: "12345678", - Name: "foo", - Namespace: "new", - }, - Spec: api.PodSpec{ - Containers: []api.Container{{Name: "container"}}, - }, - } - - fakeDocker.SetFakeRunningContainers([]*FakeContainer{ + cases := []struct { + pod *api.Pod + fakePodIP string + containerID string + infraContainerID string + networkStatusError error + expectRunning bool + expectUnknown bool + }{ { - ID: containerID, - Name: "/k8s_container_foo_new_12345678_42", - Running: true, + pod: &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "foo", + Namespace: "new", + }, + Spec: api.PodSpec{ + Containers: []api.Container{{Name: "container"}}, + }, + }, + fakePodIP: "10.10.10.10", + containerID: "123", + infraContainerID: "9876", + networkStatusError: nil, + expectRunning: true, + expectUnknown: false, }, { - ID: infraContainerID, - Name: "/k8s_POD." + strconv.FormatUint(generatePodInfraContainerHash(pod), 16) + "_foo_new_12345678_42", - Running: true, + pod: &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "foo", + Namespace: "new", + }, + Spec: api.PodSpec{ + Containers: []api.Container{{Name: "container"}}, + }, + }, + fakePodIP: "", + containerID: "123", + infraContainerID: "9876", + networkStatusError: fmt.Errorf("CNI plugin error"), + expectRunning: false, + expectUnknown: true, }, - }) - - fnp.EXPECT().Name().Return("someNetworkPlugin") - fnp.EXPECT().GetPodNetworkStatus("new", "foo", kubecontainer.DockerID(infraContainerID).ContainerID()).Return(&network.PodNetworkStatus{IP: net.ParseIP(fakePodIP)}, nil) - - podStatus, err := dm.GetPodStatus(pod.UID, pod.Name, pod.Namespace) - if err != nil { - t.Fatal(err) } - if podStatus.IP != fakePodIP { - t.Errorf("Got wrong ip, expected %v, got %v", fakePodIP, podStatus.IP) + for _, test := range cases { + dm, fakeDocker := newTestDockerManager() + ctrl := gomock.NewController(t) + fnp := mock_network.NewMockNetworkPlugin(ctrl) + dm.networkPlugin = fnp + + fakeDocker.SetFakeRunningContainers([]*FakeContainer{ + { + ID: test.containerID, + Name: fmt.Sprintf("/k8s_container_%s_%s_%s_42", test.pod.Name, test.pod.Namespace, test.pod.UID), + Running: true, + }, + { + ID: test.infraContainerID, + Name: fmt.Sprintf("/k8s_POD.%s_%s_%s_%s_42", strconv.FormatUint(generatePodInfraContainerHash(test.pod), 16), test.pod.Name, test.pod.Namespace, test.pod.UID), + Running: true, + }, + }) + + fnp.EXPECT().Name().Return("someNetworkPlugin").AnyTimes() + var podNetworkStatus *network.PodNetworkStatus + if test.fakePodIP != "" { + podNetworkStatus = &network.PodNetworkStatus{IP: net.ParseIP(test.fakePodIP)} + } + fnp.EXPECT().GetPodNetworkStatus(test.pod.Namespace, test.pod.Name, kubecontainer.DockerID(test.infraContainerID).ContainerID()).Return(podNetworkStatus, test.networkStatusError) + + podStatus, err := dm.GetPodStatus(test.pod.UID, test.pod.Name, test.pod.Namespace) + if err != nil { + t.Fatal(err) + } + if podStatus.IP != test.fakePodIP { + t.Errorf("Got wrong ip, expected %v, got %v", test.fakePodIP, podStatus.IP) + } + + expectedStatesCount := 0 + var expectedState kubecontainer.ContainerState + if test.expectRunning { + expectedState = kubecontainer.ContainerStateRunning + } else if test.expectUnknown { + expectedState = kubecontainer.ContainerStateUnknown + } else { + t.Errorf("Some state has to be expected") + } + for _, containerStatus := range podStatus.ContainerStatuses { + if containerStatus.State == expectedState { + expectedStatesCount++ + } + } + if expectedStatesCount < 1 { + t.Errorf("Invalid count of containers with expected state") + } } }