Add unit test for image history cache

pkg/kubelet/dockertools/fake_docker_client.go

@@ -30,7 +30,6 @@ import (
 	dockercontainer "github.com/docker/engine-api/types/container"
 
 	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/util/sets"
 )
 
 // FakeDockerClient is a simple fake docker client, so that kubelet can be run for testing without requiring a real docker setup.
@@ -49,7 +48,6 @@ type FakeDockerClient struct {
 	Created         []string
 	Stopped         []string
 	Removed         []string
-	RemovedImages   sets.String
 	VersionInfo     dockertypes.Version
 	Information     dockertypes.Info
 	ExecInspect     *dockertypes.ContainerExecInspect
@@ -70,7 +68,6 @@ func NewFakeDockerClientWithVersion(version, apiVersion string) *FakeDockerClien
 	return &FakeDockerClient{
 		VersionInfo:  dockertypes.Version{Version: version, APIVersion: apiVersion},
 		Errors:       make(map[string]error),
-		RemovedImages: sets.String{},
 		ContainerMap: make(map[string]*dockertypes.ContainerJSON),
 	}
 }
@@ -471,6 +468,7 @@ func (f *FakeDockerClient) InspectExec(id string) (*dockertypes.ContainerExecIns
 }
 
 func (f *FakeDockerClient) ListImages(opts dockertypes.ImageListOptions) ([]dockertypes.Image, error) {
+	f.called = append(f.called, "list_images")
 	err := f.popError("list_images")
 	return f.Images, err
 }
@@ -478,7 +476,12 @@ func (f *FakeDockerClient) ListImages(opts dockertypes.ImageListOptions) ([]dock
 func (f *FakeDockerClient) RemoveImage(image string, opts dockertypes.ImageRemoveOptions) ([]dockertypes.ImageDelete, error) {
 	err := f.popError("remove_image")
 	if err == nil {
-		f.RemovedImages.Insert(image)
+		for i := range f.Images {
+			if f.Images[i].ID == image {
+				f.Images = append(f.Images[:i], f.Images[i+1:]...)
+				break
+			}
+		}
 	}
 	return []dockertypes.ImageDelete{{Deleted: image}}, err
 }
@@ -538,6 +541,7 @@ func (f *FakeDockerPuller) IsImagePresent(name string) (bool, error) {
 func (f *FakeDockerClient) ImageHistory(id string) ([]dockertypes.ImageHistory, error) {
 	f.Lock()
 	defer f.Unlock()
+	f.called = append(f.called, "image_history")
 	history := f.ImageHistoryMap[id]
 	return history, nil
 }

pkg/kubelet/dockertools/images_test.go

@@ -25,10 +25,11 @@ import (
 
 func TestImageStatsNoImages(t *testing.T) {
 	fakeDockerClient := NewFakeDockerClientWithVersion("1.2.3", "1.2")
-	isp := &imageStatsProvider{fakeDockerClient}
+	isp := newImageStatsProvider(fakeDockerClient)
 	st, err := isp.ImageStats()
 	as := assert.New(t)
 	as.NoError(err)
+	as.NoError(fakeDockerClient.AssertCalls([]string{"list_images"}))
 	as.Equal(st.TotalStorageBytes, uint64(0))
 }
 
@@ -94,10 +95,240 @@ func TestImageStatsWithImages(t *testing.T) {
 			ID: "busybox-new",
 		},
 	})
-	isp := &imageStatsProvider{fakeDockerClient}
+	isp := newImageStatsProvider(fakeDockerClient)
 	st, err := isp.ImageStats()
 	as := assert.New(t)
 	as.NoError(err)
+	as.NoError(fakeDockerClient.AssertCalls([]string{"list_images", "image_history", "image_history", "image_history"}))
 	const expectedOutput uint64 = 1300
 	as.Equal(expectedOutput, st.TotalStorageBytes, "expected %d, got %d", expectedOutput, st.TotalStorageBytes)
 }
+
+func TestImageStatsWithCachedImages(t *testing.T) {
+	for _, test := range []struct {
+		oldLayers                map[string]*dockertypes.ImageHistory
+		oldImageToLayerIDs       map[string][]string
+		images                   []dockertypes.Image
+		history                  map[string][]dockertypes.ImageHistory
+		expectedCalls            []string
+		expectedLayers           map[string]*dockertypes.ImageHistory
+		expectedImageToLayerIDs  map[string][]string
+		expectedTotalStorageSize uint64
+	}{
+		{
+			// No cache
+			oldLayers:          make(map[string]*dockertypes.ImageHistory),
+			oldImageToLayerIDs: make(map[string][]string),
+			images: []dockertypes.Image{
+				{
+					ID: "busybox",
+				},
+				{
+					ID: "kubelet",
+				},
+			},
+			history: map[string][]dockertypes.ImageHistory{
+				"busybox": {
+					{
+						ID:        "0123456",
+						CreatedBy: "foo",
+						Size:      100,
+					},
+					{
+						ID:        "<missing>",
+						CreatedBy: "baz",
+						Size:      300,
+					},
+				},
+				"kubelet": {
+					{
+						ID:        "1123456",
+						CreatedBy: "foo",
+						Size:      200,
+					},
+					{
+						ID:        "<missing>",
+						CreatedBy: "1baz",
+						Size:      400,
+					},
+				},
+			},
+			expectedCalls: []string{"list_images", "image_history", "image_history"},
+			expectedLayers: map[string]*dockertypes.ImageHistory{
+				"0123456": {
+					ID:        "0123456",
+					CreatedBy: "foo",
+					Size:      100,
+				},
+				"1123456": {
+					ID:        "1123456",
+					CreatedBy: "foo",
+					Size:      200,
+				},
+				"<missing>baz": {
+					ID:        "<missing>",
+					CreatedBy: "baz",
+					Size:      300,
+				},
+				"<missing>1baz": {
+					ID:        "<missing>",
+					CreatedBy: "1baz",
+					Size:      400,
+				},
+			},
+			expectedImageToLayerIDs: map[string][]string{
+				"busybox": {"0123456", "<missing>baz"},
+				"kubelet": {"1123456", "<missing>1baz"},
+			},
+			expectedTotalStorageSize: 1000,
+		},
+		{
+			// Use cache value
+			oldLayers: map[string]*dockertypes.ImageHistory{
+				"0123456": {
+					ID:        "0123456",
+					CreatedBy: "foo",
+					Size:      100,
+				},
+				"<missing>baz": {
+					ID:        "<missing>",
+					CreatedBy: "baz",
+					Size:      300,
+				},
+			},
+			oldImageToLayerIDs: map[string][]string{
+				"busybox": {"0123456", "<missing>baz"},
+			},
+			images: []dockertypes.Image{
+				{
+					ID: "busybox",
+				},
+				{
+					ID: "kubelet",
+				},
+			},
+			history: map[string][]dockertypes.ImageHistory{
+				"busybox": {
+					{
+						ID:        "0123456",
+						CreatedBy: "foo",
+						Size:      100,
+					},
+					{
+						ID:        "<missing>",
+						CreatedBy: "baz",
+						Size:      300,
+					},
+				},
+				"kubelet": {
+					{
+						ID:        "1123456",
+						CreatedBy: "foo",
+						Size:      200,
+					},
+					{
+						ID:        "<missing>",
+						CreatedBy: "1baz",
+						Size:      400,
+					},
+				},
+			},
+			expectedCalls: []string{"list_images", "image_history"},
+			expectedLayers: map[string]*dockertypes.ImageHistory{
+				"0123456": {
+					ID:        "0123456",
+					CreatedBy: "foo",
+					Size:      100,
+				},
+				"1123456": {
+					ID:        "1123456",
+					CreatedBy: "foo",
+					Size:      200,
+				},
+				"<missing>baz": {
+					ID:        "<missing>",
+					CreatedBy: "baz",
+					Size:      300,
+				},
+				"<missing>1baz": {
+					ID:        "<missing>",
+					CreatedBy: "1baz",
+					Size:      400,
+				},
+			},
+			expectedImageToLayerIDs: map[string][]string{
+				"busybox": {"0123456", "<missing>baz"},
+				"kubelet": {"1123456", "<missing>1baz"},
+			},
+			expectedTotalStorageSize: 1000,
+		},
+		{
+			// Unused cache value
+			oldLayers: map[string]*dockertypes.ImageHistory{
+				"0123456": {
+					ID:        "0123456",
+					CreatedBy: "foo",
+					Size:      100,
+				},
+				"<missing>baz": {
+					ID:        "<missing>",
+					CreatedBy: "baz",
+					Size:      300,
+				},
+			},
+			oldImageToLayerIDs: map[string][]string{
+				"busybox": {"0123456", "<missing>baz"},
+			},
+			images: []dockertypes.Image{
+				{
+					ID: "kubelet",
+				},
+			},
+			history: map[string][]dockertypes.ImageHistory{
+				"kubelet": {
+					{
+						ID:        "1123456",
+						CreatedBy: "foo",
+						Size:      200,
+					},
+					{
+						ID:        "<missing>",
+						CreatedBy: "1baz",
+						Size:      400,
+					},
+				},
+			},
+			expectedCalls: []string{"list_images", "image_history"},
+			expectedLayers: map[string]*dockertypes.ImageHistory{
+				"1123456": {
+					ID:        "1123456",
+					CreatedBy: "foo",
+					Size:      200,
+				},
+				"<missing>1baz": {
+					ID:        "<missing>",
+					CreatedBy: "1baz",
+					Size:      400,
+				},
+			},
+			expectedImageToLayerIDs: map[string][]string{
+				"kubelet": {"1123456", "<missing>1baz"},
+			},
+			expectedTotalStorageSize: 600,
+		},
+	} {
+		fakeDockerClient := NewFakeDockerClientWithVersion("1.2.3", "1.2")
+		fakeDockerClient.InjectImages(test.images)
+		fakeDockerClient.InjectImageHistory(test.history)
+		isp := newImageStatsProvider(fakeDockerClient)
+		isp.layers = test.oldLayers
+		isp.imageToLayerIDs = test.oldImageToLayerIDs
+		st, err := isp.ImageStats()
+		as := assert.New(t)
+		as.NoError(err)
+		as.NoError(fakeDockerClient.AssertCalls(test.expectedCalls))
+		as.Equal(test.expectedLayers, isp.layers, "expected %+v, got %+v", test.expectedLayers, isp.layers)
+		as.Equal(test.expectedImageToLayerIDs, isp.imageToLayerIDs, "expected %+v, got %+v", test.expectedImageToLayerIDs, isp.imageToLayerIDs)
+		as.Equal(test.expectedTotalStorageSize, st.TotalStorageBytes, "expected %d, got %d", test.expectedTotalStorageSize, st.TotalStorageBytes)
+	}
+}