Merge pull request #35018 from Random-Liu/add-kuberuntime-unit-test

Automatic merge from submit-queue

CRI: Refactor kuberuntime unit test

Based on https://github.com/kubernetes/kubernetes/pull/34858

This PR:
1) Refactor the fake runtime service and some kuberuntime unit test.
2) Add better garbage collection unit test.
3) Fix init container unit test which isn't testing correctly. Some other unit tests may also need to be fixed.
4) Add pod log directory garbage collection unit test.

@feiskyer @yujuhong 
/cc @kubernetes/sig-node

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 {

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 {

pkg/kubelet/container/os.go

@@ -19,6 +19,7 @@ package container
 import (
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"time"
 )
 
@@ -35,6 +36,7 @@ type OSInterface interface {
 	Chtimes(path string, atime time.Time, mtime time.Time) error
 	Pipe() (r *os.File, w *os.File, err error)
 	ReadDir(dirname string) ([]os.FileInfo, error)
+	Glob(pattern string) ([]string, error)
 }
 
 // RealOS is used to dispatch the real system level operations.
@@ -90,3 +92,9 @@ func (RealOS) Pipe() (r *os.File, w *os.File, err error) {
 func (RealOS) ReadDir(dirname string) ([]os.FileInfo, error) {
 	return ioutil.ReadDir(dirname)
 }
+
+// Glob will call filepath.Glob to return the names of all files matching
+// pattern.
+func (RealOS) Glob(pattern string) ([]string, error) {
+	return filepath.Glob(pattern)
+}

pkg/kubelet/container/testing/mockfileinfo.go

@@ -18,7 +18,7 @@ limitations under the License.
 // Edited to include required boilerplate
 // Source: os (interfaces: FileInfo)
 
-package mock_os
+package testing
 
 import (
 	os "os"

pkg/kubelet/container/testing/os.go

@@ -74,6 +74,7 @@ func (f *FakeOS) Remove(path string) error {
 
 // RemoveAll is a fake call that just returns nil.
 func (f *FakeOS) RemoveAll(path string) error {
+	f.Removes = append(f.Removes, path)
 	return nil
 }
 
@@ -104,3 +105,8 @@ func (f *FakeOS) ReadDir(dirname string) ([]os.FileInfo, error) {
 	}
 	return nil, nil
 }
+
+// Glob is a fake call that returns nil.
+func (f *FakeOS) Glob(pattern string) ([]string, error) {
+	return nil, nil
+}

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

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

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,105 +185,8 @@ func (cgc *containerGC) evictableContainers(minAge time.Duration) (containersByE
 	return evictUnits, nil
 }
 
-// evictableSandboxes gets 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) {
-	containers, err := cgc.manager.getKubeletContainers(true)
-	if err != nil {
-		return nil, err
-	}
-
-	sandboxes, err := cgc.manager.getKubeletSandboxes(true)
-	if err != nil {
-		return nil, err
-	}
-
-	evictSandboxes := make([]string, 0)
-	newestGCTime := time.Now().Add(-minAge)
-	for _, sandbox := range sandboxes {
-		// Prune out ready sandboxes.
-		if sandbox.GetState() == runtimeApi.PodSandBoxState_READY {
-			continue
-		}
-
-		// Prune out sandboxes that still have containers.
-		found := false
-		sandboxID := sandbox.GetId()
-		for _, container := range containers {
-			if container.GetPodSandboxId() == sandboxID {
-				found = true
-				break
-			}
-		}
-		if found {
-			continue
-		}
-
-		// Only garbage collect sandboxes older than sandboxMinGCAge.
-		createdAt := time.Unix(0, sandbox.GetCreatedAt())
-		if createdAt.After(newestGCTime) {
-			continue
-		}
-
-		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
-}
-
-// evictPodLogsDirectories evicts all evictable pod logs directories. Pod logs directories
-// are evictable if there are no corresponding pods.
-func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error {
-	osInterface := cgc.manager.osInterface
-	if allSourcesReady {
-		// Only remove pod logs directories when all sources are ready.
-		dirs, err := osInterface.ReadDir(podLogsRootDirectory)
-		if err != nil {
-			return fmt.Errorf("failed to read podLogsRootDirectory %q: %v", podLogsRootDirectory, err)
-		}
-		for _, dir := range dirs {
-			podUID := types.UID(dir.Name())
-			if !cgc.isPodDeleted(podUID) {
-				continue
-			}
-			err := osInterface.RemoveAll(filepath.Join(podLogsRootDirectory, dir.Name()))
-			if err != nil {
-				glog.Errorf("Failed to remove pod logs directory %q: %v", dir.Name(), err)
-			}
-		}
-	}
-
-	// Remove dead container log symlinks.
-	// TODO(random-liu): Remove this after cluster logging supports CRI container log path.
-	logSymlinks, _ := filepath.Glob(filepath.Join(legacyContainerLogsDir, fmt.Sprintf("*.%s", legacyLogSuffix)))
-	for _, logSymlink := range logSymlinks {
-		if _, err := osInterface.Stat(logSymlink); os.IsNotExist(err) {
-			err := osInterface.Remove(logSymlink)
-			if err != nil {
-				glog.Errorf("Failed to remove container log dead symlink %q: %v", logSymlink, err)
-			}
-		}
-	}
-	return nil
-}
-
-// GarbageCollect removes dead containers using the specified container gc policy.
-// Note that gc policy is not applied to sandboxes. Sandboxes are only removed when they are
-// not ready and containing no containers.
-//
-// GarbageCollect consists of the following steps:
-// * gets evictable containers which are not active and created more than gcPolicy.MinAge ago.
-// * removes oldest dead containers for each pod by enforcing gcPolicy.MaxPerPodContainer.
-// * removes oldest dead containers by enforcing gcPolicy.MaxContainers.
-// * gets evictable sandboxes which are not ready and contains no containers.
-// * removes evictable sandboxes.
-func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy, allSourcesReady bool) error {
+// 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 {
@@ -320,17 +229,116 @@ func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy,
 			cgc.removeOldestN(flattened, numContainers-gcPolicy.MaxContainers)
 		}
 	}
+	return nil
+}
 
-	// Remove sandboxes with zero containers
-	evictSandboxes, err := cgc.evictableSandboxes(sandboxMinGCAge)
+// evictSandboxes evicts all sandboxes that are evictable. Evictable sandboxes are: not running
+// and contains no containers at all.
+func (cgc *containerGC) evictSandboxes(minAge time.Duration) error {
+	containers, err := cgc.manager.getKubeletContainers(true)
 	if err != nil {
 		return err
 	}
+
+	sandboxes, err := cgc.manager.getKubeletSandboxes(true)
+	if err != nil {
+		return err
+	}
+
+	evictSandboxes := make([]string, 0)
+	newestGCTime := time.Now().Add(-minAge)
+	for _, sandbox := range sandboxes {
+		// Prune out ready sandboxes.
+		if sandbox.GetState() == runtimeApi.PodSandBoxState_READY {
+			continue
+		}
+
+		// Prune out sandboxes that still have containers.
+		found := false
+		sandboxID := sandbox.GetId()
+		for _, container := range containers {
+			if container.GetPodSandboxId() == sandboxID {
+				found = true
+				break
+			}
+		}
+		if found {
+			continue
+		}
+
+		// Only garbage collect sandboxes older than sandboxMinGCAge.
+		createdAt := time.Unix(0, sandbox.GetCreatedAt())
+		if createdAt.After(newestGCTime) {
+			continue
+		}
+
+		evictSandboxes = append(evictSandboxes, sandboxID)
+	}
+
 	for _, sandbox := range evictSandboxes {
 		cgc.removeSandbox(sandbox)
 	}
+	return nil
+}
+
+// evictPodLogsDirectories evicts all evictable pod logs directories. Pod logs directories
+// are evictable if there are no corresponding pods.
+func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error {
+	osInterface := cgc.manager.osInterface
+	if allSourcesReady {
+		// Only remove pod logs directories when all sources are ready.
+		dirs, err := osInterface.ReadDir(podLogsRootDirectory)
+		if err != nil {
+			return fmt.Errorf("failed to read podLogsRootDirectory %q: %v", podLogsRootDirectory, err)
+		}
+		for _, dir := range dirs {
+			name := dir.Name()
+			podUID := types.UID(name)
+			if !cgc.isPodDeleted(podUID) {
+				continue
+			}
+			err := osInterface.RemoveAll(filepath.Join(podLogsRootDirectory, name))
+			if err != nil {
+				glog.Errorf("Failed to remove pod logs directory %q: %v", name, err)
+			}
+		}
+	}
+
+	// Remove dead container log symlinks.
+	// TODO(random-liu): Remove this after cluster logging supports CRI container log path.
+	logSymlinks, _ := osInterface.Glob(filepath.Join(legacyContainerLogsDir, fmt.Sprintf("*.%s", legacyLogSuffix)))
+	for _, logSymlink := range logSymlinks {
+		if _, err := osInterface.Stat(logSymlink); os.IsNotExist(err) {
+			err := osInterface.Remove(logSymlink)
+			if err != nil {
+				glog.Errorf("Failed to remove container log dead symlink %q: %v", logSymlink, err)
+			}
+		}
+	}
+	return nil
+}
+
+// GarbageCollect removes dead containers using the specified container gc policy.
+// Note that gc policy is not applied to sandboxes. Sandboxes are only removed when they are
+// not ready and containing no containers.
+//
+// GarbageCollect consists of the following steps:
+// * gets evictable containers which are not active and created more than gcPolicy.MinAge ago.
+// * removes oldest dead containers for each pod by enforcing gcPolicy.MaxPerPodContainer.
+// * removes oldest dead containers by enforcing gcPolicy.MaxContainers.
+// * gets evictable sandboxes which are not ready and contains no containers.
+// * removes evictable sandboxes.
+func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy, allSourcesReady bool) error {
+	// Remove evictable containers
+	if err := cgc.evictContainers(gcPolicy, allSourcesReady); err != nil {
+		return err
+	}
+
+	// Remove sandboxes with zero containers
+	if err := cgc.evictSandboxes(sandboxMinGCAge); err != nil {
+		return err
+	}
 
 	// Remove pod sandbox log directory
-	// TODO(random-liu): Add legacy container log localtion cleanup.
 	return cgc.evictPodLogsDirectories(allSourcesReady)
 }

pkg/kubelet/kuberuntime/kuberuntime_gc_test.go

@@ -17,125 +17,318 @@ limitations under the License.
 package kuberuntime
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
+	"github.com/golang/mock/gomock"
 	"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"
+	containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
 )
 
-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)
+		}
+	}
+}
+
+// Notice that legacy container symlink is not tested since it may be deprecated soon.
+func TestPodLogDirectoryGC(t *testing.T) {
+	_, _, m, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+	fakeOS := m.osInterface.(*containertest.FakeOS)
+	fakePodGetter := m.containerGC.podGetter.(*fakePodGetter)
+
+	// pod log directories without corresponding pods should be removed.
+	fakePodGetter.pods["123"] = makeTestPod("foo1", "new", "123", nil)
+	fakePodGetter.pods["456"] = makeTestPod("foo2", "new", "456", nil)
+	files := []string{"123", "456", "789", "012"}
+	removed := []string{filepath.Join(podLogsRootDirectory, "789"), filepath.Join(podLogsRootDirectory, "012")}
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	fakeOS.ReadDirFn = func(string) ([]os.FileInfo, error) {
+		var fileInfos []os.FileInfo
+		for _, file := range files {
+			mockFI := containertest.NewMockFileInfo(ctrl)
+			mockFI.EXPECT().Name().Return(file)
+			fileInfos = append(fileInfos, mockFI)
+		}
+		return fileInfos, nil
+	}
+
+	// allSourcesReady == true, pod log directories without corresponding pod should be removed.
+	err = m.containerGC.evictPodLogsDirectories(true)
+	assert.NoError(t, err)
+	assert.Equal(t, removed, fakeOS.Removes)
+
+	// allSourcesReady == false, pod log directories should not be removed.
+	fakeOS.Removes = []string{}
+	err = m.containerGC.evictPodLogsDirectories(false)
+	assert.NoError(t, err)
+	assert.Empty(t, fakeOS.Removes)
 }

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

pkg/kubelet/rkt/rkt_test.go

@@ -40,7 +40,6 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/network"
 	"k8s.io/kubernetes/pkg/kubelet/network/kubenet"
 	"k8s.io/kubernetes/pkg/kubelet/network/mock_network"
-	"k8s.io/kubernetes/pkg/kubelet/rkt/mock_os"
 	"k8s.io/kubernetes/pkg/kubelet/types"
 	kubetypes "k8s.io/kubernetes/pkg/types"
 	"k8s.io/kubernetes/pkg/util/errors"
@@ -813,7 +812,7 @@ func TestGetPodStatus(t *testing.T) {
 			if !ok {
 				t.Errorf("osStat called with %v, but only knew about %#v", name, podTimes)
 			}
-			mockFI := mock_os.NewMockFileInfo(ctrl)
+			mockFI := containertesting.NewMockFileInfo(ctrl)
 			mockFI.EXPECT().ModTime().Return(podTime)
 			return mockFI, nil
 		}
@@ -1781,7 +1780,7 @@ func TestGarbageCollect(t *testing.T) {
 			var fileInfos []os.FileInfo
 
 			for _, name := range serviceFileNames {
-				mockFI := mock_os.NewMockFileInfo(ctrl)
+				mockFI := containertesting.NewMockFileInfo(ctrl)
 				mockFI.EXPECT().Name().Return(name)
 				fileInfos = append(fileInfos, mockFI)
 			}