[GitHub] [yunikorn-core] wilfred-s commented on a diff in pull request #470: [YUNIKORN-1398] Remove non daemonset reservation from node before allocating daemonset pod

[GitBox <gi...@apache.org>]

wilfred-s commented on code in PR #470:
URL: https://github.com/apache/yunikorn-core/pull/470#discussion_r1042963395


##########
pkg/scheduler/objects/application.go:
##########
@@ -920,6 +926,49 @@ func (sa *Application) tryAllocate(headRoom *resources.Resource, nodeIterator fu
 	return nil
 }
 
+func (sa *Application) removeReservations(reservations map[string]*reservation) {

Review Comment:
   We need to return a value for the outcome of our cancel action: if another reservation exists with a required node you leave that reservation intact. That fact needs to be logged.
   Leaving the reservation will directly cause a nil to be returned by tryNode() that follows as the node is reserved but not for that ask. We then try to reserve the node for this app/ask. Currently we have a limit of 1 reservation per node (node.go @ line 436) so that will fail...
   If we leave a reservation we should skip or we need to check if multiple reservations work.



##########
pkg/scheduler/objects/application.go:
##########
@@ -920,6 +926,49 @@ func (sa *Application) tryAllocate(headRoom *resources.Resource, nodeIterator fu
 	return nil
 }
 
+func (sa *Application) removeReservations(reservations map[string]*reservation) {

Review Comment:
   I also think `cancelNodeReservations()` is a better name for this



##########
pkg/scheduler/objects/node.go:
##########
@@ -117,6 +117,16 @@ func (sn *Node) GetAttribute(key string) string {
 	return sn.attributes[key]
 }
 
+func (sn *Node) getReservations() map[string]*reservation {

Review Comment:
   We already have a GetReservations in the node from YUNIKORN-1448. We should re-use that one.



-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org