From f2150967152be8b6eda814dd7010f188cdc4b4c8 Mon Sep 17 00:00:00 2001
From: mattjmcnaughton <mattjmcnaughton@gmail.com>
Date: Sat, 22 Feb 2020 12:06:40 -0500
Subject: [PATCH] Add error path testing to image handling by
 `kubeGenericRuntimeManager`

In https://github.com/kubernetes/kubernetes/pull/88372, we added the
ability to inject errors to the `FakeImageService`. Use this ability to
test the error paths executed by the `kubeGenericRuntimeManager` when
underlying `ImageService` calls fail.

I don't foresee this change having a huge impact, but it should set a
good precedent for test coverage, and should the failure case behavior
become more "interesting" or risky in the future, we already will have
the scaffolding in place with which we can expand the tests.
---
 .../kuberuntime/kuberuntime_image_test.go     | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/pkg/kubelet/kuberuntime/kuberuntime_image_test.go b/pkg/kubelet/kuberuntime/kuberuntime_image_test.go
index bb210c5bb22..2857822b887 100644
--- a/pkg/kubelet/kuberuntime/kuberuntime_image_test.go
+++ b/pkg/kubelet/kuberuntime/kuberuntime_image_test.go
@@ -18,6 +18,7 @@ package kuberuntime
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -44,6 +45,20 @@ func TestPullImage(t *testing.T) {
 	assert.Equal(t, images[0].RepoTags, []string{"busybox"})
 }
 
+func TestPullImageWithError(t *testing.T) {
+	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+
+	fakeImageService.InjectError("PullImage", fmt.Errorf("test-error"))
+	imageRef, err := fakeManager.PullImage(kubecontainer.ImageSpec{Image: "busybox"}, nil, nil)
+	assert.Error(t, err)
+	assert.Equal(t, "", imageRef)
+
+	images, err := fakeManager.ListImages()
+	assert.NoError(t, err)
+	assert.Equal(t, 0, len(images))
+}
+
 func TestListImages(t *testing.T) {
 	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
 	assert.NoError(t, err)
@@ -62,6 +77,17 @@ func TestListImages(t *testing.T) {
 	assert.Equal(t, expected.List(), actual.List())
 }
 
+func TestListImagesWithError(t *testing.T) {
+	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+
+	fakeImageService.InjectError("ListImages", fmt.Errorf("test-failure"))
+
+	actualImages, err := fakeManager.ListImages()
+	assert.Error(t, err)
+	assert.Nil(t, actualImages)
+}
+
 func TestGetImageRef(t *testing.T) {
 	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
 	assert.NoError(t, err)
@@ -73,6 +99,32 @@ func TestGetImageRef(t *testing.T) {
 	assert.Equal(t, image, imageRef)
 }
 
+func TestGetImageRefImageNotAvailableLocally(t *testing.T) {
+	_, _, fakeManager, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+
+	image := "busybox"
+
+	imageRef, err := fakeManager.GetImageRef(kubecontainer.ImageSpec{Image: image})
+	assert.NoError(t, err)
+
+	imageNotAvailableLocallyRef := ""
+	assert.Equal(t, imageNotAvailableLocallyRef, imageRef)
+}
+
+func TestGetImageRefWithError(t *testing.T) {
+	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+
+	image := "busybox"
+
+	fakeImageService.InjectError("ImageStatus", fmt.Errorf("test-error"))
+
+	imageRef, err := fakeManager.GetImageRef(kubecontainer.ImageSpec{Image: image})
+	assert.Error(t, err)
+	assert.Equal(t, "", imageRef)
+}
+
 func TestRemoveImage(t *testing.T) {
 	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
 	assert.NoError(t, err)
@@ -86,6 +138,29 @@ func TestRemoveImage(t *testing.T) {
 	assert.Equal(t, 0, len(fakeImageService.Images))
 }
 
+func TestRemoveImageNoOpIfImageNotLocal(t *testing.T) {
+	_, _, fakeManager, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+
+	err = fakeManager.RemoveImage(kubecontainer.ImageSpec{Image: "busybox"})
+	assert.NoError(t, err)
+}
+
+func TestRemoveImageWithError(t *testing.T) {
+	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+
+	_, err = fakeManager.PullImage(kubecontainer.ImageSpec{Image: "busybox"}, nil, nil)
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(fakeImageService.Images))
+
+	fakeImageService.InjectError("RemoveImage", fmt.Errorf("test-failure"))
+
+	err = fakeManager.RemoveImage(kubecontainer.ImageSpec{Image: "busybox"})
+	assert.Error(t, err)
+	assert.Equal(t, 1, len(fakeImageService.Images))
+}
+
 func TestImageStats(t *testing.T) {
 	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
 	assert.NoError(t, err)
@@ -101,6 +176,17 @@ func TestImageStats(t *testing.T) {
 	assert.Equal(t, expectedStats, actualStats)
 }
 
+func TestImageStatsWithError(t *testing.T) {
+	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
+	assert.NoError(t, err)
+
+	fakeImageService.InjectError("ListImages", fmt.Errorf("test-failure"))
+
+	actualImageStats, err := fakeManager.ImageStats()
+	assert.Error(t, err)
+	assert.Nil(t, actualImageStats)
+}
+
 func TestPullWithSecrets(t *testing.T) {
 	// auth value is equivalent to: "username":"passed-user","password":"passed-password"
 	dockerCfg := map[string]map[string]string{"index.docker.io/v1/": {"email": "passed-email", "auth": "cGFzc2VkLXVzZXI6cGFzc2VkLXBhc3N3b3Jk"}}