Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubelet: Make cadvisor GetContainerInfo tests table driven #34931

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
324 changes: 169 additions & 155 deletions pkg/kubelet/kubelet_cadvisor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,47 +22,186 @@ import (

cadvisorapi "github.com/google/cadvisor/info/v1"
cadvisorapiv2 "github.com/google/cadvisor/info/v2"
"k8s.io/apimachinery/pkg/types"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
kubecontainertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
)

func TestGetContainerInfo(t *testing.T) {
containerID := "ab2cdf"
containerPath := fmt.Sprintf("/docker/%v", containerID)
containerInfo := cadvisorapi.ContainerInfo{
ContainerReference: cadvisorapi.ContainerReference{
Name: containerPath,
cadvisorApiFailure := fmt.Errorf("cAdvisor failure")
runtimeError := fmt.Errorf("List containers error")
tests := []struct {
name string
containerID string
containerPath string
cadvisorContainerInfo cadvisorapi.ContainerInfo
runtimeError error
podList []*kubecontainertest.FakePod
requestedPodFullName string
requestedPodUid types.UID
requestedContainerName string
expectDockerContainerCall bool
mockError error
expectedError error
expectStats bool
}{
{
name: "get container info",
containerID: "ab2cdf",
containerPath: "/docker/ab2cdf",
cadvisorContainerInfo: cadvisorapi.ContainerInfo{
ContainerReference: cadvisorapi.ContainerReference{
Name: "/docker/ab2cdf",
},
},
runtimeError: nil,
podList: []*kubecontainertest.FakePod{
{
Pod: &kubecontainer.Pod{
ID: "12345678",
Name: "qux",
Namespace: "ns",
Containers: []*kubecontainer.Container{
{
Name: "foo",
ID: kubecontainer.ContainerID{Type: "test", ID: "ab2cdf"},
},
},
},
},
},
requestedPodFullName: "qux_ns",
requestedPodUid: "",
requestedContainerName: "foo",
expectDockerContainerCall: true,
mockError: nil,
expectedError: nil,
expectStats: true,
},
}

testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
defer testKubelet.Cleanup()
fakeRuntime := testKubelet.fakeRuntime
kubelet := testKubelet.kubelet
cadvisorReq := &cadvisorapi.ContainerInfoRequest{}
mockCadvisor := testKubelet.fakeCadvisor
mockCadvisor.On("DockerContainer", containerID, cadvisorReq).Return(containerInfo, nil)
fakeRuntime.PodList = []*kubecontainertest.FakePod{
{Pod: &kubecontainer.Pod{
ID: "12345678",
Name: "qux",
Namespace: "ns",
Containers: []*kubecontainer.Container{
{
name: "get container info when cadvisor failed",
containerID: "ab2cdf",
containerPath: "/docker/ab2cdf",
cadvisorContainerInfo: cadvisorapi.ContainerInfo{},
runtimeError: nil,
podList: []*kubecontainertest.FakePod{
{
Name: "foo",
ID: kubecontainer.ContainerID{Type: "test", ID: containerID},
Pod: &kubecontainer.Pod{
ID: "uuid",
Name: "qux",
Namespace: "ns",
Containers: []*kubecontainer.Container{
{
Name: "foo",
ID: kubecontainer.ContainerID{Type: "test", ID: "ab2cdf"},
},
},
},
},
},
}},
}
stats, err := kubelet.GetContainerInfo("qux_ns", "", "foo", cadvisorReq)
if err != nil {
t.Errorf("unexpected error: %v", err)
requestedPodFullName: "qux_ns",
requestedPodUid: "uuid",
requestedContainerName: "foo",
expectDockerContainerCall: true,
mockError: cadvisorApiFailure,
expectedError: cadvisorApiFailure,
expectStats: false,
},
{
name: "get container info on non-existent container",
containerID: "",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: you don't need to specify default values ("", false, nil), so they can be omitted to shorten this. If you think it's more readable with all fields specified it's ok to leave them.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, I think that leaving them makes it more readable.

containerPath: "",
cadvisorContainerInfo: cadvisorapi.ContainerInfo{},
runtimeError: nil,
podList: []*kubecontainertest.FakePod{},
requestedPodFullName: "qux",
requestedPodUid: "",
requestedContainerName: "foo",
expectDockerContainerCall: false,
mockError: nil,
expectedError: kubecontainer.ErrContainerNotFound,
expectStats: false,
},
{
name: "get container info when container runtime failed",
containerID: "",
containerPath: "",
cadvisorContainerInfo: cadvisorapi.ContainerInfo{},
runtimeError: runtimeError,
podList: []*kubecontainertest.FakePod{},
requestedPodFullName: "qux",
requestedPodUid: "",
requestedContainerName: "foo",
mockError: nil,
expectedError: runtimeError,
expectStats: false,
},
{
name: "get container info with no containers",
containerID: "",
containerPath: "",
cadvisorContainerInfo: cadvisorapi.ContainerInfo{},
runtimeError: nil,
podList: []*kubecontainertest.FakePod{},
requestedPodFullName: "qux_ns",
requestedPodUid: "",
requestedContainerName: "foo",
mockError: nil,
expectedError: kubecontainer.ErrContainerNotFound,
expectStats: false,
},
{
name: "get container info with no matching containers",
containerID: "",
containerPath: "",
cadvisorContainerInfo: cadvisorapi.ContainerInfo{},
runtimeError: nil,
podList: []*kubecontainertest.FakePod{
{
Pod: &kubecontainer.Pod{
ID: "12345678",
Name: "qux",
Namespace: "ns",
Containers: []*kubecontainer.Container{
{
Name: "bar",
ID: kubecontainer.ContainerID{Type: "test", ID: "fakeID"},
},
},
},
},
},
requestedPodFullName: "qux_ns",
requestedPodUid: "",
requestedContainerName: "foo",
mockError: nil,
expectedError: kubecontainer.ErrContainerNotFound,
expectStats: false,
},
}
if stats == nil {
t.Fatalf("stats should not be nil")

for _, tc := range tests {
testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnablec */)
defer testKubelet.Cleanup()
fakeRuntime := testKubelet.fakeRuntime
kubelet := testKubelet.kubelet
cadvisorReq := &cadvisorapi.ContainerInfoRequest{}
mockCadvisor := testKubelet.fakeCadvisor
if tc.expectDockerContainerCall {
mockCadvisor.On("DockerContainer", tc.containerID, cadvisorReq).Return(tc.cadvisorContainerInfo, tc.mockError)
}
fakeRuntime.Err = tc.runtimeError
fakeRuntime.PodList = tc.podList

stats, err := kubelet.GetContainerInfo(tc.requestedPodFullName, tc.requestedPodUid, tc.requestedContainerName, cadvisorReq)
if err != tc.expectedError {
t.Errorf("test '%s' failed: expected error %#v, got %#v", tc.name, tc.expectedError, err)
}
if tc.expectStats && stats == nil {
t.Fatalf("test '%s' failed: stats should not be nil", tc.name)
}
mockCadvisor.AssertExpectations(t)
}
mockCadvisor.AssertExpectations(t)
}

func TestGetRawContainerInfoRoot(t *testing.T) {
Expand Down Expand Up @@ -117,131 +256,6 @@ func TestGetRawContainerInfoSubcontainers(t *testing.T) {
mockCadvisor.AssertExpectations(t)
}

func TestGetContainerInfoWhenCadvisorFailed(t *testing.T) {
containerID := "ab2cdf"
testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
defer testKubelet.Cleanup()
kubelet := testKubelet.kubelet
mockCadvisor := testKubelet.fakeCadvisor
fakeRuntime := testKubelet.fakeRuntime
cadvisorApiFailure := fmt.Errorf("cAdvisor failure")
containerInfo := cadvisorapi.ContainerInfo{}
cadvisorReq := &cadvisorapi.ContainerInfoRequest{}
mockCadvisor.On("DockerContainer", containerID, cadvisorReq).Return(containerInfo, cadvisorApiFailure)
fakeRuntime.PodList = []*kubecontainertest.FakePod{
{Pod: &kubecontainer.Pod{
ID: "uuid",
Name: "qux",
Namespace: "ns",
Containers: []*kubecontainer.Container{
{Name: "foo",
ID: kubecontainer.ContainerID{Type: "test", ID: containerID},
},
},
}},
}
stats, err := kubelet.GetContainerInfo("qux_ns", "uuid", "foo", cadvisorReq)
if stats != nil {
t.Errorf("non-nil stats on error")
}
if err == nil {
t.Errorf("expect error but received nil error")
return
}
if err.Error() != cadvisorApiFailure.Error() {
t.Errorf("wrong error message. expect %v, got %v", cadvisorApiFailure, err)
}
mockCadvisor.AssertExpectations(t)
}

func TestGetContainerInfoOnNonExistContainer(t *testing.T) {
testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
defer testKubelet.Cleanup()
kubelet := testKubelet.kubelet
mockCadvisor := testKubelet.fakeCadvisor
fakeRuntime := testKubelet.fakeRuntime
fakeRuntime.PodList = []*kubecontainertest.FakePod{}

stats, _ := kubelet.GetContainerInfo("qux", "", "foo", nil)
if stats != nil {
t.Errorf("non-nil stats on non exist container")
}
mockCadvisor.AssertExpectations(t)
}

func TestGetContainerInfoWhenContainerRuntimeFailed(t *testing.T) {
testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
defer testKubelet.Cleanup()
kubelet := testKubelet.kubelet
mockCadvisor := testKubelet.fakeCadvisor
fakeRuntime := testKubelet.fakeRuntime
expectedErr := fmt.Errorf("List containers error")
fakeRuntime.Err = 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")
}
mockCadvisor.AssertExpectations(t)
}

func TestGetContainerInfoWithNoContainers(t *testing.T) {
testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
defer testKubelet.Cleanup()
kubelet := testKubelet.kubelet
mockCadvisor := testKubelet.fakeCadvisor

stats, err := kubelet.GetContainerInfo("qux_ns", "", "foo", nil)
if err == nil {
t.Errorf("expected error from cadvisor client, got none")
}
if err != kubecontainer.ErrContainerNotFound {
t.Errorf("expected error %v, got %v", kubecontainer.ErrContainerNotFound.Error(), err.Error())
}
if stats != nil {
t.Errorf("non-nil stats when dockertools returned no containers")
}
mockCadvisor.AssertExpectations(t)
}

func TestGetContainerInfoWithNoMatchingContainers(t *testing.T) {
testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */)
defer testKubelet.Cleanup()
fakeRuntime := testKubelet.fakeRuntime
kubelet := testKubelet.kubelet
mockCadvisor := testKubelet.fakeCadvisor
fakeRuntime.PodList = []*kubecontainertest.FakePod{
{Pod: &kubecontainer.Pod{
ID: "12345678",
Name: "qux",
Namespace: "ns",
Containers: []*kubecontainer.Container{
{Name: "bar",
ID: kubecontainer.ContainerID{Type: "test", ID: "fakeID"},
},
}},
},
}

stats, err := kubelet.GetContainerInfo("qux_ns", "", "foo", nil)
if err == nil {
t.Errorf("Expected error from cadvisor client, got none")
}
if err != kubecontainer.ErrContainerNotFound {
t.Errorf("Expected error %v, got %v", kubecontainer.ErrContainerNotFound.Error(), err.Error())
}
if stats != nil {
t.Errorf("non-nil stats when dockertools returned no containers")
}
mockCadvisor.AssertExpectations(t)
}

func TestHasDedicatedImageFs(t *testing.T) {
testCases := map[string]struct {
imageFsInfo cadvisorapiv2.FsInfo
Expand Down