diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index f01a92ce78a..3091817a4dd 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1538,6 +1538,26 @@ func (kl *Kubelet) GetDockerVersion() ([]uint, error) { return dockerRunner.GetDockerServerVersion() } +func (kl *Kubelet) validatePodPhase(podStatus *api.PodStatus) error { + switch podStatus.Phase { + case api.PodRunning, api.PodSucceeded, api.PodFailed: + return nil + } + return fmt.Errorf("pod is not in 'Running', 'Succeeded' or 'Failed' state - State: %q", podStatus.Phase) +} + +func (kl *Kubelet) validateContainerStatus(podStatus *api.PodStatus, containerName string) (dockerID string, err error) { + for cName, cStatus := range podStatus.Info { + if containerName == cName { + if cStatus.State.Waiting != nil { + return "", fmt.Errorf("container %q is in waiting state.", containerName) + } + return strings.Replace(podStatus.Info[containerName].ContainerID, dockertools.DockerPrefix, "", 1), nil + } + } + return "", fmt.Errorf("container %q not found in pod", containerName) +} + // GetKubeletContainerLogs returns logs from the container // The second parameter of GetPodStatus and FindPodContainer methods represents pod UUID, which is allowed to be blank // TODO: this method is returning logs of random container attempts, when it should be returning the most recent attempt @@ -1551,27 +1571,14 @@ func (kl *Kubelet) GetKubeletContainerLogs(podFullName, containerName, tail stri return fmt.Errorf("failed to get status for pod %q - %v", podFullName, err) } } - switch podStatus.Phase { - case api.PodRunning, api.PodSucceeded, api.PodFailed: - break - default: - return fmt.Errorf("pod %q is not in 'Running', 'Succeeded' or 'Failed' state - State: %q", podFullName, podStatus.Phase) - } - exists := false - dockerContainerID := "" - for cName, cStatus := range podStatus.Info { - if containerName == cName { - exists = true - if !cStatus.Ready { - return fmt.Errorf("container %q is not ready.", containerName) - } - dockerContainerID = strings.Replace(podStatus.Info[containerName].ContainerID, dockertools.DockerPrefix, "", 1) - } - } - if !exists { - return fmt.Errorf("container %q not found in pod %q", containerName, podFullName) - } + if err := kl.validatePodPhase(&podStatus); err != nil { + return err + } + dockerContainerID, err := kl.validateContainerStatus(&podStatus, containerName) + if err != nil { + return err + } return dockertools.GetKubeletDockerContainerLogs(kl.dockerClient, dockerContainerID, tail, follow, stdout, stderr) } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 9606ff4489b..64d66276a47 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -3122,5 +3122,69 @@ func TestFilterHostPortConflicts(t *testing.T) { if filteredPods[0].Name != "oldpod" { t.Fatalf("Expected pod %#v. Got pod %#v", pods[1], filteredPods[0]) } - +} + +func TestValidatePodStatus(t *testing.T) { + kubelet, _, _ := newTestKubelet(t) + testCases := []struct { + podPhase api.PodPhase + success bool + }{ + {api.PodRunning, true}, + {api.PodSucceeded, true}, + {api.PodFailed, true}, + {api.PodPending, false}, + {api.PodUnknown, false}, + } + + for i, tc := range testCases { + err := kubelet.validatePodPhase(&api.PodStatus{Phase: tc.podPhase}) + if tc.success { + if err != nil { + t.Errorf("[case %d]: unexpected failure - %v", i, err) + } + } else if err == nil { + t.Errorf("[case %d]: unexpected success", i) + } + } +} + +func TestValidateContainerStatus(t *testing.T) { + kubelet, _, _ := newTestKubelet(t) + containerName := "x" + testCases := []struct { + podInfo api.PodInfo + success bool + }{ + { + podInfo: api.PodInfo{containerName: api.ContainerStatus{State: api.ContainerState{Running: &api.ContainerStateRunning{}}}}, + success: true, + }, + { + podInfo: api.PodInfo{containerName: api.ContainerStatus{State: api.ContainerState{Termination: &api.ContainerStateTerminated{}}}}, + success: true, + }, + { + podInfo: api.PodInfo{containerName: api.ContainerStatus{State: api.ContainerState{Waiting: &api.ContainerStateWaiting{}}}}, + success: false, + }, + } + + for i, tc := range testCases { + _, err := kubelet.validateContainerStatus(&api.PodStatus{ + Info: tc.podInfo, + }, containerName) + if tc.success { + if err != nil { + t.Errorf("[case %d]: unexpected failure - %v", i, err) + } + } else if err == nil { + t.Errorf("[case %d]: unexpected success", i) + } + } + if _, err := kubelet.validateContainerStatus(&api.PodStatus{ + Info: testCases[0].podInfo, + }, "blah"); err == nil { + t.Errorf("expected error with invalid container name") + } }