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

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

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


##########
pkg/scheduler/objects/application.go:
##########
@@ -886,6 +886,11 @@ 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()
+			sa.removeReservations(request, node, reservations)

Review Comment:
   I would only call this if `len(reservations) > 0`
   Need to log the fact that we are going to cancel the reservations and why.
   
   `removeReservations()` does not really cover what is done as we only cancel reservations that do not have a required node set.



##########
pkg/scheduler/objects/application.go:
##########
@@ -919,6 +924,50 @@ func (sa *Application) tryAllocate(headRoom *resources.Resource, nodeIterator fu
 	return nil
 }
 
+func (sa *Application) removeReservations(request *AllocationAsk, node *Node, reservations map[string]*reservation) {

Review Comment:
   * request is only needed for logging see earlier comment about logging, unneeded I think)
   * node is unneeded as the reservation contains the node object already
   * 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 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...



-- 
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