From d5690f12b69aac5ab4eb0e226682ef68b4059d6e Mon Sep 17 00:00:00 2001 From: Sohan Kunkerkar Date: Wed, 7 Jun 2023 14:24:24 -0400 Subject: [PATCH] pkg/kubelet: allow sandbox image pinning from CRI As part of this change, the code responsible for managing the sandbox image within the kubelet has been removed. Previously, the kubelet used to prevent sandbox image from the garbage collection process. However, with this update, the responsibility of managing the sandbox containers has been shifted to the CRI implementation itself. By allowing sandbox image pinning from CRI, we improve efficiency and simplify the kubelet's interaction with the container runtime. As a result, the kubelet can now rely on the container runtime's built-in mechanisms for sandbox container lifecycle management. Signed-off-by: Sohan Kunkerkar --- cmd/kubelet/app/server.go | 1 + pkg/kubelet/images/image_gc_manager.go | 12 +----------- pkg/kubelet/images/image_gc_manager_test.go | 10 +++++----- pkg/kubelet/kubelet.go | 2 +- pkg/kubelet/kubelet_test.go | 2 +- 5 files changed, 9 insertions(+), 18 deletions(-) diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 78b2d2c1394..45889f0b68f 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -201,6 +201,7 @@ is checked every 20 seconds (also configurable with a flag).`, if cleanFlagSet.Changed("pod-infra-container-image") { klog.InfoS("--pod-infra-container-image will not be pruned by the image garbage collector in kubelet and should also be set in the remote runtime") + _ = cmd.Flags().MarkDeprecated("pod-infra-container-image", "--pod-infra-container-image will be removed in 1.30. Image garbage collector will get sandbox image information from CRI.") } // load kubelet config file, if provided diff --git a/pkg/kubelet/images/image_gc_manager.go b/pkg/kubelet/images/image_gc_manager.go index 451dfb08390..52aa0d0ceea 100644 --- a/pkg/kubelet/images/image_gc_manager.go +++ b/pkg/kubelet/images/image_gc_manager.go @@ -106,9 +106,6 @@ type realImageGCManager struct { // imageCache is the cache of latest image list. imageCache imageCache - // sandbox image exempted from GC - sandboxImage string - // tracer for recording spans tracer trace.Tracer } @@ -160,7 +157,7 @@ type imageRecord struct { } // NewImageGCManager instantiates a new ImageGCManager object. -func NewImageGCManager(runtime container.Runtime, statsProvider StatsProvider, recorder record.EventRecorder, nodeRef *v1.ObjectReference, policy ImageGCPolicy, sandboxImage string, tracerProvider trace.TracerProvider) (ImageGCManager, error) { +func NewImageGCManager(runtime container.Runtime, statsProvider StatsProvider, recorder record.EventRecorder, nodeRef *v1.ObjectReference, policy ImageGCPolicy, tracerProvider trace.TracerProvider) (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) @@ -180,7 +177,6 @@ func NewImageGCManager(runtime container.Runtime, statsProvider StatsProvider, r recorder: recorder, nodeRef: nodeRef, initialized: false, - sandboxImage: sandboxImage, tracer: tracer, } @@ -223,12 +219,6 @@ func (im *realImageGCManager) GetImageList() ([]container.Image, error) { func (im *realImageGCManager) detectImages(ctx context.Context, detectTime time.Time) (sets.String, error) { imagesInUse := sets.NewString() - // Always consider the container runtime pod sandbox image in use - imageRef, err := im.runtime.GetImageRef(ctx, container.ImageSpec{Image: im.sandboxImage}) - if err == nil && imageRef != "" { - imagesInUse.Insert(imageRef) - } - images, err := im.runtime.ListImages(ctx) if err != nil { return imagesInUse, err diff --git a/pkg/kubelet/images/image_gc_manager_test.go b/pkg/kubelet/images/image_gc_manager_test.go index 2bb6061a125..a6844cdc798 100644 --- a/pkg/kubelet/images/image_gc_manager_test.go +++ b/pkg/kubelet/images/image_gc_manager_test.go @@ -48,7 +48,6 @@ func newRealImageGCManager(policy ImageGCPolicy, mockStatsProvider stats.Provide imageRecords: make(map[string]*imageRecord), statsProvider: mockStatsProvider, recorder: &record.FakeRecorder{}, - sandboxImage: sandboxImage, tracer: oteltrace.NewNoopTracerProvider().Tracer(""), }, fakeRuntime } @@ -202,8 +201,9 @@ func TestDeleteUnusedImagesExemptSandboxImage(t *testing.T) { manager, fakeRuntime := newRealImageGCManager(ImageGCPolicy{}, mockStatsProvider) fakeRuntime.ImageList = []container.Image{ { - ID: sandboxImage, - Size: 1024, + ID: sandboxImage, + Size: 1024, + Pinned: true, }, } @@ -233,7 +233,7 @@ func TestDeletePinnedImage(t *testing.T) { err := manager.DeleteUnusedImages(ctx) assert := assert.New(t) - assert.Len(fakeRuntime.ImageList, 2) + assert.Len(fakeRuntime.ImageList, 1) require.NoError(t, err) } @@ -719,7 +719,7 @@ func TestValidateImageGCPolicy(t *testing.T) { } for _, tc := range testCases { - if _, err := NewImageGCManager(nil, nil, nil, nil, tc.imageGCPolicy, "", oteltrace.NewNoopTracerProvider()); err != nil { + if _, err := NewImageGCManager(nil, nil, nil, nil, tc.imageGCPolicy, oteltrace.NewNoopTracerProvider()); err != nil { if err.Error() != tc.expectErr { t.Errorf("[%s:]Expected err:%v, but got:%v", tc.name, tc.expectErr, err.Error()) } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index e8918472ee8..e508951d789 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -747,7 +747,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, crOptions.PodSandboxImage, kubeDeps.TracerProvider) + imageManager, err := images.NewImageGCManager(klet.containerRuntime, klet.StatsProvider, kubeDeps.Recorder, nodeRef, imageGCPolicy, kubeDeps.TracerProvider) if err != nil { return nil, fmt.Errorf("failed to initialize image manager: %v", err) } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 1b83b320c00..18f225aade5 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -311,7 +311,7 @@ func newTestKubeletWithImageList( HighThresholdPercent: 90, LowThresholdPercent: 80, } - imageGCManager, err := images.NewImageGCManager(fakeRuntime, kubelet.StatsProvider, fakeRecorder, fakeNodeRef, fakeImageGCPolicy, "", oteltrace.NewNoopTracerProvider()) + imageGCManager, err := images.NewImageGCManager(fakeRuntime, kubelet.StatsProvider, fakeRecorder, fakeNodeRef, fakeImageGCPolicy, oteltrace.NewNoopTracerProvider()) assert.NoError(t, err) kubelet.imageManager = &fakeImageGCManager{ fakeImageService: fakeRuntime,