KEP-4210: add support for ImageMaximumGCAge field

pkg/features/kube_features.go

@@ -897,6 +897,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() {
@@ -1140,6 +1147,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/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/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/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,40 +386,12 @@ 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) {
-	imagesInUse, err := im.detectImages(ctx, freeTime)
-	if err != nil {
-		return 0, err
-	}
-
-	im.imageRecordsLock.Lock()
-	defer im.imageRecordsLock.Unlock()
-
-	// Get all images in eviction order.
-	images := make([]evictionInfo, 0, len(im.imageRecords))
-	for image, record := range im.imageRecords {
-		if isImageUsed(image, imagesInUse) {
-			klog.V(5).InfoS("Image ID is being used", "imageID", image)
-			continue
-		}
-		// Check if image is pinned, prevent garbage collection
-		if record.pinned {
-			klog.V(5).InfoS("Image is pinned, skipping garbage collection", "imageID", image)
-			continue
-
-		}
-		images = append(images, evictionInfo{
-			id:          image,
-			imageRecord: *record,
-		})
-	}
-	sort.Sort(byLastUsedAndDetected(images))
-
+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", "imageID", image.id)
+		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)
@@ -380,20 +400,15 @@ func (im *realImageGCManager) freeSpace(ctx context.Context, bytesToFree int64,
 
 		// 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 {
+		if err := im.freeImage(ctx, image); err != nil {
 			deletionErrors = append(deletionErrors, err)
 			continue
 		}
-		delete(im.imageRecords, image.id)
 		spaceFreed += image.size
 
 		if spaceFreed >= bytesToFree {
@@ -407,6 +422,50 @@ func (im *realImageGCManager) freeSpace(ctx context.Context, bytesToFree int64,
 	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 nil, err
+	}
+
+	im.imageRecordsLock.Lock()
+	defer im.imageRecordsLock.Unlock()
+
+	// Get all images in eviction order.
+	images := make([]evictionInfo, 0, len(im.imageRecords))
+	for image, record := range im.imageRecords {
+		if isImageUsed(image, imagesInUse) {
+			klog.V(5).InfoS("Image ID is being used", "imageID", image)
+			continue
+		}
+		// Check if image is pinned, prevent garbage collection
+		if record.pinned {
+			klog.V(5).InfoS("Image is pinned, skipping garbage collection", "imageID", image)
+			continue
+
+		}
+		images = append(images, evictionInfo{
+			id:          image,
+			imageRecord: *record,
+		})
+	}
+	sort.Sort(byLastUsedAndDetected(images))
+	return images, nil
+}
+
 type evictionInfo struct {
 	id string
 	imageRecord