diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index a6efdadb7a9c5..98fc1e3744b8a 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -25,7 +25,6 @@ import ( "github.com/spf13/pflag" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -117,16 +116,6 @@ type KubeletFlags struct { // This will cause the kubelet to listen to inotify events on the lock file, // releasing it and exiting when another process tries to open that file. ExitOnLockContention bool - // DEPRECATED FLAGS - // minimumGCAge is the minimum age for a finished container before it is - // garbage collected. - MinimumGCAge metav1.Duration - // maxPerPodContainerCount is the maximum number of old instances to - // retain per container. Each container takes up some disk space. - MaxPerPodContainerCount int32 - // maxContainerCount is the maximum number of old instances of containers - // to retain globally. Each container takes up some disk space. - MaxContainerCount int32 // registerSchedulable tells the kubelet to register the node as // schedulable. Won't have any effect if register-node is false. // DEPRECATED: use registerWithTaints instead @@ -141,9 +130,6 @@ func NewKubeletFlags() *KubeletFlags { ContainerRuntimeOptions: *NewContainerRuntimeOptions(), CertDirectory: "/var/lib/kubelet/pki", RootDirectory: filepath.Clean(defaultRootDir), - MaxContainerCount: -1, - MaxPerPodContainerCount: 1, - MinimumGCAge: metav1.Duration{Duration: 0}, RegisterSchedulable: true, NodeLabels: make(map[string]string), } @@ -310,12 +296,6 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) { fs.BoolVar(&f.ExitOnLockContention, "exit-on-lock-contention", f.ExitOnLockContention, "Whether kubelet should exit upon lock-file contention.") // DEPRECATED FLAGS - fs.DurationVar(&f.MinimumGCAge.Duration, "minimum-container-ttl-duration", f.MinimumGCAge.Duration, "Minimum age for a finished container before it is garbage collected. Examples: '300ms', '10s' or '2h45m'") - fs.MarkDeprecated("minimum-container-ttl-duration", "Use --eviction-hard or --eviction-soft instead. Will be removed in a future version.") - fs.Int32Var(&f.MaxPerPodContainerCount, "maximum-dead-containers-per-container", f.MaxPerPodContainerCount, "Maximum number of old instances to retain per container. Each container takes up some disk space.") - fs.MarkDeprecated("maximum-dead-containers-per-container", "Use --eviction-hard or --eviction-soft instead. Will be removed in a future version.") - fs.Int32Var(&f.MaxContainerCount, "maximum-dead-containers", f.MaxContainerCount, "Maximum number of old instances of containers to retain globally. Each container takes up some disk space. To disable, set to a negative number.") - fs.MarkDeprecated("maximum-dead-containers", "Use --eviction-hard or --eviction-soft instead. Will be removed in a future version.") fs.BoolVar(&f.RegisterSchedulable, "register-schedulable", f.RegisterSchedulable, "Register the node as schedulable. Won't have any effect if register-node is false.") fs.MarkDeprecated("register-schedulable", "will be removed in a future version") fs.StringVar(&f.ExperimentalMounterPath, "experimental-mounter-path", f.ExperimentalMounterPath, "[Experimental] Path of mounter binary. Leave empty to use the default mount.") diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 6d4bf492038f5..343152f828095 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -1333,9 +1333,6 @@ func createAndInitKubelet(kubeServer *options.KubeletServer, kubeServer.ExperimentalMounterPath, kubeServer.KernelMemcgNotification, kubeServer.ExperimentalNodeAllocatableIgnoreEvictionThreshold, - kubeServer.MinimumGCAge, - kubeServer.MaxPerPodContainerCount, - kubeServer.MaxContainerCount, kubeServer.RegisterSchedulable, kubeServer.NodeLabels, kubeServer.NodeStatusMaxImages, diff --git a/pkg/kubelet/container/container_gc.go b/pkg/kubelet/container/container_gc.go index b0a25d50058e8..9f4636f78da79 100644 --- a/pkg/kubelet/container/container_gc.go +++ b/pkg/kubelet/container/container_gc.go @@ -18,25 +18,9 @@ package container import ( "context" - "fmt" - "time" - "k8s.io/klog/v2" ) -// GCPolicy specifies a policy for garbage collecting containers. -type GCPolicy struct { - // Minimum age at which a container can be garbage collected, zero for no limit. - MinAge time.Duration - - // Max number of dead containers any single pod (UID, container name) pair is - // allowed to have, less than zero for no limit. - MaxPerPodContainer int - - // Max number of total dead containers, less than zero for no limit. - MaxContainers int -} - // GC manages garbage collection of dead containers. // // Implementation is thread-compatible. @@ -58,31 +42,23 @@ type realContainerGC struct { // Container runtime runtime Runtime - // Policy for garbage collection. - policy GCPolicy - // sourcesReadyProvider provides the readiness of kubelet configuration sources. sourcesReadyProvider SourcesReadyProvider } // NewContainerGC creates a new instance of GC with the specified policy. -func NewContainerGC(runtime Runtime, policy GCPolicy, sourcesReadyProvider SourcesReadyProvider) (GC, error) { - if policy.MinAge < 0 { - return nil, fmt.Errorf("invalid minimum garbage collection age: %v", policy.MinAge) - } - +func NewContainerGC(runtime Runtime, sourcesReadyProvider SourcesReadyProvider) (GC, error) { return &realContainerGC{ runtime: runtime, - policy: policy, sourcesReadyProvider: sourcesReadyProvider, }, nil } func (cgc *realContainerGC) GarbageCollect(ctx context.Context) error { - return cgc.runtime.GarbageCollect(ctx, cgc.policy, cgc.sourcesReadyProvider.AllReady(), false) + return cgc.runtime.GarbageCollect(ctx, cgc.sourcesReadyProvider.AllReady(), false) } func (cgc *realContainerGC) DeleteAllUnusedContainers(ctx context.Context) error { klog.InfoS("Attempting to delete unused containers") - return cgc.runtime.GarbageCollect(ctx, cgc.policy, cgc.sourcesReadyProvider.AllReady(), true) + return cgc.runtime.GarbageCollect(ctx, cgc.sourcesReadyProvider.AllReady(), true) } diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index 3b14039f46129..dbc5d9fa29026 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -96,7 +96,7 @@ type Runtime interface { // that are terminated, but not deleted will be evicted. Otherwise, only deleted pods // will be GC'd. // TODO: Revisit this method and make it cleaner. - GarbageCollect(ctx context.Context, gcPolicy GCPolicy, allSourcesReady bool, evictNonDeletedPods bool) error + GarbageCollect(ctx context.Context, allSourcesReady bool, evictNonDeletedPods bool) error // SyncPod syncs the running pod into the desired pod. SyncPod(ctx context.Context, pod *v1.Pod, podStatus *PodStatus, pullSecrets []v1.Secret, backOff *flowcontrol.Backoff) PodSyncResult // KillPod kills all the containers of a pod. Pod may be nil, running pod must not be. diff --git a/pkg/kubelet/container/testing/fake_runtime.go b/pkg/kubelet/container/testing/fake_runtime.go index 52fbd709d9944..06a1a6c25e564 100644 --- a/pkg/kubelet/container/testing/fake_runtime.go +++ b/pkg/kubelet/container/testing/fake_runtime.go @@ -401,7 +401,7 @@ func (f *FakeRuntime) RemoveImage(_ context.Context, image kubecontainer.ImageSp return f.Err } -func (f *FakeRuntime) GarbageCollect(_ context.Context, gcPolicy kubecontainer.GCPolicy, ready bool, evictNonDeletedPods bool) error { +func (f *FakeRuntime) GarbageCollect(_ context.Context, ready bool, evictNonDeletedPods bool) error { f.Lock() defer f.Unlock() diff --git a/pkg/kubelet/container/testing/runtime_mock.go b/pkg/kubelet/container/testing/runtime_mock.go index 63bd6b4e790e9..0773ea68bf046 100644 --- a/pkg/kubelet/container/testing/runtime_mock.go +++ b/pkg/kubelet/container/testing/runtime_mock.go @@ -200,17 +200,17 @@ func (_c *MockRuntime_DeleteContainer_Call) RunAndReturn(run func(context.Contex return _c } -// GarbageCollect provides a mock function with given fields: ctx, gcPolicy, allSourcesReady, evictNonDeletedPods -func (_m *MockRuntime) GarbageCollect(ctx context.Context, gcPolicy container.GCPolicy, allSourcesReady bool, evictNonDeletedPods bool) error { - ret := _m.Called(ctx, gcPolicy, allSourcesReady, evictNonDeletedPods) +// GarbageCollect provides a mock function with given fields: ctx, allSourcesReady, evictNonDeletedPods +func (_m *MockRuntime) GarbageCollect(ctx context.Context, allSourcesReady bool, evictNonDeletedPods bool) error { + ret := _m.Called(ctx, allSourcesReady, evictNonDeletedPods) if len(ret) == 0 { panic("no return value specified for GarbageCollect") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, container.GCPolicy, bool, bool) error); ok { - r0 = rf(ctx, gcPolicy, allSourcesReady, evictNonDeletedPods) + if rf, ok := ret.Get(0).(func(context.Context, bool, bool) error); ok { + r0 = rf(ctx, allSourcesReady, evictNonDeletedPods) } else { r0 = ret.Error(0) } @@ -225,16 +225,15 @@ type MockRuntime_GarbageCollect_Call struct { // GarbageCollect is a helper method to define mock.On call // - ctx context.Context -// - gcPolicy container.GCPolicy // - allSourcesReady bool // - evictNonDeletedPods bool -func (_e *MockRuntime_Expecter) GarbageCollect(ctx interface{}, gcPolicy interface{}, allSourcesReady interface{}, evictNonDeletedPods interface{}) *MockRuntime_GarbageCollect_Call { - return &MockRuntime_GarbageCollect_Call{Call: _e.mock.On("GarbageCollect", ctx, gcPolicy, allSourcesReady, evictNonDeletedPods)} +func (_e *MockRuntime_Expecter) GarbageCollect(ctx interface{}, allSourcesReady interface{}, evictNonDeletedPods interface{}) *MockRuntime_GarbageCollect_Call { + return &MockRuntime_GarbageCollect_Call{Call: _e.mock.On("GarbageCollect", ctx, allSourcesReady, evictNonDeletedPods)} } -func (_c *MockRuntime_GarbageCollect_Call) Run(run func(ctx context.Context, gcPolicy container.GCPolicy, allSourcesReady bool, evictNonDeletedPods bool)) *MockRuntime_GarbageCollect_Call { +func (_c *MockRuntime_GarbageCollect_Call) Run(run func(ctx context.Context, allSourcesReady bool, evictNonDeletedPods bool)) *MockRuntime_GarbageCollect_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(container.GCPolicy), args[2].(bool), args[3].(bool)) + run(args[0].(context.Context), args[1].(bool), args[2].(bool)) }) return _c } @@ -244,7 +243,7 @@ func (_c *MockRuntime_GarbageCollect_Call) Return(_a0 error) *MockRuntime_Garbag return _c } -func (_c *MockRuntime_GarbageCollect_Call) RunAndReturn(run func(context.Context, container.GCPolicy, bool, bool) error) *MockRuntime_GarbageCollect_Call { +func (_c *MockRuntime_GarbageCollect_Call) RunAndReturn(run func(context.Context, bool, bool) error) *MockRuntime_GarbageCollect_Call { _c.Call.Return(run) return _c } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index e22f9b8120950..b7484583c1d1b 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -214,9 +214,6 @@ const ( // ImageGCPeriod is the period for performing image garbage collection. ImageGCPeriod = 5 * time.Minute - // Minimum number of dead containers to keep in a pod - minDeadContainerInPod = 1 - // nodeLeaseRenewIntervalFraction is the fraction of lease duration to renew the lease nodeLeaseRenewIntervalFraction = 0.25 @@ -404,9 +401,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, experimentalMounterPath string, kernelMemcgNotification bool, experimentalNodeAllocatableIgnoreEvictionThreshold bool, - minimumGCAge metav1.Duration, - maxPerPodContainerCount int32, - maxContainerCount int32, registerSchedulable bool, nodeLabels map[string]string, nodeStatusMaxImages int32, @@ -461,12 +455,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, } } - containerGCPolicy := kubecontainer.GCPolicy{ - MinAge: minimumGCAge.Duration, - MaxPerPodContainer: int(maxPerPodContainerCount), - MaxContainers: int(maxContainerCount), - } - daemonEndpoints := &v1.NodeDaemonEndpoints{ KubeletEndpoint: v1.DaemonEndpoint{Port: kubeCfg.Port}, } @@ -824,12 +812,12 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, } // setup containerGC - containerGC, err := kubecontainer.NewContainerGC(klet.containerRuntime, containerGCPolicy, klet.sourcesReady) + containerGC, err := kubecontainer.NewContainerGC(klet.containerRuntime, klet.sourcesReady) if err != nil { return nil, err } klet.containerGC = containerGC - klet.containerDeletor = newPodContainerDeletor(klet.containerRuntime, max(containerGCPolicy.MaxPerPodContainer, minDeadContainerInPod)) + klet.containerDeletor = newPodContainerDeletor(klet.containerRuntime) // setup imageManager imageManager, err := images.NewImageGCManager(klet.containerRuntime, klet.StatsProvider, kubeDeps.Recorder, nodeRef, imageGCPolicy, kubeDeps.TracerProvider) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index ab64cb20ddd32..ac71e100259a8 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -326,12 +326,8 @@ func newTestKubeletWithImageList( ImageGCManager: imageGCManager, } kubelet.containerLogManager = logs.NewStubContainerLogManager() - containerGCPolicy := kubecontainer.GCPolicy{ - MinAge: time.Duration(0), - MaxPerPodContainer: 1, - MaxContainers: -1, - } - containerGC, err := kubecontainer.NewContainerGC(fakeRuntime, containerGCPolicy, kubelet.sourcesReady) + + containerGC, err := kubecontainer.NewContainerGC(fakeRuntime, kubelet.sourcesReady) assert.NoError(t, err) kubelet.containerGC = containerGC @@ -3351,9 +3347,6 @@ func TestNewMainKubeletStandAlone(t *testing.T) { "", false, false, - metav1.Duration{Duration: time.Minute}, - 1024, - 110, true, map[string]string{}, 1024, diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc.go b/pkg/kubelet/kuberuntime/kuberuntime_gc.go index 6189b1f07cae1..2dce64ef0a47e 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc.go @@ -25,12 +25,13 @@ import ( "time" "go.opentelemetry.io/otel/trace" + "k8s.io/klog/v2" + "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" internalapi "k8s.io/cri-api/pkg/apis" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" - "k8s.io/klog/v2" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) @@ -114,10 +115,11 @@ func (a sandboxByCreated) Len() int { return len(a) } func (a sandboxByCreated) Swap(i, j int) { a[i], a[j] = a[j], a[i] } func (a sandboxByCreated) Less(i, j int) bool { return a[i].createTime.After(a[j].createTime) } -// enforceMaxContainersPerEvictUnit enforces MaxPerPodContainer for each evictUnit. -func (cgc *containerGC) enforceMaxContainersPerEvictUnit(ctx context.Context, evictUnits containersByEvictUnit, MaxContainers int) { +// retainLatestContainerPerEvictUnit ensures only the most recent container is kept per evictUnit, +// removing all older containers. +func (cgc *containerGC) retainLatestContainerPerEvictUnit(ctx context.Context, evictUnits containersByEvictUnit) { for key := range evictUnits { - toRemove := len(evictUnits[key]) - MaxContainers + toRemove := len(evictUnits[key]) - 1 if toRemove > 0 { evictUnits[key] = cgc.removeOldestN(ctx, evictUnits[key], toRemove) @@ -185,32 +187,25 @@ func (cgc *containerGC) removeSandbox(ctx context.Context, sandboxID string) { } } -// evictableContainers gets all containers that are evictable. Evictable containers are: not running -// and created more than MinAge ago. -func (cgc *containerGC) evictableContainers(ctx context.Context, minAge time.Duration) (containersByEvictUnit, error) { +// evictableContainers gets all containers that are evictable. Evictable containers are: not running. +func (cgc *containerGC) evictableContainers(ctx context.Context) (containersByEvictUnit, error) { containers, err := cgc.manager.getKubeletContainers(ctx, true) if err != nil { return containersByEvictUnit{}, err } evictUnits := make(containersByEvictUnit) - newestGCTime := time.Now().Add(-minAge) for _, container := range containers { // Prune out running containers. if container.State == runtimeapi.ContainerState_CONTAINER_RUNNING { continue } - createdAt := time.Unix(0, container.CreatedAt) - if newestGCTime.Before(createdAt) { - continue - } - labeledInfo := getContainerInfoFromLabels(container.Labels) containerInfo := containerGCInfo{ id: container.Id, name: container.Metadata.Name, - createTime: createdAt, + createTime: time.Unix(0, container.CreatedAt), unknown: container.State == runtimeapi.ContainerState_CONTAINER_UNKNOWN, } key := evictUnit{ @@ -224,9 +219,9 @@ func (cgc *containerGC) evictableContainers(ctx context.Context, minAge time.Dur } // evict all containers that are evictable -func (cgc *containerGC) evictContainers(ctx context.Context, gcPolicy kubecontainer.GCPolicy, allSourcesReady bool, evictNonDeletedPods bool) error { +func (cgc *containerGC) evictContainers(ctx context.Context, allSourcesReady bool, evictNonDeletedPods bool) error { // Separate containers by evict units. - evictUnits, err := cgc.evictableContainers(ctx, gcPolicy.MinAge) + evictUnits, err := cgc.evictableContainers(ctx) if err != nil { return err } @@ -241,32 +236,9 @@ func (cgc *containerGC) evictContainers(ctx context.Context, gcPolicy kubecontai } } - // Enforce max containers per evict unit. - if gcPolicy.MaxPerPodContainer >= 0 { - cgc.enforceMaxContainersPerEvictUnit(ctx, evictUnits, gcPolicy.MaxPerPodContainer) - } - - // Enforce max total number of containers. - if gcPolicy.MaxContainers >= 0 && evictUnits.NumContainers() > gcPolicy.MaxContainers { - // Leave an equal number of containers per evict unit (min: 1). - numContainersPerEvictUnit := gcPolicy.MaxContainers / evictUnits.NumEvictUnits() - if numContainersPerEvictUnit < 1 { - numContainersPerEvictUnit = 1 - } - cgc.enforceMaxContainersPerEvictUnit(ctx, evictUnits, numContainersPerEvictUnit) - - // If we still need to evict, evict oldest first. - numContainers := evictUnits.NumContainers() - if numContainers > gcPolicy.MaxContainers { - flattened := make([]containerGCInfo, 0, numContainers) - for key := range evictUnits { - flattened = append(flattened, evictUnits[key]...) - } - sort.Sort(byCreated(flattened)) + // only the most recent container is kept per evictUnit + cgc.retainLatestContainerPerEvictUnit(ctx, evictUnits) - cgc.removeOldestN(ctx, flattened, numContainers-gcPolicy.MaxContainers) - } - } return nil } @@ -395,22 +367,18 @@ func (cgc *containerGC) evictPodLogsDirectories(ctx context.Context, allSourcesR return nil } -// GarbageCollect removes dead containers using the specified container gc policy. -// Note that gc policy is not applied to sandboxes. Sandboxes are only removed when they are -// not ready and containing no containers. +// GarbageCollect removes dead containers and sandboxes. // // GarbageCollect consists of the following steps: -// * gets evictable containers which are not active and created more than gcPolicy.MinAge ago. -// * removes oldest dead containers for each pod by enforcing gcPolicy.MaxPerPodContainer. -// * removes oldest dead containers by enforcing gcPolicy.MaxContainers. +// * gets evictable containers which are not active(only the most recent container is kept per evictUnit). // * gets evictable sandboxes which are not ready and contains no containers. // * removes evictable sandboxes. -func (cgc *containerGC) GarbageCollect(ctx context.Context, gcPolicy kubecontainer.GCPolicy, allSourcesReady bool, evictNonDeletedPods bool) error { +func (cgc *containerGC) GarbageCollect(ctx context.Context, allSourcesReady bool, evictNonDeletedPods bool) error { ctx, otelSpan := cgc.tracer.Start(ctx, "Containers/GarbageCollect") defer otelSpan.End() errors := []error{} // Remove evictable containers - if err := cgc.evictContainers(ctx, gcPolicy, allSourcesReady, evictNonDeletedPods); err != nil { + if err := cgc.evictContainers(ctx, allSourcesReady, evictNonDeletedPods); err != nil { errors = append(errors, err) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go b/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go index d0208fd07e5e0..d2352f3660567 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go @@ -18,17 +18,14 @@ package kuberuntime import ( "context" - "os" - "path/filepath" - "testing" - "time" - "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" - kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" + "os" + "path/filepath" + "testing" ) func TestSandboxGC(t *testing.T) { @@ -207,28 +204,25 @@ func TestContainerGC(t *testing.T) { assert.NoError(t, err) podStateProvider := m.containerGC.podStateProvider.(*fakePodStateProvider) - defaultGCPolicy := kubecontainer.GCPolicy{MinAge: time.Hour, MaxPerPodContainer: 2, MaxContainers: 6} for _, test := range []struct { - description string // description of the test case - containers []containerTemplate // templates of containers - policy *kubecontainer.GCPolicy // container gc policy - remain []int // template indexes of remaining containers + description string // description of the test case + containers []containerTemplate // templates of containers + remain []int // template indexes of remaining containers evictTerminatingPods bool allSourcesReady bool }{ { - description: "all containers should be removed when max container limit is 0", + description: "at least a non-running container is retained", containers: []containerTemplate{ makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), }, - policy: &kubecontainer.GCPolicy{MinAge: time.Minute, MaxPerPodContainer: 1, MaxContainers: 0}, - remain: []int{}, + remain: []int{0}, evictTerminatingPods: false, allSourcesReady: true, }, { - description: "max containers should be complied when no max per pod container limit is set", + description: "only the most recently created container will not be removed When there are no running containers within a pod.", containers: []containerTemplate{ makeGCContainer("foo", "bar", 4, 4, runtimeapi.ContainerState_CONTAINER_EXITED), makeGCContainer("foo", "bar", 3, 3, runtimeapi.ContainerState_CONTAINER_EXITED), @@ -236,68 +230,23 @@ func TestContainerGC(t *testing.T) { makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), }, - policy: &kubecontainer.GCPolicy{MinAge: time.Minute, MaxPerPodContainer: -1, MaxContainers: 4}, - remain: []int{0, 1, 2, 3}, - evictTerminatingPods: false, - allSourcesReady: true, - }, - { - description: "no containers should be removed if both max container and per pod container limits are not set", - containers: []containerTemplate{ - makeGCContainer("foo", "bar", 2, 2, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - }, - policy: &kubecontainer.GCPolicy{MinAge: time.Minute, MaxPerPodContainer: -1, MaxContainers: -1}, - remain: []int{0, 1, 2}, - evictTerminatingPods: false, - allSourcesReady: true, - }, - { - description: "recently started containers should not be removed", - containers: []containerTemplate{ - makeGCContainer("foo", "bar", 2, time.Now().UnixNano(), runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 1, time.Now().UnixNano(), runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 0, time.Now().UnixNano(), runtimeapi.ContainerState_CONTAINER_EXITED), - }, - remain: []int{0, 1, 2}, - evictTerminatingPods: false, - allSourcesReady: true, - }, - { - description: "oldest containers should be removed when per pod container limit exceeded", - containers: []containerTemplate{ - makeGCContainer("foo", "bar", 2, 2, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - }, - remain: []int{0, 1}, + remain: []int{0}, evictTerminatingPods: false, allSourcesReady: true, }, { - description: "running containers should not be removed", + description: "retain one running and one non-running container.", containers: []containerTemplate{ makeGCContainer("foo", "bar", 2, 2, runtimeapi.ContainerState_CONTAINER_EXITED), makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_RUNNING), }, - remain: []int{0, 1, 2}, - evictTerminatingPods: false, - allSourcesReady: true, - }, - { - description: "no containers should be removed when limits are not exceeded", - containers: []containerTemplate{ - makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - }, - remain: []int{0, 1}, + remain: []int{0, 2}, evictTerminatingPods: false, allSourcesReady: true, }, { - description: "max container count should apply per (UID, container) pair", + description: "gc policy should apply per (UID, container) pair", containers: []containerTemplate{ makeGCContainer("foo", "bar", 2, 2, runtimeapi.ContainerState_CONTAINER_EXITED), makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), @@ -309,43 +258,7 @@ func TestContainerGC(t *testing.T) { makeGCContainer("foo2", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), makeGCContainer("foo2", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), }, - remain: []int{0, 1, 3, 4, 6, 7}, - evictTerminatingPods: false, - allSourcesReady: true, - }, - { - description: "max limit should apply and try to keep from every pod", - containers: []containerTemplate{ - makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo1", "bar1", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo1", "bar1", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo2", "bar2", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo2", "bar2", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo3", "bar3", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo3", "bar3", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo4", "bar4", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo4", "bar4", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - }, - remain: []int{0, 2, 4, 6, 8}, - evictTerminatingPods: false, - allSourcesReady: true, - }, - { - description: "oldest pods should be removed if limit exceeded", - containers: []containerTemplate{ - makeGCContainer("foo", "bar", 2, 2, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo1", "bar1", 2, 2, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo1", "bar1", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo2", "bar2", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo3", "bar3", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo4", "bar4", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo5", "bar5", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo6", "bar6", 2, 2, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo7", "bar7", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - }, - remain: []int{0, 2, 4, 6, 8, 9}, + remain: []int{0, 3, 6}, evictTerminatingPods: false, allSourcesReady: true, }, @@ -363,20 +276,6 @@ func TestContainerGC(t *testing.T) { evictTerminatingPods: true, allSourcesReady: true, }, - { - description: "containers for deleted pods should be removed", - containers: []containerTemplate{ - makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - // deleted pods still respect MinAge. - makeGCContainer("deleted", "bar1", 2, time.Now().UnixNano(), runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("deleted", "bar1", 1, 1, runtimeapi.ContainerState_CONTAINER_EXITED), - makeGCContainer("deleted", "bar1", 0, 0, runtimeapi.ContainerState_CONTAINER_EXITED), - }, - remain: []int{0, 1, 2}, - evictTerminatingPods: false, - allSourcesReady: true, - }, { description: "containers for deleted pods may not be removed if allSourcesReady is set false ", containers: []containerTemplate{ @@ -402,10 +301,7 @@ func TestContainerGC(t *testing.T) { } fakeRuntime.SetFakeContainers(fakeContainers) - if test.policy == nil { - test.policy = &defaultGCPolicy - } - err := m.containerGC.evictContainers(ctx, *test.policy, test.allSourcesReady, test.evictTerminatingPods) + err := m.containerGC.evictContainers(ctx, test.allSourcesReady, test.evictTerminatingPods) assert.NoError(t, err) realRemain, err := fakeRuntime.ListContainers(ctx, nil) assert.NoError(t, err) @@ -466,15 +362,13 @@ func TestUnknownStateContainerGC(t *testing.T) { fakeRuntime, _, m, err := createTestRuntimeManager() assert.NoError(t, err) - // podStateProvider := m.containerGC.podStateProvider.(*fakePodStateProvider) - defaultGCPolicy := kubecontainer.GCPolicy{MinAge: time.Hour, MaxPerPodContainer: 0, MaxContainers: 0} - fakeContainers := makeFakeContainers(t, m, []containerTemplate{ + makeGCContainer("foo", "bar", 1, 1, runtimeapi.ContainerState_CONTAINER_UNKNOWN), makeGCContainer("foo", "bar", 0, 0, runtimeapi.ContainerState_CONTAINER_UNKNOWN), }) fakeRuntime.SetFakeContainers(fakeContainers) - err = m.containerGC.evictContainers(ctx, defaultGCPolicy, true, false) + err = m.containerGC.evictContainers(ctx, true, false) assert.NoError(t, err) assert.Contains(t, fakeRuntime.GetCalls(), "StopContainer", "RemoveContainer", @@ -482,5 +376,5 @@ func TestUnknownStateContainerGC(t *testing.T) { remain, err := fakeRuntime.ListContainers(ctx, nil) assert.NoError(t, err) - assert.Empty(t, remain) + assert.Len(t, remain, 1) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index dd90ef2babe20..33a35dddc0627 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -1712,8 +1712,8 @@ func (m *kubeGenericRuntimeManager) GetContainerStatus(ctx context.Context, id k } // GarbageCollect removes dead containers using the specified container gc policy. -func (m *kubeGenericRuntimeManager) GarbageCollect(ctx context.Context, gcPolicy kubecontainer.GCPolicy, allSourcesReady bool, evictNonDeletedPods bool) error { - return m.containerGC.GarbageCollect(ctx, gcPolicy, allSourcesReady, evictNonDeletedPods) +func (m *kubeGenericRuntimeManager) GarbageCollect(ctx context.Context, allSourcesReady bool, evictNonDeletedPods bool) error { + return m.containerGC.GarbageCollect(ctx, allSourcesReady, evictNonDeletedPods) } // UpdatePodCIDR is just a passthrough method to update the runtimeConfig of the shim diff --git a/pkg/kubelet/pod_container_deletor.go b/pkg/kubelet/pod_container_deletor.go index c4cecde4a874b..95fc293164c1c 100644 --- a/pkg/kubelet/pod_container_deletor.go +++ b/pkg/kubelet/pod_container_deletor.go @@ -34,8 +34,7 @@ const ( type containerStatusbyCreatedList []*kubecontainer.Status type podContainerDeletor struct { - worker chan<- kubecontainer.ContainerID - containersToKeep int + worker chan<- kubecontainer.ContainerID } func (a containerStatusbyCreatedList) Len() int { return len(a) } @@ -44,7 +43,7 @@ func (a containerStatusbyCreatedList) Less(i, j int) bool { return a[i].CreatedAt.After(a[j].CreatedAt) } -func newPodContainerDeletor(runtime kubecontainer.Runtime, containersToKeep int) *podContainerDeletor { +func newPodContainerDeletor(runtime kubecontainer.Runtime) *podContainerDeletor { buffer := make(chan kubecontainer.ContainerID, containerDeletorBufferLimit) go wait.Until(func() { for { @@ -56,8 +55,7 @@ func newPodContainerDeletor(runtime kubecontainer.Runtime, containersToKeep int) }, 0, wait.NeverStop) return &podContainerDeletor{ - worker: buffer, - containersToKeep: containersToKeep, + worker: buffer, } } @@ -101,7 +99,7 @@ func getContainersToDeleteInPod(filterContainerID string, podStatus *kubecontain // deleteContainersInPod issues container deletion requests for containers selected by getContainersToDeleteInPod. func (p *podContainerDeletor) deleteContainersInPod(filterContainerID string, podStatus *kubecontainer.PodStatus, removeAll bool) { - containersToKeep := p.containersToKeep + containersToKeep := 1 if removeAll { containersToKeep = 0 filterContainerID = "" diff --git a/pkg/kubelet/pod_container_deletor_test.go b/pkg/kubelet/pod_container_deletor_test.go index b1e7ad7bc7f86..c7424ccd650f6 100644 --- a/pkg/kubelet/pod_container_deletor_test.go +++ b/pkg/kubelet/pod_container_deletor_test.go @@ -72,10 +72,6 @@ func TestGetContainersToDeleteInPodWithFilter(t *testing.T) { 1, []*kubecontainer.Status{pod.ContainerStatuses[2], pod.ContainerStatuses[1]}, }, - { - 2, - []*kubecontainer.Status{pod.ContainerStatuses[1]}, - }, } for _, test := range testCases { diff --git a/pkg/kubemark/hollow_kubelet.go b/pkg/kubemark/hollow_kubelet.go index 523b6fcd65f42..4c14b62b3b903 100644 --- a/pkg/kubemark/hollow_kubelet.go +++ b/pkg/kubemark/hollow_kubelet.go @@ -26,7 +26,6 @@ import ( "k8s.io/klog/v2" "k8s.io/mount-utils" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" internalapi "k8s.io/cri-api/pkg/apis" @@ -156,9 +155,6 @@ func GetHollowKubeletConfig(opt *HollowKubeletOptions) (*options.KubeletFlags, * f := options.NewKubeletFlags() f.RootDirectory = testRootDir f.HostnameOverride = opt.NodeName - f.MinimumGCAge = metav1.Duration{Duration: 1 * time.Minute} - f.MaxContainerCount = 100 - f.MaxPerPodContainerCount = 2 f.NodeLabels = opt.NodeLabels f.RegisterSchedulable = true