Skip to content

Commit

Permalink
[YUNIKORN-1921] Gang-scheduled placeholder pods don't inherit Priorit…
Browse files Browse the repository at this point in the history 
…yClasses (#657)

Closes: #657

Signed-off-by: Craig Condit <[email protected]>
  • Loading branch information
@manirajv06 @craigcondit
manirajv06 authored and craigcondit committed Aug 24, 2023
1 parent a2b772e commit 5b35abc
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 0 deletions.
6 changes: 6 additions & 0 deletions pkg/cache/placeholder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ func newPlaceholder(placeholderName string, app *Application, taskGroup interfac
}
}

var priority *int32
if task := app.GetOriginatingTask(); task != nil {
priority = task.GetTaskPod().Spec.Priority
}

// prepare the resource lists
requests := utils.GetPlaceholderResourceRequests(taskGroup.MinResource)
limits := utils.GetPlaceholderResourceLimits(taskGroup.MinResource)
Expand Down Expand Up @@ -116,6 +121,7 @@ func newPlaceholder(placeholderName string, app *Application, taskGroup interfac
NodeSelector: taskGroup.NodeSelector,
Tolerations: taskGroup.Tolerations,
Affinity: taskGroup.Affinity,
Priority: priority,
},
}

Expand Down
22 changes: 22 additions & 0 deletions pkg/cache/placeholder_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package cache

import (
"fmt"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -89,6 +90,13 @@ func TestCreateAppPlaceholdersWithExistingPods(t *testing.T) {
assert.Equal(t, (*v1.Pod)(nil), createdPods["tg-test-group-1-app01-0"], "Pod should not have been created")
assert.Equal(t, (*v1.Pod)(nil), createdPods["tg-test-group-1-app01-1"], "Pod should not have been created")
assert.Equal(t, (*v1.Pod)(nil), createdPods["tg-test-group-1-app02-0"], "Pod should not have been created")
for _, tg := range []string{"tg-test-group-1-app01-", "tg-test-group-2-app01-"} {
for i := 2; i <= 9; i++ {
podName := tg + strconv.Itoa(i)
priority := createdPods[podName].Spec.Priority
assert.Equal(t, *priority, int32(10), "Pod should not have been created")
}
}
}

func createAndCheckPlaceholderCreate(mockedAPIProvider *client.MockedAPIProvider, app *Application, t *testing.T) map[string]*v1.Pod {
Expand All @@ -102,6 +110,13 @@ func createAndCheckPlaceholderCreate(mockedAPIProvider *client.MockedAPIProvider
err := placeholderMgr.createAppPlaceholders(app)
assert.NilError(t, err, "create app placeholders should be successful")
assert.Equal(t, len(createdPods), 30)
var priority *int32
for _, tg := range []string{"tg-test-group-1-app01-", "tg-test-group-2-app01-"} {
for i := 2; i <= 9; i++ {
podName := tg + strconv.Itoa(i)
assert.Equal(t, priority, createdPods[podName].Spec.Priority, "Pod should not have been created")
}
}
return createdPods
}

Expand Down Expand Up @@ -151,6 +166,8 @@ func createAppWIthTaskGroupForTest() *Application {
func createAppWIthTaskGroupAndPodsForTest() *Application {
app := createAppWIthTaskGroupForTest()
mockedContext := initContextForTest()
priority := int32(10)
specPriority := &priority
pod1 := &v1.Pod{
TypeMeta: apis.TypeMeta{
Kind: "Pod",
Expand All @@ -160,6 +177,9 @@ func createAppWIthTaskGroupAndPodsForTest() *Application {
Name: "tg-test-group-1-app01-0",
UID: "UID-01",
},
Spec: v1.PodSpec{
Priority: specPriority,
},
}
pod2 := &v1.Pod{
TypeMeta: apis.TypeMeta{
Expand All @@ -186,7 +206,9 @@ func createAppWIthTaskGroupAndPodsForTest() *Application {
task1 := NewTask(taskID1, app, mockedContext, pod1)
task1.placeholder = true
task1.pod = pod1
task1.originator = true
app.taskMap[taskID1] = task1
app.setOriginatingTask(task1)

taskID2 := "task1-02"
task2 := NewTask(taskID2, app, mockedContext, pod2)
Expand Down
51 changes: 51 additions & 0 deletions pkg/cache/placeholder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ func TestNewPlaceholder(t *testing.T) {
assert.Equal(t, len(holder.pod.Spec.ImagePullSecrets), 2, "unexpected number of pull secrets")
assert.Equal(t, "secret1", holder.pod.Spec.ImagePullSecrets[0].Name)
assert.Equal(t, "secret2", holder.pod.Spec.ImagePullSecrets[1].Name)
var priority *int32
assert.Equal(t, priority, holder.pod.Spec.Priority)
}

func TestNewPlaceholderWithLabelsAndAnnotations(t *testing.T) {
Expand All @@ -148,6 +150,8 @@ func TestNewPlaceholderWithLabelsAndAnnotations(t *testing.T) {
var taskGroupsDef []interfaces.TaskGroup
err = json.Unmarshal([]byte(holder.pod.Annotations["yunikorn.apache.org/task-groups"]), &taskGroupsDef)
assert.NilError(t, err, "taskGroupsDef unmarshal failed")
var priority *int32
assert.Equal(t, priority, holder.pod.Spec.Priority)
}

func TestNewPlaceholderWithNodeSelectors(t *testing.T) {
Expand All @@ -160,6 +164,8 @@ func TestNewPlaceholderWithNodeSelectors(t *testing.T) {
assert.Equal(t, len(holder.pod.Spec.NodeSelector), 2)
assert.Equal(t, holder.pod.Spec.NodeSelector["nodeType"], "test")
assert.Equal(t, holder.pod.Spec.NodeSelector["nodeState"], "healthy")
var priority *int32
assert.Equal(t, priority, holder.pod.Spec.Priority)
}

func TestNewPlaceholderWithTolerations(t *testing.T) {
Expand All @@ -175,6 +181,8 @@ func TestNewPlaceholderWithTolerations(t *testing.T) {
assert.Equal(t, tlr.Value, "value1")
assert.Equal(t, tlr.Operator, v1.TolerationOpEqual)
assert.Equal(t, tlr.Effect, v1.TaintEffectNoSchedule)
var priority *int32
assert.Equal(t, priority, holder.pod.Spec.Priority)
}

func TestNewPlaceholderWithAffinity(t *testing.T) {
Expand All @@ -192,6 +200,8 @@ func TestNewPlaceholderWithAffinity(t *testing.T) {
assert.Equal(t, term[0].LabelSelector.MatchExpressions[0].Key, "service")
assert.Equal(t, term[0].LabelSelector.MatchExpressions[0].Operator, metav1.LabelSelectorOpIn)
assert.Equal(t, term[0].LabelSelector.MatchExpressions[0].Values[0], "securityscan")
var priority *int32
assert.Equal(t, priority, holder.pod.Spec.Priority)
}

func TestNewPlaceholderTaskGroupsDefinition(t *testing.T) {
Expand All @@ -208,6 +218,8 @@ func TestNewPlaceholderTaskGroupsDefinition(t *testing.T) {
app.setTaskGroupsDefinition("taskGroupsDef")
holder = newPlaceholder("ph-name", app, app.taskGroups[0])
assert.Equal(t, "taskGroupsDef", holder.pod.Annotations[constants.AnnotationTaskGroups])
var priority *int32
assert.Equal(t, priority, holder.pod.Spec.Priority)
}

func TestNewPlaceholderExtendedResources(t *testing.T) {
Expand All @@ -220,4 +232,43 @@ func TestNewPlaceholderExtendedResources(t *testing.T) {
assert.Equal(t, len(holder.pod.Spec.Containers[0].Resources.Limits), 2, "limit for extended resource not found")
assert.Equal(t, holder.pod.Spec.Containers[0].Resources.Limits[gpu], holder.pod.Spec.Containers[0].Resources.Requests[gpu], "gpu: expected same value for request and limit")
assert.Equal(t, holder.pod.Spec.Containers[0].Resources.Limits[hugepages], holder.pod.Spec.Containers[0].Resources.Requests[hugepages], "hugepages: expected same value for request and limit")
var priority *int32
assert.Equal(t, priority, holder.pod.Spec.Priority)
}

func TestNewPlaceholderWithPriority(t *testing.T) {
mockedSchedulerAPI := newMockSchedulerAPI()
app := NewApplication(appID, queue,
"bob", testGroups, map[string]string{constants.AppTagNamespace: namespace}, mockedSchedulerAPI)
app.setTaskGroups(taskGroups)
mockedContext := initContextForTest()
priority := int32(10)
specPriority := &priority
pod1 := &v1.Pod{
TypeMeta: metav1.TypeMeta{
Kind: "Pod",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "tg-test-group-1-app01-0",
UID: "UID-01",
},
Spec: v1.PodSpec{
Priority: specPriority,
},
}
taskID1 := "task1-01"
task1 := NewTask(taskID1, app, mockedContext, pod1)
task1.placeholder = true
task1.pod = pod1
task1.originator = true
app.taskMap[taskID1] = task1
app.setOriginatingTask(task1)

holder := newPlaceholder("ph-name", app, app.taskGroups[0])
assert.Equal(t, len(holder.pod.Spec.Containers[0].Resources.Requests), 5, "expected requests not found")
assert.Equal(t, len(holder.pod.Spec.Containers[0].Resources.Limits), 2, "limit for extended resource not found")
assert.Equal(t, holder.pod.Spec.Containers[0].Resources.Limits[gpu], holder.pod.Spec.Containers[0].Resources.Requests[gpu], "gpu: expected same value for request and limit")
assert.Equal(t, holder.pod.Spec.Containers[0].Resources.Limits[hugepages], holder.pod.Spec.Containers[0].Resources.Requests[hugepages], "hugepages: expected same value for request and limit")
assert.Equal(t, priority, *holder.pod.Spec.Priority)
}

0 comments on commit 5b35abc

Please sign in to comment.