[YUNIKORN-2677] Rename AllocationResult to AllocationResultType (#894)

Closes: #894

pkg/scheduler/context.go

@@ -141,7 +141,7 @@ func (cc *ClusterContext) schedule() bool {
 		}
 		if alloc != nil {
 			metrics.GetSchedulerMetrics().ObserveSchedulingLatency(schedulingStart)
-			if alloc.GetResult() == objects.Replaced {
+			if alloc.GetResultType() == objects.Replaced {
 				// communicate the removal to the RM
 				cc.notifyRMAllocationReleased(psc.RmID, psc.Name, []*objects.Allocation{alloc.GetRelease()}, si.TerminationType_PLACEHOLDER_REPLACED, "replacing allocationKey: "+alloc.GetAllocationKey())
 			} else {

pkg/scheduler/objects/allocation.go

@@ -32,18 +32,18 @@ import (
 	"github.com/apache/yunikorn-scheduler-interface/lib/go/si"
 )
 
-type AllocationResult int
+type AllocationResultType int
 
 const (
-	None AllocationResult = iota
+	None AllocationResultType = iota
 	Allocated
 	AllocatedReserved
 	Reserved
 	Unreserved
 	Replaced
 )
 
-func (ar AllocationResult) String() string {
+func (ar AllocationResultType) String() string {
 	return [...]string{"None", "Allocated", "AllocatedReserved", "Reserved", "Unreserved", "Replaced"}[ar]
 }
 
@@ -66,7 +66,7 @@ type Allocation struct {
 	placeholderCreateTime time.Time
 	released              bool
 	reservedNodeID        string
-	result                AllocationResult
+	resultType            AllocationResultType
 	release               *Allocation
 	preempted             bool
 	instType              string
@@ -87,21 +87,21 @@ func NewAllocation(nodeID string, ask *AllocationAsk) *Allocation {
 		allocatedResource: ask.GetAllocatedResource().Clone(),
 		taskGroupName:     ask.GetTaskGroup(),
 		placeholder:       ask.IsPlaceholder(),
-		result:            Allocated,
+		resultType:        Allocated,
 	}
 }
 
 func newReservedAllocation(nodeID string, ask *AllocationAsk) *Allocation {
 	alloc := NewAllocation(nodeID, ask)
 	alloc.SetBindTime(time.Time{})
-	alloc.SetResult(Reserved)
+	alloc.SetResultType(Reserved)
 	return alloc
 }
 
 func newUnreservedAllocation(nodeID string, ask *AllocationAsk) *Allocation {
 	alloc := NewAllocation(nodeID, ask)
 	alloc.SetBindTime(time.Time{})
-	alloc.SetResult(Unreserved)
+	alloc.SetResultType(Unreserved)
 	return alloc
 }
 
@@ -179,7 +179,7 @@ func (a *Allocation) String() string {
 	}
 	a.RLock()
 	defer a.RUnlock()
-	return fmt.Sprintf("applicationID=%s, allocationKey=%s, Node=%s, result=%s", a.applicationID, a.allocationKey, a.nodeID, a.result.String())
+	return fmt.Sprintf("applicationID=%s, allocationKey=%s, Node=%s, resultType=%s", a.applicationID, a.allocationKey, a.nodeID, a.resultType.String())
 }
 
 // GetAsk returns the ask associated with this allocation
@@ -310,18 +310,18 @@ func (a *Allocation) GetTagsClone() map[string]string {
 	return CloneAllocationTags(a.tags)
 }
 
-// GetResult gets the result of this allocation
-func (a *Allocation) GetResult() AllocationResult {
+// GetResultType gets the result type of this allocation
+func (a *Allocation) GetResultType() AllocationResultType {
 	a.RLock()
 	defer a.RUnlock()
-	return a.result
+	return a.resultType
 }
 
-// SetResult sets the result of this allocation
-func (a *Allocation) SetResult(result AllocationResult) {
+// SetResultType sets the result type of this allocation
+func (a *Allocation) SetResultType(resultType AllocationResultType) {
 	a.Lock()
 	defer a.Unlock()
-	a.result = result
+	a.resultType = resultType
 }
 
 // GetRelease returns the associated release for this allocation

pkg/scheduler/objects/allocation_test.go

@@ -54,15 +54,15 @@ func TestNewAlloc(t *testing.T) {
 		t.Fatal("NewAllocation create failed while it should not")
 	}
 	assert.Equal(t, alloc.GetAllocationKey(), "ask-1")
-	assert.Equal(t, alloc.GetResult(), Allocated, "New alloc should default to result Allocated")
+	assert.Equal(t, alloc.GetResultType(), Allocated, "New alloc should default to result type Allocated")
 	assert.Assert(t, resources.Equals(alloc.GetAllocatedResource(), res), "Allocated resource not set correctly")
 	assert.Assert(t, !alloc.IsPlaceholder(), "ask should not have been a placeholder")
 	assert.Equal(t, time.Now().Round(time.Second), alloc.GetCreateTime().Round(time.Second))
 	assert.Equal(t, alloc.GetInstanceType(), "", "Default instance type should be empty")
 	alloc.SetInstanceType(instType1)
 	assert.Equal(t, alloc.GetInstanceType(), instType1, "Instance type not set as expected")
 	allocStr := alloc.String()
-	expected := "applicationID=app-1, allocationKey=ask-1, Node=node-1, result=Allocated"
+	expected := "applicationID=app-1, allocationKey=ask-1, Node=node-1, resultType=Allocated"
 	assert.Equal(t, allocStr, expected, "Strings should have been equal")
 	assert.Assert(t, !alloc.IsPlaceholderUsed(), fmt.Sprintf("Alloc should not be placeholder replacement by default: got %t, expected %t", alloc.IsPlaceholderUsed(), false))
 	// check that createTime is properly copied from the ask
@@ -82,10 +82,10 @@ func TestNewReservedAlloc(t *testing.T) {
 	if alloc == nil {
 		t.Fatal("NewReservedAllocation create failed while it should not")
 	}
-	assert.Equal(t, alloc.GetResult(), Reserved, "NewReservedAlloc should have Reserved result")
+	assert.Equal(t, alloc.GetResultType(), Reserved, "NewReservedAlloc should have Reserved result type")
 	assert.Assert(t, resources.Equals(alloc.GetAllocatedResource(), res), "Allocated resource not set correctly")
 	allocStr := alloc.String()
-	expected := "applicationID=app-1, allocationKey=ask-1, Node=node-1, result=Reserved"
+	expected := "applicationID=app-1, allocationKey=ask-1, Node=node-1, resultType=Reserved"
 	assert.Equal(t, allocStr, expected, "Strings should have been equal")
 }
 
@@ -97,10 +97,10 @@ func TestNewUnreservedAlloc(t *testing.T) {
 	if alloc == nil {
 		t.Fatal("NewReservedAllocation create failed while it should not")
 	}
-	assert.Equal(t, alloc.GetResult(), Unreserved, "NewReservedAlloc should have Reserved result")
+	assert.Equal(t, alloc.GetResultType(), Unreserved, "NewReservedAlloc should have Reserved result type")
 	assert.Assert(t, resources.Equals(alloc.GetAllocatedResource(), res), "Allocated resource not set correctly")
 	allocStr := alloc.String()
-	expected := "applicationID=app-1, allocationKey=ask-1, Node=node-1, result=Unreserved"
+	expected := "applicationID=app-1, allocationKey=ask-1, Node=node-1, resultType=Unreserved"
 	assert.Equal(t, allocStr, expected, "Strings should have been equal")
 }
 

pkg/scheduler/objects/application.go

@@ -1021,13 +1021,13 @@ func (sa *Application) tryAllocate(headRoom *resources.Resource, allowPreemption
 						zap.String("appID", sa.ApplicationID),
 						zap.String("nodeID", requiredNode),
 						zap.String("allocationKey", request.GetAllocationKey()))
-					alloc.SetResult(AllocatedReserved)
+					alloc.SetResultType(AllocatedReserved)
 					return alloc
 				}
 				log.Log(log.SchedApplication).Debug("allocation on required node is completed",
 					zap.String("nodeID", node.NodeID),
 					zap.String("allocationKey", request.GetAllocationKey()),
-					zap.Stringer("AllocationResult", alloc.GetResult()))
+					zap.Stringer("resultType", alloc.GetResultType()))
 				return alloc
 			}
 			return newReservedAllocation(node.NodeID, request)
@@ -1164,7 +1164,7 @@ func (sa *Application) tryPlaceholderAllocate(nodeIterator func() NodeIterator,
 				alloc.SetRelease(ph)
 				// placeholder point to the real one in the releases list
 				ph.SetRelease(alloc)
-				alloc.SetResult(Replaced)
+				alloc.SetResultType(Replaced)
 				// mark placeholder as released
 				ph.SetReleased(true)
 				_, err := sa.allocateAsk(request)
@@ -1199,14 +1199,14 @@ func (sa *Application) tryPlaceholderAllocate(nodeIterator func() NodeIterator,
 			if !node.preAllocateConditions(reqFit) {
 				return true
 			}
-			// allocation worked: on a non placeholder node update result and return
+			// allocation worked: on a non placeholder node update resultType and return
 			alloc := NewAllocation(node.NodeID, reqFit)
 			// double link to make it easier to find
 			// alloc (the real one) releases points to the placeholder in the releases list
 			alloc.SetRelease(phFit)
 			// placeholder point to the real one in the releases list
 			phFit.SetRelease(alloc)
-			alloc.SetResult(Replaced)
+			alloc.SetResultType(Replaced)
 			// mark placeholder as released
 			phFit.SetReleased(true)
 			// update just the node to make sure we keep its spot
@@ -1280,9 +1280,9 @@ func (sa *Application) tryReservedAllocate(headRoom *resources.Resource, nodeIte
 		// check allocation possibility
 		alloc := sa.tryNode(reserve.node, ask)
 
-		// allocation worked fix the result and return
+		// allocation worked fix the resultType and return
 		if alloc != nil {
-			alloc.SetResult(AllocatedReserved)
+			alloc.SetResultType(AllocatedReserved)
 			return alloc
 		}
 	}
@@ -1358,7 +1358,7 @@ func (sa *Application) tryRequiredNodePreemption(reserve *reservation, ask *Allo
 }
 
 // Try all the nodes for a reserved request that have not been tried yet.
-// This should never result in a reservation as the ask is already reserved
+// This should never resultType in a reservation as the ask is already reserved
 func (sa *Application) tryNodesNoReserve(ask *AllocationAsk, iterator NodeIterator, reservedNode string) *Allocation {
 	var allocResult *Allocation
 	iterator.ForEachNode(func(node *Node) bool {
@@ -1373,10 +1373,10 @@ func (sa *Application) tryNodesNoReserve(ask *AllocationAsk, iterator NodeIterat
 			return true
 		}
 		alloc := sa.tryNode(node, ask)
-		// allocation worked: update result and return
+		// allocation worked: update resultType and return
 		if alloc != nil {
 			alloc.SetReservedNodeID(reservedNode)
-			alloc.SetResult(AllocatedReserved)
+			alloc.SetResultType(AllocatedReserved)
 			allocResult = alloc
 			return false
 		}
@@ -1387,7 +1387,7 @@ func (sa *Application) tryNodesNoReserve(ask *AllocationAsk, iterator NodeIterat
 	return allocResult
 }
 
-// Try all the nodes for a request. The result is an allocation or reservation of a node.
+// Try all the nodes for a request. The resultType is an allocation or reservation of a node.
 // New allocations can only be reserved after a delay.
 func (sa *Application) tryNodes(ask *AllocationAsk, iterator NodeIterator) *Allocation {
 	var nodeToReserve *Node
@@ -1414,15 +1414,15 @@ func (sa *Application) tryNodes(ask *AllocationAsk, iterator NodeIterator) *Allo
 		// allocation worked so return
 		if alloc != nil {
 			metrics.GetSchedulerMetrics().ObserveTryNodeLatency(tryNodeStart)
-			// check if the node was reserved for this ask: if it is set the result and return
+			// check if the node was reserved for this ask: if it is set the resultType and return
 			// NOTE: this is a safeguard as reserved nodes should never be part of the iterator
 			// but we have no locking
 			if _, ok := sa.reservations[reservationKey(node, nil, ask)]; ok {
 				log.Log(log.SchedApplication).Debug("allocate found reserved ask during non reserved allocate",
 					zap.String("appID", sa.ApplicationID),
 					zap.String("nodeID", node.NodeID),
 					zap.String("allocationKey", allocKey))
-				alloc.SetResult(AllocatedReserved)
+				alloc.SetResultType(AllocatedReserved)
 				allocResult = alloc
 				return false
 			}
@@ -1434,7 +1434,7 @@ func (sa *Application) tryNodes(ask *AllocationAsk, iterator NodeIterator) *Allo
 					zap.String("appID", sa.ApplicationID),
 					zap.String("nodeID", nodeID),
 					zap.String("allocationKey", allocKey))
-				alloc.SetResult(AllocatedReserved)
+				alloc.SetResultType(AllocatedReserved)
 				alloc.SetReservedNodeID(nodeID)
 				allocResult = alloc
 				return false
@@ -1654,7 +1654,7 @@ func (sa *Application) addAllocationInternal(info *Allocation) {
 		// already when the last placeholder was allocated
 		// special case COMPLETING: gang with only one placeholder moves to COMPLETING and causes orphaned
 		// allocations
-		if info.GetResult() != Replaced || !resources.IsZero(sa.allocatedResource) || sa.IsCompleting() {
+		if info.GetResultType() != Replaced || !resources.IsZero(sa.allocatedResource) || sa.IsCompleting() {
 			// progress the state based on where we are, we should never fail in this case
 			// keep track of a failure in log.
 			if err := sa.HandleApplicationEvent(RunApplication); err != nil {
@@ -1737,9 +1737,9 @@ func (sa *Application) ReplaceAllocation(allocationKey string) *Allocation {
 	alloc.SetBindTime(time.Now())
 	sa.addAllocationInternal(alloc)
 	// order is important: clean up the allocation after adding it to the app
-	// we need the original Replaced allocation result.
+	// we need the original Replaced allocation resultType.
 	alloc.ClearRelease()
-	alloc.SetResult(Allocated)
+	alloc.SetResultType(Allocated)
 	if sa.placeholderData != nil {
 		sa.placeholderData[ph.GetTaskGroup()].Replaced++
 	}

pkg/scheduler/objects/application_test.go

@@ -960,15 +960,15 @@ func TestGangAllocChange(t *testing.T) {
 
 	// add a real alloc this should NOT trigger state update
 	alloc = newAllocation(appID1, nodeID1, res)
-	alloc.SetResult(Replaced)
+	alloc.SetResultType(Replaced)
 	app.AddAllocation(alloc)
 	assert.Equal(t, len(app.GetAllAllocations()), 3)
 	assert.Assert(t, app.IsRunning(), "app should still be in running state")
 	assertUserGroupResource(t, getTestUserGroup(), resources.Multiply(res, 3))
 
 	// add a second real alloc this should NOT trigger state update
 	alloc = newAllocation(appID1, nodeID1, res)
-	alloc.SetResult(Replaced)
+	alloc.SetResultType(Replaced)
 	app.AddAllocation(alloc)
 	assert.Equal(t, len(app.GetAllAllocations()), 4)
 	assert.Assert(t, app.IsRunning(), "app should still be in running state")
@@ -1271,7 +1271,7 @@ func TestReplaceAllocation(t *testing.T) {
 
 	// set the real one to replace the placeholder
 	realAlloc := newAllocation(appID1, nodeID1, res)
-	realAlloc.SetResult(Replaced)
+	realAlloc.SetResultType(Replaced)
 	ph.SetRelease(realAlloc)
 	alloc = app.ReplaceAllocation(ph.GetAllocationKey())
 	assert.Equal(t, alloc, ph, "returned allocation is not the placeholder")
@@ -1293,10 +1293,10 @@ func TestReplaceAllocation(t *testing.T) {
 
 	// set multiple real allocations to replace the placeholder
 	realAlloc = newAllocation(appID1, nodeID1, res)
-	realAlloc.SetResult(Replaced)
+	realAlloc.SetResultType(Replaced)
 	ph.SetRelease(realAlloc)
 	realAllocNoAdd := newAllocation(appID1, nodeID1, res)
-	realAllocNoAdd.SetResult(Replaced)
+	realAllocNoAdd.SetResultType(Replaced)
 	ph.SetRelease(realAlloc)
 	alloc = app.ReplaceAllocation(ph.GetAllocationKey())
 	assert.Equal(t, alloc, ph, "returned allocation is not the placeholder")
@@ -1345,21 +1345,21 @@ func TestReplaceAllocationTracking(t *testing.T) {
 
 	// replace placeholders
 	realAlloc1 := newAllocation(appID1, nodeID1, res)
-	realAlloc1.SetResult(Replaced)
+	realAlloc1.SetResultType(Replaced)
 	ph1.SetRelease(realAlloc1)
 	alloc1 := app.ReplaceAllocation(ph1.GetAllocationKey())
 	app.RemoveAllocation(ph1.GetAllocationKey(), si.TerminationType_PLACEHOLDER_REPLACED)
 	assert.Equal(t, ph1.GetAllocationKey(), alloc1.GetAllocationKey())
 	assert.Equal(t, true, app.HasPlaceholderAllocation())
 	realAlloc2 := newAllocation(appID1, nodeID1, res)
-	realAlloc2.SetResult(Replaced)
+	realAlloc2.SetResultType(Replaced)
 	ph2.SetRelease(realAlloc2)
 	alloc2 := app.ReplaceAllocation(ph2.GetAllocationKey())
 	app.RemoveAllocation(ph2.GetAllocationKey(), si.TerminationType_PLACEHOLDER_REPLACED)
 	assert.Equal(t, ph2.GetAllocationKey(), alloc2.GetAllocationKey())
 	assert.Equal(t, true, app.HasPlaceholderAllocation())
 	realAlloc3 := newAllocation(appID1, nodeID1, res)
-	realAlloc3.SetResult(Replaced)
+	realAlloc3.SetResultType(Replaced)
 	ph3.SetRelease(realAlloc3)
 	alloc3 := app.ReplaceAllocation(ph3.GetAllocationKey())
 	app.RemoveAllocation(ph3.GetAllocationKey(), si.TerminationType_PLACEHOLDER_REPLACED)
@@ -1861,7 +1861,7 @@ func TestTryAllocatePreemptQueue(t *testing.T) {
 	// pass the time and try again
 	ask3.createTime = ask3.createTime.Add(-30 * time.Second)
 	alloc3 = app2.tryAllocate(resources.NewResourceFromMap(map[string]resources.Quantity{"first": 0}), true, 30*time.Second, &preemptionAttemptsRemaining, iterator, iterator, getNode)
-	assert.Assert(t, alloc3 != nil && alloc3.result == Reserved, "alloc3 should be a reservation")
+	assert.Assert(t, alloc3 != nil && alloc3.resultType == Reserved, "alloc3 should be a reservation")
 	assert.Assert(t, alloc2.IsPreempted(), "alloc2 should have been preempted")
 }
 
@@ -1932,7 +1932,7 @@ func TestTryAllocatePreemptNode(t *testing.T) {
 	alloc3 := app2.tryAllocate(resources.NewResourceFromMap(map[string]resources.Quantity{"first": 18}), true, 30*time.Second, &preemptionAttemptsRemaining, iterator, iterator, getNode)
 	assert.Assert(t, alloc3 != nil, "alloc3 expected")
 	assert.Equal(t, "node1", alloc3.GetNodeID(), "wrong node assignment")
-	assert.Equal(t, Reserved, alloc3.GetResult(), "expected reservation")
+	assert.Equal(t, Reserved, alloc3.GetResultType(), "expected reservation")
 	assert.Assert(t, !alloc2.IsPreempted(), "alloc2 should not have been preempted")
 	err = node1.Reserve(app2, ask3)
 	assert.NilError(t, err)

pkg/scheduler/objects/node_test.go

@@ -186,7 +186,7 @@ func TestCanAllocate(t *testing.T) {
 			sn := &Node{
 				availableResource: tt.available,
 			}
-			assert.Equal(t, sn.CanAllocate(tt.request), tt.want, "unexpected node can run result")
+			assert.Equal(t, sn.CanAllocate(tt.request), tt.want, "unexpected node can run resultType")
 		})
 	}
 }
@@ -765,7 +765,7 @@ func TestNode_FitInNode(t *testing.T) {
 			sn := &Node{
 				totalResource: tt.totalRes,
 			}
-			assert.Equal(t, sn.FitInNode(tt.resRequest), tt.want, "unexpected node fit result")
+			assert.Equal(t, sn.FitInNode(tt.resRequest), tt.want, "unexpected node fit resultType")
 		})
 	}
 }