Merge pull request #57020 from dixudx/imagegc_ignore_inuse

Automatic merge from submit-queue (batch tested with PRs 57823, 58091, 58093, 58096, 57020). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

ignore images in used by running containers when GC

**What this PR does / why we need it**:
Let kubelet not attempt to remove images being used by running containers.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #57006

**Special notes for your reviewer**:
@kubernetes/sig-node-pr-reviews 

**Release note**:

```release-note
ignore images in used by running containers when GC
```
This commit is contained in:
Kubernetes Submit Queue 2018-01-10 12:37:48 -08:00 committed by GitHub
commit 07e4939b66
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 36 additions and 24 deletions

View File

@ -168,7 +168,7 @@ func (im *realImageGCManager) Start() {
if im.initialized { if im.initialized {
ts = time.Now() ts = time.Now()
} }
err := im.detectImages(ts) _, err := im.detectImages(ts)
if err != nil { if err != nil {
glog.Warningf("[imageGCManager] Failed to monitor images: %v", err) glog.Warningf("[imageGCManager] Failed to monitor images: %v", err)
} else { } else {
@ -194,18 +194,19 @@ func (im *realImageGCManager) GetImageList() ([]kubecontainer.Image, error) {
return im.imageCache.get(), nil 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() images, err := im.runtime.ListImages()
if err != nil { if err != nil {
return err return imagesInUse, err
} }
pods, err := im.runtime.GetPods(true) pods, err := im.runtime.GetPods(true)
if err != nil { if err != nil {
return err return imagesInUse, err
} }
// Make a set of images in use by containers. // Make a set of images in use by containers.
imagesInUse := sets.NewString()
for _, pod := range pods { for _, pod := range pods {
for _, container := range pod.Containers { 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) 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. // 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) glog.V(5).Infof("Setting Image ID %s lastUsed to %v", image.ID, now)
im.imageRecords[image.ID].lastUsed = 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 { 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 // Note that error may be nil and the number of bytes free may be less
// than bytesToFree. // than bytesToFree.
func (im *realImageGCManager) freeSpace(bytesToFree int64, freeTime time.Time) (int64, error) { func (im *realImageGCManager) freeSpace(bytesToFree int64, freeTime time.Time) (int64, error) {
err := im.detectImages(freeTime) imagesInUse, err := im.detectImages(freeTime)
if err != nil { if err != nil {
return 0, err return 0, err
} }
@ -320,6 +321,10 @@ func (im *realImageGCManager) freeSpace(bytesToFree int64, freeTime time.Time) (
// Get all images in eviction order. // Get all images in eviction order.
images := make([]evictionInfo, 0, len(im.imageRecords)) images := make([]evictionInfo, 0, len(im.imageRecords))
for image, record := range 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{ images = append(images, evictionInfo{
id: image, id: image,
imageRecord: *record, 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. // Check the image ID.
if _, ok := imagesInUse[image.ID]; ok { if _, ok := imagesInUse[imageID]; ok {
return true return true
} }
return false return false

View File

@ -112,7 +112,7 @@ func TestDetectImagesInitialDetect(t *testing.T) {
} }
startTime := time.Now().Add(-time.Millisecond) startTime := time.Now().Add(-time.Millisecond)
err := manager.detectImages(zero) _, err := manager.detectImages(zero)
assert := assert.New(t) assert := assert.New(t)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(manager.imageRecordsLen(), 3) 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) assert := assert.New(t)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(manager.imageRecordsLen(), 2) assert.Equal(manager.imageRecordsLen(), 2)
@ -159,7 +159,7 @@ func TestDetectImagesWithNewImage(t *testing.T) {
detectedTime := zero.Add(time.Second) detectedTime := zero.Add(time.Second)
startTime := time.Now().Add(-time.Millisecond) startTime := time.Now().Add(-time.Millisecond)
err = manager.detectImages(detectedTime) _, err = manager.detectImages(detectedTime)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(manager.imageRecordsLen(), 3) assert.Equal(manager.imageRecordsLen(), 3)
noContainer, ok := manager.getImageRecord(imageID(0)) 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) assert := assert.New(t)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(manager.imageRecordsLen(), 2) assert.Equal(manager.imageRecordsLen(), 2)
@ -199,7 +199,7 @@ func TestDetectImagesContainerStopped(t *testing.T) {
// Simulate container being stopped. // Simulate container being stopped.
fakeRuntime.AllPodList = []*containertest.FakePod{} fakeRuntime.AllPodList = []*containertest.FakePod{}
err = manager.detectImages(time.Now()) _, err = manager.detectImages(time.Now())
require.NoError(t, err) require.NoError(t, err)
assert.Equal(manager.imageRecordsLen(), 2) assert.Equal(manager.imageRecordsLen(), 2)
container1, ok := manager.getImageRecord(imageID(0)) 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) assert := assert.New(t)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(manager.imageRecordsLen(), 2) assert.Equal(manager.imageRecordsLen(), 2)
// Simulate both images being removed. // Simulate both images being removed.
fakeRuntime.ImageList = []container.Image{} fakeRuntime.ImageList = []container.Image{}
err = manager.detectImages(time.Now()) _, err = manager.detectImages(time.Now())
require.NoError(t, err) require.NoError(t, err)
assert.Equal(manager.imageRecordsLen(), 0) assert.Equal(manager.imageRecordsLen(), 0)
} }
@ -297,7 +297,8 @@ func TestFreeSpaceRemoveByLeastRecentlyUsed(t *testing.T) {
} }
// Make 1 be more recently used than 0. // 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{ fakeRuntime.AllPodList = []*containertest.FakePod{
{Pod: &container.Pod{ {Pod: &container.Pod{
Containers: []*container.Container{ 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{ fakeRuntime.AllPodList = []*containertest.FakePod{
{Pod: &container.Pod{ {Pod: &container.Pod{
Containers: []*container.Container{}, 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) require.Equal(t, manager.imageRecordsLen(), 2)
spaceFreed, err := manager.freeSpace(1024, time.Now()) 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. // 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{ fakeRuntime.ImageList = []container.Image{
makeImage(0, 1024), makeImage(0, 1024),
makeImage(1, 2048), makeImage(1, 2048),
} }
require.NoError(t, manager.detectImages(time.Now())) _, err = manager.detectImages(time.Now())
require.NoError(t, err)
fakeRuntime.AllPodList = []*containertest.FakePod{} 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) require.Equal(t, manager.imageRecordsLen(), 2)
spaceFreed, err := manager.freeSpace(1024, time.Now()) spaceFreed, err := manager.freeSpace(1024, time.Now())
@ -448,7 +454,8 @@ func TestGarbageCollectImageNotOldEnough(t *testing.T) {
fakeClock := clock.NewFakeClock(time.Now()) fakeClock := clock.NewFakeClock(time.Now())
t.Log(fakeClock.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) require.Equal(t, manager.imageRecordsLen(), 2)
// no space freed since one image is in used, and another one is not old enough // no space freed since one image is in used, and another one is not old enough
spaceFreed, err := manager.freeSpace(1024, fakeClock.Now()) spaceFreed, err := manager.freeSpace(1024, fakeClock.Now())