[YUNIKORN-3188] Skip ph released allocations in daemon set preemption (#1057)

Closes: #1057

Signed-off-by: mani <manirajv06@gmail.com>
(cherry picked from commit 63eae06)

Author: manirajv06

pkg/scheduler/objects/application.go

@@ -1438,7 +1438,8 @@ func (sa *Application) tryRequiredNodePreemption(reserve *reservation, ask *Allo
 		zap.Int("requiredNode allocations", result.requiredNodeAllocations),
 		zap.Int("allocations already preempted", result.alreadyPreemptedAllocations),
 		zap.Int("higher priority allocations", result.higherPriorityAllocations),
-		zap.Int("allocations with non-matching resources", result.atLeastOneResNotMatched))
+		zap.Int("allocations with non-matching resources", result.atLeastOneResNotMatched),
+		zap.Int("released placeholder allocations", result.releasedPhAllocations))
 	return false
 }
 

pkg/scheduler/objects/required_node_preemptor.go

@@ -34,6 +34,7 @@ type filteringResult struct {
 	atLeastOneResNotMatched     int // number of allocations where there's no single resource type that would match
 	higherPriorityAllocations   int // number of allocations with higher priority
 	alreadyPreemptedAllocations int // number of allocations already preempted
+	releasedPhAllocations       int // number of ph allocations released
 }
 
 func NewRequiredNodePreemptor(node *Node, requiredAsk *Allocation) *PreemptionContext {
@@ -73,6 +74,13 @@ func (p *PreemptionContext) filterAllocations() filteringResult {
 			result.atLeastOneResNotMatched++
 			continue
 		}
+
+		// skip placeholder tasks which are marked released
+		if allocation.IsReleased() {
+			result.releasedPhAllocations++
+			continue
+		}
+
 		p.allocations = append(p.allocations, allocation)
 	}
 

pkg/scheduler/objects/required_node_preemptor_test.go

@@ -42,7 +42,7 @@ func TestFilterAllocations(t *testing.T) {
 	p := NewRequiredNodePreemptor(node, requiredAsk)
 	asks := prepareAllocationAsks(t, node)
 	result := p.filterAllocations()
-	verifyFilterResult(t, 10, 0, 10, 0, 0, result)
+	verifyFilterResult(t, 10, 0, 10, 0, 0, 0, result)
 	filteredAllocations := p.getAllocations()
 
 	// allocations are not even considered as there is no match. of course, no victims
@@ -55,7 +55,7 @@ func TestFilterAllocations(t *testing.T) {
 	p1 := NewRequiredNodePreemptor(node, requiredAsk1)
 	asks = prepareAllocationAsks(t, node)
 	result = p1.filterAllocations()
-	verifyFilterResult(t, 10, 0, 0, 10, 0, result)
+	verifyFilterResult(t, 10, 0, 0, 10, 0, 0, result)
 	filteredAllocations = p.getAllocations()
 
 	// allocations are not even considered as there is no match. of course, no victims
@@ -68,38 +68,44 @@ func TestFilterAllocations(t *testing.T) {
 	p2 := NewRequiredNodePreemptor(node, requiredAsk2)
 	asks = prepareAllocationAsks(t, node)
 	result = p2.filterAllocations()
-	verifyFilterResult(t, 10, 0, 0, 0, 0, result)
+	verifyFilterResult(t, 10, 0, 0, 0, 0, 0, result)
 	filteredAllocations = p2.getAllocations()
 	assert.Equal(t, len(filteredAllocations), 10)
 	removeAllocationAsks(node, asks)
 
 	// case 4: allocation has been preempted
-	requiredAsk3 := createAllocationAsk("ask12", "app1", true, true, 20,
-		resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
-	p3 := NewRequiredNodePreemptor(node, requiredAsk3)
+	p3 := NewRequiredNodePreemptor(node, requiredAsk2)
 	asks = prepareAllocationAsks(t, node)
 	node.GetAllocation("ask5").MarkPreempted() // "ask5" would be the first and only victim without this
 	result = p3.filterAllocations()
 	p3.sortAllocations()
 
-	verifyFilterResult(t, 10, 0, 0, 0, 1, result)
+	verifyFilterResult(t, 10, 0, 0, 0, 1, 0, result)
 	filteredAllocations = p3.getAllocations()
 	assert.Equal(t, len(filteredAllocations), 9) // "ask5" is no longer considered as a victim
 	removeAllocationAsks(node, asks)
 
 	// case 5: existing required node allocation
-	requiredAsk4 := createAllocationAsk("ask12", "app1", true, true, 20,
-		resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
-	p4 := NewRequiredNodePreemptor(node, requiredAsk4)
-	_ = prepareAllocationAsks(t, node)
-	allocReqNode := createAllocation("ask11", "app1", node.NodeID, true, true, 5, true,
-		resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
-	assert.Assert(t, node.TryAddAllocation(allocReqNode))
+	p4 := NewRequiredNodePreemptor(node, requiredAsk2)
+	results := prepareAllocationAsks(t, node)
+	results[8].requiredNode = "node-3"
 
 	result = p4.filterAllocations()
-	verifyFilterResult(t, 11, 1, 0, 0, 0, result)
+	verifyFilterResult(t, 10, 1, 0, 0, 0, 0, result)
 	filteredAllocations = p4.getAllocations()
-	assert.Equal(t, len(filteredAllocations), 10)
+	assert.Equal(t, len(filteredAllocations), 9)
+	removeAllocationAsks(node, asks)
+
+	// case 6: release ph allocation
+	p5 := NewRequiredNodePreemptor(node, requiredAsk2)
+	results = prepareAllocationAsks(t, node)
+	results[9].released = true
+
+	result = p5.filterAllocations()
+	verifyFilterResult(t, 10, 0, 0, 0, 0, 1, result)
+	filteredAllocations = p5.getAllocations()
+	assert.Equal(t, len(filteredAllocations), 9)
+	removeAllocationAsks(node, asks)
 }
 
 func TestGetVictims(t *testing.T) {
@@ -160,11 +166,12 @@ func TestGetVictims(t *testing.T) {
 	removeAllocationAsks(node, asks)
 }
 
-func verifyFilterResult(t *testing.T, totalAllocations, requiredNodeAllocations, resourceNotEnough, higherPriorityAllocations, alreadyPreemptedAllocations int, result filteringResult) {
+func verifyFilterResult(t *testing.T, totalAllocations, requiredNodeAllocations, resourceNotEnough, higherPriorityAllocations, alreadyPreemptedAllocations int, releasedPhAllocations int, result filteringResult) {
 	t.Helper()
 	assert.Equal(t, totalAllocations, result.totalAllocations)
 	assert.Equal(t, requiredNodeAllocations, result.requiredNodeAllocations)
 	assert.Equal(t, resourceNotEnough, result.atLeastOneResNotMatched)
 	assert.Equal(t, higherPriorityAllocations, result.higherPriorityAllocations)
 	assert.Equal(t, alreadyPreemptedAllocations, result.alreadyPreemptedAllocations)
+	assert.Equal(t, releasedPhAllocations, result.releasedPhAllocations)
 }