Merge pull request #4508 from bprashanth/handle_stats_500

Cadvisor interface returns richer errors to the kubelet server #4127

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).

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, ErrCadvisorApiFailure)
-	mockCadvisor.On("DockerContainer", containerID, cadvisorReq).Return(containerInfo, expectedErr)
 
 	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() {
+	if err.Error() != ErrCadvisorApiFailure.Error() {
-		t.Errorf("wrong error message. expect %v, got %v", err, expectedErr)
+		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

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
 	}

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{}