Merge pull request #121275 from haircommander/image-max-gc

KEP-4210: add support for ImageMaximumGCAge field

pkg/features/kube_features.go

@@ -882,6 +882,13 @@ const (
 	// alpha: v1.29
 	// LoadBalancerIPMode enables the IPMode field in the LoadBalancerIngress status of a Service
 	LoadBalancerIPMode featuregate.Feature = "LoadBalancerIPMode"
+
+	// owner: @haircommander
+	// kep: http://kep.k8s.io/4210
+	// alpha: v1.29
+	// ImageMaximumGCAge enables the Kubelet configuration field of the same name, allowing an admin
+	// to specify the age after which an image will be garbage collected.
+	ImageMaximumGCAge featuregate.Feature = "ImageMaximumGCAge"
 )
 
 func init() {
@@ -1121,6 +1128,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	LoadBalancerIPMode: {Default: false, PreRelease: featuregate.Alpha},
 
+	ImageMaximumGCAge: {Default: false, PreRelease: featuregate.Alpha},
+
 	// inherited features from generic apiserver, relisted here to get a conflict if it is changed
 	// unintentionally on either side:
 

pkg/generated/openapi/zz_generated.openapi.go

@@ -55465,6 +55465,12 @@ func schema_k8sio_kubelet_config_v1beta1_KubeletConfiguration(ref common.Referen
 							Ref:         ref("k8s.io/apimachinery/pkg/apis/meta/v1.Duration"),
 						},
 					},
+					"imageMaximumGCAge": {
+						SchemaProps: spec.SchemaProps{
+							Description: "imageMaximumGCAge is the maximum age an image can be unused before it is garbage collected. The default of this field is \"0s\", which disables this field--meaning images won't be garbage collected based on being unused for too long. Default: \"0s\" (disabled)",
+							Ref:         ref("k8s.io/apimachinery/pkg/apis/meta/v1.Duration"),
+						},
+					},
 					"imageGCHighThresholdPercent": {
 						SchemaProps: spec.SchemaProps{
 							Description: "imageGCHighThresholdPercent is the percent of disk usage after which image garbage collection is always run. The percent is calculated by dividing this field value by 100, so this field must be between 0 and 100, inclusive. When specified, the value must be greater than imageGCLowThresholdPercent. Default: 85",

pkg/kubelet/apis/config/fuzzer/fuzzer.go

@@ -61,6 +61,7 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} {
 			obj.HealthzPort = 10248
 			obj.HTTPCheckFrequency = metav1.Duration{Duration: 20 * time.Second}
 			obj.ImageMinimumGCAge = metav1.Duration{Duration: 2 * time.Minute}
+			obj.ImageMaximumGCAge = metav1.Duration{}
 			obj.ImageGCHighThresholdPercent = 85
 			obj.ImageGCLowThresholdPercent = 80
 			obj.KernelMemcgNotification = false

pkg/kubelet/apis/config/helpers_test.go

@@ -230,6 +230,7 @@ var (
 		"ImageGCHighThresholdPercent",
 		"ImageGCLowThresholdPercent",
 		"ImageMinimumGCAge.Duration",
+		"ImageMaximumGCAge.Duration",
 		"KernelMemcgNotification",
 		"KubeAPIBurst",
 		"KubeAPIQPS",

pkg/kubelet/apis/config/scheme/testdata/KubeletConfiguration/after/v1beta1.yaml

@@ -43,6 +43,7 @@ healthzPort: 10248
 httpCheckFrequency: 20s
 imageGCHighThresholdPercent: 85
 imageGCLowThresholdPercent: 80
+imageMaximumGCAge: 0s
 imageMinimumGCAge: 2m0s
 iptablesDropBit: 15
 iptablesMasqueradeBit: 14

pkg/kubelet/apis/config/scheme/testdata/KubeletConfiguration/roundtrip/default/v1beta1.yaml

@@ -43,6 +43,7 @@ healthzPort: 10248
 httpCheckFrequency: 20s
 imageGCHighThresholdPercent: 85
 imageGCLowThresholdPercent: 80
+imageMaximumGCAge: 0s
 imageMinimumGCAge: 2m0s
 iptablesDropBit: 15
 iptablesMasqueradeBit: 14

pkg/kubelet/apis/config/types.go

@@ -192,9 +192,13 @@ type KubeletConfiguration struct {
 	NodeStatusReportFrequency metav1.Duration
 	// nodeLeaseDurationSeconds is the duration the Kubelet will set on its corresponding Lease.
 	NodeLeaseDurationSeconds int32
-	// imageMinimumGCAge is the minimum age for an unused image before it is
+	// ImageMinimumGCAge is the minimum age for an unused image before it is
 	// garbage collected.
 	ImageMinimumGCAge metav1.Duration
+	// ImageMaximumGCAge is the maximum age an image can be unused before it is garbage collected.
+	// The default of this field is "0s", which disables this field--meaning images won't be garbage
+	// collected based on being unused for too long.
+	ImageMaximumGCAge metav1.Duration
 	// imageGCHighThresholdPercent is the percent of disk usage after which
 	// image garbage collection is always run. The percent is calculated as
 	// this field value out of 100.

pkg/kubelet/apis/config/v1beta1/defaults_test.go

@@ -77,6 +77,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				NodeStatusReportFrequency:                 metav1.Duration{Duration: 5 * time.Minute},
 				NodeLeaseDurationSeconds:                  40,
 				ImageMinimumGCAge:                         metav1.Duration{Duration: 2 * time.Minute},
+				ImageMaximumGCAge:                         metav1.Duration{},
 				ImageGCHighThresholdPercent:               utilpointer.Int32(85),
 				ImageGCLowThresholdPercent:                utilpointer.Int32(80),
 				ContainerRuntimeEndpoint:                  "unix:///run/containerd/containerd.sock",

pkg/kubelet/apis/config/v1beta1/zz_generated.conversion.go

@@ -392,6 +392,7 @@ func autoConvert_v1beta1_KubeletConfiguration_To_config_KubeletConfiguration(in
 	out.NodeStatusReportFrequency = in.NodeStatusReportFrequency
 	out.NodeLeaseDurationSeconds = in.NodeLeaseDurationSeconds
 	out.ImageMinimumGCAge = in.ImageMinimumGCAge
+	out.ImageMaximumGCAge = in.ImageMaximumGCAge
 	if err := v1.Convert_Pointer_int32_To_int32(&in.ImageGCHighThresholdPercent, &out.ImageGCHighThresholdPercent, s); err != nil {
 		return err
 	}
@@ -579,6 +580,7 @@ func autoConvert_config_KubeletConfiguration_To_v1beta1_KubeletConfiguration(in
 	out.NodeStatusReportFrequency = in.NodeStatusReportFrequency
 	out.NodeLeaseDurationSeconds = in.NodeLeaseDurationSeconds
 	out.ImageMinimumGCAge = in.ImageMinimumGCAge
+	out.ImageMaximumGCAge = in.ImageMaximumGCAge
 	if err := v1.Convert_int32_To_Pointer_int32(&in.ImageGCHighThresholdPercent, &out.ImageGCHighThresholdPercent, s); err != nil {
 		return err
 	}

pkg/kubelet/apis/config/validation/validation.go

@@ -83,6 +83,15 @@ func ValidateKubeletConfiguration(kc *kubeletconfig.KubeletConfiguration, featur
 	if kc.ImageGCLowThresholdPercent >= kc.ImageGCHighThresholdPercent {
 		allErrors = append(allErrors, fmt.Errorf("invalid configuration: imageGCLowThresholdPercent (--image-gc-low-threshold) %v must be less than imageGCHighThresholdPercent (--image-gc-high-threshold) %v", kc.ImageGCLowThresholdPercent, kc.ImageGCHighThresholdPercent))
 	}
+	if kc.ImageMaximumGCAge.Duration != 0 && !localFeatureGate.Enabled(features.ImageMaximumGCAge) {
+		allErrors = append(allErrors, fmt.Errorf("invalid configuration: ImageMaximumGCAge feature gate is required for Kubelet configuration option ImageMaximumGCAge"))
+	}
+	if kc.ImageMaximumGCAge.Duration < 0 {
+		allErrors = append(allErrors, fmt.Errorf("invalid configuration: imageMaximumGCAge %v must not be negative", kc.ImageMaximumGCAge.Duration))
+	}
+	if kc.ImageMaximumGCAge.Duration > 0 && kc.ImageMaximumGCAge.Duration <= kc.ImageMinimumGCAge.Duration {
+		allErrors = append(allErrors, fmt.Errorf("invalid configuration: imageMaximumGCAge %v must be greater than imageMinimumGCAge %v", kc.ImageMaximumGCAge.Duration, kc.ImageMinimumGCAge.Duration))
+	}
 	if utilvalidation.IsInRange(int(kc.IPTablesDropBit), 0, 31) != nil {
 		allErrors = append(allErrors, fmt.Errorf("invalid configuration: iptablesDropBit (--iptables-drop-bit) %v must be between 0 and 31, inclusive", kc.IPTablesDropBit))
 	}

pkg/kubelet/apis/config/validation/validation_test.go

@@ -521,6 +521,30 @@ func TestValidateKubeletConfiguration(t *testing.T) {
 			return conf
 		},
 		errMsg: "invalid configuration: enableSystemLogHandler is required for enableSystemLogQuery",
+	}, {
+		name: "imageMaximumGCAge should not be specified without feature gate",
+		configure: func(conf *kubeletconfig.KubeletConfiguration) *kubeletconfig.KubeletConfiguration {
+			conf.ImageMaximumGCAge = metav1.Duration{Duration: 1}
+			return conf
+		},
+		errMsg: "invalid configuration: ImageMaximumGCAge feature gate is required for Kubelet configuration option ImageMaximumGCAge",
+	}, {
+		name: "imageMaximumGCAge should not be negative",
+		configure: func(conf *kubeletconfig.KubeletConfiguration) *kubeletconfig.KubeletConfiguration {
+			conf.FeatureGates = map[string]bool{"ImageMaximumGCAge": true}
+			conf.ImageMaximumGCAge = metav1.Duration{Duration: -1}
+			return conf
+		},
+		errMsg: "invalid configuration: imageMaximumGCAge -1ns must not be negative",
+	}, {
+		name: "imageMaximumGCAge should not be less than imageMinimumGCAge",
+		configure: func(conf *kubeletconfig.KubeletConfiguration) *kubeletconfig.KubeletConfiguration {
+			conf.FeatureGates = map[string]bool{"ImageMaximumGCAge": true}
+			conf.ImageMaximumGCAge = metav1.Duration{Duration: 1}
+			conf.ImageMinimumGCAge = metav1.Duration{Duration: 2}
+			return conf
+		},
+		errMsg: "invalid configuration: imageMaximumGCAge 1ns must be greater than imageMinimumGCAge 2ns",
 	}}
 
 	for _, tc := range cases {

pkg/kubelet/apis/config/zz_generated.deepcopy.go

@@ -202,6 +202,7 @@ func (in *KubeletConfiguration) DeepCopyInto(out *KubeletConfiguration) {
 	out.NodeStatusUpdateFrequency = in.NodeStatusUpdateFrequency
 	out.NodeStatusReportFrequency = in.NodeStatusReportFrequency
 	out.ImageMinimumGCAge = in.ImageMinimumGCAge
+	out.ImageMaximumGCAge = in.ImageMaximumGCAge
 	out.VolumeStatsAggPeriod = in.VolumeStatsAggPeriod
 	if in.CPUManagerPolicyOptions != nil {
 		in, out := &in.CPUManagerPolicyOptions, &out.CPUManagerPolicyOptions

pkg/kubelet/images/image_gc_manager.go

@@ -36,6 +36,7 @@ import (
 	statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
 	"k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/kubelet/events"
+	"k8s.io/kubernetes/pkg/kubelet/metrics"
 	"k8s.io/kubernetes/pkg/kubelet/util/sliceutils"
 )
 
@@ -78,6 +79,11 @@ type ImageGCPolicy struct {
 
 	// Minimum age at which an image can be garbage collected.
 	MinAge time.Duration
+
+	// Maximum age after which an image can be garbage collected, regardless of disk usage.
+	// Currently gated by MaximumImageGCAge feature gate and Kubelet configuration.
+	// If 0, the feature is disabled.
+	MaxAge time.Duration
 }
 
 type realImageGCManager struct {
@@ -280,6 +286,18 @@ func (im *realImageGCManager) detectImages(ctx context.Context, detectTime time.
 func (im *realImageGCManager) GarbageCollect(ctx context.Context) error {
 	ctx, otelSpan := im.tracer.Start(ctx, "Images/GarbageCollect")
 	defer otelSpan.End()
+
+	freeTime := time.Now()
+	images, err := im.imagesInEvictionOrder(ctx, freeTime)
+	if err != nil {
+		return err
+	}
+
+	images, err = im.freeOldImages(ctx, images, freeTime)
+	if err != nil {
+		return err
+	}
+
 	// Get disk usage on disk holding images.
 	fsStats, err := im.statsProvider.ImageFsStats(ctx)
 	if err != nil {
@@ -311,7 +329,7 @@ func (im *realImageGCManager) GarbageCollect(ctx context.Context) error {
 	if usagePercent >= im.policy.HighThresholdPercent {
 		amountToFree := capacity*int64(100-im.policy.LowThresholdPercent)/100 - available
 		klog.InfoS("Disk usage on image filesystem is over the high threshold, trying to free bytes down to the low threshold", "usage", usagePercent, "highThreshold", im.policy.HighThresholdPercent, "amountToFree", amountToFree, "lowThreshold", im.policy.LowThresholdPercent)
-		freed, err := im.freeSpace(ctx, amountToFree, time.Now())
+		freed, err := im.freeSpace(ctx, amountToFree, freeTime, images)
 		if err != nil {
 			return err
 		}
@@ -326,9 +344,39 @@ func (im *realImageGCManager) GarbageCollect(ctx context.Context) error {
 	return nil
 }
 
+func (im *realImageGCManager) freeOldImages(ctx context.Context, images []evictionInfo, freeTime time.Time) ([]evictionInfo, error) {
+	if im.policy.MaxAge == 0 {
+		return images, nil
+	}
+	var deletionErrors []error
+	remainingImages := make([]evictionInfo, 0)
+	for _, image := range images {
+		klog.V(5).InfoS("Evaluating image ID for possible garbage collection based on image age", "imageID", image.id)
+		// Evaluate whether image is older than MaxAge.
+		if freeTime.Sub(image.lastUsed) > im.policy.MaxAge {
+			if err := im.freeImage(ctx, image); err != nil {
+				deletionErrors = append(deletionErrors, err)
+				remainingImages = append(remainingImages, image)
+				continue
+			}
+			continue
+		}
+		remainingImages = append(remainingImages, image)
+	}
+	if len(deletionErrors) > 0 {
+		return remainingImages, fmt.Errorf("wanted to free images older than %v, encountered errors in image deletion: %v", im.policy.MaxAge, errors.NewAggregate(deletionErrors))
+	}
+	return remainingImages, nil
+}
+
 func (im *realImageGCManager) DeleteUnusedImages(ctx context.Context) error {
 	klog.InfoS("Attempting to delete unused images")
-	_, err := im.freeSpace(ctx, math.MaxInt64, time.Now())
+	freeTime := time.Now()
+	images, err := im.imagesInEvictionOrder(ctx, freeTime)
+	if err != nil {
+		return err
+	}
+	_, err = im.freeSpace(ctx, math.MaxInt64, freeTime, images)
 	return err
 }
 
@@ -338,10 +386,59 @@ func (im *realImageGCManager) DeleteUnusedImages(ctx context.Context) error {
 // bytes freed is always returned.
 // Note that error may be nil and the number of bytes free may be less
 // than bytesToFree.
-func (im *realImageGCManager) freeSpace(ctx context.Context, bytesToFree int64, freeTime time.Time) (int64, error) {
+func (im *realImageGCManager) freeSpace(ctx context.Context, bytesToFree int64, freeTime time.Time, images []evictionInfo) (int64, error) {
+	// Delete unused images until we've freed up enough space.
+	var deletionErrors []error
+	spaceFreed := int64(0)
+	for _, image := range images {
+		klog.V(5).InfoS("Evaluating image ID for possible garbage collection based on disk usage", "imageID", image.id)
+		// Images that are currently in used were given a newer lastUsed.
+		if image.lastUsed.Equal(freeTime) || image.lastUsed.After(freeTime) {
+			klog.V(5).InfoS("Image ID was used too recently, not eligible for garbage collection", "imageID", image.id, "lastUsed", image.lastUsed, "freeTime", freeTime)
+			continue
+		}
+
+		// 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 freeTime.Sub(image.firstDetected) < im.policy.MinAge {
+			klog.V(5).InfoS("Image ID's age is less than the policy's minAge, not eligible for garbage collection", "imageID", image.id, "age", freeTime.Sub(image.firstDetected), "minAge", im.policy.MinAge)
+			continue
+		}
+
+		if err := im.freeImage(ctx, image); err != nil {
+			deletionErrors = append(deletionErrors, err)
+			continue
+		}
+		spaceFreed += image.size
+
+		if spaceFreed >= bytesToFree {
+			break
+		}
+	}
+
+	if len(deletionErrors) > 0 {
+		return spaceFreed, fmt.Errorf("wanted to free %d bytes, but freed %d bytes space with errors in image deletion: %v", bytesToFree, spaceFreed, errors.NewAggregate(deletionErrors))
+	}
+	return spaceFreed, nil
+}
+
+func (im *realImageGCManager) freeImage(ctx context.Context, image evictionInfo) error {
+	// Remove image. Continue despite errors.
+	klog.InfoS("Removing image to free bytes", "imageID", image.id, "size", image.size)
+	err := im.runtime.RemoveImage(ctx, container.ImageSpec{Image: image.id})
+	if err != nil {
+		return err
+	}
+	delete(im.imageRecords, image.id)
+	metrics.ImageGarbageCollectedTotal.Inc()
+	return err
+}
+
+// Queries all of the image records and arranges them in a slice of evictionInfo, sorted based on last time used, ignoring images pinned by the runtime.
+func (im *realImageGCManager) imagesInEvictionOrder(ctx context.Context, freeTime time.Time) ([]evictionInfo, error) {
 	imagesInUse, err := im.detectImages(ctx, freeTime)
 	if err != nil {
-		return 0, err
+		return nil, err
 	}
 
 	im.imageRecordsLock.Lock()
@@ -366,45 +463,7 @@ func (im *realImageGCManager) freeSpace(ctx context.Context, bytesToFree int64,
 		})
 	}
 	sort.Sort(byLastUsedAndDetected(images))
-
-	// Delete unused images until we've freed up enough space.
-	var deletionErrors []error
-	spaceFreed := int64(0)
-	for _, image := range images {
-		klog.V(5).InfoS("Evaluating image ID for possible garbage collection", "imageID", image.id)
-		// Images that are currently in used were given a newer lastUsed.
-		if image.lastUsed.Equal(freeTime) || image.lastUsed.After(freeTime) {
-			klog.V(5).InfoS("Image ID was used too recently, not eligible for garbage collection", "imageID", image.id, "lastUsed", image.lastUsed, "freeTime", freeTime)
-			continue
-		}
-
-		// 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 freeTime.Sub(image.firstDetected) < im.policy.MinAge {
-			klog.V(5).InfoS("Image ID's age is less than the policy's minAge, not eligible for garbage collection", "imageID", image.id, "age", freeTime.Sub(image.firstDetected), "minAge", im.policy.MinAge)
-			continue
-		}
-
-		// Remove image. Continue despite errors.
-		klog.InfoS("Removing image to free bytes", "imageID", image.id, "size", image.size)
-		err := im.runtime.RemoveImage(ctx, container.ImageSpec{Image: image.id})
-		if err != nil {
-			deletionErrors = append(deletionErrors, err)
-			continue
-		}
-		delete(im.imageRecords, image.id)
-		spaceFreed += image.size
-
-		if spaceFreed >= bytesToFree {
-			break
-		}
-	}
-
-	if len(deletionErrors) > 0 {
-		return spaceFreed, fmt.Errorf("wanted to free %d bytes, but freed %d bytes space with errors in image deletion: %v", bytesToFree, spaceFreed, errors.NewAggregate(deletionErrors))
-	}
-	return spaceFreed, nil
+	return images, nil
 }
 
 type evictionInfo struct {

pkg/kubelet/images/image_gc_manager_test.go

@@ -255,11 +255,8 @@ func TestDoNotDeletePinnedImage(t *testing.T) {
 		},
 	}
 
-	spaceFreed, err := manager.freeSpace(ctx, 4096, time.Now())
 	assert := assert.New(t)
-	require.NoError(t, err)
-	assert.EqualValues(1024, spaceFreed)
-	assert.Len(fakeRuntime.ImageList, 1)
+	getImagesAndFreeSpace(ctx, t, assert, manager, fakeRuntime, 4096, 1024, 1, time.Now())
 }
 
 func TestDeleteUnPinnedImage(t *testing.T) {
@@ -280,11 +277,8 @@ func TestDeleteUnPinnedImage(t *testing.T) {
 		},
 	}
 
-	spaceFreed, err := manager.freeSpace(ctx, 2048, time.Now())
 	assert := assert.New(t)
-	require.NoError(t, err)
-	assert.EqualValues(2048, spaceFreed)
-	assert.Len(fakeRuntime.ImageList, 0)
+	getImagesAndFreeSpace(ctx, t, assert, manager, fakeRuntime, 2048, 2048, 0, time.Now())
 }
 
 func TestAllPinnedImages(t *testing.T) {
@@ -306,11 +300,8 @@ func TestAllPinnedImages(t *testing.T) {
 		},
 	}
 
-	spaceFreed, err := manager.freeSpace(ctx, 2048, time.Now())
 	assert := assert.New(t)
-	require.NoError(t, err)
-	assert.EqualValues(0, spaceFreed)
-	assert.Len(fakeRuntime.ImageList, 2)
+	getImagesAndFreeSpace(ctx, t, assert, manager, fakeRuntime, 2048, 0, 2, time.Now())
 }
 
 func TestDetectImagesContainerStopped(t *testing.T) {
@@ -404,11 +395,8 @@ func TestFreeSpaceImagesInUseContainersAreIgnored(t *testing.T) {
 		}},
 	}
 
-	spaceFreed, err := manager.freeSpace(ctx, 2048, time.Now())
 	assert := assert.New(t)
-	require.NoError(t, err)
-	assert.EqualValues(1024, spaceFreed)
-	assert.Len(fakeRuntime.ImageList, 1)
+	getImagesAndFreeSpace(ctx, t, assert, manager, fakeRuntime, 2048, 1024, 1, time.Now())
 }
 
 func TestDeleteUnusedImagesRemoveAllUnusedImages(t *testing.T) {
@@ -487,11 +475,8 @@ func TestFreeSpaceRemoveByLeastRecentlyUsed(t *testing.T) {
 
 	// We're setting the delete time one minute in the future, so the time the image
 	// was first detected and the delete time are different.
-	spaceFreed, err := manager.freeSpace(ctx, 1024, time.Now().Add(time.Minute))
 	assert := assert.New(t)
-	require.NoError(t, err)
-	assert.EqualValues(1024, spaceFreed)
-	assert.Len(fakeRuntime.ImageList, 1)
+	getImagesAndFreeSpace(ctx, t, assert, manager, fakeRuntime, 1024, 1024, 1, time.Now().Add(time.Minute))
 }
 
 func TestFreeSpaceTiesBrokenByDetectedTime(t *testing.T) {
@@ -526,11 +511,8 @@ func TestFreeSpaceTiesBrokenByDetectedTime(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, manager.imageRecordsLen(), 2)
 
-	spaceFreed, err := manager.freeSpace(ctx, 1024, time.Now())
 	assert := assert.New(t)
-	require.NoError(t, err)
-	assert.EqualValues(2048, spaceFreed)
-	assert.Len(fakeRuntime.ImageList, 1)
+	getImagesAndFreeSpace(ctx, t, assert, manager, fakeRuntime, 1024, 2048, 1, time.Now())
 }
 
 func TestGarbageCollectBelowLowThreshold(t *testing.T) {
@@ -653,20 +635,136 @@ func TestGarbageCollectImageNotOldEnough(t *testing.T) {
 	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(ctx, 1024, fakeClock.Now())
 	assert := assert.New(t)
-	require.NoError(t, err)
-	assert.EqualValues(0, spaceFreed)
-	assert.Len(fakeRuntime.ImageList, 2)
+	getImagesAndFreeSpace(ctx, t, assert, manager, fakeRuntime, 1024, 0, 2, fakeClock.Now())
 
 	// move clock by minAge duration, then 1 image will be garbage collected
 	fakeClock.Step(policy.MinAge)
-	spaceFreed, err = manager.freeSpace(ctx, 1024, fakeClock.Now())
+	getImagesAndFreeSpace(ctx, t, assert, manager, fakeRuntime, 1024, 1024, 1, fakeClock.Now())
+}
+
+func getImagesAndFreeSpace(ctx context.Context, t *testing.T, assert *assert.Assertions, im *realImageGCManager, fakeRuntime *containertest.FakeRuntime, spaceToFree, expectedSpaceFreed int64, imagesLen int, freeTime time.Time) {
+	images, err := im.imagesInEvictionOrder(ctx, freeTime)
 	require.NoError(t, err)
-	assert.EqualValues(1024, spaceFreed)
+	spaceFreed, err := im.freeSpace(ctx, spaceToFree, freeTime, images)
+	require.NoError(t, err)
+	assert.EqualValues(expectedSpaceFreed, spaceFreed)
+	assert.Len(fakeRuntime.ImageList, imagesLen)
+}
+
+func TestGarbageCollectImageTooOld(t *testing.T) {
+	ctx := context.Background()
+	policy := ImageGCPolicy{
+		HighThresholdPercent: 90,
+		LowThresholdPercent:  80,
+		MinAge:               0,
+		MaxAge:               time.Minute * 1,
+	}
+	fakeRuntime := &containertest.FakeRuntime{}
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	mockStatsProvider := statstest.NewMockProvider(mockCtrl)
+	manager := &realImageGCManager{
+		runtime:       fakeRuntime,
+		policy:        policy,
+		imageRecords:  make(map[string]*imageRecord),
+		statsProvider: mockStatsProvider,
+		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 = []*containertest.FakePod{
+		{Pod: &container.Pod{
+			Containers: []*container.Container{
+				makeContainer(1),
+			},
+		}},
+	}
+
+	fakeClock := testingclock.NewFakeClock(time.Now())
+	t.Log(fakeClock.Now())
+	images, err := manager.imagesInEvictionOrder(ctx, fakeClock.Now())
+	require.NoError(t, err)
+	require.Equal(t, len(images), 1)
+	// Simulate pod having just used this image, but having been GC'd
+	images[0].lastUsed = fakeClock.Now()
+
+	// First GC round should not GC remaining image, as it was used too recently.
+	assert := assert.New(t)
+	images, err = manager.freeOldImages(ctx, images, fakeClock.Now())
+	require.NoError(t, err)
+	assert.Len(images, 1)
+	assert.Len(fakeRuntime.ImageList, 2)
+
+	// move clock by a millisecond past maxAge duration, then 1 image will be garbage collected
+	fakeClock.Step(policy.MaxAge + 1)
+	images, err = manager.freeOldImages(ctx, images, fakeClock.Now())
+	require.NoError(t, err)
+	assert.Len(images, 0)
 	assert.Len(fakeRuntime.ImageList, 1)
 }
 
+func TestGarbageCollectImageMaxAgeDisabled(t *testing.T) {
+	ctx := context.Background()
+	policy := ImageGCPolicy{
+		HighThresholdPercent: 90,
+		LowThresholdPercent:  80,
+		MinAge:               0,
+		MaxAge:               0,
+	}
+	fakeRuntime := &containertest.FakeRuntime{}
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	mockStatsProvider := statstest.NewMockProvider(mockCtrl)
+	manager := &realImageGCManager{
+		runtime:       fakeRuntime,
+		policy:        policy,
+		imageRecords:  make(map[string]*imageRecord),
+		statsProvider: mockStatsProvider,
+		recorder:      &record.FakeRecorder{},
+	}
+
+	assert := assert.New(t)
+	fakeRuntime.ImageList = []container.Image{
+		makeImage(0, 1024),
+		makeImage(1, 2048),
+	}
+	assert.Len(fakeRuntime.ImageList, 2)
+	// 1 image is in use, and another one is not old enough
+	fakeRuntime.AllPodList = []*containertest.FakePod{
+		{Pod: &container.Pod{
+			Containers: []*container.Container{
+				makeContainer(1),
+			},
+		}},
+	}
+
+	fakeClock := testingclock.NewFakeClock(time.Now())
+	t.Log(fakeClock.Now())
+	images, err := manager.imagesInEvictionOrder(ctx, fakeClock.Now())
+	require.NoError(t, err)
+	require.Equal(t, len(images), 1)
+	assert.Len(fakeRuntime.ImageList, 2)
+
+	// First GC round should not GC remaining image, as it was used too recently.
+	images, err = manager.freeOldImages(ctx, images, fakeClock.Now())
+	require.NoError(t, err)
+	assert.Len(images, 1)
+	assert.Len(fakeRuntime.ImageList, 2)
+
+	// Move clock by a lot, and the images should continue to not be garbage colleced
+	// See https://stackoverflow.com/questions/25065055/what-is-the-maximum-time-time-in-go
+	fakeClock.SetTime(time.Unix(1<<63-62135596801, 999999999))
+	images, err = manager.freeOldImages(ctx, images, fakeClock.Now())
+	require.NoError(t, err)
+	assert.Len(images, 1)
+	assert.Len(fakeRuntime.ImageList, 2)
+}
+
 func TestValidateImageGCPolicy(t *testing.T) {
 	testCases := []struct {
 		name          string

pkg/kubelet/kubelet.go

@@ -424,6 +424,12 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 		LowThresholdPercent:  int(kubeCfg.ImageGCLowThresholdPercent),
 	}
 
+	if utilfeature.DefaultFeatureGate.Enabled(features.ImageMaximumGCAge) {
+		imageGCPolicy.MaxAge = kubeCfg.ImageMaximumGCAge.Duration
+	} else if kubeCfg.ImageMaximumGCAge.Duration != 0 {
+		klog.InfoS("ImageMaximumGCAge flag enabled, but corresponding feature gate is not enabled. Ignoring flag.")
+	}
+
 	enforceNodeAllocatable := kubeCfg.EnforceNodeAllocatable
 	if experimentalNodeAllocatableIgnoreEvictionThreshold {
 		// Do not provide kubeCfg.EnforceNodeAllocatable to eviction threshold parsing if we are not enforcing Evictions

pkg/kubelet/metrics/metrics.go

@@ -116,6 +116,9 @@ const (
 	orphanPodCleanedVolumesKey       = "orphan_pod_cleaned_volumes"
 	orphanPodCleanedVolumesErrorsKey = "orphan_pod_cleaned_volumes_errors"
 
+	// Metric for tracking garbage collected images
+	ImageGarbageCollectedTotalKey = "image_garbage_collected_total"
+
 	// Values used in metric labels
 	Container          = "container"
 	InitContainer      = "init_container"
@@ -786,6 +789,15 @@ var (
 			StabilityLevel: metrics.ALPHA,
 		},
 	)
+
+	ImageGarbageCollectedTotal = metrics.NewCounter(
+		&metrics.CounterOpts{
+			Subsystem:      KubeletSubsystem,
+			Name:           ImageGarbageCollectedTotalKey,
+			Help:           "Total number of images garbage collected by the kubelet, whether through disk usage or image age.",
+			StabilityLevel: metrics.ALPHA,
+		},
+	)
 )
 
 var registerMetrics sync.Once

staging/src/k8s.io/kubelet/config/v1beta1/types.go

@@ -290,6 +290,12 @@ type KubeletConfiguration struct {
 	// Default: "2m"
 	// +optional
 	ImageMinimumGCAge metav1.Duration `json:"imageMinimumGCAge,omitempty"`
+	// imageMaximumGCAge is the maximum age an image can be unused before it is garbage collected.
+	// The default of this field is "0s", which disables this field--meaning images won't be garbage
+	// collected based on being unused for too long.
+	// Default: "0s" (disabled)
+	// +optional
+	ImageMaximumGCAge metav1.Duration `json:"imageMaximumGCAge,omitempty"`
 	// imageGCHighThresholdPercent is the percent of disk usage after which
 	// image garbage collection is always run. The percent is calculated by
 	// dividing this field value by 100, so this field must be between 0 and

staging/src/k8s.io/kubelet/config/v1beta1/zz_generated.deepcopy.go

@@ -237,6 +237,7 @@ func (in *KubeletConfiguration) DeepCopyInto(out *KubeletConfiguration) {
 	out.NodeStatusUpdateFrequency = in.NodeStatusUpdateFrequency
 	out.NodeStatusReportFrequency = in.NodeStatusReportFrequency
 	out.ImageMinimumGCAge = in.ImageMinimumGCAge
+	out.ImageMaximumGCAge = in.ImageMaximumGCAge
 	if in.ImageGCHighThresholdPercent != nil {
 		in, out := &in.ImageGCHighThresholdPercent, &out.ImageGCHighThresholdPercent
 		*out = new(int32)