From 1980b18c4589d62ffc08531dc81fa59eb45695fb Mon Sep 17 00:00:00 2001
From: boenn <13752080613@163.com>
Date: Fri, 9 Jul 2021 11:56:43 +0800
Subject: [PATCH] Solved the test problem and added update comment

---
 pkg/scheduler/internal/queue/scheduling_queue.go      | 11 +++++++----
 pkg/scheduler/internal/queue/scheduling_queue_test.go | 11 ++++++++---
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/pkg/scheduler/internal/queue/scheduling_queue.go b/pkg/scheduler/internal/queue/scheduling_queue.go
index b0bac0a128b..438518cee75 100644
--- a/pkg/scheduler/internal/queue/scheduling_queue.go
+++ b/pkg/scheduler/internal/queue/scheduling_queue.go
@@ -700,14 +700,17 @@ func (npm *nominator) AddNominatedPod(pi *framework.PodInfo, nodeName string) {
 	npm.Unlock()
 }
 
-// NominatedPodsForNode returns pods that are nominated to run on the given node,
+// NominatedPodsForNode returns a copy of pods that are nominated to run on the given node,
 // but they are waiting for other pods to be removed from the node.
 func (npm *nominator) NominatedPodsForNode(nodeName string) []*framework.PodInfo {
 	npm.RLock()
 	defer npm.RUnlock()
-	// TODO: we may need to return a copy of []*Pods to avoid modification
-	// on the caller side.
-	return npm.nominatedPods[nodeName]
+	// Make a copy of the nominated Pods so the caller can mutate safely.
+	pods := make([]*framework.PodInfo, len(npm.nominatedPods[nodeName]))
+	for i := 0; i < len(pods); i++ {
+		pods[i] = npm.nominatedPods[nodeName][i].DeepCopy()
+	}
+	return pods
 }
 
 func (p *PriorityQueue) podsCompareBackoffCompleted(podInfo1, podInfo2 interface{}) bool {
diff --git a/pkg/scheduler/internal/queue/scheduling_queue_test.go b/pkg/scheduler/internal/queue/scheduling_queue_test.go
index 84e4c386e1c..79e1656fa0c 100644
--- a/pkg/scheduler/internal/queue/scheduling_queue_test.go
+++ b/pkg/scheduler/internal/queue/scheduling_queue_test.go
@@ -650,10 +650,15 @@ func TestPriorityQueue_NominatedPodsForNode(t *testing.T) {
 		t.Errorf("Expected: %v after Pop, but got: %v", highPriorityPodInfo.Pod.Name, p.Pod.Name)
 	}
 	expectedList := []*framework.PodInfo{medPriorityPodInfo, unschedulablePodInfo}
-	if !reflect.DeepEqual(expectedList, q.NominatedPodsForNode("node1")) {
-		t.Error("Unexpected list of nominated Pods for node.")
+	podInfos := q.NominatedPodsForNode("node1")
+	if diff := cmp.Diff(expectedList, podInfos); diff != "" {
+		t.Errorf("Unexpected list of nominated Pods for node: (-want, +got):\n%s", diff)
 	}
-	if q.NominatedPodsForNode("node2") != nil {
+	podInfos[0].Pod.Name = "not mpp"
+	if diff := cmp.Diff(podInfos, q.NominatedPodsForNode("node1")); diff == "" {
+		t.Error("Expected list of nominated Pods for node2 is different from podInfos")
+	}
+	if len(q.NominatedPodsForNode("node2")) != 0 {
 		t.Error("Expected list of nominated Pods for node2 to be empty.")
 	}
 }