From ce4e1197766ad06754c310b5db791a0f48f9c6a9 Mon Sep 17 00:00:00 2001 From: Craig Condit Date: Tue, 30 Jul 2024 17:27:10 -0500 Subject: [PATCH] [YUNIKORN-2780] Remove references to SI ExistingAllocations 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. --- go.mod | 2 +- go.sum | 4 ++-- pkg/cache/context.go | 1 - pkg/common/si_helper.go | 33 --------------------------------- pkg/common/si_helper_test.go | 28 ---------------------------- pkg/shim/scheduler_mock_test.go | 32 +++++++++++++++++++++++++++++++- 6 files changed, 34 insertions(+), 66 deletions(-) diff --git a/go.mod b/go.mod index 9240134aa..102904654 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ 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-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 diff --git a/go.sum b/go.sum index 0354b1fb5..db6379dae 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,8 @@ github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df h 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-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= diff --git a/pkg/cache/context.go b/pkg/cache/context.go index 0f7764ade..243b1847e 100644 --- a/pkg/cache/context.go +++ b/pkg/cache/context.go @@ -1558,7 +1558,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 } diff --git a/pkg/common/si_helper.go b/pkg/common/si_helper.go index 9c4520a07..c3c1a7796 100644 --- a/pkg/common/si_helper.go +++ b/pkg/common/si_helper.go @@ -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{ diff --git a/pkg/common/si_helper_test.go b/pkg/common/si_helper_test.go index 63c603f7d..92f5e2407 100644 --- a/pkg/common/si_helper_test.go +++ b/pkg/common/si_helper_test.go @@ -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" ) @@ -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() diff --git a/pkg/shim/scheduler_mock_test.go b/pkg/shim/scheduler_mock_test.go index 1e8f19106..b67746d72 100644 --- a/pkg/shim/scheduler_mock_test.go +++ b/pkg/shim/scheduler_mock_test.go @@ -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" @@ -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) } @@ -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, + } +}