From f84c07e8f48ddc45bfeba66cdbe80a3f672e12f6 Mon Sep 17 00:00:00 2001 From: caden Date: Fri, 19 Nov 2021 14:29:47 +0800 Subject: [PATCH] restore NumPDBViolations info of nodes, when HTTPExtender ProcessPreemption. This info will be used in subsequent filtering steps - pick OneNodeForPreemption. add unit tests for HTTPExtender.ProcessPreemption - make sure the NumPDBViolations info of node is return as it is. --- pkg/scheduler/extender.go | 18 +++-- pkg/scheduler/extender_test.go | 137 ++++++++++++++++++++++++++++++++- 2 files changed, 146 insertions(+), 9 deletions(-) diff --git a/pkg/scheduler/extender.go b/pkg/scheduler/extender.go index a685299c618..e29026ad155 100644 --- a/pkg/scheduler/extender.go +++ b/pkg/scheduler/extender.go @@ -178,7 +178,7 @@ func (h *HTTPExtender) ProcessPreemption( if h.nodeCacheCapable { // If extender has cached node info, pass NodeNameToMetaVictims in args. - nodeNameToMetaVictims := convertToNodeNameToMetaVictims(nodeNameToVictims) + nodeNameToMetaVictims := convertToMetaVictims(nodeNameToVictims) args = &extenderv1.ExtenderPreemptionArgs{ Pod: pod, NodeNameToMetaVictims: nodeNameToMetaVictims, @@ -196,7 +196,7 @@ func (h *HTTPExtender) ProcessPreemption( // Extender will always return NodeNameToMetaVictims. // So let's convert it to NodeNameToVictims by using . - newNodeNameToVictims, err := h.convertToNodeNameToVictims(result.NodeNameToMetaVictims, nodeInfos) + newNodeNameToVictims, err := h.convertToVictims(result.NodeNameToMetaVictims, nodeInfos) if err != nil { return nil, err } @@ -204,9 +204,9 @@ func (h *HTTPExtender) ProcessPreemption( return newNodeNameToVictims, nil } -// convertToNodeNameToVictims converts "nodeNameToMetaVictims" from object identifiers, +// convertToVictims converts "nodeNameToMetaVictims" from object identifiers, // such as UIDs and names, to object pointers. -func (h *HTTPExtender) convertToNodeNameToVictims( +func (h *HTTPExtender) convertToVictims( nodeNameToMetaVictims map[string]*extenderv1.MetaVictims, nodeInfos framework.NodeInfoLister, ) (map[string]*extenderv1.Victims, error) { @@ -217,7 +217,8 @@ func (h *HTTPExtender) convertToNodeNameToVictims( return nil, err } victims := &extenderv1.Victims{ - Pods: []*v1.Pod{}, + Pods: []*v1.Pod{}, + NumPDBViolations: metaVictims.NumPDBViolations, } for _, metaPod := range metaVictims.Pods { pod, err := h.convertPodUIDToPod(metaPod, nodeInfo) @@ -247,14 +248,15 @@ func (h *HTTPExtender) convertPodUIDToPod( h.extenderURL, metaPod, nodeInfo.Node().Name) } -// convertToNodeNameToMetaVictims converts from struct type to meta types. -func convertToNodeNameToMetaVictims( +// convertToMetaVictims converts from struct type to meta types. +func convertToMetaVictims( nodeNameToVictims map[string]*extenderv1.Victims, ) map[string]*extenderv1.MetaVictims { nodeNameToMetaVictims := map[string]*extenderv1.MetaVictims{} for node, victims := range nodeNameToVictims { metaVictims := &extenderv1.MetaVictims{ - Pods: []*extenderv1.MetaPod{}, + Pods: []*extenderv1.MetaPod{}, + NumPDBViolations: victims.NumPDBViolations, } for _, pod := range victims.Pods { metaPod := &extenderv1.MetaPod{ diff --git a/pkg/scheduler/extender_test.go b/pkg/scheduler/extender_test.go index 74494ae7c0b..83e4fa29a57 100644 --- a/pkg/scheduler/extender_test.go +++ b/pkg/scheduler/extender_test.go @@ -22,15 +22,17 @@ import ( "testing" "time" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" clientsetfake "k8s.io/client-go/kubernetes/fake" + extenderv1 "k8s.io/kube-scheduler/extender/v1" schedulerapi "k8s.io/kubernetes/pkg/scheduler/apis/config" "k8s.io/kubernetes/pkg/scheduler/framework" + "k8s.io/kubernetes/pkg/scheduler/framework/fake" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultbinder" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/queuesort" "k8s.io/kubernetes/pkg/scheduler/framework/runtime" @@ -388,3 +390,136 @@ func TestIsInterested(t *testing.T) { }) } } + +func TestConvertToMetaVictims(t *testing.T) { + tests := []struct { + name string + nodeNameToVictims map[string]*extenderv1.Victims + want map[string]*extenderv1.MetaVictims + }{ + { + name: "test NumPDBViolations is transferred from nodeNameToVictims to nodeNameToMetaVictims", + nodeNameToVictims: map[string]*extenderv1.Victims{ + "node1": { + Pods: []*v1.Pod{ + {ObjectMeta: metav1.ObjectMeta{Name: "pod1", UID: "uid1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod3", UID: "uid3"}}, + }, + NumPDBViolations: 1, + }, + "node2": { + Pods: []*v1.Pod{ + {ObjectMeta: metav1.ObjectMeta{Name: "pod2", UID: "uid2"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod4", UID: "uid4"}}, + }, + NumPDBViolations: 2, + }, + }, + want: map[string]*extenderv1.MetaVictims{ + "node1": { + Pods: []*extenderv1.MetaPod{ + {UID: "uid1"}, + {UID: "uid3"}, + }, + NumPDBViolations: 1, + }, + "node2": { + Pods: []*extenderv1.MetaPod{ + {UID: "uid2"}, + {UID: "uid4"}, + }, + NumPDBViolations: 2, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := convertToMetaVictims(tt.nodeNameToVictims); !reflect.DeepEqual(got, tt.want) { + t.Errorf("convertToMetaVictims() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestConvertToVictims(t *testing.T) { + tests := []struct { + name string + httpExtender *HTTPExtender + nodeNameToMetaVictims map[string]*extenderv1.MetaVictims + nodeNames []string + podsInNodeList []*v1.Pod + nodeInfos framework.NodeInfoLister + want map[string]*extenderv1.Victims + wantErr bool + }{ + { + name: "test NumPDBViolations is transferred from NodeNameToMetaVictims to newNodeNameToVictims", + httpExtender: &HTTPExtender{}, + nodeNameToMetaVictims: map[string]*extenderv1.MetaVictims{ + "node1": { + Pods: []*extenderv1.MetaPod{ + {UID: "uid1"}, + {UID: "uid3"}, + }, + NumPDBViolations: 1, + }, + "node2": { + Pods: []*extenderv1.MetaPod{ + {UID: "uid2"}, + {UID: "uid4"}, + }, + NumPDBViolations: 2, + }, + }, + nodeNames: []string{"node1", "node2"}, + podsInNodeList: []*v1.Pod{ + {ObjectMeta: metav1.ObjectMeta{Name: "pod1", UID: "uid1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod2", UID: "uid2"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod3", UID: "uid3"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod4", UID: "uid4"}}, + }, + nodeInfos: nil, + want: map[string]*extenderv1.Victims{ + "node1": { + Pods: []*v1.Pod{ + {ObjectMeta: metav1.ObjectMeta{Name: "pod1", UID: "uid1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod3", UID: "uid3"}}, + }, + NumPDBViolations: 1, + }, + "node2": { + Pods: []*v1.Pod{ + {ObjectMeta: metav1.ObjectMeta{Name: "pod2", UID: "uid2"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod4", UID: "uid4"}}, + }, + NumPDBViolations: 2, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // nodeInfos instantiations + nodeInfoList := make([]*framework.NodeInfo, 0, len(tt.nodeNames)) + for i, nm := range tt.nodeNames { + nodeInfo := framework.NewNodeInfo() + node := createNode(nm) + nodeInfo.SetNode(node) + nodeInfo.AddPod(tt.podsInNodeList[i]) + nodeInfo.AddPod(tt.podsInNodeList[i+2]) + nodeInfoList = append(nodeInfoList, nodeInfo) + } + tt.nodeInfos = fake.NodeInfoLister(nodeInfoList) + + got, err := tt.httpExtender.convertToVictims(tt.nodeNameToMetaVictims, tt.nodeInfos) + if (err != nil) != tt.wantErr { + t.Errorf("convertToVictims() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("convertToVictims() got = %v, want %v", got, tt.want) + } + }) + } +}