[YUNIKORN-2616] Remove unused bool return from PreemptionPredicates() (…

…#841) Closes: #841 Signed-off-by: Chia-Ping Tsai <[email protected]>
apache · May 17, 2024 · 6f2800f · 6f2800f
1 parent 5f80f49
commit 6f2800f
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 17 deletions.
diff --git a/pkg/cache/context.go b/pkg/cache/context.go
@@ -681,7 +681,7 @@ func (ctx *Context) IsPodFitNodeViaPreemption(name, node string, allocations []s
 			}
 
 			// check predicates for a match
-			if index, _ := ctx.predManager.PreemptionPredicates(pod, targetNode, victims, startIndex); index != -1 {
+			if index := ctx.predManager.PreemptionPredicates(pod, targetNode, victims, startIndex); index != -1 {
 				return index, true
 			}
 		}

diff --git a/pkg/cache/scheduler_callback_test.go b/pkg/cache/scheduler_callback_test.go
@@ -573,8 +573,8 @@ func (m *mockPredicateManager) Predicates(_ *v1.Pod, _ *framework.NodeInfo, _ bo
 	return "", nil
 }
 
-func (m *mockPredicateManager) PreemptionPredicates(_ *v1.Pod, _ *framework.NodeInfo, _ []*v1.Pod, _ int) (index int, ok bool) {
-	return 0, true
+func (m *mockPredicateManager) PreemptionPredicates(_ *v1.Pod, _ *framework.NodeInfo, _ []*v1.Pod, _ int) (index int) {
+	return 0
 }
 
 func initCallbackTest(t *testing.T, podAssigned, placeholder bool) (*AsyncRMCallback, *Context) {

diff --git a/pkg/plugin/predicates/predicate_manager.go b/pkg/plugin/predicates/predicate_manager.go
@@ -43,7 +43,7 @@ import (
 type PredicateManager interface {
 	EventsToRegister(queueingHintFn framework.QueueingHintFn) []framework.ClusterEventWithHint
 	Predicates(pod *v1.Pod, node *framework.NodeInfo, allocate bool) (plugin string, error error)
-	PreemptionPredicates(pod *v1.Pod, node *framework.NodeInfo, victims []*v1.Pod, startIndex int) (index int, ok bool)
+	PreemptionPredicates(pod *v1.Pod, node *framework.NodeInfo, victims []*v1.Pod, startIndex int) (index int)
 }
 
 var _ PredicateManager = &predicateManagerImpl{}
@@ -125,7 +125,7 @@ func (p *predicateManagerImpl) Predicates(pod *v1.Pod, node *framework.NodeInfo,
 	return p.predicatesReserve(pod, node)
 }
 
-func (p *predicateManagerImpl) PreemptionPredicates(pod *v1.Pod, node *framework.NodeInfo, victims []*v1.Pod, startIndex int) (int, bool) {
+func (p *predicateManagerImpl) PreemptionPredicates(pod *v1.Pod, node *framework.NodeInfo, victims []*v1.Pod, startIndex int) int {
 	ctx := context.Background()
 	state := framework.NewCycleState()
 
@@ -138,7 +138,7 @@ func (p *predicateManagerImpl) PreemptionPredicates(pod *v1.Pod, node *framework
 			zap.String("plugin", plugin),
 			zap.String("message", s.Message()))
 
-		return -1, false
+		return -1
 	}
 
 	// clone node so that we can modify it here for predicate checks
@@ -154,15 +154,15 @@ func (p *predicateManagerImpl) PreemptionPredicates(pod *v1.Pod, node *framework
 		p.removePodFromNodeNoFail(preemptingNode, victims[i])
 		status, _ := p.runFilterPlugins(ctx, *p.allocationFilters, state, pod, preemptingNode, skip)
 		if status.IsSuccess() {
-			return i, true
+			return i
 		}
 	}
 
 	// no fit, log and return
 	log.Log(log.ShimPredicates).Debug("Filter checks failed during preemption check, no fit",
 		zap.String("podUID", string(pod.UID)),
 		zap.String("nodeID", node.Node().Name))
-	return -1, false
+	return -1
 }
 
 func (p *predicateManagerImpl) removePodFromNodeNoFail(node *framework.NodeInfo, pod *v1.Pod) {

diff --git a/pkg/plugin/predicates/predicate_manager_test.go b/pkg/plugin/predicates/predicate_manager_test.go
@@ -69,9 +69,8 @@ func TestPreemptionPredicatesEmpty(t *testing.T) {
 	node := framework.NewNodeInfo()
 	node.SetNode(&v1.Node{})
 	victims := make([]*v1.Pod, 0)
-	index, ok := predicateManager.PreemptionPredicates(pod, node, victims, 0)
-	assert.Check(t, !ok, "check should have failed")
-	assert.Equal(t, index, -1, "wrong index")
+	index := predicateManager.PreemptionPredicates(pod, node, victims, 0)
+	assert.Equal(t, index, -1, "should not find any victim index after preemption check")
 }
 
 func TestPreemptionPredicates(t *testing.T) {
@@ -115,18 +114,16 @@ func TestPreemptionPredicates(t *testing.T) {
 	node.AddPod(victims[3])
 
 	// all but 1 existing pod should need removing
-	index, ok := predicateManager.PreemptionPredicates(pod, node, victims, 1)
-	assert.Check(t, ok, "check failed")
-	assert.Equal(t, index, 2, "wrong index")
+	index := predicateManager.PreemptionPredicates(pod, node, victims, 1)
+	assert.Equal(t, index, 2, "wrong victim index")
 
 	// try again, but with too many resources requested
 	pod = newResourcePod(framework.Resource{MilliCPU: 1500, Memory: 15000000})
 	pod.Name = "largepod"
 	pod.UID = "largepod"
 
-	index, ok = predicateManager.PreemptionPredicates(pod, node, victims, 1)
-	assert.Check(t, !ok, "check should have failed")
-	assert.Equal(t, index, -1, "wrong index")
+	index = predicateManager.PreemptionPredicates(pod, node, victims, 1)
+	assert.Equal(t, index, -1, "should not find any victim index after preemption check")
 }
 
 func TestEventsToRegister(t *testing.T) {