Fix return value of PriorityQueue.Add.

This function was returning a non-nil error for the common, non-failure
case. The fix is to properly scope local error values and add early
returns.

pkg/scheduler/internal/queue/scheduling_queue.go

@@ -317,23 +317,23 @@ func (p *PriorityQueue) updateNominatedPod(oldPod, newPod *v1.Pod) {
 func (p *PriorityQueue) Add(pod *v1.Pod) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
-	err := p.activeQ.Add(pod)
+	if err := p.activeQ.Add(pod); err != nil {
-	if err != nil {
 		klog.Errorf("Error adding pod %v/%v to the scheduling queue: %v", pod.Namespace, pod.Name, err)
-	} else {
+		return err
+	}
 	if p.unschedulableQ.get(pod) != nil {
 		klog.Errorf("Error: pod %v/%v is already in the unschedulable queue.", pod.Namespace, pod.Name)
 		p.deleteNominatedPodIfExists(pod)
 		p.unschedulableQ.delete(pod)
 	}
 	// Delete pod from backoffQ if it is backing off
-		if err = p.podBackoffQ.Delete(pod); err == nil {
+	if err := p.podBackoffQ.Delete(pod); err == nil {
 		klog.Errorf("Error: pod %v/%v is already in the podBackoff queue.", pod.Namespace, pod.Name)
 	}
 	p.addNominatedPodIfNeeded(pod)
 	p.cond.Broadcast()
-	}
+
-	return err
+	return nil
 }
 
 // AddIfNotPresent adds a pod to the active queue if it is not present in any of

pkg/scheduler/internal/queue/scheduling_queue_test.go

@@ -96,9 +96,15 @@ var highPriorityPod, highPriNominatedPod, medPriorityPod, unschedulablePod = v1.
 
 func TestPriorityQueue_Add(t *testing.T) {
 	q := NewPriorityQueue(nil)
-	q.Add(&medPriorityPod)
+	if err := q.Add(&medPriorityPod); err != nil {
-	q.Add(&unschedulablePod)
+		t.Errorf("add failed: %v", err)
-	q.Add(&highPriorityPod)
+	}
+	if err := q.Add(&unschedulablePod); err != nil {
+		t.Errorf("add failed: %v", err)
+	}
+	if err := q.Add(&highPriorityPod); err != nil {
+		t.Errorf("add failed: %v", err)
+	}
 	expectedNominatedPods := map[string][]*v1.Pod{
 		"node1": {&medPriorityPod, &unschedulablePod},
 	}
@@ -228,7 +234,9 @@ func TestPriorityQueue_Delete(t *testing.T) {
 	q := NewPriorityQueue(nil)
 	q.Update(&highPriorityPod, &highPriNominatedPod)
 	q.Add(&unschedulablePod)
-	q.Delete(&highPriNominatedPod)
+	if err := q.Delete(&highPriNominatedPod); err != nil {
+		t.Errorf("delete failed: %v", err)
+	}
 	if _, exists, _ := q.activeQ.Get(&unschedulablePod); !exists {
 		t.Errorf("Expected %v to be in activeQ.", unschedulablePod.Name)
 	}
@@ -238,7 +246,9 @@ func TestPriorityQueue_Delete(t *testing.T) {
 	if len(q.nominatedPods) != 1 {
 		t.Errorf("Expected nomindatePods to have only 'unschedulablePod': %v", q.nominatedPods)
 	}
-	q.Delete(&unschedulablePod)
+	if err := q.Delete(&unschedulablePod); err != nil {
+		t.Errorf("delete failed: %v", err)
+	}
 	if len(q.nominatedPods) != 0 {
 		t.Errorf("Expected nomindatePods to be empty: %v", q.nominatedPods)
 	}