diff --git a/pkg/koordlet/qosmanager/framework/config.go b/pkg/koordlet/qosmanager/framework/config.go index f6ac0ca6e..c6c462840 100644 --- a/pkg/koordlet/qosmanager/framework/config.go +++ b/pkg/koordlet/qosmanager/framework/config.go @@ -27,6 +27,7 @@ type Config struct { MemoryEvictIntervalSeconds int MemoryEvictCoolTimeSeconds int CPUEvictCoolTimeSeconds int + OnlyEvictByAPI bool QOSExtensionCfg *QOSExtensionConfig } @@ -38,6 +39,7 @@ func NewDefaultConfig() *Config { MemoryEvictIntervalSeconds: 1, MemoryEvictCoolTimeSeconds: 4, CPUEvictCoolTimeSeconds: 20, + OnlyEvictByAPI: false, QOSExtensionCfg: &QOSExtensionConfig{FeatureGates: map[string]bool{}}, } } @@ -49,5 +51,6 @@ func (c *Config) InitFlags(fs *flag.FlagSet) { fs.IntVar(&c.MemoryEvictIntervalSeconds, "memory-evict-interval-seconds", c.MemoryEvictIntervalSeconds, "evict be pod(memory) interval by seconds") fs.IntVar(&c.MemoryEvictCoolTimeSeconds, "memory-evict-cool-time-seconds", c.MemoryEvictCoolTimeSeconds, "cooling time: memory next evict time should after lastEvictTime + MemoryEvictCoolTimeSeconds") fs.IntVar(&c.CPUEvictCoolTimeSeconds, "cpu-evict-cool-time-seconds", c.CPUEvictCoolTimeSeconds, "cooltime: CPU next evict time should after lastEvictTime + CPUEvictCoolTimeSeconds") + fs.BoolVar(&c.OnlyEvictByAPI, "only-evict-by-api", c.OnlyEvictByAPI, "only evict pod if call eviction api successed") c.QOSExtensionCfg.InitFlags(fs) } diff --git a/pkg/koordlet/qosmanager/framework/config_test.go b/pkg/koordlet/qosmanager/framework/config_test.go index 5ebb97836..122bf2f04 100644 --- a/pkg/koordlet/qosmanager/framework/config_test.go +++ b/pkg/koordlet/qosmanager/framework/config_test.go @@ -31,6 +31,7 @@ func Test_NewDefaultConfig(t *testing.T) { MemoryEvictIntervalSeconds: 1, MemoryEvictCoolTimeSeconds: 4, CPUEvictCoolTimeSeconds: 20, + OnlyEvictByAPI: false, QOSExtensionCfg: &QOSExtensionConfig{FeatureGates: map[string]bool{}}, } defaultConfig := NewDefaultConfig() @@ -47,6 +48,7 @@ func Test_InitFlags(t *testing.T) { "--memory-evict-cool-time-seconds=8", "--cpu-evict-cool-time-seconds=40", "--qos-extension-plugins=test-plugin=true", + "--only-evict-by-api=false", } fs := flag.NewFlagSet(cmdArgs[0], flag.ExitOnError) @@ -57,6 +59,7 @@ func Test_InitFlags(t *testing.T) { MemoryEvictIntervalSeconds int MemoryEvictCoolTimeSeconds int CPUEvictCoolTimeSeconds int + OnlyEvictByAPI bool QOSExtensionCfg *QOSExtensionConfig } type args struct { @@ -76,6 +79,7 @@ func Test_InitFlags(t *testing.T) { MemoryEvictIntervalSeconds: 2, MemoryEvictCoolTimeSeconds: 8, CPUEvictCoolTimeSeconds: 40, + OnlyEvictByAPI: false, QOSExtensionCfg: &QOSExtensionConfig{FeatureGates: map[string]bool{"test-plugin": true}}, }, args: args{fs: fs}, @@ -90,6 +94,7 @@ func Test_InitFlags(t *testing.T) { MemoryEvictIntervalSeconds: tt.fields.MemoryEvictIntervalSeconds, MemoryEvictCoolTimeSeconds: tt.fields.MemoryEvictCoolTimeSeconds, CPUEvictCoolTimeSeconds: tt.fields.CPUEvictCoolTimeSeconds, + OnlyEvictByAPI: tt.fields.OnlyEvictByAPI, QOSExtensionCfg: tt.fields.QOSExtensionCfg, } c := NewDefaultConfig() diff --git a/pkg/koordlet/qosmanager/framework/context.go b/pkg/koordlet/qosmanager/framework/context.go index e4d86e9a1..8df0a3e21 100644 --- a/pkg/koordlet/qosmanager/framework/context.go +++ b/pkg/koordlet/qosmanager/framework/context.go @@ -62,7 +62,7 @@ func (r *Evictor) Start(stopCh <-chan struct{}) error { func (r *Evictor) EvictPodsIfNotEvicted(evictPods []*corev1.Pod, node *corev1.Node, reason string, message string) { for _, evictPod := range evictPods { - r.evictPodIfNotEvicted(evictPod, node, reason, message) + r.EvictPodIfNotEvicted(evictPod, node, reason, message) } } @@ -74,16 +74,18 @@ func (r *Evictor) IsPodEvicted(pod *corev1.Pod) bool { return evicted } -func (r *Evictor) evictPodIfNotEvicted(evictPod *corev1.Pod, node *corev1.Node, reason string, message string) { +func (r *Evictor) EvictPodIfNotEvicted(evictPod *corev1.Pod, node *corev1.Node, reason string, message string) bool { _, evicted := r.podsEvicted.Get(string(evictPod.UID)) if evicted { klog.V(5).Infof("Pod has been evicted! podID: %v, evict reason: %s", evictPod.UID, reason) - return + return true } success := r.evictPod(evictPod, reason, message) if success { _ = r.podsEvicted.SetDefault(string(evictPod.UID), evictPod.UID) } + + return success } func (r *Evictor) evictPod(evictPod *corev1.Pod, reason string, message string) bool { diff --git a/pkg/koordlet/qosmanager/framework/context_test.go b/pkg/koordlet/qosmanager/framework/context_test.go index 247d8f65f..e17f459c0 100644 --- a/pkg/koordlet/qosmanager/framework/context_test.go +++ b/pkg/koordlet/qosmanager/framework/context_test.go @@ -32,11 +32,101 @@ import ( apiext "github.com/koordinator-sh/koordinator/apis/extension" "github.com/koordinator-sh/koordinator/pkg/koordlet/qosmanager/helpers" + "github.com/koordinator-sh/koordinator/pkg/koordlet/resourceexecutor" mock_statesinformer "github.com/koordinator-sh/koordinator/pkg/koordlet/statesinformer/mockstatesinformer" "github.com/koordinator-sh/koordinator/pkg/koordlet/util/testutil" "github.com/koordinator-sh/koordinator/pkg/util" ) +func Test_EvictPodIfNotEvicted(t *testing.T) { + testpod := testutil.MockTestPod(apiext.QoSBE, "test_be_pod") + testnode := testutil.MockTestNode("80", "120G") + + type args struct { + pod *corev1.Pod + node *corev1.Node + reason string + } + type wants struct { + result bool + evictObjectError bool + eventReason string + } + + tests := []struct { + name string + pod *corev1.Pod + node *corev1.Node + args args + wanted wants + }{ + { + name: "evict ok", + pod: testpod, + node: testnode, + args: args{ + pod: testpod, + node: testnode, + reason: resourceexecutor.EvictPodByNodeMemoryUsage, + }, + wanted: wants{ + result: true, + evictObjectError: false, + eventReason: helpers.EvictPodSuccess, + }, + }, + { + name: "evict not found", + node: testnode, + args: args{ + pod: testpod, + node: testnode, + reason: resourceexecutor.EvictPodByNodeMemoryUsage, + }, + wanted: wants{ + result: false, + evictObjectError: true, + eventReason: helpers.EvictPodFail, + }, + }, + // TODO add test for evict failed by forbidden + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // prepare + ctl := gomock.NewController(t) + defer ctl.Finish() + fakeRecorder := &testutil.FakeRecorder{} + client := clientsetfake.NewSimpleClientset() + r := NewEvictor(client, fakeRecorder, policyv1beta1.SchemeGroupVersion.Version) + stop := make(chan struct{}) + err := r.podsEvicted.Run(stop) + assert.NoError(t, err) + defer func() { stop <- struct{}{} }() + + if tt.pod != nil { + _, err = client.CoreV1().Pods(tt.pod.Namespace).Create(context.TODO(), tt.pod, metav1.CreateOptions{}) + assert.NoError(t, err) + } + + got := r.EvictPodIfNotEvicted(tt.args.pod, tt.args.node, tt.args.reason, "") + assert.Equal(t, tt.wanted.result, got) + + getEvictObject, err := client.Tracker().Get(testutil.PodsResource, tt.args.pod.Namespace, tt.args.pod.Name) + if tt.wanted.evictObjectError { + assert.Error(t, err) + assert.Nil(t, getEvictObject) + assert.Equal(t, tt.wanted.eventReason, fakeRecorder.EventReason, "expect evict failed event! but got %s", fakeRecorder.EventReason) + } else { + assert.NoError(t, err) + assert.NotNil(t, getEvictObject) + assert.Equal(t, tt.wanted.eventReason, fakeRecorder.EventReason, "expect evict success event! but got %s", fakeRecorder.EventReason) + } + }) + } +} + func Test_EvictPodsIfNotEvicted(t *testing.T) { // test data pod := testutil.MockTestPod(apiext.QoSBE, "test_be_pod") diff --git a/pkg/koordlet/qosmanager/plugins/cpuevict/cpu_evict.go b/pkg/koordlet/qosmanager/plugins/cpuevict/cpu_evict.go index b124c3e0f..a44790583 100644 --- a/pkg/koordlet/qosmanager/plugins/cpuevict/cpu_evict.go +++ b/pkg/koordlet/qosmanager/plugins/cpuevict/cpu_evict.go @@ -58,6 +58,7 @@ type cpuEvictor struct { metricCache metriccache.MetricCache evictor *framework.Evictor lastEvictTime time.Time + onlyEvictByAPI bool } func New(opt *framework.Options) framework.QOSStrategy { @@ -68,6 +69,7 @@ func New(opt *framework.Options) framework.QOSStrategy { statesInformer: opt.StatesInformer, metricCache: opt.MetricCache, lastEvictTime: time.Now(), + onlyEvictByAPI: opt.Config.OnlyEvictByAPI, } } @@ -295,24 +297,30 @@ func (c *cpuEvictor) killAndEvictBEPodsRelease(node *corev1.Node, bePodInfos []* node.Name, cpuNeedMilliRelease) cpuMilliReleased := int64(0) - var killedPods []*corev1.Pod + hasKillPods := false for _, bePod := range bePodInfos { if cpuMilliReleased >= cpuNeedMilliRelease { break } - podKillMsg := fmt.Sprintf("%s, kill pod: %s", message, util.GetPodKey(bePod.pod)) - helpers.KillContainers(bePod.pod, podKillMsg) - - killedPods = append(killedPods, bePod.pod) - cpuMilliReleased = cpuMilliReleased + bePod.milliRequest - - klog.V(5).Infof("cpuEvict pick pod %s to evict", util.GetPodKey(bePod.pod)) + if c.onlyEvictByAPI { + if c.evictor.EvictPodIfNotEvicted(bePod.pod, node, resourceexecutor.EvictPodByBECPUSatisfaction, message) { + cpuMilliReleased = cpuMilliReleased + bePod.milliRequest + klog.V(5).Infof("cpuEvict pick pod %s to evict", util.GetPodKey(bePod.pod)) + hasKillPods = true + } else { + klog.V(5).Infof("cpuEvict pick pod %s to evict, failed", util.GetPodKey(bePod.pod)) + } + } else { + podKillMsg := fmt.Sprintf("%s, kill pod: %s", message, util.GetPodKey(bePod.pod)) + helpers.KillContainers(bePod.pod, podKillMsg) + cpuMilliReleased = cpuMilliReleased + bePod.milliRequest + klog.V(5).Infof("cpuEvict pick pod %s to evict", util.GetPodKey(bePod.pod)) + hasKillPods = true + } } - c.evictor.EvictPodsIfNotEvicted(killedPods, node, resourceexecutor.EvictPodByBECPUSatisfaction, message) - - if len(killedPods) > 0 { + if hasKillPods { c.lastEvictTime = time.Now() } klog.V(5).Infof("killAndEvictBEPodsRelease finished! cpuNeedMilliRelease(%d) cpuMilliReleased(%d)", diff --git a/pkg/koordlet/qosmanager/plugins/cpuevict/cpu_evict_test.go b/pkg/koordlet/qosmanager/plugins/cpuevict/cpu_evict_test.go index da3013683..b9ff70bc4 100644 --- a/pkg/koordlet/qosmanager/plugins/cpuevict/cpu_evict_test.go +++ b/pkg/koordlet/qosmanager/plugins/cpuevict/cpu_evict_test.go @@ -576,12 +576,71 @@ func Test_killAndEvictBEPodsRelease(t *testing.T) { // evict subresource will not be creat or update in client go testing, check evict object // https://github.com/kubernetes/client-go/blob/v0.28.7/testing/fixture.go#L117 - assert.True(t, cpuEvictor.evictor.IsPodEvicted(podEvictInfosSorted[0].pod)) - assert.True(t, cpuEvictor.evictor.IsPodEvicted(podEvictInfosSorted[1].pod)) + + // evict by API is false, so podsEvicted cache will not record killed pod + assert.False(t, cpuEvictor.evictor.IsPodEvicted(podEvictInfosSorted[0].pod)) + assert.False(t, cpuEvictor.evictor.IsPodEvicted(podEvictInfosSorted[1].pod)) assert.False(t, cpuEvictor.evictor.IsPodEvicted(podEvictInfosSorted[2].pod)) +} - assert.True(t, cpuEvictor.lastEvictTime.After(time.Now().Add(-5*time.Second)), "checkLastTime") +func Test_killAndEvictBEPodsReleaseWithEvictionAPIOnly(t *testing.T) { + podEvictInfosSorted := []*podEvictCPUInfo{ + { + pod: mockBEPodForCPUEvict("pod_be_3_priority10", 16*1000, 10), + milliRequest: 16 * 1000, + }, + { + pod: mockBEPodForCPUEvict("pod_be_2_priority100", 16*1000, 100), + milliRequest: 16 * 1000, + }, + { + pod: mockBEPodForCPUEvict("pod_be_1_priority100", 16*1000, 100), + milliRequest: 16 * 1000, + }, + } + + ctl := gomock.NewController(t) + defer ctl.Finish() + + fakeRecorder := &testutil.FakeRecorder{} + client := clientsetfake.NewSimpleClientset() + + stop := make(chan struct{}) + evictor := framework.NewEvictor(client, fakeRecorder, policyv1beta1.SchemeGroupVersion.Version) + evictor.Start(stop) + defer func() { stop <- struct{}{} }() + node := testutil.MockTestNode("100", "500G") + runtime.DockerHandler = handler.NewFakeRuntimeHandler() + // create pod + var containers []*critesting.FakeContainer + for _, podInfo := range podEvictInfosSorted { + pod := podInfo.pod + _, _ = client.CoreV1().Pods(pod.Namespace).Create(context.TODO(), pod, metav1.CreateOptions{}) + for _, containerStatus := range pod.Status.ContainerStatuses { + _, containerId, _ := util.ParseContainerId(containerStatus.ContainerID) + fakeContainer := &critesting.FakeContainer{ + SandboxID: string(pod.UID), + ContainerStatus: runtimeapi.ContainerStatus{Id: containerId}, + } + containers = append(containers, fakeContainer) + } + } + runtime.DockerHandler.(*handler.FakeRuntimeHandler).SetFakeContainers(containers) + + cpuEvictor := &cpuEvictor{ + evictor: evictor, + onlyEvictByAPI: true, + lastEvictTime: time.Now().Add(-5 * time.Minute), + } + + cpuEvictor.killAndEvictBEPodsRelease(node, podEvictInfosSorted, 18*1000) + + // evict subresource will not be creat or update in client go testing, check evict object + // https://github.com/kubernetes/client-go/blob/v0.28.7/testing/fixture.go#L117 + assert.True(t, cpuEvictor.evictor.IsPodEvicted(podEvictInfosSorted[0].pod)) + assert.True(t, cpuEvictor.evictor.IsPodEvicted(podEvictInfosSorted[1].pod)) + assert.False(t, cpuEvictor.evictor.IsPodEvicted(podEvictInfosSorted[2].pod)) } func Test_isSatisfactionConfigValid(t *testing.T) { diff --git a/pkg/koordlet/qosmanager/plugins/memoryevict/memory_evict.go b/pkg/koordlet/qosmanager/plugins/memoryevict/memory_evict.go index 72616337c..a3bf03ef2 100644 --- a/pkg/koordlet/qosmanager/plugins/memoryevict/memory_evict.go +++ b/pkg/koordlet/qosmanager/plugins/memoryevict/memory_evict.go @@ -32,6 +32,7 @@ import ( "github.com/koordinator-sh/koordinator/pkg/koordlet/qosmanager/helpers" "github.com/koordinator-sh/koordinator/pkg/koordlet/resourceexecutor" "github.com/koordinator-sh/koordinator/pkg/koordlet/statesinformer" + "github.com/koordinator-sh/koordinator/pkg/util" ) const ( @@ -50,6 +51,7 @@ type memoryEvictor struct { metricCache metriccache.MetricCache evictor *framework.Evictor lastEvictTime time.Time + onlyEvictByAPI bool } type podInfo struct { @@ -64,6 +66,7 @@ func New(opt *framework.Options) framework.QOSStrategy { metricCollectInterval: opt.MetricAdvisorConfig.CollectResUsedInterval, statesInformer: opt.StatesInformer, metricCache: opt.MetricCache, + onlyEvictByAPI: opt.Config.OnlyEvictByAPI, } } @@ -164,24 +167,36 @@ func (m *memoryEvictor) killAndEvictBEPods(node *corev1.Node, podMetrics map[str bePodInfos := m.getSortedBEPodInfos(podMetrics) message := fmt.Sprintf("killAndEvictBEPods for node, need to release memory: %v", memoryNeedRelease) memoryReleased := int64(0) - - var killedPods []*corev1.Pod + hasKillPods := false for _, bePod := range bePodInfos { if memoryReleased >= memoryNeedRelease { break } - killMsg := fmt.Sprintf("%v, kill pod: %v", message, bePod.pod.Name) - helpers.KillContainers(bePod.pod, killMsg) - killedPods = append(killedPods, bePod.pod) - if bePod.memUsed != 0 { - memoryReleased += int64(bePod.memUsed) + if m.onlyEvictByAPI { + if m.evictor.EvictPodIfNotEvicted(bePod.pod, node, resourceexecutor.EvictPodByNodeMemoryUsage, message) { + hasKillPods = true + if bePod.memUsed != 0 { + memoryReleased += int64(bePod.memUsed) + } + klog.V(5).Infof("memoryEvict pick pod %s to evict", util.GetPodKey(bePod.pod)) + } else { + klog.V(5).Infof("memoryEvict pick pod %s to evict", util.GetPodKey(bePod.pod)) + } + } else { + killMsg := fmt.Sprintf("%v, kill pod: %v", message, bePod.pod.Name) + helpers.KillContainers(bePod.pod, killMsg) + hasKillPods = true + if bePod.memUsed != 0 { + memoryReleased += int64(bePod.memUsed) + } + klog.V(5).Infof("memoryEvict pick pod %s to evict", util.GetPodKey(bePod.pod)) } } + if hasKillPods { + m.lastEvictTime = time.Now() + } - m.evictor.EvictPodsIfNotEvicted(killedPods, node, resourceexecutor.EvictPodByNodeMemoryUsage, message) - - m.lastEvictTime = time.Now() klog.Infof("killAndEvictBEPods completed, memoryNeedRelease(%v) memoryReleased(%v)", memoryNeedRelease, memoryReleased) } diff --git a/pkg/koordlet/qosmanager/plugins/memoryevict/memory_evict_test.go b/pkg/koordlet/qosmanager/plugins/memoryevict/memory_evict_test.go index 623d490b7..3ed8e22b5 100644 --- a/pkg/koordlet/qosmanager/plugins/memoryevict/memory_evict_test.go +++ b/pkg/koordlet/qosmanager/plugins/memoryevict/memory_evict_test.go @@ -408,6 +408,7 @@ func Test_memoryEvict(t *testing.T) { memoryEvictor := m.(*memoryEvictor) memoryEvictor.Setup(&framework.Context{Evictor: evictor}) memoryEvictor.lastEvictTime = time.Now().Add(-30 * time.Second) + memoryEvictor.onlyEvictByAPI = true memoryEvictor.memoryEvict() // evict subresource will not be creat or update in client go testing, check evict object