Merge pull request kubernetes#18189 from sjenning/pick-58229

Automatic merge from submit-queue (batch tested with PRs 18189, 18010).

UPSTREAM: 58229: kubelet: imagegc: exempt sandbox image

kubernetes#58229

Prevents the imageGC from removing the locally built origin-pod sandbox image

@derekwaynecarr @bparees @stevekuznetsov @pweil- @jupierce

Origin-commit: ffab5c9958893ab0faa606fd8949176f970d8fe4

Author: k8s-publishing-bot

pkg/kubelet/images/image_gc_manager.go

@@ -101,6 +101,9 @@ type realImageGCManager struct {
 
 	// imageCache is the cache of latest image list.
 	imageCache imageCache
+
+	// sandbox image exempted from GC
+	sandboxImage string
 }
 
 // imageCache caches latest result of ListImages.
@@ -137,7 +140,7 @@ type imageRecord struct {
 	size int64
 }
 
-func NewImageGCManager(runtime container.Runtime, statsProvider StatsProvider, recorder record.EventRecorder, nodeRef *v1.ObjectReference, policy ImageGCPolicy) (ImageGCManager, error) {
+func NewImageGCManager(runtime container.Runtime, statsProvider StatsProvider, recorder record.EventRecorder, nodeRef *v1.ObjectReference, policy ImageGCPolicy, sandboxImage string) (ImageGCManager, error) {
 	// Validate policy.
 	if policy.HighThresholdPercent < 0 || policy.HighThresholdPercent > 100 {
 		return nil, fmt.Errorf("invalid HighThresholdPercent %d, must be in range [0-100]", policy.HighThresholdPercent)
@@ -156,6 +159,7 @@ func NewImageGCManager(runtime container.Runtime, statsProvider StatsProvider, r
 		recorder:      recorder,
 		nodeRef:       nodeRef,
 		initialized:   false,
+		sandboxImage:  sandboxImage,
 	}
 
 	return im, nil
@@ -206,6 +210,13 @@ func (im *realImageGCManager) detectImages(detectTime time.Time) error {
 
 	// Make a set of images in use by containers.
 	imagesInUse := sets.NewString()
+
+	// Always consider the container runtime pod sandbox image in use
+	imageRef, err := im.runtime.GetImageRef(container.ImageSpec{Image: im.sandboxImage})
+	if err == nil && imageRef != "" {
+		imagesInUse.Insert(imageRef)
+	}
+
 	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)

pkg/kubelet/images/image_gc_manager_test.go

@@ -33,6 +33,7 @@ import (
 )
 
 var zero time.Time
+var sandboxImage = "gcr.io/google_containers/pause-amd64:latest"
 
 func newRealImageGCManager(policy ImageGCPolicy) (*realImageGCManager, *containertest.FakeRuntime, *statstest.StatsProvider) {
 	fakeRuntime := &containertest.FakeRuntime{}
@@ -43,6 +44,7 @@ func newRealImageGCManager(policy ImageGCPolicy) (*realImageGCManager, *containe
 		imageRecords:  make(map[string]*imageRecord),
 		statsProvider: mockStatsProvider,
 		recorder:      &record.FakeRecorder{},
+		sandboxImage:  sandboxImage,
 	}, fakeRuntime, mockStatsProvider
 }
 
@@ -176,6 +178,21 @@ func TestDetectImagesWithNewImage(t *testing.T) {
 	assert.Equal(zero, noContainer.lastUsed)
 }
 
+func TestDeleteUnusedImagesExemptSandboxImage(t *testing.T) {
+	manager, fakeRuntime, _ := newRealImageGCManager(ImageGCPolicy{})
+	fakeRuntime.ImageList = []container.Image{
+		{
+			ID:   sandboxImage,
+			Size: 1024,
+		},
+	}
+
+	spaceFreed, err := manager.DeleteUnusedImages()
+	assert := assert.New(t)
+	require.NoError(t, err)
+	assert.EqualValues(0, spaceFreed)
+}
+
 func TestDetectImagesContainerStopped(t *testing.T) {
 	manager, fakeRuntime, _ := newRealImageGCManager(ImageGCPolicy{})
 	fakeRuntime.ImageList = []container.Image{
@@ -517,7 +534,7 @@ func TestValidateImageGCPolicy(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		if _, err := NewImageGCManager(nil, nil, nil, nil, tc.imageGCPolicy); err != nil {
+		if _, err := NewImageGCManager(nil, nil, nil, nil, tc.imageGCPolicy, ""); err != nil {
 			if err.Error() != tc.expectErr {
 				t.Errorf("[%s:]Expected err:%v, but got:%v", tc.name, tc.expectErr, err.Error())
 			}

pkg/kubelet/kubelet.go

@@ -767,7 +767,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 	klet.containerDeletor = newPodContainerDeletor(klet.containerRuntime, integer.IntMax(containerGCPolicy.MaxPerPodContainer, minDeadContainerInPod))
 
 	// setup imageManager
-	imageManager, err := images.NewImageGCManager(klet.containerRuntime, klet.StatsProvider, kubeDeps.Recorder, nodeRef, imageGCPolicy)
+	imageManager, err := images.NewImageGCManager(klet.containerRuntime, klet.StatsProvider, kubeDeps.Recorder, nodeRef, imageGCPolicy, crOptions.PodSandboxImage)
 	if err != nil {
 		return nil, fmt.Errorf("failed to initialize image manager: %v", err)
 	}

pkg/kubelet/kubelet_test.go

@@ -256,7 +256,7 @@ func newTestKubeletWithImageList(
 		HighThresholdPercent: 90,
 		LowThresholdPercent:  80,
 	}
-	imageGCManager, err := images.NewImageGCManager(fakeRuntime, kubelet.StatsProvider, fakeRecorder, fakeNodeRef, fakeImageGCPolicy)
+	imageGCManager, err := images.NewImageGCManager(fakeRuntime, kubelet.StatsProvider, fakeRecorder, fakeNodeRef, fakeImageGCPolicy, "")
 	assert.NoError(t, err)
 	kubelet.imageManager = &fakeImageGCManager{
 		fakeImageService: fakeRuntime,

pkg/kubelet/kuberuntime/kuberuntime_image.go

@@ -78,7 +78,7 @@ func (m *kubeGenericRuntimeManager) PullImage(image kubecontainer.ImageSpec, pul
 	return "", utilerrors.NewAggregate(pullErrs)
 }
 
-// GetImageRef gets the reference (digest or ID) of the image which has already been in
+// GetImageRef gets the ID of the image which has already been in
 // the local storage. It returns ("", nil) if the image isn't in the local storage.
 func (m *kubeGenericRuntimeManager) GetImageRef(image kubecontainer.ImageSpec) (string, error) {
 	status, err := m.imageService.ImageStatus(&runtimeapi.ImageSpec{Image: image.Image})
@@ -89,12 +89,7 @@ func (m *kubeGenericRuntimeManager) GetImageRef(image kubecontainer.ImageSpec) (
 	if status == nil {
 		return "", nil
 	}
-
-	imageRef := status.Id
-	if len(status.RepoDigests) > 0 {
-		imageRef = status.RepoDigests[0]
-	}
-	return imageRef, nil
+	return status.Id, nil
 }
 
 // ListImages gets all images currently on the machine.