Skip to content

Commit

Permalink
[YUNIKORN-2780] Remove references to SI ExistingAllocations (#886)
Browse files Browse the repository at this point in the history
Now that node registration has been simplified, we no longer need to
send ExistingAllocations via the SI. Remove references so these can
be removed from the interface.

Closes: #886
  • Loading branch information
craigcondit committed Aug 2, 2024
1 parent acbc966 commit 7f9901f
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 69 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ module github.com/apache/yunikorn-k8shim
go 1.21

require (
github.com/apache/yunikorn-core v0.0.0-20240711165824-d96cd583305b
github.com/apache/yunikorn-scheduler-interface v0.0.0-20240425182941-07f5695119a1
github.com/apache/yunikorn-core v0.0.0-20240802210614-4aec626c6bf9
github.com/apache/yunikorn-scheduler-interface v0.0.0-20240731203810-92032b13d586
github.com/google/go-cmp v0.6.0
github.com/google/uuid v1.6.0
github.com/looplab/fsm v1.0.1
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cq
github.com/NYTimes/gziphandler v1.1.1/go.mod h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMomdKFjzJNB0c=
github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df h1:7RFfzj4SSt6nnvCPbCqijJi1nWCd+TqAT3bYCStRC18=
github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df/go.mod h1:pSwJ0fSY5KhvocuWSx4fz3BA8OrA1bQn+K1Eli3BRwM=
github.com/apache/yunikorn-core v0.0.0-20240711165824-d96cd583305b h1:GDizY3dcE+hkfik/+NY3Zdw71A/V4dWGp9Pl6k5Ii2M=
github.com/apache/yunikorn-core v0.0.0-20240711165824-d96cd583305b/go.mod h1:pSi7AFBRiGCGQ7RwQffpD4m6dvA5lc1HuCrg7LpJJqs=
github.com/apache/yunikorn-scheduler-interface v0.0.0-20240425182941-07f5695119a1 h1:v4J9L3MlW8BQfYnbq6FV2l3uyay3SqMS2Ffpo+SFat4=
github.com/apache/yunikorn-scheduler-interface v0.0.0-20240425182941-07f5695119a1/go.mod h1:WuHJpVk34t8N5+1ErYGj/5Qq33/cRzL4YtuoAsbMtWc=
github.com/apache/yunikorn-core v0.0.0-20240802210614-4aec626c6bf9 h1:s1Co/K+cR9Q/GW0e974dToW9eyLQZxYoCp0TCoEuEj0=
github.com/apache/yunikorn-core v0.0.0-20240802210614-4aec626c6bf9/go.mod h1:S9yGBGA2i2hAajtEc2t4lmiPJDZz3Ek8eVxz5KhJqGI=
github.com/apache/yunikorn-scheduler-interface v0.0.0-20240731203810-92032b13d586 h1:ZVpo9Qj2/gvwX6Rl44UxkZBm2pZWEJDYWTramc9hwF0=
github.com/apache/yunikorn-scheduler-interface v0.0.0-20240731203810-92032b13d586/go.mod h1:WuHJpVk34t8N5+1ErYGj/5Qq33/cRzL4YtuoAsbMtWc=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a h1:idn718Q4B6AGu/h5Sxe66HYVdqdGu2l9Iebqhi/AEoA=
Expand Down
1 change: 0 additions & 1 deletion pkg/cache/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -1530,7 +1530,6 @@ func (ctx *Context) registerNodes(nodes []*v1.Node) ([]*v1.Node, error) {
},
SchedulableResource: common.GetNodeResource(&nodeStatus),
OccupiedResource: common.NewResourceBuilder().Build(),
ExistingAllocations: make([]*si.Allocation, 0),
})
pendingNodes[node.Name] = node
}
Expand Down
33 changes: 0 additions & 33 deletions pkg/common/si_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,39 +153,6 @@ func CreateReleaseRequestForTask(appID, taskID, allocationKey, partition, termin
}
}

// CreateUpdateRequestForNewNode builds a NodeRequest for new node addition and restoring existing node
func CreateUpdateRequestForNewNode(nodeID string, nodeLabels map[string]string, capacity *si.Resource, occupied *si.Resource,
existingAllocations []*si.Allocation) *si.NodeRequest {
// Use node's name as the NodeID, this is because when bind pod to node,
// name of node is required but uid is optional.
nodeInfo := &si.NodeInfo{
NodeID: nodeID,
SchedulableResource: capacity,
OccupiedResource: occupied,
Attributes: map[string]string{
constants.DefaultNodeAttributeHostNameKey: nodeID,
constants.DefaultNodeAttributeRackNameKey: constants.DefaultRackName,
},
ExistingAllocations: existingAllocations,
Action: si.NodeInfo_CREATE,
}

// Add nodeLabels key value to Attributes map
for k, v := range nodeLabels {
nodeInfo.Attributes[k] = v
}

// Add instanceType to Attributes map
nodeInfo.Attributes[common.InstanceType] = nodeLabels[conf.GetSchedulerConf().InstanceTypeNodeLabelKey]

nodes := make([]*si.NodeInfo, 1)
nodes[0] = nodeInfo
return &si.NodeRequest{
Nodes: nodes,
RmID: conf.GetSchedulerConf().ClusterID,
}
}

// CreateUpdateRequestForUpdatedNode builds a NodeRequest for capacity and occupied resource updates
func CreateUpdateRequestForUpdatedNode(nodeID string, capacity *si.Resource, occupied *si.Resource) *si.NodeRequest {
nodeInfo := &si.NodeInfo{
Expand Down
28 changes: 0 additions & 28 deletions pkg/common/si_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
v1 "k8s.io/api/core/v1"
apis "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/apache/yunikorn-k8shim/pkg/common/constants"
"github.com/apache/yunikorn-scheduler-interface/lib/go/common"
"github.com/apache/yunikorn-scheduler-interface/lib/go/si"
)
Expand Down Expand Up @@ -230,33 +229,6 @@ func TestCreateTagsForTask(t *testing.T) {
assert.Equal(t, len(result4), 4)
}

func TestCreateUpdateRequestForNewNode(t *testing.T) {
capacity := NewResourceBuilder().AddResource(common.Memory, 200).AddResource(common.CPU, 2).Build()
occupied := NewResourceBuilder().AddResource(common.Memory, 50).AddResource(common.CPU, 1).Build()
var existingAllocations []*si.Allocation
nodeLabels := map[string]string{
"label1": "key1",
"label2": "key2",
"node.kubernetes.io/instance-type": "HighMem",
}
request := CreateUpdateRequestForNewNode(nodeID, nodeLabels, capacity, occupied, existingAllocations)
assert.Equal(t, len(request.Nodes), 1)
assert.Equal(t, request.Nodes[0].NodeID, nodeID)
assert.Equal(t, request.Nodes[0].SchedulableResource, capacity)
assert.Equal(t, request.Nodes[0].OccupiedResource, occupied)
assert.Equal(t, len(request.Nodes[0].Attributes), 6)
assert.Equal(t, request.Nodes[0].Attributes[constants.DefaultNodeAttributeHostNameKey], nodeID)
assert.Equal(t, request.Nodes[0].Attributes[constants.DefaultNodeAttributeRackNameKey], constants.DefaultRackName)

// Make sure include nodeLabel
assert.Equal(t, request.Nodes[0].Attributes["label1"], "key1")
assert.Equal(t, request.Nodes[0].Attributes["label2"], "key2")
assert.Equal(t, request.Nodes[0].Attributes["node.kubernetes.io/instance-type"], "HighMem")

// Make sure include the instanceType
assert.Equal(t, request.Nodes[0].Attributes[common.InstanceType], "HighMem")
}

func TestCreateUpdateRequestForUpdatedNode(t *testing.T) {
capacity := NewResourceBuilder().AddResource(common.Memory, 200).AddResource(common.CPU, 2).Build()
occupied := NewResourceBuilder().AddResource(common.Memory, 50).AddResource(common.CPU, 1).Build()
Expand Down
32 changes: 31 additions & 1 deletion pkg/shim/scheduler_mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/apache/yunikorn-k8shim/pkg/cache"
"github.com/apache/yunikorn-k8shim/pkg/client"
"github.com/apache/yunikorn-k8shim/pkg/common"
"github.com/apache/yunikorn-k8shim/pkg/common/constants"
"github.com/apache/yunikorn-k8shim/pkg/common/events"
"github.com/apache/yunikorn-k8shim/pkg/common/utils"
"github.com/apache/yunikorn-k8shim/pkg/conf"
Expand Down Expand Up @@ -124,7 +125,7 @@ func (fc *MockScheduler) addNode(nodeName string, nodeLabels map[string]string,
AddResource(siCommon.CPU, cpu).
AddResource("pods", pods).
Build()
request := common.CreateUpdateRequestForNewNode(nodeName, nodeLabels, nodeResource, nil, nil)
request := createUpdateRequestForNewNode(nodeName, nodeLabels, nodeResource, nil)
fmt.Printf("report new nodes to scheduler, request: %s", request.String())
return fc.apiProvider.GetAPIs().SchedulerAPI.UpdateNode(request)
}
Expand Down Expand Up @@ -335,3 +336,32 @@ func (fc *MockScheduler) ensureStarted() {
panic("mock scheduler is not started - call start() first")
}
}
func createUpdateRequestForNewNode(nodeID string, nodeLabels map[string]string, capacity *si.Resource, occupied *si.Resource) *si.NodeRequest {
// Use node's name as the NodeID, this is because when bind pod to node,
// name of node is required but uid is optional.
nodeInfo := &si.NodeInfo{
NodeID: nodeID,
SchedulableResource: capacity,
OccupiedResource: occupied,
Attributes: map[string]string{
constants.DefaultNodeAttributeHostNameKey: nodeID,
constants.DefaultNodeAttributeRackNameKey: constants.DefaultRackName,
},
Action: si.NodeInfo_CREATE,
}

// Add nodeLabels key value to Attributes map
for k, v := range nodeLabels {
nodeInfo.Attributes[k] = v
}

// Add instanceType to Attributes map
nodeInfo.Attributes[siCommon.InstanceType] = nodeLabels[conf.GetSchedulerConf().InstanceTypeNodeLabelKey]

nodes := make([]*si.NodeInfo, 1)
nodes[0] = nodeInfo
return &si.NodeRequest{
Nodes: nodes,
RmID: conf.GetSchedulerConf().ClusterID,
}
}

0 comments on commit 7f9901f

Please sign in to comment.