Skip to content

Commit

Permalink
feature:add preemptionpolicy in preempt and reclaim
Browse files Browse the repository at this point in the history
Signed-off-by: jessestutler <jesseincomparable@hotmail.com>
  • Loading branch information
JesseStutler committed Sep 23, 2024
1 parent 0843c0d commit b791571
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/controllers/job/job_controller_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ func createJobPod(job *batch.Job, template *v1.PodTemplateSpec, topologyPolicy b
pod.Spec.SchedulerName = job.Spec.SchedulerName
}

// If no priority class specified in pod template, use priority class specified in job
if len(pod.Spec.PriorityClassName) == 0 && len(job.Spec.PriorityClassName) != 0 {
pod.Spec.PriorityClassName = job.Spec.PriorityClassName
}

volumeMap := make(map[string]string)
for _, volume := range job.Spec.Volumes {
vcName := volume.VolumeClaimName
Expand Down
5 changes: 5 additions & 0 deletions pkg/scheduler/actions/preempt/preempt.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package preempt
import (
"fmt"

v1 "k8s.io/api/core/v1"
"k8s.io/klog/v2"

"volcano.sh/volcano/pkg/scheduler/api"
Expand Down Expand Up @@ -231,6 +232,10 @@ func (pmpt *Action) preempt(
filter func(*api.TaskInfo) bool,
predicateHelper util.PredicateHelper,
) (bool, error) {
if preemptor.Pod.Spec.PreemptionPolicy != nil && *preemptor.Pod.Spec.PreemptionPolicy == v1.PreemptNever {
return false, fmt.Errorf("not eligible to preempt other tasks due to preemptionPolicy is Never")
}

assigned := false

if err := ssn.PrePredicateFn(preemptor); err != nil {
Expand Down
20 changes: 20 additions & 0 deletions pkg/scheduler/actions/preempt/preempt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,26 @@ func TestPreempt(t *testing.T) {
ExpectEvicted: []string{"c1/preemptee1"},
ExpectEvictNum: 1,
},
{
// case about #3642
Name: "can not preempt resources when task preemption policy is never",
PodGroups: []*schedulingv1beta1.PodGroup{
util.BuildPodGroupWithPrio("pg1", "c1", "q1", 0, nil, schedulingv1beta1.PodGroupInqueue, "low-priority"),
util.BuildPodGroupWithPrio("pg2", "c1", "q1", 0, nil, schedulingv1beta1.PodGroupInqueue, "high-priority"),
},
Pods: []*v1.Pod{
util.BuildPod("c1", "preemptee1", "n1", v1.PodRunning, api.BuildResourceList("1", "1G"), "pg1", map[string]string{schedulingv1beta1.PodPreemptable: "true"}, make(map[string]string)),
util.BuildPodWithPreeemptionPolicy("c1", "preemptor1", "", v1.PodPending, api.BuildResourceList("1", "1G"), "pg2", make(map[string]string), make(map[string]string), v1.PreemptNever),
},
Nodes: []*v1.Node{
util.BuildNode("n1", api.BuildResourceList("1", "1Gi", []api.ScalarResource{{Name: "pods", Value: "1"}}...), make(map[string]string)),
},
Queues: []*schedulingv1beta1.Queue{
util.BuildQueue("q1", 1, nil),
},
ExpectEvictNum: 0,
ExpectEvicted: []string{}, // no victims should be reclaimed
},
}

trueValue := true
Expand Down
10 changes: 10 additions & 0 deletions pkg/scheduler/actions/reclaim/reclaim.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package reclaim

import (
v1 "k8s.io/api/core/v1"
"k8s.io/klog/v2"

"volcano.sh/volcano/pkg/scheduler/api"
Expand Down Expand Up @@ -113,6 +114,15 @@ func (ra *Action) Execute(ssn *framework.Session) {
task = tasks.Pop().(*api.TaskInfo)
}

if task.Pod.Spec.PreemptionPolicy != nil && *task.Pod.Spec.PreemptionPolicy == v1.PreemptNever {
klog.V(3).Infof("Task %s/%s is not eligible to preempt other tasks due to preemptionPolicy is Never", task.Namespace, task.Name)
// TODO: In order to avoid blocking other tasks in the job or other jobs in the queue to reclaim resources, the job and queue need
// to be pushed back to the priority queue. Need to refactor the framework of reclaim action, see issue: https://github.com/volcano-sh/volcano/issues/3738
jobs.Push(job)
queues.Push(queue)
continue
}

if !ssn.Allocatable(queue, task) {
klog.V(3).Infof("Queue <%s> is overused when considering task <%s>, ignore it.", queue.Name, task.Name)
continue
Expand Down
30 changes: 30 additions & 0 deletions pkg/scheduler/actions/reclaim/reclaim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,36 @@ func TestReclaim(t *testing.T) {
ExpectEvictNum: 1,
ExpectEvicted: []string{"c1/preemptee1-1"}, // low queue priority job's preemptable pod is evicted
},
{
// case about #3642
Name: "can not reclaim resources when task preemption policy is never",
Plugins: map[string]framework.PluginBuilder{
conformance.PluginName: conformance.New,
gang.PluginName: gang.New,
proportion.PluginName: proportion.New,
},
PriClass: []*schedulingv1.PriorityClass{
util.BuildPriorityClass("low-priority", 100),
util.BuildPriorityClassWithPreemptionPolicy("high-priority", 1000, v1.PreemptNever),
},
PodGroups: []*schedulingv1beta1.PodGroup{
util.BuildPodGroupWithPrio("pg1", "c1", "q1", 0, nil, schedulingv1beta1.PodGroupInqueue, "low-priority"),
util.BuildPodGroupWithPrio("pg2", "c1", "q2", 0, nil, schedulingv1beta1.PodGroupInqueue, "high-priority"),
},
Pods: []*v1.Pod{
util.BuildPod("c1", "preemptee1", "n1", v1.PodRunning, api.BuildResourceList("1", "1G"), "pg1", map[string]string{schedulingv1beta1.PodPreemptable: "true"}, make(map[string]string)),
util.BuildPodWithPreeemptionPolicy("c1", "preemptor1", "", v1.PodPending, api.BuildResourceList("1", "1G"), "pg2", make(map[string]string), make(map[string]string), v1.PreemptNever),
},
Nodes: []*v1.Node{
util.BuildNode("n1", api.BuildResourceList("1", "1Gi", []api.ScalarResource{{Name: "pods", Value: "1"}}...), make(map[string]string)),
},
Queues: []*schedulingv1beta1.Queue{
util.BuildQueue("q1", 5, nil),
util.BuildQueue("q2", 10, nil),
},
ExpectEvictNum: 0,
ExpectEvicted: []string{}, // no victims should be reclaimed
},
}

reclaim := New()
Expand Down
14 changes: 14 additions & 0 deletions pkg/scheduler/util/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,13 @@ func BuildPodWithPriority(namespace, name, nodeName string, p v1.PodPhase, req v
return pod
}

// BuildPodWithPreemptionPolicy builds a pod with preemptionPolicy
func BuildPodWithPreeemptionPolicy(namespace, name, nodeName string, p v1.PodPhase, req v1.ResourceList, groupName string, labels map[string]string, selector map[string]string, preemptionPolicy v1.PreemptionPolicy) *v1.Pod {
pod := BuildPod(namespace, name, nodeName, p, req, groupName, labels, selector)
pod.Spec.PreemptionPolicy = &preemptionPolicy
return pod
}

// BuildPodGroup return podgroup with base spec and phase status
func BuildPodGroup(name, ns, queue string, minMember int32, taskMinMember map[string]int32, status schedulingv1beta1.PodGroupPhase) *schedulingv1beta1.PodGroup {
return &schedulingv1beta1.PodGroup{
Expand Down Expand Up @@ -304,6 +311,13 @@ func BuildPriorityClass(name string, value int32) *schedulingv1.PriorityClass {
}
}

// BuildPriorityClassWithPreemptionPolicy return a priorityClass with value and preemptionPolicy
func BuildPriorityClassWithPreemptionPolicy(name string, value int32, preemptionPolicy v1.PreemptionPolicy) *schedulingv1.PriorityClass {
pc := BuildPriorityClass(name, value)
pc.PreemptionPolicy = &preemptionPolicy
return pc
}

// FakeBinder is used as fake binder
type FakeBinder struct {
sync.RWMutex
Expand Down

0 comments on commit b791571

Please sign in to comment.