From 9e7a2a2a4b410d2a5a67b726eb6a855f3647187a Mon Sep 17 00:00:00 2001 From: lowang-bh Date: Sat, 26 Aug 2023 16:32:56 +0800 Subject: [PATCH] keep the origin logic of calculate podgorup minresource when jobMinAvailable < totalTask's Signed-off-by: lowang-bh --- pkg/controllers/job/job_controller_actions.go | 15 +++ pkg/controllers/job/job_controller_util.go | 17 +++ .../job/job_controller_util_test.go | 100 +++++++++++++++++- 3 files changed, 129 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/job/job_controller_actions.go b/pkg/controllers/job/job_controller_actions.go index acc05dee041..fe7c79159ab 100644 --- a/pkg/controllers/job/job_controller_actions.go +++ b/pkg/controllers/job/job_controller_actions.go @@ -766,6 +766,7 @@ func (cc *jobcontroller) deleteJobPod(jobName string, pod *v1.Pod) error { func (cc *jobcontroller) calcPGMinResources(job *batch.Job) *v1.ResourceList { // sort task by priorityClasses var tasksPriority TasksPriority + totalMinAvailable := int32(0) for _, task := range job.Spec.Tasks { tp := TaskPriority{0, task} pc := task.Template.Spec.PriorityClassName @@ -779,8 +780,22 @@ func (cc *jobcontroller) calcPGMinResources(job *batch.Job) *v1.ResourceList { } } tasksPriority = append(tasksPriority, tp) + if task.MinAvailable != nil { // actually, it can not be nil, because nil value will be patched in webhook + totalMinAvailable += *task.MinAvailable + } else { + totalMinAvailable += task.Replicas + } + } + + // see docs https://github.com/volcano-sh/volcano/pull/2945 + // 1. job.MinAvailable < sum(task.MinAvailable), regard podgroup's min resource as sum of the first minAvailable, + // according to https://github.com/volcano-sh/volcano/blob/c91eb07f2c300e4d5c826ff11a63b91781b3ac11/pkg/scheduler/api/job_info.go#L738-L740 + if job.Spec.MinAvailable < totalMinAvailable { + minReq := tasksPriority.CalcFirstCountResources(job.Spec.MinAvailable) + return &minReq } + // 2. job.MinAvailable >= sum(task.MinAvailable) minReq := tasksPriority.CalcPGMinResources(job.Spec.MinAvailable) return &minReq diff --git a/pkg/controllers/job/job_controller_util.go b/pkg/controllers/job/job_controller_util.go index 356b38eca54..584da84cdcf 100644 --- a/pkg/controllers/job/job_controller_util.go +++ b/pkg/controllers/job/job_controller_util.go @@ -259,6 +259,23 @@ func isControlledBy(obj metav1.Object, gvk schema.GroupVersionKind) bool { return false } +// CalcFirstCountResources return the first count tasks resource, sorted by priority +func (p TasksPriority) CalcFirstCountResources(count int32) v1.ResourceList { + sort.Sort(p) + minReq := v1.ResourceList{} + + for _, task := range p { + if count <= task.Replicas { + minReq = quotav1.Add(minReq, calTaskRequests(&v1.Pod{Spec: task.Template.Spec}, count)) + break + } else { + minReq = quotav1.Add(minReq, calTaskRequests(&v1.Pod{Spec: task.Template.Spec}, task.Replicas)) + count -= task.Replicas + } + } + return minReq +} + // CalcPGMinResources sums up all task's min available; if not enough, then fill up to jobMinAvailable via task's replicas func (p TasksPriority) CalcPGMinResources(jobMinAvailable int32) v1.ResourceList { sort.Sort(p) diff --git a/pkg/controllers/job/job_controller_util_test.go b/pkg/controllers/job/job_controller_util_test.go index d8a8447dbe2..77e30a6dc43 100644 --- a/pkg/controllers/job/job_controller_util_test.go +++ b/pkg/controllers/job/job_controller_util_test.go @@ -25,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "volcano.sh/apis/pkg/apis/batch/v1alpha1" + batch "volcano.sh/apis/pkg/apis/batch/v1alpha1" busv1alpha1 "volcano.sh/apis/pkg/apis/bus/v1alpha1" "volcano.sh/volcano/pkg/controllers/apis" ) @@ -685,8 +686,8 @@ func TestTasksPriority_Swap(t *testing.T) { } } -func TestTaskPriority_CalcPGMin(t *testing.T) { - worker := v1alpha1.TaskSpec{ +var ( + worker = v1alpha1.TaskSpec{ Name: "worker", Replicas: 2, Template: v1.PodTemplateSpec{ @@ -704,7 +705,7 @@ func TestTaskPriority_CalcPGMin(t *testing.T) { }, }, } - master := v1alpha1.TaskSpec{ + master = v1alpha1.TaskSpec{ Name: "master", Replicas: 2, Template: v1.PodTemplateSpec{ @@ -722,6 +723,9 @@ func TestTaskPriority_CalcPGMin(t *testing.T) { }, }, } +) + +func TestTaskPriority_CalcPGMin(t *testing.T) { oneMinAvailable := int32(1) zeroMinAvailable := int32(0) @@ -976,3 +980,93 @@ func TestTaskPriority_CalcPGMin(t *testing.T) { } } } + +func TestCalcPGMinResources(t *testing.T) { + jc := newFakeController() + job := &batch.Job{ + TypeMeta: metav1.TypeMeta{}, + Spec: batch.JobSpec{ + Tasks: []batch.TaskSpec{ + master, worker, + }, + }, + } + + oneMinAvailable := int32(1) + //zeroMinAvailable := int32(0) + + tests := []struct { + TasksMinAvailable []*int32 + JobMinMember int32 + ExpectValue v1.ResourceList + }{ + // jobMinAvailable < sum(taskMinAvailable) + { + JobMinMember: 2, + TasksMinAvailable: []*int32{&oneMinAvailable, nil}, + ExpectValue: v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(100, resource.DecimalSI), "requests.cpu": *resource.NewMilliQuantity(100, resource.DecimalSI), + "pods": *resource.NewQuantity(2, resource.DecimalSI), "count/pods": *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + { + JobMinMember: 2, + TasksMinAvailable: []*int32{nil, &oneMinAvailable}, + ExpectValue: v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(100, resource.DecimalSI), "requests.cpu": *resource.NewMilliQuantity(100, resource.DecimalSI), + "pods": *resource.NewQuantity(2, resource.DecimalSI), "count/pods": *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + { + JobMinMember: 3, + TasksMinAvailable: []*int32{nil, nil}, + ExpectValue: v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(200, resource.DecimalSI), "requests.cpu": *resource.NewMilliQuantity(200, resource.DecimalSI), + "pods": *resource.NewQuantity(3, resource.DecimalSI), "count/pods": *resource.NewQuantity(3, resource.DecimalSI), + }, + }, + { // jobMinAvailable > sum(taskMinAvailable) + JobMinMember: 3, + TasksMinAvailable: []*int32{&oneMinAvailable, &oneMinAvailable}, + ExpectValue: v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(200, resource.DecimalSI), "requests.cpu": *resource.NewMilliQuantity(200, resource.DecimalSI), + "pods": *resource.NewQuantity(3, resource.DecimalSI), "count/pods": *resource.NewQuantity(3, resource.DecimalSI), + }, + }, + // jobMinAvailable = sum(taskMinAvailable) + { + JobMinMember: 3, + TasksMinAvailable: []*int32{&oneMinAvailable, nil}, + ExpectValue: v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(250, resource.DecimalSI), "requests.cpu": *resource.NewMilliQuantity(250, resource.DecimalSI), + "pods": *resource.NewQuantity(3, resource.DecimalSI), "count/pods": *resource.NewQuantity(3, resource.DecimalSI), + }, + }, + { + JobMinMember: 2, + TasksMinAvailable: []*int32{&oneMinAvailable, &oneMinAvailable}, + ExpectValue: v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(150, resource.DecimalSI), "requests.cpu": *resource.NewMilliQuantity(150, resource.DecimalSI), + "pods": *resource.NewQuantity(2, resource.DecimalSI), "count/pods": *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + } + for i, tt := range tests { + job.Spec.MinAvailable = tt.JobMinMember + for i := range tt.TasksMinAvailable { + job.Spec.Tasks[i].MinAvailable = tt.TasksMinAvailable[i] + } + // simulating patch in webhook + for i, task := range job.Spec.Tasks { + if task.MinAvailable == nil { + min := &task.Replicas + job.Spec.Tasks[i].MinAvailable = min + } + } + gotMin := jc.calcPGMinResources(job) + if !reflect.DeepEqual(gotMin, &tt.ExpectValue) { + t.Fatalf("case %d: expected %v got %v", i, tt.ExpectValue, gotMin) + } + + } +}