wilfred-s commented on code in PR #452:
URL: https://github.com/apache/yunikorn-core/pull/452#discussion_r1030999658
##########
pkg/scheduler/partition_test.go:
##########
@@ -361,6 +361,7 @@ func TestRemoveNodeWithPlaceholders(t *testing.T) {
assert.Equal(t, 0, len(allocs), "expected no allocations for the app")
assert.Assert(t, resources.Equals(app.GetPendingResource(), appRes),
"app should have updated pending resources")
// check the interim state of the placeholder involved
+ // check the interim state of the placeholder involved
Review Comment:
merge error? remove duplicated line
##########
pkg/scheduler/objects/node_test.go:
##########
@@ -231,12 +231,14 @@ func TestNodeReservation(t *testing.T) {
if node.IsReserved() && !node.isReservedForApp(appID1) {
t.Errorf("node should have reservations for app-1")
}
+ assert.Equal(t, 1, len(node.getReservations()), "node should have 1
reservations")
Review Comment:
nit: move up to line 225 directly after the error check
##########
pkg/scheduler/objects/application.go:
##########
@@ -886,6 +886,40 @@ func (sa *Application) tryAllocate(headRoom
*resources.Resource, nodeIterator fu
zap.String("required node",
requiredNode))
return nil
}
+
+ // Are there any non daemon set reservations on
specific required node?
+ // Cancel those reservations to run daemon set pods
+ reservations := node.getReservations()
+ if len(reservations) > 0 {
+ unreserve := true
+ for _, res := range node.getReservations() {
+ // skip the node
+ if res.ask.GetRequiredNode() != "" {
+ unreserve = false
+ break
+ }
+ }
+ if unreserve {
+ // un reserve all the apps that were
reserved on the node
+ reservedKeys, releasedAsks :=
node.UnReserveApps()
+ if len(reservedKeys) == 0 ||
len(releasedAsks) == 0 {
Review Comment:
The comments on the `node.UnReserveApps()` method are wide ranging: we do
not lock the node or the app inside this method. It also does a callback into
the application which is unlocked. This can cause data races. We *must* not use
this method.
The implementation should, if we do not skip the node, iterate over the list
of reservations returned and call `app.UnReserve(node, ask)` to make sure we do
the correct locking etc.
Logging is simply handed for each returning call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
