Merge pull request #91885 from iamchuckss/test-throttling-image-service

Add test coverage for throttledImageService

pkg/kubelet/images/image_manager_test.go

@@ -37,29 +37,38 @@ type pullerExpects struct {
 }
 
 type pullerTestCase struct {
+	testName       string
 	containerImage string
 	policy         v1.PullPolicy
 	inspectErr     error
 	pullerErr      error
+	qps            float32
+	burst          int
 	expected       []pullerExpects
 }
 
 func pullerTestCases() []pullerTestCase {
 	return []pullerTestCase{
 		{ // pull missing image
+			testName:       "image missing, pull",
 			containerImage: "missing_image",
 			policy:         v1.PullIfNotPresent,
 			inspectErr:     nil,
 			pullerErr:      nil,
+			qps:            0.0,
+			burst:          0,
 			expected: []pullerExpects{
 				{[]string{"GetImageRef", "PullImage"}, nil},
 			}},
 
 		{ // image present, don't pull
+			testName:       "image present, don't pull ",
 			containerImage: "present_image",
 			policy:         v1.PullIfNotPresent,
 			inspectErr:     nil,
 			pullerErr:      nil,
+			qps:            0.0,
+			burst:          0,
 			expected: []pullerExpects{
 				{[]string{"GetImageRef"}, nil},
 				{[]string{"GetImageRef"}, nil},
@@ -67,9 +76,12 @@ func pullerTestCases() []pullerTestCase {
 			}},
 		// image present, pull it
 		{containerImage: "present_image",
+			testName:   "image present, pull ",
 			policy:     v1.PullAlways,
 			inspectErr: nil,
 			pullerErr:  nil,
+			qps:        0.0,
+			burst:      0,
 			expected: []pullerExpects{
 				{[]string{"GetImageRef", "PullImage"}, nil},
 				{[]string{"GetImageRef", "PullImage"}, nil},
@@ -77,9 +89,12 @@ func pullerTestCases() []pullerTestCase {
 			}},
 		// missing image, error PullNever
 		{containerImage: "missing_image",
+			testName:   "image missing, never pull",
 			policy:     v1.PullNever,
 			inspectErr: nil,
 			pullerErr:  nil,
+			qps:        0.0,
+			burst:      0,
 			expected: []pullerExpects{
 				{[]string{"GetImageRef"}, ErrImageNeverPull},
 				{[]string{"GetImageRef"}, ErrImageNeverPull},
@@ -87,9 +102,12 @@ func pullerTestCases() []pullerTestCase {
 			}},
 		// missing image, unable to inspect
 		{containerImage: "missing_image",
+			testName:   "image missing, pull if not present",
 			policy:     v1.PullIfNotPresent,
 			inspectErr: errors.New("unknown inspectError"),
 			pullerErr:  nil,
+			qps:        0.0,
+			burst:      0,
 			expected: []pullerExpects{
 				{[]string{"GetImageRef"}, ErrImageInspect},
 				{[]string{"GetImageRef"}, ErrImageInspect},
@@ -97,9 +115,12 @@ func pullerTestCases() []pullerTestCase {
 			}},
 		// missing image, unable to fetch
 		{containerImage: "typo_image",
+			testName:   "image missing, unable to fetch",
 			policy:     v1.PullIfNotPresent,
 			inspectErr: nil,
 			pullerErr:  errors.New("404"),
+			qps:        0.0,
+			burst:      0,
 			expected: []pullerExpects{
 				{[]string{"GetImageRef", "PullImage"}, ErrImagePull},
 				{[]string{"GetImageRef", "PullImage"}, ErrImagePull},
@@ -108,6 +129,32 @@ func pullerTestCases() []pullerTestCase {
 				{[]string{"GetImageRef"}, ErrImagePullBackOff},
 				{[]string{"GetImageRef"}, ErrImagePullBackOff},
 			}},
+		// image present, non-zero qps, try to pull
+		{containerImage: "present_image",
+			testName:   "image present and qps>0, pull",
+			policy:     v1.PullAlways,
+			inspectErr: nil,
+			pullerErr:  nil,
+			qps:        400.0,
+			burst:      600,
+			expected: []pullerExpects{
+				{[]string{"GetImageRef", "PullImage"}, nil},
+				{[]string{"GetImageRef", "PullImage"}, nil},
+				{[]string{"GetImageRef", "PullImage"}, nil},
+			}},
+		// image present, non-zero qps, try to pull when qps exceeded
+		{containerImage: "present_image",
+			testName:   "image present and excessive qps rate, pull",
+			policy:     v1.PullAlways,
+			inspectErr: nil,
+			pullerErr:  nil,
+			qps:        2000.0,
+			burst:      0,
+			expected: []pullerExpects{
+				{[]string{"GetImageRef"}, ErrImagePull},
+				{[]string{"GetImageRef"}, ErrImagePull},
+				{[]string{"GetImageRef"}, ErrImagePullBackOff},
+			}},
 	}
 }
 
@@ -129,7 +176,7 @@ func pullerTestEnv(c pullerTestCase, serialized bool) (puller ImageManager, fake
 	fakeRuntime.Err = c.pullerErr
 	fakeRuntime.InspectErr = c.inspectErr
 
-	puller = NewImageManager(fakeRecorder, fakeRuntime, backOff, serialized, 0, 0)
+	puller = NewImageManager(fakeRecorder, fakeRuntime, backOff, serialized, c.qps, c.burst)
 	return
 }
 
@@ -146,16 +193,18 @@ func TestParallelPuller(t *testing.T) {
 	cases := pullerTestCases()
 
 	useSerializedEnv := false
-	for i, c := range cases {
+	for _, c := range cases {
 		puller, fakeClock, fakeRuntime, container := pullerTestEnv(c, useSerializedEnv)
 
-		for tick, expected := range c.expected {
+		t.Run(c.testName, func(t *testing.T) {
+			for _, expected := range c.expected {
 				fakeRuntime.CalledFunctions = nil
 				fakeClock.Step(time.Second)
 				_, _, err := puller.EnsureImageExists(pod, container, nil, nil)
-			assert.NoError(t, fakeRuntime.AssertCalls(expected.calls), "in test %d tick=%d", i, tick)
+				assert.NoError(t, fakeRuntime.AssertCalls(expected.calls))
-			assert.Equal(t, expected.err, err, "in test %d tick=%d", i, tick)
+				assert.Equal(t, expected.err, err)
 			}
+		})
 	}
 }
 
@@ -172,34 +221,39 @@ func TestSerializedPuller(t *testing.T) {
 	cases := pullerTestCases()
 
 	useSerializedEnv := true
-	for i, c := range cases {
+	for _, c := range cases {
 		puller, fakeClock, fakeRuntime, container := pullerTestEnv(c, useSerializedEnv)
 
-		for tick, expected := range c.expected {
+		t.Run(c.testName, func(t *testing.T) {
+			for _, expected := range c.expected {
 				fakeRuntime.CalledFunctions = nil
 				fakeClock.Step(time.Second)
 				_, _, err := puller.EnsureImageExists(pod, container, nil, nil)
-			assert.NoError(t, fakeRuntime.AssertCalls(expected.calls), "in test %d tick=%d", i, tick)
+				assert.NoError(t, fakeRuntime.AssertCalls(expected.calls))
-			assert.Equal(t, expected.err, err, "in test %d tick=%d", i, tick)
+				assert.Equal(t, expected.err, err)
 			}
+		})
 	}
 }
 
 func TestApplyDefaultImageTag(t *testing.T) {
 	for _, testCase := range []struct {
+		testName string
 		Input    string
 		Output   string
 	}{
-		{Input: "root", Output: "root:latest"},
+		{testName: "root", Input: "root", Output: "root:latest"},
-		{Input: "root:tag", Output: "root:tag"},
+		{testName: "root:tag", Input: "root:tag", Output: "root:tag"},
-		{Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Output: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
+		{testName: "root@sha", Input: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Output: "root@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
 	} {
+		t.Run(testCase.testName, func(t *testing.T) {
 			image, err := applyDefaultImageTag(testCase.Input)
 			if err != nil {
 				t.Errorf("applyDefaultImageTag(%s) failed: %v", testCase.Input, err)
 			} else if image != testCase.Output {
 				t.Errorf("Expected image reference: %q, got %q", testCase.Output, image)
 			}
+		})
 	}
 }
 
@@ -216,6 +270,7 @@ func TestPullAndListImageWithPodAnnotations(t *testing.T) {
 			},
 		}}
 	c := pullerTestCase{ // pull missing image
+		testName:       "test pull and list image with pod annotations",
 		containerImage: "missing_image",
 		policy:         v1.PullIfNotPresent,
 		inspectErr:     nil,
@@ -230,6 +285,7 @@ func TestPullAndListImageWithPodAnnotations(t *testing.T) {
 	fakeRuntime.ImageList = []Image{}
 	fakeClock.Step(time.Second)
 
+	t.Run(c.testName, func(t *testing.T) {
 		_, _, err := puller.EnsureImageExists(pod, container, nil, nil)
 		assert.NoError(t, fakeRuntime.AssertCalls(c.expected[0].calls), "tick=%d", 0)
 		assert.Equal(t, c.expected[0].err, err, "tick=%d", 0)
@@ -246,4 +302,5 @@ func TestPullAndListImageWithPodAnnotations(t *testing.T) {
 				Value: "handler_name",
 			}}
 		assert.Equal(t, expectedAnnotations, image.Spec.Annotations, "image spec annotations")
+	})
 }