Merge pull request #34945 from nebril/image-manager-tests-cleanup

Automatic merge from submit-queue

Clean up redundant tests in image_manager_test

There was a lot of overlap between parallel and serialized puller tests,
extracted most of these tests internals to separate functions.

pkg/kubelet/images/image_manager_test.go

@@ -30,24 +30,17 @@ import (
 	"k8s.io/kubernetes/pkg/util/flowcontrol"
 )
 
-func TestParallelPuller(t *testing.T) {
+type pullerTestCase struct {
-	pod := &v1.Pod{
+	containerImage  string
-		ObjectMeta: v1.ObjectMeta{
+	policy          v1.PullPolicy
-			Name:            "test_pod",
+	calledFunctions []string
-			Namespace:       "test-ns",
+	inspectErr      error
-			UID:             "bar",
+	pullerErr       error
-			ResourceVersion: "42",
+	expectedErr     []error
-			SelfLink:        "/api/v1/pods/foo",
+}
-		}}
 
-	cases := []struct {
+func pullerTestCases() []pullerTestCase {
-		containerImage  string
+	return []pullerTestCase{
-		policy          v1.PullPolicy
-		calledFunctions []string
-		inspectErr      error
-		pullerErr       error
-		expectedErr     []error
-	}{
 		{ // pull missing image
 			containerImage:  "missing_image",
 			policy:          v1.PullIfNotPresent,
@@ -92,25 +85,44 @@ func TestParallelPuller(t *testing.T) {
 			pullerErr:       errors.New("404"),
 			expectedErr:     []error{ErrImagePull, ErrImagePull, ErrImagePullBackOff, ErrImagePull, ErrImagePullBackOff, ErrImagePullBackOff}},
 	}
+}
+
+func pullerTestEnv(c pullerTestCase, serialized bool) (puller ImageManager, fakeClock *clock.FakeClock, fakeRuntime *ctest.FakeRuntime, container *v1.Container) {
+	container = &v1.Container{
+		Name:            "container_name",
+		Image:           c.containerImage,
+		ImagePullPolicy: c.policy,
+	}
+
+	backOff := flowcontrol.NewBackOff(time.Second, time.Minute)
+	fakeClock = clock.NewFakeClock(time.Now())
+	backOff.Clock = fakeClock
+
+	fakeRuntime = &ctest.FakeRuntime{}
+	fakeRecorder := &record.FakeRecorder{}
+
+	fakeRuntime.ImageList = []Image{{ID: "present_image"}}
+	fakeRuntime.Err = c.pullerErr
+	fakeRuntime.InspectErr = c.inspectErr
+
+	puller = NewImageManager(fakeRecorder, fakeRuntime, backOff, serialized, 0, 0)
+	return
+}
+
+func TestParallelPuller(t *testing.T) {
+	pod := &v1.Pod{
+		ObjectMeta: v1.ObjectMeta{
+			Name:            "test_pod",
+			Namespace:       "test-ns",
+			UID:             "bar",
+			ResourceVersion: "42",
+			SelfLink:        "/api/v1/pods/foo",
+		}}
+
+	cases := pullerTestCases()
 
 	for i, c := range cases {
-		container := &v1.Container{
+		puller, fakeClock, fakeRuntime, container := pullerTestEnv(c, false)
-			Name:            "container_name",
-			Image:           c.containerImage,
-			ImagePullPolicy: c.policy,
-		}
-
-		backOff := flowcontrol.NewBackOff(time.Second, time.Minute)
-		fakeClock := clock.NewFakeClock(time.Now())
-		backOff.Clock = fakeClock
-
-		fakeRuntime := &ctest.FakeRuntime{}
-		fakeRecorder := &record.FakeRecorder{}
-		puller := NewImageManager(fakeRecorder, fakeRuntime, backOff, false, 0, 0)
-
-		fakeRuntime.ImageList = []Image{{ID: "present_image", Size: 1}}
-		fakeRuntime.Err = c.pullerErr
-		fakeRuntime.InspectErr = c.inspectErr
 
 		for tick, expected := range c.expectedErr {
 			fakeClock.Step(time.Second)
@@ -131,77 +143,10 @@ func TestSerializedPuller(t *testing.T) {
 			SelfLink:        "/api/v1/pods/foo",
 		}}
 
-	cases := []struct {
+	cases := pullerTestCases()
-		containerImage  string
-		policy          v1.PullPolicy
-		calledFunctions []string
-		inspectErr      error
-		pullerErr       error
-		expectedErr     []error
-	}{
-		{ // pull missing image
-			containerImage:  "missing_image",
-			policy:          v1.PullIfNotPresent,
-			calledFunctions: []string{"IsImagePresent", "PullImage"},
-			inspectErr:      nil,
-			pullerErr:       nil,
-			expectedErr:     []error{nil}},
-
-		{ // image present, don't pull
-			containerImage:  "present_image",
-			policy:          v1.PullIfNotPresent,
-			calledFunctions: []string{"IsImagePresent"},
-			inspectErr:      nil,
-			pullerErr:       nil,
-			expectedErr:     []error{nil, nil, nil}},
-		// image present, pull it
-		{containerImage: "present_image",
-			policy:          v1.PullAlways,
-			calledFunctions: []string{"IsImagePresent", "PullImage"},
-			inspectErr:      nil,
-			pullerErr:       nil,
-			expectedErr:     []error{nil, nil, nil}},
-		// missing image, error PullNever
-		{containerImage: "missing_image",
-			policy:          v1.PullNever,
-			calledFunctions: []string{"IsImagePresent"},
-			inspectErr:      nil,
-			pullerErr:       nil,
-			expectedErr:     []error{ErrImageNeverPull, ErrImageNeverPull, ErrImageNeverPull}},
-		// missing image, unable to inspect
-		{containerImage: "missing_image",
-			policy:          v1.PullIfNotPresent,
-			calledFunctions: []string{"IsImagePresent"},
-			inspectErr:      errors.New("unknown inspectError"),
-			pullerErr:       nil,
-			expectedErr:     []error{ErrImageInspect, ErrImageInspect, ErrImageInspect}},
-		// missing image, unable to fetch
-		{containerImage: "typo_image",
-			policy:          v1.PullIfNotPresent,
-			calledFunctions: []string{"IsImagePresent", "PullImage"},
-			inspectErr:      nil,
-			pullerErr:       errors.New("404"),
-			expectedErr:     []error{ErrImagePull, ErrImagePull, ErrImagePullBackOff, ErrImagePull, ErrImagePullBackOff, ErrImagePullBackOff}},
-	}
 
 	for i, c := range cases {
-		container := &v1.Container{
+		puller, fakeClock, fakeRuntime, container := pullerTestEnv(c, true)
-			Name:            "container_name",
-			Image:           c.containerImage,
-			ImagePullPolicy: c.policy,
-		}
-
-		backOff := flowcontrol.NewBackOff(time.Second, time.Minute)
-		fakeClock := clock.NewFakeClock(time.Now())
-		backOff.Clock = fakeClock
-
-		fakeRuntime := &ctest.FakeRuntime{}
-		fakeRecorder := &record.FakeRecorder{}
-		puller := NewImageManager(fakeRecorder, fakeRuntime, backOff, true, 0, 0)
-
-		fakeRuntime.ImageList = []Image{{ID: "present_image"}}
-		fakeRuntime.Err = c.pullerErr
-		fakeRuntime.InspectErr = c.inspectErr
 
 		for tick, expected := range c.expectedErr {
 			fakeClock.Step(time.Second)