From e87fa5e0ffadf4b1437fd34cb2691d0035c392f7 Mon Sep 17 00:00:00 2001 From: Random Liu Date: Mon, 17 Oct 2016 09:22:42 +0000 Subject: [PATCH] * Refactor kuberuntime unit test * Add gc unit test * Fix init container unit test --- .../api/testing/fake_runtime_service.go | 43 +-- pkg/kubelet/api/testing/utils.go | 5 +- pkg/kubelet/dockertools/container_gc_test.go | 1 + .../kuberuntime/kuberuntime_container_test.go | 3 +- pkg/kubelet/kuberuntime/kuberuntime_gc.go | 121 +++--- .../kuberuntime/kuberuntime_gc_test.go | 363 +++++++++++++----- .../kuberuntime/kuberuntime_manager_test.go | 250 +++++++----- 7 files changed, 501 insertions(+), 285 deletions(-) diff --git a/pkg/kubelet/api/testing/fake_runtime_service.go b/pkg/kubelet/api/testing/fake_runtime_service.go index 887129d7b03..40764ab237a 100644 --- a/pkg/kubelet/api/testing/fake_runtime_service.go +++ b/pkg/kubelet/api/testing/fake_runtime_service.go @@ -123,10 +123,13 @@ func (r *FakeRuntimeService) RunPodSandbox(config *runtimeApi.PodSandboxConfig) readyState := runtimeApi.PodSandBoxState_READY r.Sandboxes[podSandboxID] = &FakePodSandbox{ PodSandboxStatus: runtimeApi.PodSandboxStatus{ - Id: &podSandboxID, - Metadata: config.Metadata, - State: &readyState, - CreatedAt: &createdAt, + Id: &podSandboxID, + Metadata: config.Metadata, + State: &readyState, + CreatedAt: &createdAt, + Network: &runtimeApi.PodSandboxNetworkStatus{ + Ip: &FakePodSandboxIP, + }, Labels: config.Labels, Annotations: config.Annotations, }, @@ -174,17 +177,8 @@ func (r *FakeRuntimeService) PodSandboxStatus(podSandboxID string) (*runtimeApi. return nil, fmt.Errorf("pod sandbox %q not found", podSandboxID) } - return &runtimeApi.PodSandboxStatus{ - Id: &podSandboxID, - Metadata: s.Metadata, - CreatedAt: s.CreatedAt, - State: s.State, - Network: &runtimeApi.PodSandboxNetworkStatus{ - Ip: &FakePodSandboxIP, - }, - Labels: s.Labels, - Annotations: s.Annotations, - }, nil + status := s.PodSandboxStatus + return &status, nil } func (r *FakeRuntimeService) ListPodSandbox(filter *runtimeApi.PodSandboxFilter) ([]*runtimeApi.PodSandbox, error) { @@ -228,7 +222,7 @@ func (r *FakeRuntimeService) CreateContainer(podSandboxID string, config *runtim // ContainerID should be randomized for real container runtime, but here just use // fixed BuildContainerName() for easily making fake containers. - containerID := BuildContainerName(config.Metadata) + containerID := BuildContainerName(config.Metadata, podSandboxID) createdAt := time.Now().Unix() createdState := runtimeApi.ContainerState_CREATED imageRef := config.Image.GetImage() @@ -351,21 +345,8 @@ func (r *FakeRuntimeService) ContainerStatus(containerID string) (*runtimeApi.Co return nil, fmt.Errorf("container %q not found", containerID) } - return &runtimeApi.ContainerStatus{ - Id: c.Id, - Metadata: c.Metadata, - State: c.State, - CreatedAt: c.CreatedAt, - Image: c.Image, - ImageRef: c.ImageRef, - Labels: c.Labels, - Annotations: c.Annotations, - ExitCode: c.ExitCode, - StartedAt: c.StartedAt, - FinishedAt: c.FinishedAt, - Reason: c.Reason, - Mounts: c.Mounts, - }, nil + status := c.ContainerStatus + return &status, nil } func (r *FakeRuntimeService) Exec(containerID string, cmd []string, tty bool, stdin io.Reader, stdout, stderr io.WriteCloser) error { diff --git a/pkg/kubelet/api/testing/utils.go b/pkg/kubelet/api/testing/utils.go index 447386f2743..3566243aac6 100644 --- a/pkg/kubelet/api/testing/utils.go +++ b/pkg/kubelet/api/testing/utils.go @@ -22,8 +22,9 @@ import ( runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" ) -func BuildContainerName(metadata *runtimeApi.ContainerMetadata) string { - return fmt.Sprintf("%s_%d", metadata.GetName(), metadata.GetAttempt()) +func BuildContainerName(metadata *runtimeApi.ContainerMetadata, sandboxID string) string { + // include the sandbox ID to make the container ID unique. + return fmt.Sprintf("%s_%s_%d", sandboxID, metadata.GetName(), metadata.GetAttempt()) } func BuildSandboxName(metadata *runtimeApi.PodSandboxMetadata) string { diff --git a/pkg/kubelet/dockertools/container_gc_test.go b/pkg/kubelet/dockertools/container_gc_test.go index f2731616485..f654b263a21 100644 --- a/pkg/kubelet/dockertools/container_gc_test.go +++ b/pkg/kubelet/dockertools/container_gc_test.go @@ -148,6 +148,7 @@ func TestGarbageCollectNoMaxLimit(t *testing.T) { }) addPods(gc.podGetter, "foo", "foo1", "foo2", "foo3", "foo4") + assert.Nil(t, gc.GarbageCollect(kubecontainer.ContainerGCPolicy{MinAge: time.Minute, MaxPerPodContainer: -1, MaxContainers: -1}, true)) assert.Len(t, fakeDocker.Removed, 0) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go index bcf3ef33324..05bf6996ea2 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go @@ -47,8 +47,7 @@ func TestRemoveContainer(t *testing.T) { } // Create fake sandbox and container - _, fakeContainers, err := makeAndSetFakePod(m, fakeRuntime, pod) - assert.NoError(t, err) + _, fakeContainers := makeAndSetFakePod(t, m, fakeRuntime, pod) assert.Equal(t, len(fakeContainers), 1) containerId := fakeContainers[0].GetId() diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc.go b/pkg/kubelet/kuberuntime/kuberuntime_gc.go index 2fa766c8e5f..59227954ef4 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc.go @@ -136,6 +136,12 @@ func (cgc *containerGC) removeSandbox(sandboxID string) { } } +// isPodDeleted returns true if the pod is already deleted. +func (cgc *containerGC) isPodDeleted(podUID types.UID) bool { + _, found := cgc.podGetter.GetPodByUID(podUID) + return !found +} + // evictableContainers gets all containers that are evictable. Evictable containers are: not running // and created more than MinAge ago. func (cgc *containerGC) evictableContainers(minAge time.Duration) (containersByEvictUnit, error) { @@ -179,17 +185,64 @@ func (cgc *containerGC) evictableContainers(minAge time.Duration) (containersByE return evictUnits, nil } -// evictableSandboxes gets all sandboxes that are evictable. Evictable sandboxes are: not running +// evict all containers that are evictable +func (cgc *containerGC) evictContainers(gcPolicy kubecontainer.ContainerGCPolicy, allSourcesReady bool) error { + // Separate containers by evict units. + evictUnits, err := cgc.evictableContainers(gcPolicy.MinAge) + if err != nil { + return err + } + + // Remove deleted pod containers if all sources are ready. + if allSourcesReady { + for key, unit := range evictUnits { + if cgc.isPodDeleted(key.uid) { + cgc.removeOldestN(unit, len(unit)) // Remove all. + delete(evictUnits, key) + } + } + } + + // Enforce max containers per evict unit. + if gcPolicy.MaxPerPodContainer >= 0 { + cgc.enforceMaxContainersPerEvictUnit(evictUnits, gcPolicy.MaxPerPodContainer) + } + + // Enforce max total number of containers. + if gcPolicy.MaxContainers >= 0 && evictUnits.NumContainers() > gcPolicy.MaxContainers { + // Leave an equal number of containers per evict unit (min: 1). + numContainersPerEvictUnit := gcPolicy.MaxContainers / evictUnits.NumEvictUnits() + if numContainersPerEvictUnit < 1 { + numContainersPerEvictUnit = 1 + } + cgc.enforceMaxContainersPerEvictUnit(evictUnits, numContainersPerEvictUnit) + + // If we still need to evict, evict oldest first. + numContainers := evictUnits.NumContainers() + if numContainers > gcPolicy.MaxContainers { + flattened := make([]containerGCInfo, 0, numContainers) + for key := range evictUnits { + flattened = append(flattened, evictUnits[key]...) + } + sort.Sort(byCreated(flattened)) + + cgc.removeOldestN(flattened, numContainers-gcPolicy.MaxContainers) + } + } + return nil +} + +// evictSandboxes evicts all sandboxes that are evictable. Evictable sandboxes are: not running // and contains no containers at all. -func (cgc *containerGC) evictableSandboxes(minAge time.Duration) ([]string, error) { +func (cgc *containerGC) evictSandboxes(minAge time.Duration) error { containers, err := cgc.manager.getKubeletContainers(true) if err != nil { - return nil, err + return err } sandboxes, err := cgc.manager.getKubeletSandboxes(true) if err != nil { - return nil, err + return err } evictSandboxes := make([]string, 0) @@ -222,13 +275,10 @@ func (cgc *containerGC) evictableSandboxes(minAge time.Duration) ([]string, erro evictSandboxes = append(evictSandboxes, sandboxID) } - return evictSandboxes, nil -} - -// isPodDeleted returns true if the pod is already deleted. -func (cgc *containerGC) isPodDeleted(podUID types.UID) bool { - _, found := cgc.podGetter.GetPodByUID(podUID) - return !found + for _, sandbox := range evictSandboxes { + cgc.removeSandbox(sandbox) + } + return nil } // evictPodLogsDirectories evicts all evictable pod logs directories. Pod logs directories @@ -278,59 +328,16 @@ func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error { // * gets evictable sandboxes which are not ready and contains no containers. // * removes evictable sandboxes. func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy, allSourcesReady bool) error { - // Separate containers by evict units. - evictUnits, err := cgc.evictableContainers(gcPolicy.MinAge) - if err != nil { + // Remove evictable containers + if err := cgc.evictContainers(gcPolicy, allSourcesReady); err != nil { return err } - // Remove deleted pod containers if all sources are ready. - if allSourcesReady { - for key, unit := range evictUnits { - if cgc.isPodDeleted(key.uid) { - cgc.removeOldestN(unit, len(unit)) // Remove all. - delete(evictUnits, key) - } - } - } - - // Enforce max containers per evict unit. - if gcPolicy.MaxPerPodContainer >= 0 { - cgc.enforceMaxContainersPerEvictUnit(evictUnits, gcPolicy.MaxPerPodContainer) - } - - // Enforce max total number of containers. - if gcPolicy.MaxContainers >= 0 && evictUnits.NumContainers() > gcPolicy.MaxContainers { - // Leave an equal number of containers per evict unit (min: 1). - numContainersPerEvictUnit := gcPolicy.MaxContainers / evictUnits.NumEvictUnits() - if numContainersPerEvictUnit < 1 { - numContainersPerEvictUnit = 1 - } - cgc.enforceMaxContainersPerEvictUnit(evictUnits, numContainersPerEvictUnit) - - // If we still need to evict, evict oldest first. - numContainers := evictUnits.NumContainers() - if numContainers > gcPolicy.MaxContainers { - flattened := make([]containerGCInfo, 0, numContainers) - for key := range evictUnits { - flattened = append(flattened, evictUnits[key]...) - } - sort.Sort(byCreated(flattened)) - - cgc.removeOldestN(flattened, numContainers-gcPolicy.MaxContainers) - } - } - // Remove sandboxes with zero containers - evictSandboxes, err := cgc.evictableSandboxes(sandboxMinGCAge) - if err != nil { + if err := cgc.evictSandboxes(sandboxMinGCAge); err != nil { return err } - for _, sandbox := range evictSandboxes { - cgc.removeSandbox(sandbox) - } // Remove pod sandbox log directory - // TODO(random-liu): Add legacy container log localtion cleanup. return cgc.evictPodLogsDirectories(allSourcesReady) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go b/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go index d4e9b88ca15..a56f6d42e65 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go @@ -22,120 +22,271 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/kubernetes/pkg/api" - apitest "k8s.io/kubernetes/pkg/kubelet/api/testing" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" - "k8s.io/kubernetes/pkg/types" ) -type apiPodWithCreatedAt struct { - apiPod *api.Pod - createdAt int64 -} - -func makeAndSetFakeEvictablePod(m *kubeGenericRuntimeManager, fakeRuntime *apitest.FakeRuntimeService, pods []*apiPodWithCreatedAt) error { - sandboxes := make([]*apitest.FakePodSandbox, 0) - containers := make([]*apitest.FakeContainer, 0) - for _, pod := range pods { - fakePodSandbox, err := makeFakePodSandbox(m, pod.apiPod, pod.createdAt) - if err != nil { - return err - } - - fakeContainers, err := makeFakeContainers(m, pod.apiPod, pod.apiPod.Spec.Containers, pod.createdAt, runtimeApi.ContainerState_EXITED) - if err != nil { - return err - } - - // Set sandbox to not ready state - sandboxNotReady := runtimeApi.PodSandBoxState_NOTREADY - fakePodSandbox.State = &sandboxNotReady - sandboxes = append(sandboxes, fakePodSandbox) - - // Set containers to exited state - containerExited := runtimeApi.ContainerState_EXITED - for _, c := range fakeContainers { - c.State = &containerExited - containers = append(containers, c) - } - - } - - fakeRuntime.SetFakeSandboxes(sandboxes) - fakeRuntime.SetFakeContainers(containers) - return nil -} - -func makeTestContainer(name, image string) api.Container { - return api.Container{ - Name: name, - Image: image, - } -} - -func makeTestPod(podName, podNamespace, podUID string, containers []api.Container, createdAt int64) *apiPodWithCreatedAt { - return &apiPodWithCreatedAt{ - createdAt: createdAt, - apiPod: &api.Pod{ - ObjectMeta: api.ObjectMeta{ - UID: types.UID(podUID), - Name: podName, - Namespace: podNamespace, - }, - Spec: api.PodSpec{ - Containers: containers, - }, - }, - } -} - -func TestGarbageCollect(t *testing.T) { +func TestSandboxGC(t *testing.T) { fakeRuntime, _, m, err := createTestRuntimeManager() assert.NoError(t, err) - pods := []*apiPodWithCreatedAt{ - makeTestPod("123456", "foo1", "new", []api.Container{ - makeTestContainer("foo1", "busybox"), - makeTestContainer("foo2", "busybox"), - makeTestContainer("foo3", "busybox"), - }, 1), - makeTestPod("1234567", "foo2", "new", []api.Container{ - makeTestContainer("foo4", "busybox"), - makeTestContainer("foo5", "busybox"), - }, 2), - makeTestPod("12345678", "foo3", "new", []api.Container{ - makeTestContainer("foo6", "busybox"), - }, 3), + pods := []*api.Pod{ + makeTestPod("foo1", "new", "1234", []api.Container{ + makeTestContainer("bar1", "busybox"), + makeTestContainer("bar2", "busybox"), + }), + makeTestPod("foo2", "new", "5678", []api.Container{ + makeTestContainer("bar3", "busybox"), + }), } - err = makeAndSetFakeEvictablePod(m, fakeRuntime, pods) - assert.NoError(t, err) - assert.NoError(t, m.GarbageCollect(kubecontainer.ContainerGCPolicy{ - MinAge: time.Second, - MaxPerPodContainer: 1, - MaxContainers: 3, - }, false)) - assert.Equal(t, 3, len(fakeRuntime.Containers)) - assert.Equal(t, 2, len(fakeRuntime.Sandboxes)) - // no containers should be removed. - err = makeAndSetFakeEvictablePod(m, fakeRuntime, pods) - assert.NoError(t, err) - assert.NoError(t, m.GarbageCollect(kubecontainer.ContainerGCPolicy{ - MinAge: time.Second, - MaxPerPodContainer: 10, - MaxContainers: 100, - }, false)) - assert.Equal(t, 6, len(fakeRuntime.Containers)) - assert.Equal(t, 3, len(fakeRuntime.Sandboxes)) + for c, test := range []struct { + description string // description of the test case + sandboxes []sandboxTemplate // templates of sandboxes + containers []containerTemplate // templates of containers + minAge time.Duration // sandboxMinGCAge + remain []int // template indexes of remaining sandboxes + }{ + { + description: "sandbox with no containers should be garbage collected.", + sandboxes: []sandboxTemplate{ + {pod: pods[0], state: runtimeApi.PodSandBoxState_NOTREADY}, + }, + containers: []containerTemplate{}, + remain: []int{}, + }, + { + description: "running sandbox should not be garbage collected.", + sandboxes: []sandboxTemplate{ + {pod: pods[0], state: runtimeApi.PodSandBoxState_READY}, + }, + containers: []containerTemplate{}, + remain: []int{0}, + }, + { + description: "sandbox with containers should not be garbage collected.", + sandboxes: []sandboxTemplate{ + {pod: pods[0], state: runtimeApi.PodSandBoxState_NOTREADY}, + }, + containers: []containerTemplate{ + {pod: pods[0], container: &pods[0].Spec.Containers[0], state: runtimeApi.ContainerState_EXITED}, + }, + remain: []int{0}, + }, + { + description: "sandbox within min age should not be garbage collected.", + sandboxes: []sandboxTemplate{ + {pod: pods[0], createdAt: time.Now().UnixNano(), state: runtimeApi.PodSandBoxState_NOTREADY}, + {pod: pods[1], createdAt: time.Now().Add(-2 * time.Hour).UnixNano(), state: runtimeApi.PodSandBoxState_NOTREADY}, + }, + containers: []containerTemplate{}, + minAge: time.Hour, // assume the test won't take an hour + remain: []int{0}, + }, + { + description: "multiple sandboxes should be handled properly.", + sandboxes: []sandboxTemplate{ + // running sandbox. + {pod: pods[0], attempt: 1, state: runtimeApi.PodSandBoxState_READY}, + // exited sandbox with containers. + {pod: pods[1], attempt: 1, state: runtimeApi.PodSandBoxState_NOTREADY}, + // exited sandbox without containers. + {pod: pods[1], attempt: 0, state: runtimeApi.PodSandBoxState_NOTREADY}, + }, + containers: []containerTemplate{ + {pod: pods[1], container: &pods[1].Spec.Containers[0], sandboxAttempt: 1, state: runtimeApi.ContainerState_EXITED}, + }, + remain: []int{0, 1}, + }, + } { + t.Logf("TestCase #%d: %+v", c, test) + fakeSandboxes := makeFakePodSandboxes(t, m, test.sandboxes) + fakeContainers := makeFakeContainers(t, m, test.containers) + fakeRuntime.SetFakeSandboxes(fakeSandboxes) + fakeRuntime.SetFakeContainers(fakeContainers) - // all containers should be removed. - err = makeAndSetFakeEvictablePod(m, fakeRuntime, pods) - assert.NoError(t, err) - assert.NoError(t, m.GarbageCollect(kubecontainer.ContainerGCPolicy{ - MinAge: time.Second, - MaxPerPodContainer: 0, - MaxContainers: 0, - }, false)) - assert.Equal(t, 0, len(fakeRuntime.Containers)) - assert.Equal(t, 0, len(fakeRuntime.Sandboxes)) + err := m.containerGC.evictSandboxes(test.minAge) + assert.NoError(t, err) + realRemain, err := fakeRuntime.ListPodSandbox(nil) + assert.NoError(t, err) + assert.Len(t, realRemain, len(test.remain)) + for _, remain := range test.remain { + status, err := fakeRuntime.PodSandboxStatus(fakeSandboxes[remain].GetId()) + assert.NoError(t, err) + assert.Equal(t, &fakeSandboxes[remain].PodSandboxStatus, status) + } + } +} + +func TestContainerGC(t *testing.T) { + fakeRuntime, _, m, err := createTestRuntimeManager() + assert.NoError(t, err) + + fakePodGetter := m.containerGC.podGetter.(*fakePodGetter) + makeGCContainer := func(podName, containerName string, attempt int, createdAt int64, state runtimeApi.ContainerState) containerTemplate { + container := makeTestContainer(containerName, "test-image") + pod := makeTestPod(podName, "test-ns", podName, []api.Container{container}) + if podName != "deleted" { + // initialize the pod getter, explicitly exclude deleted pod + fakePodGetter.pods[pod.UID] = pod + } + return containerTemplate{ + pod: pod, + container: &container, + attempt: attempt, + createdAt: createdAt, + state: state, + } + } + defaultGCPolicy := kubecontainer.ContainerGCPolicy{MinAge: time.Hour, MaxPerPodContainer: 2, MaxContainers: 6} + + for c, test := range []struct { + description string // description of the test case + containers []containerTemplate // templates of containers + policy *kubecontainer.ContainerGCPolicy // container gc policy + remain []int // template indexes of remaining containers + }{ + { + description: "all containers should be removed when max container limit is 0", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + }, + policy: &kubecontainer.ContainerGCPolicy{MinAge: time.Minute, MaxPerPodContainer: 1, MaxContainers: 0}, + remain: []int{}, + }, + { + description: "max containers should be complied when no max per pod container limit is set", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 4, 4, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 3, 3, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + }, + policy: &kubecontainer.ContainerGCPolicy{MinAge: time.Minute, MaxPerPodContainer: -1, MaxContainers: 4}, + remain: []int{0, 1, 2, 3}, + }, + { + description: "no containers should be removed if both max container and per pod container limits are not set", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + }, + policy: &kubecontainer.ContainerGCPolicy{MinAge: time.Minute, MaxPerPodContainer: -1, MaxContainers: -1}, + remain: []int{0, 1, 2}, + }, + { + description: "recently started containers should not be removed", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 2, time.Now().UnixNano(), runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 1, time.Now().UnixNano(), runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, time.Now().UnixNano(), runtimeApi.ContainerState_EXITED), + }, + remain: []int{0, 1, 2}, + }, + { + description: "oldest containers should be removed when per pod container limit exceeded", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + }, + remain: []int{0, 1}, + }, + { + description: "running containers should not be removed", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_RUNNING), + }, + remain: []int{0, 1, 2}, + }, + { + description: "no containers should be removed when limits are not exceeded", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + }, + remain: []int{0, 1}, + }, + { + description: "max container count should apply per (UID, container) pair", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo1", "baz", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo1", "baz", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo1", "baz", 0, 0, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo2", "bar", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo2", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo2", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + }, + remain: []int{0, 1, 3, 4, 6, 7}, + }, + { + description: "max limit should apply and try to keep from every pod", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo1", "bar1", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo1", "bar1", 0, 0, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo2", "bar2", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo2", "bar2", 0, 0, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo3", "bar3", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo3", "bar3", 0, 0, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo4", "bar4", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo4", "bar4", 0, 0, runtimeApi.ContainerState_EXITED), + }, + remain: []int{0, 2, 4, 6, 8}, + }, + { + description: "oldest pods should be removed if limit exceeded", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo1", "bar1", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo1", "bar1", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo2", "bar2", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo3", "bar3", 0, 0, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo4", "bar4", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo5", "bar5", 0, 0, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo6", "bar6", 2, 2, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo7", "bar7", 1, 1, runtimeApi.ContainerState_EXITED), + }, + remain: []int{0, 2, 4, 6, 8, 9}, + }, + { + description: "containers for deleted pods should be removed", + containers: []containerTemplate{ + makeGCContainer("foo", "bar", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("foo", "bar", 0, 0, runtimeApi.ContainerState_EXITED), + // deleted pods still respect MinAge. + makeGCContainer("deleted", "bar1", 2, time.Now().UnixNano(), runtimeApi.ContainerState_EXITED), + makeGCContainer("deleted", "bar1", 1, 1, runtimeApi.ContainerState_EXITED), + makeGCContainer("deleted", "bar1", 0, 0, runtimeApi.ContainerState_EXITED), + }, + remain: []int{0, 1, 2}, + }, + } { + t.Logf("TestCase #%d: %+v", c, test) + fakeContainers := makeFakeContainers(t, m, test.containers) + fakeRuntime.SetFakeContainers(fakeContainers) + + if test.policy == nil { + test.policy = &defaultGCPolicy + } + err := m.containerGC.evictContainers(*test.policy, true) + assert.NoError(t, err) + realRemain, err := fakeRuntime.ListContainers(nil) + assert.NoError(t, err) + assert.Len(t, realRemain, len(test.remain)) + for _, remain := range test.remain { + status, err := fakeRuntime.ContainerStatus(fakeContainers[remain].GetId()) + assert.NoError(t, err) + assert.Equal(t, &fakeContainers[remain].ContainerStatus, status) + } + } } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index 78befa2948d..1e549284a7d 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -32,6 +32,7 @@ import ( containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" "k8s.io/kubernetes/pkg/kubelet/network" nettest "k8s.io/kubernetes/pkg/kubelet/network/testing" + "k8s.io/kubernetes/pkg/types" kubetypes "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/flowcontrol" ) @@ -60,55 +61,95 @@ func createTestRuntimeManager() (*apitest.FakeRuntimeService, *apitest.FakeImage return fakeRuntimeService, fakeImageService, manager, err } -func makeAndSetFakePod(m *kubeGenericRuntimeManager, fakeRuntime *apitest.FakeRuntimeService, pod *api.Pod) (*apitest.FakePodSandbox, []*apitest.FakeContainer, error) { - fakePodSandbox, err := makeFakePodSandbox(m, pod, fakeCreatedAt) - if err != nil { - return nil, nil, err - } - - fakeContainers, err := makeFakeContainers(m, pod, pod.Spec.Containers, fakeCreatedAt, runtimeApi.ContainerState_RUNNING) - if err != nil { - return nil, nil, err - } - - fakeInitContainers, err := makeFakeContainers(m, pod, pod.Spec.InitContainers, fakeCreatedAt, runtimeApi.ContainerState_EXITED) - if err != nil { - return nil, nil, err - } - - fakeContainers = append(fakeContainers, fakeInitContainers...) - fakeRuntime.SetFakeSandboxes([]*apitest.FakePodSandbox{fakePodSandbox}) - fakeRuntime.SetFakeContainers(fakeContainers) - return fakePodSandbox, fakeContainers, nil +// sandboxTemplate is a sandbox template to create fake sandbox. +type sandboxTemplate struct { + pod *api.Pod + attempt uint32 + createdAt int64 + state runtimeApi.PodSandBoxState } -func makeFakePodSandbox(m *kubeGenericRuntimeManager, pod *api.Pod, createdAt int64) (*apitest.FakePodSandbox, error) { - config, err := m.generatePodSandboxConfig(pod, 0) - if err != nil { - return nil, err +// containerTemplate is a container template to create fake container. +type containerTemplate struct { + pod *api.Pod + container *api.Container + sandboxAttempt uint32 + attempt int + createdAt int64 + state runtimeApi.ContainerState +} + +// makeAndSetFakePod is a helper function to create and set one fake sandbox for a pod and +// one fake container for each of its container. +func makeAndSetFakePod(t *testing.T, m *kubeGenericRuntimeManager, fakeRuntime *apitest.FakeRuntimeService, + pod *api.Pod) (*apitest.FakePodSandbox, []*apitest.FakeContainer) { + sandbox := makeFakePodSandbox(t, m, sandboxTemplate{ + pod: pod, + createdAt: fakeCreatedAt, + state: runtimeApi.PodSandBoxState_READY, + }) + + var containers []*apitest.FakeContainer + newTemplate := func(c *api.Container) containerTemplate { + return containerTemplate{ + pod: pod, + container: c, + createdAt: fakeCreatedAt, + state: runtimeApi.ContainerState_RUNNING, + } + } + for i := range pod.Spec.Containers { + containers = append(containers, makeFakeContainer(t, m, newTemplate(&pod.Spec.Containers[i]))) + } + for i := range pod.Spec.InitContainers { + containers = append(containers, makeFakeContainer(t, m, newTemplate(&pod.Spec.InitContainers[i]))) } + fakeRuntime.SetFakeSandboxes([]*apitest.FakePodSandbox{sandbox}) + fakeRuntime.SetFakeContainers(containers) + return sandbox, containers +} + +// makeFakePodSandbox creates a fake pod sandbox based on a sandbox template. +func makeFakePodSandbox(t *testing.T, m *kubeGenericRuntimeManager, template sandboxTemplate) *apitest.FakePodSandbox { + config, err := m.generatePodSandboxConfig(template.pod, template.attempt) + assert.NoError(t, err, "generatePodSandboxConfig for sandbox template %+v", template) + podSandboxID := apitest.BuildSandboxName(config.Metadata) - readyState := runtimeApi.PodSandBoxState_READY return &apitest.FakePodSandbox{ PodSandboxStatus: runtimeApi.PodSandboxStatus{ Id: &podSandboxID, Metadata: config.Metadata, - State: &readyState, - CreatedAt: &createdAt, - Labels: config.Labels, + State: &template.state, + CreatedAt: &template.createdAt, + Network: &runtimeApi.PodSandboxNetworkStatus{ + Ip: &apitest.FakePodSandboxIP, + }, + Labels: config.Labels, }, - }, nil + } } -func makeFakeContainer(m *kubeGenericRuntimeManager, pod *api.Pod, container api.Container, sandboxConfig *runtimeApi.PodSandboxConfig, createdAt int64, state runtimeApi.ContainerState) (*apitest.FakeContainer, error) { - containerConfig, err := m.generateContainerConfig(&container, pod, 0, "") - if err != nil { - return nil, err +// makeFakePodSandboxes creates a group of fake pod sandboxes based on the sandbox templates. +// The function guarantees the order of the fake pod sandboxes is the same with the templates. +func makeFakePodSandboxes(t *testing.T, m *kubeGenericRuntimeManager, templates []sandboxTemplate) []*apitest.FakePodSandbox { + var fakePodSandboxes []*apitest.FakePodSandbox + for _, template := range templates { + fakePodSandboxes = append(fakePodSandboxes, makeFakePodSandbox(t, m, template)) } + return fakePodSandboxes +} + +// makeFakeContainer creates a fake container based on a container template. +func makeFakeContainer(t *testing.T, m *kubeGenericRuntimeManager, template containerTemplate) *apitest.FakeContainer { + sandboxConfig, err := m.generatePodSandboxConfig(template.pod, template.sandboxAttempt) + assert.NoError(t, err, "generatePodSandboxConfig for container template %+v", template) + + containerConfig, err := m.generateContainerConfig(template.container, template.pod, template.attempt, "") + assert.NoError(t, err, "generateContainerConfig for container template %+v", template) - containerID := apitest.BuildContainerName(containerConfig.Metadata) podSandboxID := apitest.BuildSandboxName(sandboxConfig.Metadata) + containerID := apitest.BuildContainerName(containerConfig.Metadata, podSandboxID) imageRef := containerConfig.Image.GetImage() return &apitest.FakeContainer{ ContainerStatus: runtimeApi.ContainerStatus{ @@ -116,32 +157,45 @@ func makeFakeContainer(m *kubeGenericRuntimeManager, pod *api.Pod, container api Metadata: containerConfig.Metadata, Image: containerConfig.Image, ImageRef: &imageRef, - CreatedAt: &createdAt, - State: &state, + CreatedAt: &template.createdAt, + State: &template.state, Labels: containerConfig.Labels, Annotations: containerConfig.Annotations, }, SandboxID: podSandboxID, - }, nil + } } -func makeFakeContainers(m *kubeGenericRuntimeManager, pod *api.Pod, containers []api.Container, createdAt int64, state runtimeApi.ContainerState) ([]*apitest.FakeContainer, error) { - sandboxConfig, err := m.generatePodSandboxConfig(pod, 0) - if err != nil { - return nil, err +// makeFakeContainers creates a group of fake containers based on the container templates. +// The function guarantees the order of the fake containers is the same with the templates. +func makeFakeContainers(t *testing.T, m *kubeGenericRuntimeManager, templates []containerTemplate) []*apitest.FakeContainer { + var fakeContainers []*apitest.FakeContainer + for _, template := range templates { + fakeContainers = append(fakeContainers, makeFakeContainer(t, m, template)) } + return fakeContainers +} - result := make([]*apitest.FakeContainer, len(containers)) - for idx, c := range containers { - containerWithState, err := makeFakeContainer(m, pod, c, sandboxConfig, createdAt, state) - if err != nil { - return nil, err - } - - result[idx] = containerWithState +// makeTestContainer creates a test api container. +func makeTestContainer(name, image string) api.Container { + return api.Container{ + Name: name, + Image: image, } +} - return result, nil +// makeTestPod creates a test api pod. +func makeTestPod(podName, podNamespace, podUID string, containers []api.Container) *api.Pod { + return &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: types.UID(podUID), + Name: podName, + Namespace: podNamespace, + }, + Spec: api.PodSpec{ + Containers: containers, + }, + } } // verifyPods returns true if the two pod slices are equal. @@ -226,8 +280,7 @@ func TestGetPodStatus(t *testing.T) { } // Set fake sandbox and faked containers to fakeRuntime. - _, _, err = makeAndSetFakePod(m, fakeRuntime, pod) - assert.NoError(t, err) + makeAndSetFakePod(t, m, fakeRuntime, pod) podStatus, err := m.GetPodStatus(pod.UID, pod.Name, pod.Namespace) assert.NoError(t, err) @@ -262,8 +315,7 @@ func TestGetPods(t *testing.T) { } // Set fake sandbox and fake containers to fakeRuntime. - fakeSandbox, fakeContainers, err := makeAndSetFakePod(m, fakeRuntime, pod) - assert.NoError(t, err) + fakeSandbox, fakeContainers := makeAndSetFakePod(t, m, fakeRuntime, pod) // Convert the fakeContainers to kubecontainer.Container containers := make([]*kubecontainer.Container, len(fakeContainers)) @@ -338,8 +390,7 @@ func TestGetPodContainerID(t *testing.T) { }, } // Set fake sandbox and fake containers to fakeRuntime. - fakeSandbox, _, err := makeAndSetFakePod(m, fakeRuntime, pod) - assert.NoError(t, err) + fakeSandbox, _ := makeAndSetFakePod(t, m, fakeRuntime, pod) // Convert fakeSandbox to kubecontainer.Container sandbox, err := m.sandboxToKubeContainer(&runtimeApi.PodSandbox{ @@ -387,8 +438,7 @@ func TestGetNetNS(t *testing.T) { } // Set fake sandbox and fake containers to fakeRuntime. - sandbox, _, err := makeAndSetFakePod(m, fakeRuntime, pod) - assert.NoError(t, err) + sandbox, _ := makeAndSetFakePod(t, m, fakeRuntime, pod) actual, err := m.GetNetNS(kubecontainer.ContainerID{ID: sandbox.GetId()}) assert.Equal(t, "", actual) @@ -420,8 +470,7 @@ func TestKillPod(t *testing.T) { } // Set fake sandbox and fake containers to fakeRuntime. - fakeSandbox, fakeContainers, err := makeAndSetFakePod(m, fakeRuntime, pod) - assert.NoError(t, err) + fakeSandbox, fakeContainers := makeAndSetFakePod(t, m, fakeRuntime, pod) // Convert the fakeContainers to kubecontainer.Container containers := make([]*kubecontainer.Container, len(fakeContainers)) @@ -512,6 +561,8 @@ func TestPruneInitContainers(t *testing.T) { fakeRuntime, _, m, err := createTestRuntimeManager() assert.NoError(t, err) + init1 := makeTestContainer("init1", "busybox") + init2 := makeTestContainer("init2", "busybox") pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ UID: "12345678", @@ -519,36 +570,25 @@ func TestPruneInitContainers(t *testing.T) { Namespace: "new", }, Spec: api.PodSpec{ - InitContainers: []api.Container{ - { - Name: "init1", - Image: "busybox", - }, - { - Name: "init2", - Image: "busybox", - }, - }, + InitContainers: []api.Container{init1, init2}, }, } - // Set fake sandbox and fake containers to fakeRuntime. - _, _, err = makeAndSetFakePod(m, fakeRuntime, pod) + templates := []containerTemplate{ + {pod: pod, container: &init1, attempt: 2, createdAt: 2, state: runtimeApi.ContainerState_EXITED}, + {pod: pod, container: &init1, attempt: 1, createdAt: 1, state: runtimeApi.ContainerState_EXITED}, + {pod: pod, container: &init2, attempt: 1, createdAt: 1, state: runtimeApi.ContainerState_EXITED}, + {pod: pod, container: &init2, attempt: 0, createdAt: 0, state: runtimeApi.ContainerState_EXITED}, + {pod: pod, container: &init1, attempt: 0, createdAt: 0, state: runtimeApi.ContainerState_EXITED}, + } + fakes := makeFakeContainers(t, m, templates) + fakeRuntime.SetFakeContainers(fakes) + podStatus, err := m.GetPodStatus(pod.UID, pod.Name, pod.Namespace) assert.NoError(t, err) - status := &kubecontainer.PodStatus{ - ContainerStatuses: []*kubecontainer.ContainerStatus{ - {Name: "init2", ID: kubecontainer.ContainerID{ID: "init2_0"}, State: kubecontainer.ContainerStateExited}, - {Name: "init1", ID: kubecontainer.ContainerID{ID: "init1_0"}, State: kubecontainer.ContainerStateExited}, - {Name: "init1", ID: kubecontainer.ContainerID{ID: "init1_1"}, State: kubecontainer.ContainerStateExited}, - {Name: "init1", ID: kubecontainer.ContainerID{ID: "init1_2"}, State: kubecontainer.ContainerStateExited}, - {Name: "init2", ID: kubecontainer.ContainerID{ID: "init2_1"}, State: kubecontainer.ContainerStateExited}, - }, - } - keep := map[kubecontainer.ContainerID]int{} - m.pruneInitContainersBeforeStart(pod, status, keep) - expectedContainers := []string{"init1_0", "init2_0"} + m.pruneInitContainersBeforeStart(pod, podStatus, keep) + expectedContainers := []string{fakes[0].GetId(), fakes[2].GetId()} if actual, ok := verifyFakeContainerList(fakeRuntime, expectedContainers); !ok { t.Errorf("expected %q, got %q", expectedContainers, actual) } @@ -589,16 +629,52 @@ func TestSyncPodWithInitContainers(t *testing.T) { }, } - _, _, err = makeAndSetFakePod(m, fakeRuntime, pod) - assert.NoError(t, err) + // buildContainerID is an internal helper function to build container id from api pod + // and container with default attempt number 0. + buildContainerID := func(pod *api.Pod, container api.Container) string { + uid := string(pod.UID) + sandboxID := apitest.BuildSandboxName(&runtimeApi.PodSandboxMetadata{ + Name: &pod.Name, + Uid: &uid, + Namespace: &pod.Namespace, + }) + return apitest.BuildContainerName(&runtimeApi.ContainerMetadata{Name: &container.Name}, sandboxID) + } backOff := flowcontrol.NewBackOff(time.Second, time.Minute) + + // 1. should only create the init container. podStatus, err := m.GetPodStatus(pod.UID, pod.Name, pod.Namespace) assert.NoError(t, err) result := m.SyncPod(pod, api.PodStatus{}, podStatus, []api.Secret{}, backOff) assert.NoError(t, result.Error()) + assert.Equal(t, 1, len(fakeRuntime.Containers)) + initContainerID := buildContainerID(pod, initContainers[0]) + expectedContainers := []string{initContainerID} + if actual, ok := verifyFakeContainerList(fakeRuntime, expectedContainers); !ok { + t.Errorf("expected %q, got %q", expectedContainers, actual) + } + + // 2. should not create app container because init container is still running. + podStatus, err = m.GetPodStatus(pod.UID, pod.Name, pod.Namespace) + assert.NoError(t, err) + result = m.SyncPod(pod, api.PodStatus{}, podStatus, []api.Secret{}, backOff) + assert.NoError(t, result.Error()) + assert.Equal(t, 1, len(fakeRuntime.Containers)) + expectedContainers = []string{initContainerID} + if actual, ok := verifyFakeContainerList(fakeRuntime, expectedContainers); !ok { + t.Errorf("expected %q, got %q", expectedContainers, actual) + } + + // 3. should create all app containers because init container finished. + fakeRuntime.StopContainer(initContainerID, 0) + podStatus, err = m.GetPodStatus(pod.UID, pod.Name, pod.Namespace) + assert.NoError(t, err) + result = m.SyncPod(pod, api.PodStatus{}, podStatus, []api.Secret{}, backOff) + assert.NoError(t, result.Error()) assert.Equal(t, 3, len(fakeRuntime.Containers)) - expectedContainers := []string{"foo1_0", "foo2_0", "init1_0"} + expectedContainers = []string{initContainerID, buildContainerID(pod, containers[0]), + buildContainerID(pod, containers[1])} if actual, ok := verifyFakeContainerList(fakeRuntime, expectedContainers); !ok { t.Errorf("expected %q, got %q", expectedContainers, actual) }