diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index bbe6b86f0b..ba397f0fbf 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -513,7 +513,7 @@ func GetDockerPodInfo(client DockerInterface, manifest api.PodSpec, podFullName if len(info) < (len(manifest.Containers) + 1) { var containerStatus api.ContainerStatus - // Not all containers expected are created, verify if there are + // Not all containers expected are created, check if there are // image related issues for _, container := range manifest.Containers { if _, found := info[container.Name]; found { diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index e0d1405d27..e47bc1e2df 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1415,19 +1415,33 @@ func (kl *Kubelet) GetDockerVersion() ([]uint, error) { // TODO: this method is returning logs of random container attempts, when it should be returning the most recent attempt // or all of them. func (kl *Kubelet) GetKubeletContainerLogs(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error { - _, err := kl.GetPodStatus(podFullName, "") - if err == dockertools.ErrNoContainersInPod { - return fmt.Errorf("pod not found (%q)\n", podFullName) - } - dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient, true) + podStatus, err := kl.GetPodStatus(podFullName, "") if err != nil { - return err + if err == dockertools.ErrNoContainersInPod { + return fmt.Errorf("pod %q not found\n", podFullName) + } else { + return fmt.Errorf("failed to get status for pod %q - %v", podFullName, err) + } } - dockerContainer, found, _ := dockerContainers.FindPodContainer(podFullName, "", containerName) - if !found { - return fmt.Errorf("container not found (%q)\n", containerName) + if podStatus.Phase != api.PodRunning { + return fmt.Errorf("pod %q is not in 'Running' state - State: %q", podFullName, podStatus.Phase) } - return dockertools.GetKubeletDockerContainerLogs(kl.dockerClient, dockerContainer.ID, tail, follow, stdout, stderr) + 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) + } + + return dockertools.GetKubeletDockerContainerLogs(kl.dockerClient, dockerContainerID, tail, follow, stdout, stderr) } // GetBoundPods returns all pods bound to the kubelet and their spec diff --git a/pkg/kubelet/server.go b/pkg/kubelet/server.go index 35953a15bf..5f08674f97 100644 --- a/pkg/kubelet/server.go +++ b/pkg/kubelet/server.go @@ -185,7 +185,18 @@ func (s *Server) handleContainerLogs(w http.ResponseWriter, req *http.Request) { pod, ok := s.host.GetPodByName(podNamespace, podID) if !ok { - http.Error(w, "Pod does not exist", http.StatusNotFound) + http.Error(w, fmt.Sprintf("Pod %q does not exist", podID), http.StatusNotFound) + return + } + // Check if containerName is valid. + containerExists := false + for _, container := range pod.Spec.Containers { + if container.Name == containerName { + containerExists = true + } + } + if !containerExists { + http.Error(w, fmt.Sprintf("Container %q not found in Pod %q", containerName, podID), http.StatusNotFound) return } diff --git a/pkg/kubelet/server_test.go b/pkg/kubelet/server_test.go index 32c5f9967e..710949c3e2 100644 --- a/pkg/kubelet/server_test.go +++ b/pkg/kubelet/server_test.go @@ -380,15 +380,28 @@ func TestServeRunInContainerWithUID(t *testing.T) { } } -func TestContainerLogs(t *testing.T) { - fw := newServerTest() - output := "foo bar" - podNamespace := "other" - podName := "foo" - expectedPodName := podName + ".other.etcd" - expectedContainerName := "baz" - expectedTail := "" - expectedFollow := false +func setPodByNameFunc(fw *serverTestFramework, namespace, pod, container string) { + fw.fakeKubelet.podByNameFunc = func(namespace, name string) (*api.BoundPod, bool) { + return &api.BoundPod{ + ObjectMeta: api.ObjectMeta{ + Namespace: namespace, + Name: pod, + Annotations: map[string]string{ + ConfigSourceAnnotationKey: "etcd", + }, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: container, + }, + }, + }, + }, true + } +} + +func setGetContainerLogsFunc(fw *serverTestFramework, t *testing.T, expectedPodName, expectedContainerName, expectedTail string, expectedFollow bool, output string) { fw.fakeKubelet.containerLogsFunc = func(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error { if podFullName != expectedPodName { t.Errorf("expected %s, got %s", expectedPodName, podFullName) @@ -402,8 +415,22 @@ func TestContainerLogs(t *testing.T) { if follow != expectedFollow { t.Errorf("expected %t, got %t", expectedFollow, follow) } + io.WriteString(stdout, output) return nil } +} + +func TestContainerLogs(t *testing.T) { + fw := newServerTest() + output := "foo bar" + podNamespace := "other" + podName := "foo" + expectedPodName := podName + ".other.etcd" + expectedContainerName := "baz" + expectedTail := "" + expectedFollow := false + setPodByNameFunc(fw, podNamespace, podName, expectedContainerName) + setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, output) resp, err := http.Get(fw.testHTTPServer.URL + "/containerLogs/" + podNamespace + "/" + podName + "/" + expectedContainerName) if err != nil { t.Errorf("Got error GETing: %v", err) @@ -415,7 +442,7 @@ func TestContainerLogs(t *testing.T) { t.Errorf("Error reading container logs: %v", err) } result := string(body) - if result != string(body) { + if result != output { t.Errorf("Expected: '%v', got: '%v'", output, result) } } @@ -429,21 +456,8 @@ func TestContainerLogsWithTail(t *testing.T) { expectedContainerName := "baz" expectedTail := "5" expectedFollow := false - fw.fakeKubelet.containerLogsFunc = func(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error { - if podFullName != expectedPodName { - t.Errorf("expected %s, got %s", expectedPodName, podFullName) - } - if containerName != expectedContainerName { - t.Errorf("expected %s, got %s", expectedContainerName, containerName) - } - if tail != expectedTail { - t.Errorf("expected %s, got %s", expectedTail, tail) - } - if follow != expectedFollow { - t.Errorf("expected %t, got %t", expectedFollow, follow) - } - return nil - } + setPodByNameFunc(fw, podNamespace, podName, expectedContainerName) + setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, output) resp, err := http.Get(fw.testHTTPServer.URL + "/containerLogs/" + podNamespace + "/" + podName + "/" + expectedContainerName + "?tail=5") if err != nil { t.Errorf("Got error GETing: %v", err) @@ -455,7 +469,7 @@ func TestContainerLogsWithTail(t *testing.T) { t.Errorf("Error reading container logs: %v", err) } result := string(body) - if result != string(body) { + if result != output { t.Errorf("Expected: '%v', got: '%v'", output, result) } } @@ -469,21 +483,8 @@ func TestContainerLogsWithFollow(t *testing.T) { expectedContainerName := "baz" expectedTail := "" expectedFollow := true - fw.fakeKubelet.containerLogsFunc = func(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error { - if podFullName != expectedPodName { - t.Errorf("expected %s, got %s", expectedPodName, podFullName) - } - if containerName != expectedContainerName { - t.Errorf("expected %s, got %s", expectedContainerName, containerName) - } - if tail != expectedTail { - t.Errorf("expected %s, got %s", expectedTail, tail) - } - if follow != expectedFollow { - t.Errorf("expected %t, got %t", expectedFollow, follow) - } - return nil - } + setPodByNameFunc(fw, podNamespace, podName, expectedContainerName) + setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, output) resp, err := http.Get(fw.testHTTPServer.URL + "/containerLogs/" + podNamespace + "/" + podName + "/" + expectedContainerName + "?follow=1") if err != nil { t.Errorf("Got error GETing: %v", err) @@ -495,7 +496,7 @@ func TestContainerLogsWithFollow(t *testing.T) { t.Errorf("Error reading container logs: %v", err) } result := string(body) - if result != string(body) { + if result != output { t.Errorf("Expected: '%v', got: '%v'", output, result) } }