You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by cc...@apache.org on 2024/08/07 16:10:42 UTC
(yunikorn-core) branch master updated: [YUNIKORN-2788] Ensure Node AddAllocation() is error checked (#932)
This is an automated email from the ASF dual-hosted git repository.
ccondit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-core.git
The following commit(s) were added to refs/heads/master by this push:
new e08a258a [YUNIKORN-2788] Ensure Node AddAllocation() is error checked (#932)
e08a258a is described below
commit e08a258a8a48aadc731af91dd64be93202aae051
Author: Craig Condit <cc...@apache.org>
AuthorDate: Wed Aug 7 11:10:30 2024 -0500
[YUNIKORN-2788] Ensure Node AddAllocation() is error checked (#932)
Split Node.AddAllocation() into two functions, one which forces the
allocation and a new Node.TryAddAllocation() function which behaves as
before. Update callers to use the appropriate function depending on
context.
Closes: #932
---
pkg/scheduler/objects/application.go | 4 +-
pkg/scheduler/objects/node.go | 30 +++++--
pkg/scheduler/objects/node_collection_test.go | 6 +-
pkg/scheduler/objects/node_test.go | 39 ++++++++-
pkg/scheduler/objects/preemption_test.go | 93 +++++++++++-----------
.../objects/required_node_preemptor_test.go | 67 +++++++++++-----
pkg/webservice/handlers_test.go | 6 +-
7 files changed, 162 insertions(+), 83 deletions(-)
diff --git a/pkg/scheduler/objects/application.go b/pkg/scheduler/objects/application.go
index 563db8f4..504b6b0d 100644
--- a/pkg/scheduler/objects/application.go
+++ b/pkg/scheduler/objects/application.go
@@ -1204,7 +1204,7 @@ func (sa *Application) tryPlaceholderAllocate(nodeIterator func() NodeIterator,
}
// update just the node to make sure we keep its spot
// no queue update as we're releasing the placeholder and are just temp over the size
- if !node.AddAllocation(reqFit) {
+ if !node.TryAddAllocation(reqFit) {
log.Log(log.SchedApplication).Debug("Node update failed unexpectedly",
zap.String("applicationID", sa.ApplicationID),
zap.Stringer("ask", reqFit),
@@ -1519,7 +1519,7 @@ func (sa *Application) tryNode(node *Node, ask *Allocation) (*AllocationResult,
}
// everything OK really allocate
- if node.AddAllocation(ask) {
+ if node.TryAddAllocation(ask) {
if err := sa.queue.IncAllocatedResource(ask.GetAllocatedResource(), false); err != nil {
log.Log(log.SchedApplication).DPanic("queue update failed unexpectedly",
zap.Error(err))
diff --git a/pkg/scheduler/objects/node.go b/pkg/scheduler/objects/node.go
index 196e6995..3ef64a1e 100644
--- a/pkg/scheduler/objects/node.go
+++ b/pkg/scheduler/objects/node.go
@@ -312,26 +312,46 @@ func (sn *Node) RemoveAllocation(allocationKey string) *Allocation {
return nil
}
+// TryAddAllocation attempts to add the allocation to the node. Used resources will increase available will decrease.
+// A nil Allocation makes no changes. Preempted resources must have been released already.
+// Do a sanity check to make sure it still fits in the node and nothing has changed
+func (sn *Node) TryAddAllocation(alloc *Allocation) bool {
+ return sn.addAllocationInternal(alloc, false)
+}
+
// AddAllocation adds the allocation to the node. Used resources will increase available will decrease.
// A nil Allocation makes no changes. Preempted resources must have been released already.
// Do a sanity check to make sure it still fits in the node and nothing has changed
-func (sn *Node) AddAllocation(alloc *Allocation) bool {
+func (sn *Node) AddAllocation(alloc *Allocation) {
+ _ = sn.addAllocationInternal(alloc, true)
+}
+
+func (sn *Node) addAllocationInternal(alloc *Allocation, force bool) bool {
if alloc == nil {
return false
}
- defer sn.notifyListeners()
+ result := false
+ defer func() {
+ // check result to ensure we don't notify listeners unnecessarily
+ if result {
+ sn.notifyListeners()
+ }
+ }()
+
sn.Lock()
defer sn.Unlock()
// check if this still fits: it might have changed since pre-check
res := alloc.GetAllocatedResource()
- if sn.availableResource.FitIn(res) {
+ if force || sn.availableResource.FitIn(res) {
sn.allocations[alloc.GetAllocationKey()] = alloc
sn.allocatedResource.AddTo(res)
sn.availableResource.SubFrom(res)
sn.nodeEvents.SendAllocationAddedEvent(sn.NodeID, alloc.allocationKey, res)
- return true
+ result = true
+ return result
}
- return false
+ result = false
+ return result
}
// ReplaceAllocation replaces the placeholder with the real allocation on the node.
diff --git a/pkg/scheduler/objects/node_collection_test.go b/pkg/scheduler/objects/node_collection_test.go
index c3abccd1..30d73e67 100644
--- a/pkg/scheduler/objects/node_collection_test.go
+++ b/pkg/scheduler/objects/node_collection_test.go
@@ -163,7 +163,7 @@ func TestSetNodeSortingPolicy(t *testing.T) {
node := newNode(nodesInfo[id].nodeID, map[string]resources.Quantity{"vcore": resources.Quantity(defaultCapicity[0]), "memory": resources.Quantity(defaultCapicity[1])})
res := resources.NewResourceFromMap(map[string]resources.Quantity{"vcore": resources.Quantity(nodesInfo[id].allocatedVcore), "memory": resources.Quantity(nodesInfo[id].allocatedMem)})
alloc := newAllocation(fmt.Sprintf("test-app-%d", id+1), fmt.Sprintf("test-%d", id+1), res)
- if ok := node.AddAllocation(alloc); !ok {
+ if ok := node.TryAddAllocation(alloc); !ok {
t.Error("Allocation error happen in node.")
}
@@ -298,7 +298,7 @@ func TestGetFullNodeIterator(t *testing.T) {
} else {
res := resources.NewResourceFromMap(map[string]resources.Quantity{"vcore": resources.Quantity(i)})
alloc := newAllocation(fmt.Sprintf("test-app-%d", i), fmt.Sprintf("test-%d", i), res)
- if ok := node.AddAllocation(alloc); !ok {
+ if ok := node.TryAddAllocation(alloc); !ok {
t.Error("Allocation error in node.")
}
}
@@ -347,7 +347,7 @@ func TestGetNodeIterator(t *testing.T) {
} else {
res := resources.NewResourceFromMap(map[string]resources.Quantity{"vcore": resources.Quantity(i)})
alloc := newAllocation(fmt.Sprintf("test-app-%d", i), fmt.Sprintf("test-%d", i), res)
- if ok := node.AddAllocation(alloc); !ok {
+ if ok := node.TryAddAllocation(alloc); !ok {
t.Error("Allocation error happen in node.")
}
}
diff --git a/pkg/scheduler/objects/node_test.go b/pkg/scheduler/objects/node_test.go
index b09a5d67..1f9d6121 100644
--- a/pkg/scheduler/objects/node_test.go
+++ b/pkg/scheduler/objects/node_test.go
@@ -390,16 +390,47 @@ func TestAddAllocation(t *testing.T) {
}
// check nil alloc
- assert.Assert(t, !node.AddAllocation(nil), "nil allocation should not have been added: %v", node)
+ node.AddAllocation(nil)
+ assert.Assert(t, resources.IsZero(node.GetAllocatedResource()), "nil allocation should not have been added: %v", node)
+
+ // check alloc that is over quota
+ large := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 200})
+ alloc := newAllocation(appID1, nodeID1, large)
+ node.AddAllocation(alloc)
+ if !resources.Equals(node.GetAllocatedResource(), large) {
+ t.Errorf("failed to add large allocation expected %v, got %v", large, node.GetAllocatedResource())
+ }
+ node.RemoveAllocation(alloc.GetAllocationKey())
+ assert.Assert(t, resources.IsZero(node.GetAllocatedResource()), "removal of large allocation should return to zero %v, got %v", node, node.GetAllocatedResource())
+
+ // check alloc that is over quota
+ half := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 50, "second": 100})
+ alloc = newAllocation(appID1, nodeID1, half)
+ node.AddAllocation(alloc)
+ if !resources.Equals(node.GetAllocatedResource(), half) {
+ t.Errorf("failed to add half allocation expected %v, got %v", half, node.GetAllocatedResource())
+ }
+ node.RemoveAllocation(alloc.GetAllocationKey())
+ assert.Assert(t, resources.IsZero(node.GetAllocatedResource()), "removal of half allocation should return to zero %v, got %v", node, node.GetAllocatedResource())
+}
+
+func TestTryAddAllocation(t *testing.T) {
+ node := newNode("node-123", map[string]resources.Quantity{"first": 100, "second": 200})
+ if !resources.IsZero(node.GetAllocatedResource()) {
+ t.Fatal("Failed to initialize resource")
+ }
+
+ // check nil alloc
+ assert.Assert(t, !node.TryAddAllocation(nil), "nil allocation should not have been added: %v", node)
// check alloc that does not match
unknown := resources.NewResourceFromMap(map[string]resources.Quantity{"unknown": 1})
alloc := newAllocation(appID1, nodeID1, unknown)
- assert.Assert(t, !node.AddAllocation(alloc), "unmatched resource type in allocation should not have been added: %v", node)
+ assert.Assert(t, !node.TryAddAllocation(alloc), "unmatched resource type in allocation should not have been added: %v", node)
// allocate half of the resources available and check the calculation
half := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 50, "second": 100})
alloc = newAllocation(appID1, nodeID1, half)
- assert.Assert(t, node.AddAllocation(alloc), "add allocation 1 should not have failed")
+ assert.Assert(t, node.TryAddAllocation(alloc), "add allocation 1 should not have failed")
if node.GetAllocation(alloc.GetAllocationKey()) == nil {
t.Fatal("failed to add allocations: allocation not returned")
}
@@ -416,7 +447,7 @@ func TestAddAllocation(t *testing.T) {
// second and check calculation
piece := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 25, "second": 50})
alloc = newAllocation(appID1, nodeID1, piece)
- assert.Assert(t, node.AddAllocation(alloc), "add allocation 2 should not have failed")
+ assert.Assert(t, node.TryAddAllocation(alloc), "add allocation 2 should not have failed")
if node.GetAllocation(alloc.GetAllocationKey()) == nil {
t.Fatal("failed to add allocations: allocation not returned")
}
diff --git a/pkg/scheduler/objects/preemption_test.go b/pkg/scheduler/objects/preemption_test.go
index 702bad36..a7903e5c 100644
--- a/pkg/scheduler/objects/preemption_test.go
+++ b/pkg/scheduler/objects/preemption_test.go
@@ -198,10 +198,9 @@ func TestTryPreemption(t *testing.T) {
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated("node1", ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated("node1", ask2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
- node.AddAllocation(alloc2)
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ1.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ1.IncAllocatedResource(ask2.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.child2")
@@ -262,10 +261,10 @@ func TestTryPreemptionOnNode(t *testing.T) {
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node1.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node1.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID2, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node2.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node2.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ1.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ1.IncAllocatedResource(ask2.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.child2")
@@ -325,10 +324,10 @@ func TestTryPreemptionOnQueue(t *testing.T) {
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node1.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node1.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID2, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node2.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node2.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ1.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ1.IncAllocatedResource(ask2.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.child2")
@@ -387,10 +386,10 @@ func TestTryPreemption_VictimsAvailable_InsufficientResource(t *testing.T) {
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node1.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node1.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID2, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node2.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node2.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ1.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ1.IncAllocatedResource(ask2.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.child2")
@@ -440,10 +439,10 @@ func TestTryPreemption_VictimsOnDifferentNodes_InsufficientResource(t *testing.T
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node1.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node1.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID2, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node2.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node2.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ1.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ1.IncAllocatedResource(ask2.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.child2")
@@ -503,10 +502,10 @@ func TestTryPreemption_VictimsAvailableOnDifferentNodes(t *testing.T) {
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node1.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node1.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID2, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node2.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node2.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ1.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ1.IncAllocatedResource(ask2.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.child2")
@@ -570,10 +569,10 @@ func TestTryPreemption_OnQueue_VictimsOnDifferentNodes(t *testing.T) {
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node1.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node1.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID2, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node2.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node2.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ1.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ1.IncAllocatedResource(ask2.GetAllocatedResource(), false))
@@ -588,7 +587,7 @@ func TestTryPreemption_OnQueue_VictimsOnDifferentNodes(t *testing.T) {
alloc4 := markAllocated(nodeID2, ask4)
app3.AddAllocation(alloc4)
- assert.Check(t, node2.AddAllocation(alloc4), "node alloc2 failed")
+ assert.Check(t, node2.TryAddAllocation(alloc4), "node alloc2 failed")
assert.NilError(t, childQ3.IncAllocatedResource(ask4.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.child2")
@@ -656,10 +655,10 @@ func TestTryPreemption_OnQueue_VictimsAvailable_LowerPriority(t *testing.T) {
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node1.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node1.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node1.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node1.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ1.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ1.IncAllocatedResource(ask2.GetAllocatedResource(), false))
@@ -674,7 +673,7 @@ func TestTryPreemption_OnQueue_VictimsAvailable_LowerPriority(t *testing.T) {
alloc4 := markAllocated(nodeID2, ask4)
app3.AddAllocation(alloc4)
- assert.Check(t, node2.AddAllocation(alloc4), "node alloc2 failed")
+ assert.Check(t, node2.TryAddAllocation(alloc4), "node alloc2 failed")
assert.NilError(t, childQ3.IncAllocatedResource(ask4.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.child2")
@@ -744,10 +743,10 @@ func TestTryPreemption_AskResTypesDifferent_GuaranteedSetOnPreemptorSide(t *test
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ2.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ2.IncAllocatedResource(ask2.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.parent1.child1")
@@ -814,10 +813,10 @@ func TestTryPreemption_OnNode_AskResTypesDifferent_GuaranteedSetOnPreemptorSide(
assert.NilError(t, app1.AddAllocationAsk(ask2))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app1.AddAllocation(alloc2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
assert.NilError(t, childQ2.IncAllocatedResource(ask1.GetAllocatedResource(), false))
assert.NilError(t, childQ2.IncAllocatedResource(ask2.GetAllocatedResource(), false))
app2 := newApplication(appID2, "default", "root.parent.parent1.child1")
@@ -890,7 +889,7 @@ func TestTryPreemption_AskResTypesDifferent_GuaranteedSetOnVictimAndPreemptorSid
assert.NilError(t, app1.AddAllocationAsk(askN))
allocN := markAllocated(nodeID1, askN)
app1.AddAllocation(allocN)
- assert.Check(t, node.AddAllocation(allocN), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(allocN), "node alloc1 failed")
}
ask1 := newAllocationAsk("alloc1", appID1, resources.NewResourceFromMap(map[string]resources.Quantity{"vcores": 1, "mem": 100}))
@@ -904,13 +903,13 @@ func TestTryPreemption_AskResTypesDifferent_GuaranteedSetOnVictimAndPreemptorSid
assert.NilError(t, app1.AddAllocationAsk(ask3))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app2.AddAllocation(alloc2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
alloc3 := markAllocated(nodeID1, ask3)
app3.AddAllocation(alloc3)
- assert.Check(t, node.AddAllocation(alloc3), "node alloc3 failed")
+ assert.Check(t, node.TryAddAllocation(alloc3), "node alloc3 failed")
for i := 5; i < 8; i++ {
assert.NilError(t, childQ2.IncAllocatedResource(resources.NewResourceFromMap(map[string]resources.Quantity{"mem": 100}), false))
@@ -992,7 +991,7 @@ func TestTryPreemption_OnNode_AskResTypesDifferent_GuaranteedSetOnVictimAndPreem
assert.NilError(t, app1.AddAllocationAsk(askN))
allocN := markAllocated(nodeID1, askN)
app1.AddAllocation(allocN)
- assert.Check(t, node.AddAllocation(allocN), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(allocN), "node alloc1 failed")
}
ask1 := newAllocationAsk("alloc1", appID1, resources.NewResourceFromMap(map[string]resources.Quantity{"vcores": 1, "mem": 100}))
@@ -1006,13 +1005,13 @@ func TestTryPreemption_OnNode_AskResTypesDifferent_GuaranteedSetOnVictimAndPreem
assert.NilError(t, app1.AddAllocationAsk(ask3))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app2.AddAllocation(alloc2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
alloc3 := markAllocated(nodeID1, ask3)
app3.AddAllocation(alloc3)
- assert.Check(t, node.AddAllocation(alloc3), "node alloc3 failed")
+ assert.Check(t, node.TryAddAllocation(alloc3), "node alloc3 failed")
for i := 5; i < 8; i++ {
assert.NilError(t, childQ2.IncAllocatedResource(resources.NewResourceFromMap(map[string]resources.Quantity{"mem": 100}), false))
@@ -1093,7 +1092,7 @@ func TestTryPreemption_AskResTypesSame_GuaranteedSetOnPreemptorSide(t *testing.T
assert.NilError(t, app1.AddAllocationAsk(askN))
allocN := markAllocated(nodeID1, askN)
app1.AddAllocation(allocN)
- assert.Check(t, node.AddAllocation(allocN), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(allocN), "node alloc1 failed")
}
ask1 := newAllocationAsk("alloc1", appID1, resources.NewResourceFromMap(map[string]resources.Quantity{"vcores": 1}))
@@ -1107,13 +1106,13 @@ func TestTryPreemption_AskResTypesSame_GuaranteedSetOnPreemptorSide(t *testing.T
assert.NilError(t, app1.AddAllocationAsk(ask3))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app2.AddAllocation(alloc2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
alloc3 := markAllocated(nodeID1, ask3)
app3.AddAllocation(alloc3)
- assert.Check(t, node.AddAllocation(alloc3), "node alloc3 failed")
+ assert.Check(t, node.TryAddAllocation(alloc3), "node alloc3 failed")
for i := 5; i < 8; i++ {
assert.NilError(t, childQ2.IncAllocatedResource(resources.NewResourceFromMap(map[string]resources.Quantity{"gpu": 100}), false))
@@ -1195,7 +1194,7 @@ func TestTryPreemption_OnNode_AskResTypesSame_GuaranteedSetOnPreemptorSide(t *te
assert.NilError(t, app1.AddAllocationAsk(askN))
allocN := markAllocated(nodeID1, askN)
app1.AddAllocation(allocN)
- assert.Check(t, node.AddAllocation(allocN), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(allocN), "node alloc1 failed")
}
ask1 := newAllocationAsk("alloc1", appID1, resources.NewResourceFromMap(map[string]resources.Quantity{"vcores": 1}))
@@ -1209,13 +1208,13 @@ func TestTryPreemption_OnNode_AskResTypesSame_GuaranteedSetOnPreemptorSide(t *te
assert.NilError(t, app1.AddAllocationAsk(ask3))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app2.AddAllocation(alloc2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
alloc3 := markAllocated(nodeID1, ask3)
app3.AddAllocation(alloc3)
- assert.Check(t, node.AddAllocation(alloc3), "node alloc3 failed")
+ assert.Check(t, node.TryAddAllocation(alloc3), "node alloc3 failed")
for i := 5; i < 8; i++ {
assert.NilError(t, childQ2.IncAllocatedResource(resources.NewResourceFromMap(map[string]resources.Quantity{"gpu": 100}), false))
@@ -1296,7 +1295,7 @@ func TestTryPreemption_AskResTypesSame_GuaranteedSetOnVictimAndPreemptorSides(t
assert.NilError(t, app1.AddAllocationAsk(askN))
allocN := markAllocated(nodeID1, askN)
app1.AddAllocation(allocN)
- assert.Check(t, node.AddAllocation(allocN), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(allocN), "node alloc1 failed")
}
ask1 := newAllocationAsk("alloc1", appID1, resources.NewResourceFromMap(map[string]resources.Quantity{"vcores": 1}))
@@ -1310,13 +1309,13 @@ func TestTryPreemption_AskResTypesSame_GuaranteedSetOnVictimAndPreemptorSides(t
assert.NilError(t, app1.AddAllocationAsk(ask3))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app2.AddAllocation(alloc2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
alloc3 := markAllocated(nodeID1, ask3)
app3.AddAllocation(alloc3)
- assert.Check(t, node.AddAllocation(alloc3), "node alloc3 failed")
+ assert.Check(t, node.TryAddAllocation(alloc3), "node alloc3 failed")
for i := 5; i < 8; i++ {
assert.NilError(t, childQ2.IncAllocatedResource(resources.NewResourceFromMap(map[string]resources.Quantity{"gpu": 100}), false))
@@ -1399,7 +1398,7 @@ func TestTryPreemption_OnNode_AskResTypesSame_GuaranteedSetOnVictimAndPreemptorS
assert.NilError(t, app1.AddAllocationAsk(askN))
allocN := markAllocated(nodeID1, askN)
app1.AddAllocation(allocN)
- assert.Check(t, node.AddAllocation(allocN), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(allocN), "node alloc1 failed")
}
ask1 := newAllocationAsk("alloc1", appID1, resources.NewResourceFromMap(map[string]resources.Quantity{"vcores": 1}))
@@ -1413,13 +1412,13 @@ func TestTryPreemption_OnNode_AskResTypesSame_GuaranteedSetOnVictimAndPreemptorS
assert.NilError(t, app1.AddAllocationAsk(ask3))
alloc1 := markAllocated(nodeID1, ask1)
app1.AddAllocation(alloc1)
- assert.Check(t, node.AddAllocation(alloc1), "node alloc1 failed")
+ assert.Check(t, node.TryAddAllocation(alloc1), "node alloc1 failed")
alloc2 := markAllocated(nodeID1, ask2)
app2.AddAllocation(alloc2)
- assert.Check(t, node.AddAllocation(alloc2), "node alloc2 failed")
+ assert.Check(t, node.TryAddAllocation(alloc2), "node alloc2 failed")
alloc3 := markAllocated(nodeID1, ask3)
app3.AddAllocation(alloc3)
- assert.Check(t, node.AddAllocation(alloc3), "node alloc3 failed")
+ assert.Check(t, node.TryAddAllocation(alloc3), "node alloc3 failed")
for i := 5; i < 8; i++ {
assert.NilError(t, childQ2.IncAllocatedResource(resources.NewResourceFromMap(map[string]resources.Quantity{"gpu": 100}), false))
diff --git a/pkg/scheduler/objects/required_node_preemptor_test.go b/pkg/scheduler/objects/required_node_preemptor_test.go
index f0ad10fe..8da78ebf 100644
--- a/pkg/scheduler/objects/required_node_preemptor_test.go
+++ b/pkg/scheduler/objects/required_node_preemptor_test.go
@@ -44,55 +44,75 @@ func createAllocationAsk(allocationKey string, app string, allowPreemption bool,
return ask
}
-func prepareAllocationAsks(node *Node) {
+func prepareAllocationAsks(t *testing.T, node *Node) []*Allocation {
createTime := time.Now()
+ result := make([]*Allocation, 0)
+
// regular pods
ask1 := createAllocationAsk("ask1", "app1", true, false, 10,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10}))
- node.AddAllocation(markAllocated(node.NodeID, ask1))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask1)))
+ result = append(result, ask1)
ask2 := createAllocationAsk("ask2", "app1", true, false, 10,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 8}))
ask2.createTime = createTime
- node.AddAllocation(markAllocated(node.NodeID, ask2))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask2)))
+ result = append(result, ask2)
ask3 := createAllocationAsk("ask3", "app1", true, false, 15,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10}))
- node.AddAllocation(markAllocated(node.NodeID, ask3))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask3)))
+ result = append(result, ask3)
ask4 := createAllocationAsk("ask4", "app1", true, false, 10,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
ask4.createTime = createTime
- node.AddAllocation(markAllocated(node.NodeID, ask4))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask4)))
+ result = append(result, ask4)
ask5 := createAllocationAsk("ask5", "app1", true, false, 5,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
- node.AddAllocation(markAllocated(node.NodeID, ask5))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask5)))
+ result = append(result, ask5)
// opted out pods
ask6 := createAllocationAsk("ask6", "app1", false, false, 10,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10}))
- node.AddAllocation(markAllocated(node.NodeID, ask6))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask6)))
+ result = append(result, ask6)
ask7 := createAllocationAsk("ask7", "app1", false, false, 10,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 8}))
ask7.createTime = createTime
- node.AddAllocation(markAllocated(node.NodeID, ask7))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask7)))
+ result = append(result, ask7)
ask8 := createAllocationAsk("ask8", "app1", false, false, 15,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10}))
- node.AddAllocation(markAllocated(node.NodeID, ask8))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask8)))
+ result = append(result, ask8)
// driver/owner pods
ask9 := createAllocationAsk("ask9", "app1", false, true, 10,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
ask9.createTime = createTime
- node.AddAllocation(markAllocated(node.NodeID, ask9))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask9)))
+ result = append(result, ask9)
ask10 := createAllocationAsk("ask10", "app1", true, true, 5,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
- node.AddAllocation(markAllocated(node.NodeID, ask10))
+ assert.Assert(t, node.TryAddAllocation(markAllocated(node.NodeID, ask10)))
+ result = append(result, ask10)
+
+ return result
+}
+
+func removeAllocationAsks(node *Node, asks []*Allocation) {
+ for _, ask := range asks {
+ node.RemoveAllocation(ask.GetAllocationKey())
+ }
}
// regular pods
@@ -126,7 +146,7 @@ func TestSortAllocations(t *testing.T) {
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
p := NewRequiredNodePreemptor(node, requiredAsk)
- prepareAllocationAsks(node)
+ asks := prepareAllocationAsks(t, node)
p.filterAllocations()
p.sortAllocations()
sortedAsks := p.getAllocations()
@@ -146,6 +166,8 @@ func TestSortAllocations(t *testing.T) {
// assert driver/owner pods
assert.Equal(t, sortedAsks[8].GetAllocationKey(), "ask10")
assert.Equal(t, sortedAsks[9].GetAllocationKey(), "ask9")
+
+ removeAllocationAsks(node, asks)
}
func TestFilterAllocations(t *testing.T) {
@@ -161,38 +183,41 @@ func TestFilterAllocations(t *testing.T) {
requiredAsk := createAllocationAsk("ask12", "app1", true, true, 20,
resources.NewResourceFromMap(map[string]resources.Quantity{"second": 5}))
p := NewRequiredNodePreemptor(node, requiredAsk)
- prepareAllocationAsks(node)
+ asks := prepareAllocationAsks(t, node)
p.filterAllocations()
filteredAllocations := p.getAllocations()
// allocations are not even considered as there is no match. of course, no victims
assert.Equal(t, len(filteredAllocations), 0)
+ removeAllocationAsks(node, asks)
// case 2: allocations are available but priority is higher than ds request priority, hence no allocations considered
requiredAsk1 := createAllocationAsk("ask12", "app1", true, true, 1,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
p1 := NewRequiredNodePreemptor(node, requiredAsk1)
- prepareAllocationAsks(node)
+ asks = prepareAllocationAsks(t, node)
p1.filterAllocations()
filteredAllocations = p.getAllocations()
// allocations are not even considered as there is no match. of course, no victims
assert.Equal(t, len(filteredAllocations), 0)
+ removeAllocationAsks(node, asks)
// case 3: victims are available as there are allocations with lower priority and resource match
requiredAsk2 := createAllocationAsk("ask12", "app1", true, true, 20,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
p2 := NewRequiredNodePreemptor(node, requiredAsk2)
- prepareAllocationAsks(node)
+ asks = prepareAllocationAsks(t, node)
p2.filterAllocations()
filteredAllocations = p2.getAllocations()
assert.Equal(t, len(filteredAllocations), 10)
+ removeAllocationAsks(node, asks)
// case 4: allocation has preempted
requiredAsk3 := createAllocationAsk("ask12", "app1", true, true, 20,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
p3 := NewRequiredNodePreemptor(node, requiredAsk3)
- prepareAllocationAsks(node)
+ asks = prepareAllocationAsks(t, node)
p3.filterAllocations()
p3.sortAllocations()
@@ -203,6 +228,7 @@ func TestFilterAllocations(t *testing.T) {
p3.filterAllocations()
filteredAllocations = p3.getAllocations()
assert.Equal(t, len(filteredAllocations), 19)
+ removeAllocationAsks(node, asks)
}
func TestGetVictims(t *testing.T) {
@@ -219,7 +245,7 @@ func TestGetVictims(t *testing.T) {
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 25}))
p := NewRequiredNodePreemptor(node, requiredAsk)
- prepareAllocationAsks(node)
+ asks := prepareAllocationAsks(t, node)
p.filterAllocations()
p.sortAllocations()
victims := p.GetVictims()
@@ -232,22 +258,24 @@ func TestGetVictims(t *testing.T) {
assert.Equal(t, resources.Equals(victims[2].GetAllocatedResource(), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5})), true)
assert.Equal(t, victims[3].GetAllocationKey(), "ask2")
assert.Equal(t, resources.Equals(victims[3].GetAllocatedResource(), resources.NewResourceFromMap(map[string]resources.Quantity{"first": 8})), true)
+ removeAllocationAsks(node, asks)
// case 2: victims are available and its resources are matching with ds request ask (but with different quantity)
requiredAsk2 := createAllocationAsk("ask13", "app1", true, true, 20,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5}))
p2 := NewRequiredNodePreemptor(node, requiredAsk2)
- prepareAllocationAsks(node)
+ asks = prepareAllocationAsks(t, node)
p2.filterAllocations()
p2.sortAllocations()
victims2 := p2.GetVictims()
assert.Equal(t, len(victims2), 1)
+ removeAllocationAsks(node, asks)
// case 3: allocations are available and its resources are matching partially with ds request ask (because of different resource types), hence no victims
requiredAsk3 := createAllocationAsk("ask13", "app1", true, true, 20,
resources.NewResourceFromMap(map[string]resources.Quantity{"first": 5, "second": 5}))
p3 := NewRequiredNodePreemptor(node, requiredAsk3)
- prepareAllocationAsks(node)
+ asks = prepareAllocationAsks(t, node)
p3.filterAllocations()
filteredAllocations := p3.getAllocations()
@@ -258,4 +286,5 @@ func TestGetVictims(t *testing.T) {
// allocations are available but no exact match for choosing victims
victims3 := p3.GetVictims()
assert.Equal(t, len(victims3), 0)
+ removeAllocationAsks(node, asks)
}
diff --git a/pkg/webservice/handlers_test.go b/pkg/webservice/handlers_test.go
index b7c2bea4..9bf55648 100644
--- a/pkg/webservice/handlers_test.go
+++ b/pkg/webservice/handlers_test.go
@@ -772,7 +772,7 @@ func TestGetNodeUtilisation(t *testing.T) {
resAlloc := resources.NewResourceFromMap(map[string]resources.Quantity{"first": 10})
ask := objects.NewAllocationAsk("alloc-1", "app", resAlloc)
alloc := markAllocated(node1.NodeID, ask)
- assert.Assert(t, node1.AddAllocation(alloc), "unexpected failure adding allocation to node")
+ assert.Assert(t, node1.TryAddAllocation(alloc), "unexpected failure adding allocation to node")
rootQ := partition.GetQueue("root")
err = rootQ.IncAllocatedResource(resAlloc, false)
assert.NilError(t, err, "unexpected error returned setting allocated resource on queue")
@@ -790,7 +790,7 @@ func TestGetNodeUtilisation(t *testing.T) {
resAlloc = resources.NewResourceFromMap(map[string]resources.Quantity{"second": 5})
ask = objects.NewAllocationAsk("alloc-2", "app", resAlloc)
alloc = markAllocated(node2.NodeID, ask)
- assert.Assert(t, node2.AddAllocation(alloc), "unexpected failure adding allocation to node")
+ assert.Assert(t, node2.TryAddAllocation(alloc), "unexpected failure adding allocation to node")
err = rootQ.IncAllocatedResource(resAlloc, false)
assert.NilError(t, err, "unexpected error returned setting allocated resource on queue")
// get nodes utilization
@@ -817,7 +817,7 @@ func addAllocatedResource(t *testing.T, node *objects.Node, allocationKey string
resAlloc := resources.NewResourceFromMap(quantityMap)
ask := objects.NewAllocationAsk(allocationKey, appID, resAlloc)
alloc := markAllocated(node.NodeID, ask)
- assert.Assert(t, node.AddAllocation(alloc), "unexpected failure adding allocation to node")
+ assert.Assert(t, node.TryAddAllocation(alloc), "unexpected failure adding allocation to node")
}
func confirmNodeCount(info []*dao.NodeUtilDAOInfo, count int64) bool {
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org