You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by cc...@apache.org on 2023/08/24 20:33:31 UTC
[yunikorn-k8shim] branch master updated: [YUNIKORN-1921] Gang-scheduled placeholder pods don't inherit PriorityClasses (#657)
This is an automated email from the ASF dual-hosted git repository.
ccondit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git
The following commit(s) were added to refs/heads/master by this push:
new 5b35abcd [YUNIKORN-1921] Gang-scheduled placeholder pods don't inherit PriorityClasses (#657)
5b35abcd is described below
commit 5b35abcd16f4c489f2aaaa3197f9c2a685db393b
Author: Manikandan R <ma...@gmail.com>
AuthorDate: Thu Aug 24 15:32:45 2023 -0500
[YUNIKORN-1921] Gang-scheduled placeholder pods don't inherit PriorityClasses (#657)
Closes: #657
Signed-off-by: Craig Condit <cc...@apache.org>
---
pkg/cache/placeholder.go | 6 +++++
pkg/cache/placeholder_manager_test.go | 22 +++++++++++++++
pkg/cache/placeholder_test.go | 51 +++++++++++++++++++++++++++++++++++
3 files changed, 79 insertions(+)
diff --git a/pkg/cache/placeholder.go b/pkg/cache/placeholder.go
index 292ddd4a..f3b91bd8 100644
--- a/pkg/cache/placeholder.go
+++ b/pkg/cache/placeholder.go
@@ -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)
@@ -116,6 +121,7 @@ func newPlaceholder(placeholderName string, app *Application, taskGroup interfac
NodeSelector: taskGroup.NodeSelector,
Tolerations: taskGroup.Tolerations,
Affinity: taskGroup.Affinity,
+ Priority: priority,
},
}
diff --git a/pkg/cache/placeholder_manager_test.go b/pkg/cache/placeholder_manager_test.go
index 33db90e6..978f1b75 100644
--- a/pkg/cache/placeholder_manager_test.go
+++ b/pkg/cache/placeholder_manager_test.go
@@ -20,6 +20,7 @@ package cache
import (
"fmt"
+ "strconv"
"testing"
"time"
@@ -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 {
@@ -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
}
@@ -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",
@@ -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{
@@ -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)
diff --git a/pkg/cache/placeholder_test.go b/pkg/cache/placeholder_test.go
index bc5cb647..e7ebdc09 100644
--- a/pkg/cache/placeholder_test.go
+++ b/pkg/cache/placeholder_test.go
@@ -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) {
@@ -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) {
@@ -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) {
@@ -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) {
@@ -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) {
@@ -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) {
@@ -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)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org