diff --git a/pkg/kubelet/cadvisor.go b/pkg/kubelet/cadvisor.go index e784be6977..c60bec0ba6 100644 --- a/pkg/kubelet/cadvisor.go +++ b/pkg/kubelet/cadvisor.go @@ -17,6 +17,7 @@ limitations under the License. package kubelet import ( + "errors" "fmt" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools" @@ -24,6 +25,17 @@ import ( cadvisor "github.com/google/cadvisor/info" ) +var ( + // ErrNoKubeletContainers returned when there are not containers managed by the kubelet (ie: either no containers on the node, or none that the kubelet cares about). + ErrNoKubeletContainers = errors.New("No containers managed by kubelet") + + // ErrContainerNotFound returned when a container in the given pod with the given container name was not found, amongst those managed by the kubelet. + ErrContainerNotFound = errors.New("No matching container") + + // ErrCadvisorApiFailure returned when cadvisor couldn't retrieve stats for the given container, either because it isn't running or it was confused by the request + ErrCadvisorApiFailure = errors.New("Failed to retrieve cadvisor stats") +) + // cadvisorInterface is an abstract interface for testability. It abstracts the interface of "github.com/google/cadvisor/client".Client. type cadvisorInterface interface { DockerContainer(name string, req *cadvisor.ContainerInfoRequest) (cadvisor.ContainerInfo, error) @@ -63,11 +75,19 @@ func (kl *Kubelet) GetContainerInfo(podFullName string, uid types.UID, container if err != nil { return nil, err } + if len(dockerContainers) == 0 { + return nil, ErrNoKubeletContainers + } dockerContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uid, containerName) if !found { - return nil, fmt.Errorf("couldn't find container") + return nil, ErrContainerNotFound } - return kl.statsFromDockerContainer(cc, dockerContainer.ID, req) + + ci, err := kl.statsFromDockerContainer(cc, dockerContainer.ID, req) + if err != nil { + return nil, ErrCadvisorApiFailure + } + return ci, nil } // GetRootInfo returns stats (from Cadvisor) of current machine (root container). diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 90c11038f1..7c0a2c92cb 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -1265,6 +1265,16 @@ type mockCadvisorClient struct { mock.Mock } +type errorTestingDockerClient struct { + dockertools.FakeDockerClient + listContainersError error + containerList []docker.APIContainers +} + +func (f *errorTestingDockerClient) ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error) { + return f.containerList, f.listContainersError +} + // ContainerInfo is a mock implementation of CadvisorInterface.ContainerInfo. func (c *mockCadvisorClient) ContainerInfo(name string, req *info.ContainerInfoRequest) (*info.ContainerInfo, error) { args := c.Called(name, req) @@ -1369,8 +1379,7 @@ func TestGetContainerInfoWhenCadvisorFailed(t *testing.T) { containerInfo := info.ContainerInfo{} mockCadvisor := &mockCadvisorClient{} cadvisorReq := &info.ContainerInfoRequest{} - expectedErr := fmt.Errorf("some error") - mockCadvisor.On("DockerContainer", containerID, cadvisorReq).Return(containerInfo, expectedErr) + mockCadvisor.On("DockerContainer", containerID, cadvisorReq).Return(containerInfo, ErrCadvisorApiFailure) kubelet, fakeDocker := newTestKubelet(t) kubelet.cadvisorClient = mockCadvisor @@ -1391,8 +1400,8 @@ func TestGetContainerInfoWhenCadvisorFailed(t *testing.T) { t.Errorf("expect error but received nil error") return } - if err.Error() != expectedErr.Error() { - t.Errorf("wrong error message. expect %v, got %v", err, expectedErr) + if err.Error() != ErrCadvisorApiFailure.Error() { + t.Errorf("wrong error message. expect %v, got %v", ErrCadvisorApiFailure, err) } mockCadvisor.AssertExpectations(t) } @@ -1411,6 +1420,71 @@ func TestGetContainerInfoOnNonExistContainer(t *testing.T) { mockCadvisor.AssertExpectations(t) } +func TestGetContainerInfoWhenDockerToolsFailed(t *testing.T) { + mockCadvisor := &mockCadvisorClient{} + + kubelet, _ := newTestKubelet(t) + kubelet.cadvisorClient = mockCadvisor + expectedErr := fmt.Errorf("List containers error") + kubelet.dockerClient = &errorTestingDockerClient{listContainersError: expectedErr} + + stats, err := kubelet.GetContainerInfo("qux", "", "foo", nil) + if err == nil { + t.Errorf("Expected error from dockertools, got none") + } + if err.Error() != expectedErr.Error() { + t.Errorf("Expected error %v got %v", expectedErr.Error(), err.Error()) + } + if stats != nil { + t.Errorf("non-nil stats when dockertools failed") + } +} + +func TestGetContainerInfoWithNoContainers(t *testing.T) { + mockCadvisor := &mockCadvisorClient{} + + kubelet, _ := newTestKubelet(t) + kubelet.cadvisorClient = mockCadvisor + + kubelet.dockerClient = &errorTestingDockerClient{listContainersError: nil} + stats, err := kubelet.GetContainerInfo("qux", "", "foo", nil) + if err == nil { + t.Errorf("Expected error from cadvisor client, got none") + } + if err != ErrNoKubeletContainers { + t.Errorf("Expected error %v, got %v", ErrNoKubeletContainers.Error(), err.Error()) + } + if stats != nil { + t.Errorf("non-nil stats when dockertools returned no containers") + } +} + +func TestGetContainerInfoWithNoMatchingContainers(t *testing.T) { + mockCadvisor := &mockCadvisorClient{} + + kubelet, _ := newTestKubelet(t) + kubelet.cadvisorClient = mockCadvisor + + containerList := []docker.APIContainers{ + { + ID: "fakeId", + Names: []string{"/k8s_bar_qux_1234_42"}, + }, + } + + kubelet.dockerClient = &errorTestingDockerClient{listContainersError: nil, containerList: containerList} + stats, err := kubelet.GetContainerInfo("qux", "", "foo", nil) + if err == nil { + t.Errorf("Expected error from cadvisor client, got none") + } + if err != ErrContainerNotFound { + t.Errorf("Expected error %v, got %v", ErrContainerNotFound.Error(), err.Error()) + } + if stats != nil { + t.Errorf("non-nil stats when dockertools returned no containers") + } +} + type fakeContainerCommandRunner struct { Cmd []string ID string diff --git a/pkg/kubelet/server.go b/pkg/kubelet/server.go index 5f08674f97..475453050f 100644 --- a/pkg/kubelet/server.go +++ b/pkg/kubelet/server.go @@ -400,7 +400,13 @@ func (s *Server) serveStats(w http.ResponseWriter, req *http.Request) { http.Error(w, "unknown resource.", http.StatusNotFound) return } - if err != nil { + switch err { + case nil: + break + case ErrNoKubeletContainers, ErrContainerNotFound: + http.Error(w, err.Error(), http.StatusNotFound) + return + default: s.error(w, err) return } diff --git a/pkg/kubelet/server_test.go b/pkg/kubelet/server_test.go index 710949c3e2..f13740bfec 100644 --- a/pkg/kubelet/server_test.go +++ b/pkg/kubelet/server_test.go @@ -221,6 +221,25 @@ func TestContainerInfoWithUidNamespace(t *testing.T) { } } +func TestContainerNotFound(t *testing.T) { + fw := newServerTest() + podID := "somepod" + expectedNamespace := "custom" + expectedContainerName := "slowstartcontainer" + expectedUid := "9b01b80f-8fb4-11e4-95ab-4200af06647" + fw.fakeKubelet.containerInfoFunc = func(podID string, uid types.UID, containerName string, req *info.ContainerInfoRequest) (*info.ContainerInfo, error) { + return nil, ErrContainerNotFound + } + resp, err := http.Get(fw.testHTTPServer.URL + fmt.Sprintf("/stats/%v/%v/%v/%v", expectedNamespace, podID, expectedUid, expectedContainerName)) + if err != nil { + t.Fatalf("Got error GETing: %v", err) + } + if resp.StatusCode != http.StatusNotFound { + t.Fatalf("Received status %d expecting %d", resp.StatusCode, http.StatusNotFound) + } + defer resp.Body.Close() +} + func TestRootInfo(t *testing.T) { fw := newServerTest() expectedInfo := &info.ContainerInfo{}