Fix AssertCalls usage for kubelet fake runtimes

Despite its name, AssertCalls() does not assert anything. It returns an
error that must be checked. This was causing false negatives for
a handful of unit tests.

pkg/kubelet/images/image_manager_test.go

@@ -31,60 +31,83 @@ import (
 	ctest "k8s.io/kubernetes/pkg/kubelet/container/testing"
 )
 
+type pullerExpects struct {
+	calls []string
+	err   error
+}
+
 type pullerTestCase struct {
-	containerImage  string
+	containerImage string
-	policy          v1.PullPolicy
+	policy         v1.PullPolicy
-	calledFunctions []string
+	inspectErr     error
-	inspectErr      error
+	pullerErr      error
-	pullerErr       error
+	expected       []pullerExpects
-	expectedErr     []error
 }
 
 func pullerTestCases() []pullerTestCase {
 	return []pullerTestCase{
 		{ // pull missing image
-			containerImage:  "missing_image",
+			containerImage: "missing_image",
-			policy:          v1.PullIfNotPresent,
+			policy:         v1.PullIfNotPresent,
-			calledFunctions: []string{"GetImageRef", "PullImage"},
+			inspectErr:     nil,
-			inspectErr:      nil,
+			pullerErr:      nil,
-			pullerErr:       nil,
+			expected: []pullerExpects{
-			expectedErr:     []error{nil}},
+				{[]string{"GetImageRef", "PullImage"}, nil},
+			}},
 
 		{ // image present, don't pull
-			containerImage:  "present_image",
+			containerImage: "present_image",
-			policy:          v1.PullIfNotPresent,
+			policy:         v1.PullIfNotPresent,
-			calledFunctions: []string{"GetImageRef"},
+			inspectErr:     nil,
-			inspectErr:      nil,
+			pullerErr:      nil,
-			pullerErr:       nil,
+			expected: []pullerExpects{
-			expectedErr:     []error{nil, nil, nil}},
+				{[]string{"GetImageRef"}, nil},
+				{[]string{"GetImageRef"}, nil},
+				{[]string{"GetImageRef"}, nil},
+			}},
 		// image present, pull it
 		{containerImage: "present_image",
-			policy:          v1.PullAlways,
+			policy:     v1.PullAlways,
-			calledFunctions: []string{"GetImageRef", "PullImage"},
+			inspectErr: nil,
-			inspectErr:      nil,
+			pullerErr:  nil,
-			pullerErr:       nil,
+			expected: []pullerExpects{
-			expectedErr:     []error{nil, nil, nil}},
+				{[]string{"GetImageRef", "PullImage"}, nil},
+				{[]string{"GetImageRef", "PullImage"}, nil},
+				{[]string{"GetImageRef", "PullImage"}, nil},
+			}},
 		// missing image, error PullNever
 		{containerImage: "missing_image",
-			policy:          v1.PullNever,
+			policy:     v1.PullNever,
-			calledFunctions: []string{"GetImageRef"},
+			inspectErr: nil,
-			inspectErr:      nil,
+			pullerErr:  nil,
-			pullerErr:       nil,
+			expected: []pullerExpects{
-			expectedErr:     []error{ErrImageNeverPull, ErrImageNeverPull, ErrImageNeverPull}},
+				{[]string{"GetImageRef"}, ErrImageNeverPull},
+				{[]string{"GetImageRef"}, ErrImageNeverPull},
+				{[]string{"GetImageRef"}, ErrImageNeverPull},
+			}},
 		// missing image, unable to inspect
 		{containerImage: "missing_image",
-			policy:          v1.PullIfNotPresent,
+			policy:     v1.PullIfNotPresent,
-			calledFunctions: []string{"GetImageRef"},
+			inspectErr: errors.New("unknown inspectError"),
-			inspectErr:      errors.New("unknown inspectError"),
+			pullerErr:  nil,
-			pullerErr:       nil,
+			expected: []pullerExpects{
-			expectedErr:     []error{ErrImageInspect, ErrImageInspect, ErrImageInspect}},
+				{[]string{"GetImageRef"}, ErrImageInspect},
+				{[]string{"GetImageRef"}, ErrImageInspect},
+				{[]string{"GetImageRef"}, ErrImageInspect},
+			}},
 		// missing image, unable to fetch
 		{containerImage: "typo_image",
-			policy:          v1.PullIfNotPresent,
+			policy:     v1.PullIfNotPresent,
-			calledFunctions: []string{"GetImageRef", "PullImage"},
+			inspectErr: nil,
-			inspectErr:      nil,
+			pullerErr:  errors.New("404"),
-			pullerErr:       errors.New("404"),
+			expected: []pullerExpects{
-			expectedErr:     []error{ErrImagePull, ErrImagePull, ErrImagePullBackOff, ErrImagePull, ErrImagePullBackOff, ErrImagePullBackOff}},
+				{[]string{"GetImageRef", "PullImage"}, ErrImagePull},
+				{[]string{"GetImageRef", "PullImage"}, ErrImagePull},
+				{[]string{"GetImageRef"}, ErrImagePullBackOff},
+				{[]string{"GetImageRef", "PullImage"}, ErrImagePull},
+				{[]string{"GetImageRef"}, ErrImagePullBackOff},
+				{[]string{"GetImageRef"}, ErrImagePullBackOff},
+			}},
 	}
 }
 
@@ -102,7 +125,7 @@ func pullerTestEnv(c pullerTestCase, serialized bool) (puller ImageManager, fake
 	fakeRuntime = &ctest.FakeRuntime{}
 	fakeRecorder := &record.FakeRecorder{}
 
-	fakeRuntime.ImageList = []Image{{ID: "present_image"}}
+	fakeRuntime.ImageList = []Image{{ID: "present_image:latest"}}
 	fakeRuntime.Err = c.pullerErr
 	fakeRuntime.InspectErr = c.inspectErr
 
@@ -125,11 +148,12 @@ func TestParallelPuller(t *testing.T) {
 	for i, c := range cases {
 		puller, fakeClock, fakeRuntime, container := pullerTestEnv(c, false)
 
-		for tick, expected := range c.expectedErr {
+		for tick, expected := range c.expected {
+			fakeRuntime.CalledFunctions = nil
 			fakeClock.Step(time.Second)
 			_, _, err := puller.EnsureImageExists(pod, container, nil)
-			fakeRuntime.AssertCalls(c.calledFunctions)
+			assert.NoError(t, fakeRuntime.AssertCalls(expected.calls), "in test %d tick=%d", i, tick)
-			assert.Equal(t, expected, err, "in test %d tick=%d", i, tick)
+			assert.Equal(t, expected.err, err, "in test %d tick=%d", i, tick)
 		}
 	}
 }
@@ -149,11 +173,12 @@ func TestSerializedPuller(t *testing.T) {
 	for i, c := range cases {
 		puller, fakeClock, fakeRuntime, container := pullerTestEnv(c, true)
 
-		for tick, expected := range c.expectedErr {
+		for tick, expected := range c.expected {
+			fakeRuntime.CalledFunctions = nil
 			fakeClock.Step(time.Second)
 			_, _, err := puller.EnsureImageExists(pod, container, nil)
-			fakeRuntime.AssertCalls(c.calledFunctions)
+			assert.NoError(t, fakeRuntime.AssertCalls(expected.calls), "in test %d tick=%d", i, tick)
-			assert.Equal(t, expected, err, "in test %d tick=%d", i, tick)
+			assert.Equal(t, expected.err, err, "in test %d tick=%d", i, tick)
 		}
 	}
 }

pkg/kubelet/kuberuntime/kuberuntime_container_test.go

@@ -60,7 +60,7 @@ func TestRemoveContainer(t *testing.T) {
 	expectedContainerLogSymlink := legacyLogSymlink(containerId, "foo", "bar", "new")
 	assert.Equal(t, fakeOS.Removes, []string{expectedContainerLogPath, expectedContainerLogSymlink})
 	// Verify container is removed
-	fakeRuntime.AssertCalls([]string{"RemoveContainer"})
+	assert.Contains(t, fakeRuntime.Called, "RemoveContainer")
 	containers, err := fakeRuntime.ListContainers(&runtimeapi.ContainerFilter{Id: containerId})
 	assert.NoError(t, err)
 	assert.Empty(t, containers)

pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go

@@ -57,7 +57,7 @@ func TestCreatePodSandbox(t *testing.T) {
 	}
 	id, _, err := m.createPodSandbox(pod, 1)
 	assert.NoError(t, err)
-	fakeRuntime.AssertCalls([]string{"RunPodSandbox"})
+	assert.Contains(t, fakeRuntime.Called, "RunPodSandbox")
 	sandboxes, err := fakeRuntime.ListPodSandbox(&runtimeapi.PodSandboxFilter{Id: id})
 	assert.NoError(t, err)
 	assert.Equal(t, len(sandboxes), 1)