From 4f400e5d2f35f47f98dcb57635c80e86dbe6484a Mon Sep 17 00:00:00 2001 From: Di Xu Date: Mon, 11 Dec 2017 15:20:55 +0800 Subject: [PATCH] ignore images in used by running containers when GC --- pkg/kubelet/images/image_gc_manager.go | 25 +++++++++------ pkg/kubelet/images/image_gc_manager_test.go | 35 ++++++++++++--------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/pkg/kubelet/images/image_gc_manager.go b/pkg/kubelet/images/image_gc_manager.go index 344e0156a498b..b8503968bef29 100644 --- a/pkg/kubelet/images/image_gc_manager.go +++ b/pkg/kubelet/images/image_gc_manager.go @@ -168,7 +168,7 @@ func (im *realImageGCManager) Start() { if im.initialized { ts = time.Now() } - err := im.detectImages(ts) + _, err := im.detectImages(ts) if err != nil { glog.Warningf("[imageGCManager] Failed to monitor images: %v", err) } else { @@ -194,18 +194,19 @@ func (im *realImageGCManager) GetImageList() ([]kubecontainer.Image, error) { return im.imageCache.get(), nil } -func (im *realImageGCManager) detectImages(detectTime time.Time) error { +func (im *realImageGCManager) detectImages(detectTime time.Time) (sets.String, error) { + imagesInUse := sets.NewString() + images, err := im.runtime.ListImages() if err != nil { - return err + return imagesInUse, err } pods, err := im.runtime.GetPods(true) if err != nil { - return err + return imagesInUse, err } // Make a set of images in use by containers. - imagesInUse := sets.NewString() for _, pod := range pods { for _, container := range pod.Containers { glog.V(5).Infof("Pod %s/%s, container %s uses image %s(%s)", pod.Namespace, pod.Name, container.Name, container.Image, container.ImageID) @@ -231,7 +232,7 @@ func (im *realImageGCManager) detectImages(detectTime time.Time) error { } // Set last used time to now if the image is being used. - if isImageUsed(image, imagesInUse) { + if isImageUsed(image.ID, imagesInUse) { glog.V(5).Infof("Setting Image ID %s lastUsed to %v", image.ID, now) im.imageRecords[image.ID].lastUsed = now } @@ -248,7 +249,7 @@ func (im *realImageGCManager) detectImages(detectTime time.Time) error { } } - return nil + return imagesInUse, nil } func (im *realImageGCManager) GarbageCollect() error { @@ -309,7 +310,7 @@ func (im *realImageGCManager) DeleteUnusedImages() (int64, error) { // Note that error may be nil and the number of bytes free may be less // than bytesToFree. func (im *realImageGCManager) freeSpace(bytesToFree int64, freeTime time.Time) (int64, error) { - err := im.detectImages(freeTime) + imagesInUse, err := im.detectImages(freeTime) if err != nil { return 0, err } @@ -320,6 +321,10 @@ func (im *realImageGCManager) freeSpace(bytesToFree int64, freeTime time.Time) ( // Get all images in eviction order. images := make([]evictionInfo, 0, len(im.imageRecords)) for image, record := range im.imageRecords { + if isImageUsed(image, imagesInUse) { + glog.V(5).Infof("Image ID %s is being used", image) + continue + } images = append(images, evictionInfo{ id: image, imageRecord: *record, @@ -385,9 +390,9 @@ func (ev byLastUsedAndDetected) Less(i, j int) bool { } } -func isImageUsed(image container.Image, imagesInUse sets.String) bool { +func isImageUsed(imageID string, imagesInUse sets.String) bool { // Check the image ID. - if _, ok := imagesInUse[image.ID]; ok { + if _, ok := imagesInUse[imageID]; ok { return true } return false diff --git a/pkg/kubelet/images/image_gc_manager_test.go b/pkg/kubelet/images/image_gc_manager_test.go index aac3bad0f4748..fe680f45d8e70 100644 --- a/pkg/kubelet/images/image_gc_manager_test.go +++ b/pkg/kubelet/images/image_gc_manager_test.go @@ -112,7 +112,7 @@ func TestDetectImagesInitialDetect(t *testing.T) { } startTime := time.Now().Add(-time.Millisecond) - err := manager.detectImages(zero) + _, err := manager.detectImages(zero) assert := assert.New(t) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 3) @@ -145,7 +145,7 @@ func TestDetectImagesWithNewImage(t *testing.T) { }}, } - err := manager.detectImages(zero) + _, err := manager.detectImages(zero) assert := assert.New(t) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 2) @@ -159,7 +159,7 @@ func TestDetectImagesWithNewImage(t *testing.T) { detectedTime := zero.Add(time.Second) startTime := time.Now().Add(-time.Millisecond) - err = manager.detectImages(detectedTime) + _, err = manager.detectImages(detectedTime) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 3) noContainer, ok := manager.getImageRecord(imageID(0)) @@ -190,7 +190,7 @@ func TestDetectImagesContainerStopped(t *testing.T) { }}, } - err := manager.detectImages(zero) + _, err := manager.detectImages(zero) assert := assert.New(t) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 2) @@ -199,7 +199,7 @@ func TestDetectImagesContainerStopped(t *testing.T) { // Simulate container being stopped. fakeRuntime.AllPodList = []*containertest.FakePod{} - err = manager.detectImages(time.Now()) + _, err = manager.detectImages(time.Now()) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 2) container1, ok := manager.getImageRecord(imageID(0)) @@ -226,14 +226,14 @@ func TestDetectImagesWithRemovedImages(t *testing.T) { }}, } - err := manager.detectImages(zero) + _, err := manager.detectImages(zero) assert := assert.New(t) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 2) // Simulate both images being removed. fakeRuntime.ImageList = []container.Image{} - err = manager.detectImages(time.Now()) + _, err = manager.detectImages(time.Now()) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 0) } @@ -297,7 +297,8 @@ func TestFreeSpaceRemoveByLeastRecentlyUsed(t *testing.T) { } // Make 1 be more recently used than 0. - require.NoError(t, manager.detectImages(zero)) + _, err := manager.detectImages(zero) + require.NoError(t, err) fakeRuntime.AllPodList = []*containertest.FakePod{ {Pod: &container.Pod{ Containers: []*container.Container{ @@ -305,13 +306,15 @@ func TestFreeSpaceRemoveByLeastRecentlyUsed(t *testing.T) { }, }}, } - require.NoError(t, manager.detectImages(time.Now())) + _, err = manager.detectImages(time.Now()) + require.NoError(t, err) fakeRuntime.AllPodList = []*containertest.FakePod{ {Pod: &container.Pod{ Containers: []*container.Container{}, }}, } - require.NoError(t, manager.detectImages(time.Now())) + _, err = manager.detectImages(time.Now()) + require.NoError(t, err) require.Equal(t, manager.imageRecordsLen(), 2) spaceFreed, err := manager.freeSpace(1024, time.Now()) @@ -335,14 +338,17 @@ func TestFreeSpaceTiesBrokenByDetectedTime(t *testing.T) { } // Make 1 more recently detected but used at the same time as 0. - require.NoError(t, manager.detectImages(zero)) + _, err := manager.detectImages(zero) + require.NoError(t, err) fakeRuntime.ImageList = []container.Image{ makeImage(0, 1024), makeImage(1, 2048), } - require.NoError(t, manager.detectImages(time.Now())) + _, err = manager.detectImages(time.Now()) + require.NoError(t, err) fakeRuntime.AllPodList = []*containertest.FakePod{} - require.NoError(t, manager.detectImages(time.Now())) + _, err = manager.detectImages(time.Now()) + require.NoError(t, err) require.Equal(t, manager.imageRecordsLen(), 2) spaceFreed, err := manager.freeSpace(1024, time.Now()) @@ -448,7 +454,8 @@ func TestGarbageCollectImageNotOldEnough(t *testing.T) { fakeClock := clock.NewFakeClock(time.Now()) t.Log(fakeClock.Now()) - require.NoError(t, manager.detectImages(fakeClock.Now())) + _, err := manager.detectImages(fakeClock.Now()) + require.NoError(t, err) require.Equal(t, manager.imageRecordsLen(), 2) // no space freed since one image is in used, and another one is not old enough spaceFreed, err := manager.freeSpace(1024, fakeClock.Now())