From 5b383dad62605f1b0c442bc820ed3f229e569b6a Mon Sep 17 00:00:00 2001 From: combk8s Date: Thu, 17 Dec 2015 20:46:56 +0800 Subject: [PATCH] fix image gc bug --- Godeps/Godeps.json | 4 + .../src/github.com/ssoroka/ttime/README.md | 42 +++++++ .../src/github.com/ssoroka/ttime/ttime.go | 108 ++++++++++++++++++ pkg/kubelet/image_manager.go | 21 +++- pkg/kubelet/image_manager_test.go | 62 ++++++++-- 5 files changed, 227 insertions(+), 10 deletions(-) create mode 100644 Godeps/_workspace/src/github.com/ssoroka/ttime/README.md create mode 100644 Godeps/_workspace/src/github.com/ssoroka/ttime/ttime.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index a43a081e630..f12e0edbb8e 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -831,6 +831,10 @@ "ImportPath": "github.com/spf13/pflag", "Rev": "08b1a584251b5b62f458943640fc8ebd4d50aaa5" }, + { + "ImportPath": "github.com/ssoroka/ttime", + "Rev": "881f221816e0300201ac24f6c31e54e3bb958de7" + }, { "ImportPath": "github.com/stretchr/objx", "Rev": "d40df0cc104c06eae2dfe03d7dddb83802d52f9a" diff --git a/Godeps/_workspace/src/github.com/ssoroka/ttime/README.md b/Godeps/_workspace/src/github.com/ssoroka/ttime/README.md new file mode 100644 index 00000000000..788ddceb34a --- /dev/null +++ b/Godeps/_workspace/src/github.com/ssoroka/ttime/README.md @@ -0,0 +1,42 @@ +This is an experiment in making time easier to mock in Go tests. + +You should be able to alias the ttime library to time to avoid having to change all your time.Now() methods to ttime.Now() throughout your code. + +All methods return actual time.Time structs (if they were supposed to). + +example code: + + import ( + time "github.com/ssoroka/ttime" + ) + + fmt.Printf("The starting time is %v", time.Now().UTC()) + + // in test this will not sleep at all, but it will advance the clock 5 seconds. + // in production, it's identical to time.Sleep + time.Sleep(5 * time.Second) + fmt.Printf("The time after sleeping for 5 seconds is %v", time.Now().UTC()) + + time.After(10 * time.Second, func() { + // This will execute after 10 seconds in production and immediately in tests. + fmt.Printf("It is now %v", time.Now().UTC()) + }) + +example test: + + func TestFreezingTime(t *testing.T) { + time.Freeze(time.Now()) // freeze the system clock, at least as far as ttime is concerned. + + // or freeze time at a specific date/time (eg, test leap-year support!): + now, err := time.Parse(time.RFC3339, "2012-02-29T00:00:00Z") + if err != nil { panic("date time parse failed") } + time.Freeze(now) + defer time.Unfreeze() + + // test leap-year-specific code + if !isLeapYear(time.Now()) { + t.Error("Oh no! isLeapYear is broken!") + } + + t.Logf("It is now %v", time.Now().UTC()) + } diff --git a/Godeps/_workspace/src/github.com/ssoroka/ttime/ttime.go b/Godeps/_workspace/src/github.com/ssoroka/ttime/ttime.go new file mode 100644 index 00000000000..d22730413bc --- /dev/null +++ b/Godeps/_workspace/src/github.com/ssoroka/ttime/ttime.go @@ -0,0 +1,108 @@ +package ttime + +import "time" + +var ( + currentTime time.Time + timeFrozen bool +) + +type Duration time.Duration +type Location time.Location +type Month time.Month +type ParseError time.ParseError +type Ticker time.Ticker +type Time time.Time +type Timer time.Timer +type Weekday time.Weekday + +var ( + // import a ton of constants so we can act like the time library. + Parse = time.Parse + ParseDuration = time.ParseDuration + Date = time.Date + ParseInLocation = time.ParseInLocation + FixedZone = time.FixedZone + LoadLocation = time.LoadLocation + Sunday = time.Sunday + Monday = time.Monday + Tuesday = time.Tuesday + Wednesday = time.Wednesday + Thursday = time.Thursday + Friday = time.Friday + Saturday = time.Saturday + ANSIC = time.ANSIC + UnixDate = time.UnixDate + RubyDate = time.RubyDate + RFC822 = time.RFC822 + RFC822Z = time.RFC822Z + RFC850 = time.RFC850 + RFC1123 = time.RFC1123 + RFC1123Z = time.RFC1123Z + RFC3339 = time.RFC3339 + RFC3339Nano = time.RFC3339Nano + Kitchen = time.Kitchen + Stamp = time.Stamp + StampMilli = time.StampMilli + StampMicro = time.StampMicro + StampNano = time.StampNano + // constants that I really should redefine: + NewTimer = time.NewTimer + NewTicker = time.NewTicker + Unix = time.Unix +) + +func Freeze(t time.Time) { + currentTime = t + timeFrozen = true +} + +func Unfreeze() { + timeFrozen = false +} + +func IsFrozen() bool { + return timeFrozen +} + +func Now() time.Time { + if timeFrozen { + return currentTime + } else { + return time.Now() + } +} + +func After(d time.Duration) <-chan time.Time { + if timeFrozen { + currentTime = currentTime.Add(d) + c := make(chan time.Time, 1) + c <- currentTime + return c + } else { + return time.After(d) + } +} + +func Tick(d time.Duration) <-chan time.Time { + if timeFrozen { + c := make(chan time.Time, 1) + go func() { + for { + currentTime = currentTime.Add(d) + c <- currentTime + } + }() + return c + } else { + return time.Tick(d) + } +} + +func Sleep(d time.Duration) { + if timeFrozen && d > 0 { + currentTime = currentTime.Add(d) + } else { + time.Sleep(d) + } +} diff --git a/pkg/kubelet/image_manager.go b/pkg/kubelet/image_manager.go index cb8e6bfd190..59a895fe2a7 100644 --- a/pkg/kubelet/image_manager.go +++ b/pkg/kubelet/image_manager.go @@ -32,6 +32,10 @@ import ( "k8s.io/kubernetes/pkg/util/sets" ) +const ( + defaultGCAge = time.Minute * 1 +) + // Manages lifecycle of all images. // // Implementation is thread-safe. @@ -71,6 +75,10 @@ type realImageManager struct { // The image garbage collection policy in use. policy ImageGCPolicy + // Minimum age at which a image can be garbage collected, zero for no limit. + // TODO(mqliang): move it to ImageGCPolicy and make it configurable + minAge time.Duration + // cAdvisor instance. cadvisor cadvisor.Interface @@ -87,7 +95,7 @@ type realImageManager struct { // Information about the images we track. type imageRecord struct { // Time when this image was first detected. - detected time.Time + firstDetected time.Time // Time when we last saw this image being used. lastUsed time.Time @@ -110,6 +118,7 @@ func newImageManager(runtime container.Runtime, cadvisorInterface cadvisor.Inter im := &realImageManager{ runtime: runtime, policy: policy, + minAge: defaultGCAge, imageRecords: make(map[string]*imageRecord), cadvisor: cadvisorInterface, recorder: recorder, @@ -176,7 +185,7 @@ func (im *realImageManager) detectImages(detected time.Time) error { // New image, set it as detected now. if _, ok := im.imageRecords[image.ID]; !ok { im.imageRecords[image.ID] = &imageRecord{ - detected: detected, + firstDetected: detected, } } @@ -269,6 +278,12 @@ func (im *realImageManager) freeSpace(bytesToFree int64) (int64, error) { break } + // Avoid garbage collect the image if the image is not old enough. + // In such a case, the image may have just been pulled down, and will be used by a container right away. + if startTime.Sub(image.firstDetected) < im.minAge { + continue + } + // Remove image. Continue despite errors. glog.Infof("[ImageManager]: Removing image %q to free %d bytes", image.id, image.size) err := im.runtime.RemoveImage(container.ImageSpec{Image: image.id}) @@ -299,7 +314,7 @@ func (ev byLastUsedAndDetected) Swap(i, j int) { ev[i], ev[j] = ev[j], ev[i] } func (ev byLastUsedAndDetected) Less(i, j int) bool { // Sort by last used, break ties by detected. if ev[i].lastUsed.Equal(ev[j].lastUsed) { - return ev[i].detected.Before(ev[j].detected) + return ev[i].firstDetected.Before(ev[j].firstDetected) } else { return ev[i].lastUsed.Before(ev[j].lastUsed) } diff --git a/pkg/kubelet/image_manager_test.go b/pkg/kubelet/image_manager_test.go index abd21a26462..02f9f2d9eb1 100644 --- a/pkg/kubelet/image_manager_test.go +++ b/pkg/kubelet/image_manager_test.go @@ -22,6 +22,7 @@ import ( "time" cadvisorapiv2 "github.com/google/cadvisor/info/v2" + "github.com/ssoroka/ttime" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/kubernetes/pkg/client/record" @@ -37,6 +38,7 @@ func newRealImageManager(policy ImageGCPolicy) (*realImageManager, *container.Fa return &realImageManager{ runtime: fakeRuntime, policy: policy, + minAge: 0, imageRecords: make(map[string]*imageRecord), cadvisor: mockCadvisor, recorder: &record.FakeRecorder{}, @@ -99,11 +101,11 @@ func TestDetectImagesInitialDetect(t *testing.T) { assert.Equal(manager.imageRecordsLen(), 2) noContainer, ok := manager.getImageRecord(imageName(0)) require.True(t, ok) - assert.Equal(zero, noContainer.detected) + assert.Equal(zero, noContainer.firstDetected) assert.Equal(zero, noContainer.lastUsed) withContainer, ok := manager.getImageRecord(imageName(1)) require.True(t, ok) - assert.Equal(zero, withContainer.detected) + assert.Equal(zero, withContainer.firstDetected) assert.True(withContainer.lastUsed.After(startTime)) } @@ -141,15 +143,15 @@ func TestDetectImagesWithNewImage(t *testing.T) { assert.Equal(manager.imageRecordsLen(), 3) noContainer, ok := manager.getImageRecord(imageName(0)) require.True(t, ok) - assert.Equal(zero, noContainer.detected) + assert.Equal(zero, noContainer.firstDetected) assert.Equal(zero, noContainer.lastUsed) withContainer, ok := manager.getImageRecord(imageName(1)) require.True(t, ok) - assert.Equal(zero, withContainer.detected) + assert.Equal(zero, withContainer.firstDetected) assert.True(withContainer.lastUsed.After(startTime)) newContainer, ok := manager.getImageRecord(imageName(2)) require.True(t, ok) - assert.Equal(detectedTime, newContainer.detected) + assert.Equal(detectedTime, newContainer.firstDetected) assert.Equal(zero, noContainer.lastUsed) } @@ -181,11 +183,11 @@ func TestDetectImagesContainerStopped(t *testing.T) { assert.Equal(manager.imageRecordsLen(), 2) container1, ok := manager.getImageRecord(imageName(0)) require.True(t, ok) - assert.Equal(zero, container1.detected) + assert.Equal(zero, container1.firstDetected) assert.Equal(zero, container1.lastUsed) container2, ok := manager.getImageRecord(imageName(1)) require.True(t, ok) - assert.Equal(zero, container2.detected) + assert.Equal(zero, container2.firstDetected) assert.True(container2.lastUsed.Equal(withContainer.lastUsed)) } @@ -399,3 +401,49 @@ func TestGarbageCollectNotEnoughFreed(t *testing.T) { assert.NotNil(t, manager.GarbageCollect()) } + +func TestGarbageCollectImageNotOldEnough(t *testing.T) { + policy := ImageGCPolicy{ + HighThresholdPercent: 90, + LowThresholdPercent: 80, + } + fakeRuntime := &container.FakeRuntime{} + mockCadvisor := new(cadvisor.Mock) + manager := &realImageManager{ + runtime: fakeRuntime, + policy: policy, + minAge: defaultGCAge, + imageRecords: make(map[string]*imageRecord), + cadvisor: mockCadvisor, + recorder: &record.FakeRecorder{}, + } + + fakeRuntime.ImageList = []container.Image{ + makeImage(0, 1024), + makeImage(1, 2048), + } + // 1 image is in use, and another one is not old enough + fakeRuntime.AllPodList = []*container.Pod{ + { + Containers: []*container.Container{ + makeContainer(1), + }, + }, + } + + require.NoError(t, manager.detectImages(ttime.Now())) + 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) + assert := assert.New(t) + require.NoError(t, err) + assert.EqualValues(0, spaceFreed) + assert.Len(fakeRuntime.ImageList, 2) + + // sleep 1 minute, then 1 image will be garbage collected + ttime.Sleep(defaultGCAge) + spaceFreed, err = manager.freeSpace(1024) + require.NoError(t, err) + assert.EqualValues(1024, spaceFreed) + assert.Len(fakeRuntime.ImageList, 1) +}