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 <[email protected]>
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 <[email protected]>
---
 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: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to