Add image garbage collection policy to imageManager.

pkg/kubelet/cadvisor/cadvisor_fake.go

@@ -18,7 +18,7 @@ package cadvisor
 
 import (
 	cadvisorApi "github.com/google/cadvisor/info/v1"
-	cadvisorApi2 "github.com/google/cadvisor/info/v2"
+	cadvisorApiV2 "github.com/google/cadvisor/info/v2"
 )
 
 // Fake cAdvisor implementation.
@@ -39,6 +39,6 @@ func (c *Fake) MachineInfo() (*cadvisorApi.MachineInfo, error) {
 	return new(cadvisorApi.MachineInfo), nil
 }
 
-func (c *Fake) DockerImagesFsInfo() (cadvisorApi2.FsInfo, error) {
+func (c *Fake) DockerImagesFsInfo() (cadvisorApiV2.FsInfo, error) {
-	return cadvisorApi2.FsInfo{}, nil
+	return cadvisorApiV2.FsInfo{}, nil
 }

pkg/kubelet/cadvisor/cadvisor_linux.go

@@ -28,7 +28,7 @@ import (
 	cadvisorFs "github.com/google/cadvisor/fs"
 	cadvisorHttp "github.com/google/cadvisor/http"
 	cadvisorApi "github.com/google/cadvisor/info/v1"
-	cadvisorApi2 "github.com/google/cadvisor/info/v2"
+	cadvisorApiV2 "github.com/google/cadvisor/info/v2"
 	"github.com/google/cadvisor/manager"
 	"github.com/google/cadvisor/storage/memory"
 	"github.com/google/cadvisor/utils/sysfs"
@@ -113,13 +113,13 @@ func (self *cadvisorClient) MachineInfo() (*cadvisorApi.MachineInfo, error) {
 	return self.GetMachineInfo()
 }
 
-func (self *cadvisorClient) DockerImagesFsInfo() (cadvisorApi2.FsInfo, error) {
+func (self *cadvisorClient) DockerImagesFsInfo() (cadvisorApiV2.FsInfo, error) {
 	res, err := self.GetFsInfo(cadvisorFs.LabelDockerImages)
 	if err != nil {
-		return cadvisorApi2.FsInfo{}, err
+		return cadvisorApiV2.FsInfo{}, err
 	}
 	if len(res) == 0 {
-		return cadvisorApi2.FsInfo{}, fmt.Errorf("failed to find information for the filesystem containing Docker images")
+		return cadvisorApiV2.FsInfo{}, fmt.Errorf("failed to find information for the filesystem containing Docker images")
 	}
 	// TODO(vmarmol): Handle this better when Docker has more than one image filesystem.
 	if len(res) > 1 {

pkg/kubelet/cadvisor/cadvisor_mock.go

@@ -18,7 +18,7 @@ package cadvisor
 
 import (
 	cadvisorApi "github.com/google/cadvisor/info/v1"
-	cadvisorApi2 "github.com/google/cadvisor/info/v2"
+	cadvisorApiV2 "github.com/google/cadvisor/info/v2"
 	"github.com/stretchr/testify/mock"
 )
 
@@ -46,7 +46,7 @@ func (c *Mock) MachineInfo() (*cadvisorApi.MachineInfo, error) {
 	return args.Get(0).(*cadvisorApi.MachineInfo), args.Error(1)
 }
 
-func (c *Mock) DockerImagesFsInfo() (cadvisorApi2.FsInfo, error) {
+func (c *Mock) DockerImagesFsInfo() (cadvisorApiV2.FsInfo, error) {
 	args := c.Called()
-	return args.Get(0).(cadvisorApi2.FsInfo), args.Error(1)
+	return args.Get(0).(cadvisorApiV2.FsInfo), args.Error(1)
 }

pkg/kubelet/cadvisor/cadvisor_unsupported.go

@@ -22,7 +22,7 @@ import (
 	"errors"
 
 	cadvisorApi "github.com/google/cadvisor/info/v1"
-	cadvisorApi2 "github.com/google/cadvisor/info/v2"
+	cadvisorApiV2 "github.com/google/cadvisor/info/v2"
 )
 
 type cadvisorUnsupported struct {
@@ -48,6 +48,6 @@ func (self *cadvisorUnsupported) MachineInfo() (*cadvisorApi.MachineInfo, error)
 	return nil, unsupportedErr
 }
 
-func (self *cadvisorUnsupported) DockerImagesFsInfo() (cadvisorApi2.FsInfo, error) {
+func (self *cadvisorUnsupported) DockerImagesFsInfo() (cadvisorApiV2.FsInfo, error) {
-	return cadvisorApi2.FsInfo{}, unsupportedErr
+	return cadvisorApiV2.FsInfo{}, unsupportedErr
 }

pkg/kubelet/cadvisor/types.go

@@ -18,7 +18,7 @@ package cadvisor
 
 import (
 	cadvisorApi "github.com/google/cadvisor/info/v1"
-	cadvisorApi2 "github.com/google/cadvisor/info/v2"
+	cadvisorApiV2 "github.com/google/cadvisor/info/v2"
 )
 
 // Interface is an abstract interface for testability.  It abstracts the interface to cAdvisor.
@@ -28,5 +28,5 @@ type Interface interface {
 	MachineInfo() (*cadvisorApi.MachineInfo, error)
 
 	// Returns usage information about the filesystem holding Docker images.
-	DockerImagesFsInfo() (cadvisorApi2.FsInfo, error)
+	DockerImagesFsInfo() (cadvisorApiV2.FsInfo, error)
 }

pkg/kubelet/image_manager.go

@@ -17,10 +17,12 @@ limitations under the License.
 package kubelet
 
 import (
+	"fmt"
 	"sort"
 	"sync"
 	"time"
 
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/cadvisor"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	docker "github.com/fsouza/go-dockerclient"
@@ -29,31 +31,40 @@ import (
 
 // Manages lifecycle of all images.
 //
-// Class is thread-safe.
+// Implementation is thread-safe.
 type imageManager interface {
-	// Starts the image manager.
+	// Applies the garbage collection policy. Errors include being unable to free
-	Start() error
+	// enough space as per the garbage collection policy.
-
+	GarbageCollect() error
-	// Tries to free bytesToFree worth of images on the disk.
-	//
-	// Returns the number of bytes free and an error if any occured. The number of
-	// bytes freed is always returned.
-	// Note that error may be nil and the number of bytes free may be less
-	// than bytesToFree.
-	FreeSpace(bytesToFree int64) (int64, error)
 
 	// TODO(vmarmol): Have this subsume pulls as well.
 }
 
+// A policy for garbage collecting images. Policy defines an allowed band in
+// which garbage collection will be run.
+type ImageGCPolicy struct {
+	// Any usage above this threshold will always trigger garbage collection.
+	// This is the highest usage we will allow.
+	HighThresholdPercent int
+
+	// Any usage below this threshold will never trigger garbage collection.
+	// This is the lowest threshold we will try to garbage collect to.
+	LowThresholdPercent int
+}
+
 type realImageManager struct {
 	// Connection to the Docker daemon.
 	dockerClient dockertools.DockerInterface
 
 	// Records of images and their use.
-	imageRecords map[string]*imageRecord
+	imageRecords     map[string]*imageRecord
-
-	// Lock for imageRecords.
 	imageRecordsLock sync.Mutex
+
+	// The image garbage collection policy in use.
+	policy ImageGCPolicy
+
+	// cAdvisor instance.
+	cadvisor cadvisor.Interface
 }
 
 // Information about the images we track.
@@ -68,14 +79,30 @@ type imageRecord struct {
 	size int64
 }
 
-func newImageManager(dockerClient dockertools.DockerInterface) imageManager {
+func newImageManager(dockerClient dockertools.DockerInterface, cadvisorInterface cadvisor.Interface, policy ImageGCPolicy) (imageManager, error) {
-	return &realImageManager{
+	// Validate policy.
-		dockerClient: dockerClient,
+	if policy.HighThresholdPercent < 0 || policy.HighThresholdPercent > 100 {
-		imageRecords: make(map[string]*imageRecord),
+		return nil, fmt.Errorf("invalid HighThresholdPercent %d, must be in range [0-100]", policy.HighThresholdPercent)
 	}
+	if policy.LowThresholdPercent < 0 || policy.LowThresholdPercent > 100 {
+		return nil, fmt.Errorf("invalid LowThresholdPercent %d, must be in range [0-100]", policy.LowThresholdPercent)
+	}
+	im := &realImageManager{
+		dockerClient: dockerClient,
+		policy:       policy,
+		imageRecords: make(map[string]*imageRecord),
+		cadvisor:     cadvisorInterface,
+	}
+
+	err := im.start()
+	if err != nil {
+		return nil, fmt.Errorf("failed to start image manager: %v", err)
+	}
+
+	return im, nil
 }
 
-func (self *realImageManager) Start() error {
+func (self *realImageManager) start() error {
 	// Initial detection make detected time "unknown" in the past.
 	var zero time.Time
 	err := self.detectImages(zero)
@@ -83,7 +110,7 @@ func (self *realImageManager) Start() error {
 		return err
 	}
 
-	util.Forever(func() {
+	go util.Forever(func() {
 		err := self.detectImages(time.Now())
 		if err != nil {
 			glog.Warningf("[ImageManager] Failed to monitor images: %v", err)
@@ -144,7 +171,40 @@ func (self *realImageManager) detectImages(detected time.Time) error {
 	return nil
 }
 
-func (self *realImageManager) FreeSpace(bytesToFree int64) (int64, error) {
+func (self *realImageManager) GarbageCollect() error {
+	// Get disk usage on disk holding images.
+	fsInfo, err := self.cadvisor.DockerImagesFsInfo()
+	if err != nil {
+		return err
+	}
+	usage := int64(fsInfo.Usage)
+	capacity := int64(fsInfo.Capacity)
+	usagePercent := int(usage * 100 / capacity)
+
+	// If over the max threshold, free enough to place us at the lower threshold.
+	if usagePercent >= self.policy.HighThresholdPercent {
+		amountToFree := usage - (int64(self.policy.LowThresholdPercent) * capacity / 100)
+		freed, err := self.freeSpace(amountToFree)
+		if err != nil {
+			return err
+		}
+
+		if freed < amountToFree {
+			// TODO(vmarmol): Surface event.
+			return fmt.Errorf("failed to garbage collect required amount of images. Wanted to free %d, but freed %d", amountToFree, freed)
+		}
+	}
+
+	return nil
+}
+
+// Tries to free bytesToFree worth of images on the disk.
+//
+// Returns the number of bytes free and an error if any occured. The number of
+// bytes freed is always returned.
+// Note that error may be nil and the number of bytes free may be less
+// than bytesToFree.
+func (self *realImageManager) freeSpace(bytesToFree int64) (int64, error) {
 	startTime := time.Now()
 	err := self.detectImages(startTime)
 	if err != nil {

pkg/kubelet/image_manager_test.go

@@ -21,17 +21,39 @@ import (
 	"testing"
 	"time"
 
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/cadvisor"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	docker "github.com/fsouza/go-dockerclient"
+	cadvisorApiV2 "github.com/google/cadvisor/info/v2"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 var zero time.Time
 
-func newRealImageManager(dockerClient dockertools.DockerInterface) *realImageManager {
+func newRealImageManager(t *testing.T, policy ImageGCPolicy) (*realImageManager, *dockertools.FakeDockerClient, *cadvisor.Mock) {
-	return newImageManager(dockerClient).(*realImageManager)
+	fakeDocker := &dockertools.FakeDockerClient{
+		RemovedImages: util.NewStringSet(),
+	}
+	mockCadvisor := new(cadvisor.Mock)
+	im, err := newImageManager(fakeDocker, mockCadvisor, policy)
+	require.NoError(t, err)
+	return im.(*realImageManager), fakeDocker, mockCadvisor
+}
+
+// Accessors used for thread-safe testing.
+func (self *realImageManager) imageRecordsLen() int {
+	self.imageRecordsLock.Lock()
+	defer self.imageRecordsLock.Unlock()
+	return len(self.imageRecords)
+}
+func (self *realImageManager) getImageRecord(name string) (*imageRecord, bool) {
+	self.imageRecordsLock.Lock()
+	defer self.imageRecordsLock.Unlock()
+	v, ok := self.imageRecords[name]
+	vCopy := *v
+	return &vCopy, ok
 }
 
 // Returns the name of the image with the given ID.
@@ -56,27 +78,25 @@ func makeContainer(id int) docker.APIContainers {
 }
 
 func TestDetectImagesInitialDetect(t *testing.T) {
-	fakeDocker := &dockertools.FakeDockerClient{
+	manager, fakeDocker, _ := newRealImageManager(t, ImageGCPolicy{})
-		Images: []docker.APIImages{
+	fakeDocker.Images = []docker.APIImages{
-			makeImage(0, 1024),
+		makeImage(0, 1024),
-			makeImage(1, 2048),
+		makeImage(1, 2048),
-		},
+	}
-		ContainerList: []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
-			makeContainer(1),
+		makeContainer(1),
-		},
 	}
-	manager := newRealImageManager(fakeDocker)
 
 	startTime := time.Now().Add(-time.Millisecond)
 	err := manager.detectImages(zero)
 	assert := assert.New(t)
-	require.Nil(t, err)
+	require.NoError(t, err)
-	assert.Len(manager.imageRecords, 2)
+	assert.Equal(manager.imageRecordsLen(), 2)
-	noContainer, ok := manager.imageRecords[imageName(0)]
+	noContainer, ok := manager.getImageRecord(imageName(0))
 	require.True(t, ok)
 	assert.Equal(zero, noContainer.detected)
 	assert.Equal(zero, noContainer.lastUsed)
-	withContainer, ok := manager.imageRecords[imageName(1)]
+	withContainer, ok := manager.getImageRecord(imageName(1))
 	require.True(t, ok)
 	assert.Equal(zero, withContainer.detected)
 	assert.True(withContainer.lastUsed.After(startTime))
@@ -84,21 +104,19 @@ func TestDetectImagesInitialDetect(t *testing.T) {
 
 func TestDetectImagesWithNewImage(t *testing.T) {
 	// Just one image initially.
-	fakeDocker := &dockertools.FakeDockerClient{
+	manager, fakeDocker, _ := newRealImageManager(t, ImageGCPolicy{})
-		Images: []docker.APIImages{
+	fakeDocker.Images = []docker.APIImages{
-			makeImage(0, 1024),
+		makeImage(0, 1024),
-			makeImage(1, 2048),
+		makeImage(1, 2048),
-		},
+	}
-		ContainerList: []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
-			makeContainer(1),
+		makeContainer(1),
-		},
 	}
-	manager := newRealImageManager(fakeDocker)
 
 	err := manager.detectImages(zero)
 	assert := assert.New(t)
-	require.Nil(t, err)
+	require.NoError(t, err)
-	assert.Len(manager.imageRecords, 2)
+	assert.Equal(manager.imageRecordsLen(), 2)
 
 	// Add a new image.
 	fakeDocker.Images = []docker.APIImages{
@@ -110,147 +128,96 @@ func TestDetectImagesWithNewImage(t *testing.T) {
 	detectedTime := zero.Add(time.Second)
 	startTime := time.Now().Add(-time.Millisecond)
 	err = manager.detectImages(detectedTime)
-	require.Nil(t, err)
+	require.NoError(t, err)
-	assert.Len(manager.imageRecords, 3)
+	assert.Equal(manager.imageRecordsLen(), 3)
-	noContainer, ok := manager.imageRecords[imageName(0)]
+	noContainer, ok := manager.getImageRecord(imageName(0))
 	require.True(t, ok)
 	assert.Equal(zero, noContainer.detected)
 	assert.Equal(zero, noContainer.lastUsed)
-	withContainer, ok := manager.imageRecords[imageName(1)]
+	withContainer, ok := manager.getImageRecord(imageName(1))
 	require.True(t, ok)
 	assert.Equal(zero, withContainer.detected)
 	assert.True(withContainer.lastUsed.After(startTime))
-	newContainer, ok := manager.imageRecords[imageName(2)]
+	newContainer, ok := manager.getImageRecord(imageName(2))
 	require.True(t, ok)
 	assert.Equal(detectedTime, newContainer.detected)
 	assert.Equal(zero, noContainer.lastUsed)
 }
 
 func TestDetectImagesContainerStopped(t *testing.T) {
-	fakeDocker := &dockertools.FakeDockerClient{
+	manager, fakeDocker, _ := newRealImageManager(t, ImageGCPolicy{})
-		Images: []docker.APIImages{
+	fakeDocker.Images = []docker.APIImages{
-			makeImage(0, 1024),
+		makeImage(0, 1024),
-			makeImage(1, 2048),
+		makeImage(1, 2048),
-		},
+	}
-		ContainerList: []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
-			makeContainer(1),
+		makeContainer(1),
-		},
 	}
-	manager := newRealImageManager(fakeDocker)
 
 	err := manager.detectImages(zero)
 	assert := assert.New(t)
-	require.Nil(t, err)
+	require.NoError(t, err)
-	assert.Len(manager.imageRecords, 2)
+	assert.Equal(manager.imageRecordsLen(), 2)
-	withContainer, ok := manager.imageRecords[imageName(1)]
+	withContainer, ok := manager.getImageRecord(imageName(1))
 	require.True(t, ok)
 
 	// Simulate container being stopped.
 	fakeDocker.ContainerList = []docker.APIContainers{}
 	err = manager.detectImages(time.Now())
-	require.Nil(t, err)
+	require.NoError(t, err)
-	assert.Len(manager.imageRecords, 2)
+	assert.Equal(manager.imageRecordsLen(), 2)
-	container1, ok := manager.imageRecords[imageName(0)]
+	container1, ok := manager.getImageRecord(imageName(0))
 	require.True(t, ok)
 	assert.Equal(zero, container1.detected)
 	assert.Equal(zero, container1.lastUsed)
-	container2, ok := manager.imageRecords[imageName(1)]
+	container2, ok := manager.getImageRecord(imageName(1))
 	require.True(t, ok)
 	assert.Equal(zero, container2.detected)
 	assert.True(container2.lastUsed.Equal(withContainer.lastUsed))
 }
 
 func TestDetectImagesWithRemovedImages(t *testing.T) {
-	fakeDocker := &dockertools.FakeDockerClient{
+	manager, fakeDocker, _ := newRealImageManager(t, ImageGCPolicy{})
-		Images: []docker.APIImages{
+	fakeDocker.Images = []docker.APIImages{
-			makeImage(0, 1024),
+		makeImage(0, 1024),
-			makeImage(1, 2048),
+		makeImage(1, 2048),
-		},
+	}
-		ContainerList: []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
-			makeContainer(1),
+		makeContainer(1),
-		},
 	}
-	manager := newRealImageManager(fakeDocker)
 
 	err := manager.detectImages(zero)
 	assert := assert.New(t)
-	require.Nil(t, err)
+	require.NoError(t, err)
-	assert.Len(manager.imageRecords, 2)
+	assert.Equal(manager.imageRecordsLen(), 2)
 
 	// Simulate both images being removed.
 	fakeDocker.Images = []docker.APIImages{}
 	err = manager.detectImages(time.Now())
-	require.Nil(t, err)
+	require.NoError(t, err)
-	assert.Len(manager.imageRecords, 0)
+	assert.Equal(manager.imageRecordsLen(), 0)
 }
 
 func TestFreeSpaceImagesInUseContainersAreIgnored(t *testing.T) {
-	fakeDocker := &dockertools.FakeDockerClient{
+	manager, fakeDocker, _ := newRealImageManager(t, ImageGCPolicy{})
-		Images: []docker.APIImages{
+	fakeDocker.Images = []docker.APIImages{
-			makeImage(0, 1024),
+		makeImage(0, 1024),
-			makeImage(1, 2048),
+		makeImage(1, 2048),
-		},
+	}
-		ContainerList: []docker.APIContainers{
+	fakeDocker.ContainerList = []docker.APIContainers{
-			makeContainer(1),
+		makeContainer(1),
-		},
-		RemovedImages: util.NewStringSet(),
 	}
-	manager := newRealImageManager(fakeDocker)
 
-	spaceFreed, err := manager.FreeSpace(2048)
+	spaceFreed, err := manager.freeSpace(2048)
 	assert := assert.New(t)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	assert.Equal(1024, spaceFreed)
 	assert.Len(fakeDocker.RemovedImages, 1)
 	assert.True(fakeDocker.RemovedImages.Has(imageName(0)))
 }
 
 func TestFreeSpaceRemoveByLeastRecentlyUsed(t *testing.T) {
-	fakeDocker := &dockertools.FakeDockerClient{
+	manager, fakeDocker, _ := newRealImageManager(t, ImageGCPolicy{})
-		Images: []docker.APIImages{
-			makeImage(0, 1024),
-			makeImage(1, 2048),
-		},
-		ContainerList: []docker.APIContainers{
-			makeContainer(0),
-			makeContainer(1),
-		},
-		RemovedImages: util.NewStringSet(),
-	}
-	manager := newRealImageManager(fakeDocker)
-
-	// Make 1 be more recently used than 0.
-	require.Nil(t, manager.detectImages(zero))
-	fakeDocker.ContainerList = []docker.APIContainers{
-		makeContainer(1),
-	}
-	require.Nil(t, manager.detectImages(time.Now()))
-	fakeDocker.ContainerList = []docker.APIContainers{}
-	require.Nil(t, manager.detectImages(time.Now()))
-	require.Len(t, manager.imageRecords, 2)
-
-	spaceFreed, err := manager.FreeSpace(1024)
-	assert := assert.New(t)
-	require.Nil(t, err)
-	assert.Equal(1024, spaceFreed)
-	assert.Len(fakeDocker.RemovedImages, 1)
-	assert.True(fakeDocker.RemovedImages.Has(imageName(0)))
-}
-
-func TestFreeSpaceTiesBrokenByDetectedTime(t *testing.T) {
-	fakeDocker := &dockertools.FakeDockerClient{
-		Images: []docker.APIImages{
-			makeImage(0, 1024),
-		},
-		ContainerList: []docker.APIContainers{
-			makeContainer(0),
-		},
-		RemovedImages: util.NewStringSet(),
-	}
-	manager := newRealImageManager(fakeDocker)
-
-	// Make 1 more recently detected but used at the same time as 0.
-	require.Nil(t, manager.detectImages(zero))
 	fakeDocker.Images = []docker.APIImages{
 		makeImage(0, 1024),
 		makeImage(1, 2048),
@@ -259,43 +226,143 @@ func TestFreeSpaceTiesBrokenByDetectedTime(t *testing.T) {
 		makeContainer(0),
 		makeContainer(1),
 	}
-	require.Nil(t, manager.detectImages(time.Now()))
-	fakeDocker.ContainerList = []docker.APIContainers{}
-	require.Nil(t, manager.detectImages(time.Now()))
-	require.Len(t, manager.imageRecords, 2)
 
-	spaceFreed, err := manager.FreeSpace(1024)
+	// Make 1 be more recently used than 0.
+	require.NoError(t, manager.detectImages(zero))
+	fakeDocker.ContainerList = []docker.APIContainers{
+		makeContainer(1),
+	}
+	require.NoError(t, manager.detectImages(time.Now()))
+	fakeDocker.ContainerList = []docker.APIContainers{}
+	require.NoError(t, manager.detectImages(time.Now()))
+	require.Equal(t, manager.imageRecordsLen(), 2)
+
+	spaceFreed, err := manager.freeSpace(1024)
 	assert := assert.New(t)
-	require.Nil(t, err)
+	require.NoError(t, err)
+	assert.Equal(1024, spaceFreed)
+	assert.Len(fakeDocker.RemovedImages, 1)
+	assert.True(fakeDocker.RemovedImages.Has(imageName(0)))
+}
+
+func TestFreeSpaceTiesBrokenByDetectedTime(t *testing.T) {
+	manager, fakeDocker, _ := newRealImageManager(t, ImageGCPolicy{})
+	fakeDocker.Images = []docker.APIImages{
+		makeImage(0, 1024),
+	}
+	fakeDocker.ContainerList = []docker.APIContainers{
+		makeContainer(0),
+	}
+
+	// Make 1 more recently detected but used at the same time as 0.
+	require.NoError(t, manager.detectImages(zero))
+	fakeDocker.Images = []docker.APIImages{
+		makeImage(0, 1024),
+		makeImage(1, 2048),
+	}
+	fakeDocker.ContainerList = []docker.APIContainers{
+		makeContainer(0),
+		makeContainer(1),
+	}
+	require.NoError(t, manager.detectImages(time.Now()))
+	fakeDocker.ContainerList = []docker.APIContainers{}
+	require.NoError(t, manager.detectImages(time.Now()))
+	require.Equal(t, manager.imageRecordsLen(), 2)
+
+	spaceFreed, err := manager.freeSpace(1024)
+	assert := assert.New(t)
+	require.NoError(t, err)
 	assert.Equal(1024, spaceFreed)
 	assert.Len(fakeDocker.RemovedImages, 1)
 	assert.True(fakeDocker.RemovedImages.Has(imageName(0)))
 }
 
 func TestFreeSpaceImagesAlsoDoesLookupByRepoTags(t *testing.T) {
-	fakeDocker := &dockertools.FakeDockerClient{
+	manager, fakeDocker, _ := newRealImageManager(t, ImageGCPolicy{})
-		Images: []docker.APIImages{
+	fakeDocker.Images = []docker.APIImages{
-			makeImage(0, 1024),
+		makeImage(0, 1024),
-			{
+		{
-				ID:          "5678",
+			ID:          "5678",
-				RepoTags:    []string{"potato", "salad"},
+			RepoTags:    []string{"potato", "salad"},
-				VirtualSize: 2048,
+			VirtualSize: 2048,
-			},
+		},
+	}
+	fakeDocker.ContainerList = []docker.APIContainers{
+		{
+			ID:    "c5678",
+			Image: "salad",
 		},
-		ContainerList: []docker.APIContainers{
-			{
-				ID:    "c5678",
-				Image: "salad",
-			},
-		},
-		RemovedImages: util.NewStringSet(),
 	}
-	manager := newRealImageManager(fakeDocker)
 
-	spaceFreed, err := manager.FreeSpace(1024)
+	spaceFreed, err := manager.freeSpace(1024)
 	assert := assert.New(t)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	assert.Equal(1024, spaceFreed)
 	assert.Len(fakeDocker.RemovedImages, 1)
 	assert.True(fakeDocker.RemovedImages.Has(imageName(0)))
 }
+
+func TestGarbageCollectBelowLowThreshold(t *testing.T) {
+	policy := ImageGCPolicy{
+		HighThresholdPercent: 90,
+		LowThresholdPercent:  80,
+	}
+	manager, _, mockCadvisor := newRealImageManager(t, policy)
+
+	// Expect 40% usage.
+	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApiV2.FsInfo{
+		Usage:    400,
+		Capacity: 1000,
+	}, nil)
+
+	assert.NoError(t, manager.GarbageCollect())
+}
+
+func TestGarbageCollectCadvisorFailure(t *testing.T) {
+	policy := ImageGCPolicy{
+		HighThresholdPercent: 90,
+		LowThresholdPercent:  80,
+	}
+	manager, _, mockCadvisor := newRealImageManager(t, policy)
+
+	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApiV2.FsInfo{}, fmt.Errorf("error"))
+	assert.NotNil(t, manager.GarbageCollect())
+}
+
+func TestGarbageCollectBelowSuccess(t *testing.T) {
+	policy := ImageGCPolicy{
+		HighThresholdPercent: 90,
+		LowThresholdPercent:  80,
+	}
+	manager, fakeDocker, mockCadvisor := newRealImageManager(t, policy)
+
+	// Expect 95% usage and most of it gets freed.
+	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApiV2.FsInfo{
+		Usage:    950,
+		Capacity: 1000,
+	}, nil)
+	fakeDocker.Images = []docker.APIImages{
+		makeImage(0, 450),
+	}
+
+	assert.NoError(t, manager.GarbageCollect())
+}
+
+func TestGarbageCollectNotEnoughFreed(t *testing.T) {
+	policy := ImageGCPolicy{
+		HighThresholdPercent: 90,
+		LowThresholdPercent:  80,
+	}
+	manager, fakeDocker, mockCadvisor := newRealImageManager(t, policy)
+
+	// Expect 95% usage and little of it gets freed.
+	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApiV2.FsInfo{
+		Usage:    950,
+		Capacity: 1000,
+	}, nil)
+	fakeDocker.Images = []docker.APIImages{
+		makeImage(0, 50),
+	}
+
+	assert.NotNil(t, manager.GarbageCollect())
+}