craigcondit commented on code in PR #975:
URL: https://github.com/apache/yunikorn-core/pull/975#discussion_r1779115443
##########
pkg/scheduler/objects/node.go:
##########
@@ -243,6 +243,31 @@ func (sn *Node) GetAllAllocations() []*Allocation {
return arr
}
+// GetYunikornAllocations returns a copy of Yunikorn allocations on this node
+func (sn *Node) GetYunikornAllocations() []*Allocation {
+ sn.RLock()
+ defer sn.RUnlock()
+ return sn.getAllocations(false)
+}
+
+// GetForeignAllocations returns a copy of non-Yunikorn allocations on this
node
+func (sn *Node) GetForeignAllocations() []*Allocation {
+ sn.RLock()
+ defer sn.RUnlock()
+ return sn.getAllocations(true)
+}
+
+func (sn *Node) getAllocations(foreign bool) []*Allocation {
+ arr := make([]*Allocation, 0)
+ for _, v := range sn.allocations {
+ if (v.IsForeign() && foreign) || (!v.IsForeign() && !foreign) {
Review Comment:
This can be simplified to: `if v.IsForeign() == foreign {`
##########
pkg/scheduler/partition.go:
##########
@@ -1286,12 +1291,54 @@ func (pc *PartitionContext) UpdateAllocation(alloc
*objects.Allocation) (request
return false, false, nil
}
+func (pc *PartitionContext) handleForeignAllocation(allocationKey,
applicationID, nodeID string, node *objects.Node, alloc *objects.Allocation)
(requestCreated bool, allocCreated bool, err error) {
+ allocated := alloc.IsAllocated()
+ if !allocated {
+ return false, false, fmt.Errorf("trying to add a foreign
request (non-allocation) %s", allocationKey)
+ }
+ if alloc.GetNodeID() == "" {
+ return false, false, fmt.Errorf("node ID is empty for
allocation %s", allocationKey)
+ }
+ if node == nil {
+ return false, false, fmt.Errorf("failed to find node %s for
allocation %s", nodeID, allocationKey)
+ }
+
+ existingNodeID := pc.getOrSetNodeIDForAlloc(allocationKey, nodeID)
+ if existingNodeID == "" {
+ log.Log(log.SchedPartition).Info("handling new foreign
allocation",
+ zap.String("partitionName", pc.Name),
+ zap.String("nodeID", nodeID),
+ zap.String("allocationKey", allocationKey))
+ node.AddAllocation(alloc)
+ return false, true, nil
+ }
+
+ log.Log(log.SchedPartition).Info("handling foreign allocation update",
+ zap.String("partitionName", pc.Name),
+ zap.String("appID", applicationID),
+ zap.String("allocationKey", allocationKey))
+ // this is a placeholder for eventual resource updates; nothing to do
yet
+ return false, false, nil
+}
+
func (pc *PartitionContext) convertUGI(ugi *si.UserGroupInformation, forced
bool) (security.UserGroup, error) {
pc.RLock()
defer pc.RUnlock()
return pc.userGroupCache.ConvertUGI(ugi, forced)
}
+// getOrSetNodeIDForAlloc returns the nodeID for a given foreign allocation,
or sets is if it's unset
Review Comment:
How is this meant to be used?
##########
pkg/scheduler/partition.go:
##########
@@ -1496,6 +1547,28 @@ func (pc *PartitionContext) removeAllocation(release
*si.AllocationRelease) ([]*
return released, confirmed
}
+func (pc *PartitionContext) removeForeignAllocation(allocID string) {
+ nodeID := pc.foreignAllocs[allocID]
Review Comment:
Do we really need all these warning cases?
##########
pkg/scheduler/objects/allocation.go:
##########
@@ -135,9 +151,19 @@ func NewAllocationFromSI(alloc *si.Allocation) *Allocation
{
allocated: allocated,
nodeID: nodeID,
bindTime: bindTime,
+ foreign: foreign,
+ preemptable: preemptable,
}
}
+// NewAllocationFromSIAllocated creates an Allocation where the "allocated"
flag is always true,
+// regardless whehether the NodeID if empty or not. Used for testing.
+func NewAllocationFromSIAllocated(siAlloc *si.Allocation) *Allocation {
Review Comment:
I understand the temptation to add this, but I'd rather we didn't.
--
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]
