This is an automated email from the ASF dual-hosted git repository.
mani pushed a commit to branch branch-1.5
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git
The following commit(s) were added to refs/heads/branch-1.5 by this push:
new 700e2953 [YUNIKORN-2665] Gang app originator pod changes after restart
(#855)
700e2953 is described below
commit 700e29533f343537e2a86984cb19c9b2138c54df
Author: Manikandan R <maniraj...@gmail.com>
AuthorDate: Fri Jun 7 14:27:47 2024 +0530
[YUNIKORN-2665] Gang app originator pod changes after restart (#855)
Closes: #855
Signed-off-by: Manikandan R <maniraj...@gmail.com>
(cherry picked from commit a2deb72675bdb1c1c1819b307330c502f7004b48)
---
pkg/cache/context.go | 3 +-
pkg/cache/context_test.go | 88 +++++++++++++++++++++++++++++++++++++++++++++++
pkg/cache/metadata.go | 11 +++++-
3 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/pkg/cache/context.go b/pkg/cache/context.go
index 3f162856..ca15fc8e 100644
--- a/pkg/cache/context.go
+++ b/pkg/cache/context.go
@@ -1115,7 +1115,8 @@ func (ctx *Context) addTask(request *AddTaskRequest)
*Task {
// Is this task the originator of the application?
// If yes, then make it as "first pod/owner/driver" of
the application and set the task as originator
- if app.GetOriginatingTask() == nil {
+ // At any cost, placeholder cannot become originator
+ if !request.Metadata.Placeholder &&
app.GetOriginatingTask() == nil {
for _, ownerReference := range
app.getPlaceholderOwnerReferences() {
referenceID :=
string(ownerReference.UID)
if request.Metadata.TaskID ==
referenceID {
diff --git a/pkg/cache/context_test.go b/pkg/cache/context_test.go
index 7b277cbb..1926683b 100644
--- a/pkg/cache/context_test.go
+++ b/pkg/cache/context_test.go
@@ -2232,6 +2232,94 @@ func TestAssumePod_PodNotFound(t *testing.T) {
assert.Equal(t, podInCache.Spec.NodeName, "", "NodeName in pod spec was
set unexpectedly")
}
+// TestOriginatorPodAfterRestart Test to ensure originator pod remains same
even after restart. After restart, ordering of pods may change which can lead to
+// incorrect originator pod selection. Instead of doing actual restart, create
a situation where in pods are being processed in any random order.
+// For example, placeholders are processed first and then real driver pod.
+// Ensure ordering of pods doesn't have any impact on the originator.
+func TestOriginatorPodAfterRestart(t *testing.T) {
+ context := initContextForTest()
+ controller := false
+ blockOwnerDeletion := true
+ ref := apis.OwnerReference{
+ APIVersion: "v1",
+ Kind: "Pod",
+ Name: "originator-01",
+ UID: "UID-00001",
+ Controller: &controller,
+ BlockOwnerDeletion: &blockOwnerDeletion,
+ }
+ ownerRefs := []apis.OwnerReference{ref}
+
+ // Real driver pod
+ pod1 := &v1.Pod{
+ TypeMeta: apis.TypeMeta{
+ Kind: "Pod",
+ APIVersion: "v1",
+ },
+ ObjectMeta: apis.ObjectMeta{
+ Name: "originator-01",
+ UID: "UID-00001",
+ Labels: map[string]string{
+ "applicationId": "spark-app-01",
+ "queue": "root.a",
+ },
+ },
+ Spec: v1.PodSpec{SchedulerName: "yunikorn"},
+ }
+
+ // Placeholder pod 1
+ pod2 := &v1.Pod{
+ TypeMeta: apis.TypeMeta{
+ Kind: "Pod",
+ APIVersion: "v1",
+ },
+ ObjectMeta: apis.ObjectMeta{
+ Name: "placeholder-01",
+ UID: "placeholder-01",
+ Labels: map[string]string{
+ "applicationId": "spark-app-01",
+ "queue": "root.a",
+ },
+ Annotations: map[string]string{
+ constants.AnnotationPlaceholderFlag: "true",
+ },
+ OwnerReferences: ownerRefs, // Add owner references
because every ph reuse the app placeholder owner references.
+ },
+ Spec: v1.PodSpec{SchedulerName: "yunikorn"},
+ }
+
+ // Placeholder pod 1
+ pod3 := &v1.Pod{
+ TypeMeta: apis.TypeMeta{
+ Kind: "Pod",
+ APIVersion: "v1",
+ },
+ ObjectMeta: apis.ObjectMeta{
+ Name: "placeholder-02",
+ UID: "placeholder-02",
+ Labels: map[string]string{
+ "applicationId": "spark-app-01",
+ "queue": "root.a",
+ },
+ Annotations: map[string]string{
+ constants.AnnotationPlaceholderFlag: "true",
+ },
+ OwnerReferences: ownerRefs, // Add owner references
because every ph reuse the app placeholder owner references.
+ },
+ Spec: v1.PodSpec{SchedulerName: "yunikorn"},
+ }
+
+ // Add the ph pods first and then real driver pod at the last
+ context.AddPod(pod3)
+ context.AddPod(pod2)
+ context.AddPod(pod1)
+
+ app := context.getApplication("spark-app-01")
+ assert.Equal(t, app.originatingTask.taskID, "UID-00001")
+ assert.Equal(t, len(app.GetPlaceHolderTasks()), 2)
+ assert.Equal(t, len(app.GetAllocatedTasks()), 0)
+}
+
func initAssumePodTest(binder *test.VolumeBinderMock) *Context {
context, apiProvider := initContextAndAPIProviderForTest()
if binder != nil {
diff --git a/pkg/cache/metadata.go b/pkg/cache/metadata.go
index b3b03873..39533fa3 100644
--- a/pkg/cache/metadata.go
+++ b/pkg/cache/metadata.go
@@ -110,7 +110,16 @@ func getAppMetadata(pod *v1.Pod) (ApplicationMetadata,
bool) {
tags[constants.AnnotationTaskGroups] =
pod.Annotations[constants.AnnotationTaskGroups]
}
- ownerReferences := getOwnerReference(pod)
+ var ownerReferences []metav1.OwnerReference
+ // When app created for the first time, app owner references has been
set based on the real driver pod.
+ // Same owner references would be used for all placeholders. During
recovery, when processing any placeholder pod,
+ // don't derive the owner references unlike real driver pod. Just use
owner references as is because it has been copied from app owner references
itself.
+ if utils.GetPlaceholderFlagFromPodSpec(pod) {
+ ownerReferences = pod.GetOwnerReferences()
+ } else {
+ ownerReferences = getOwnerReference(pod)
+ }
+
schedulingPolicyParams := GetSchedulingPolicyParam(pod)
tags[constants.AnnotationSchedulingPolicyParam] =
pod.Annotations[constants.AnnotationSchedulingPolicyParam]
creationTime := pod.CreationTimestamp.Unix()
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: issues-h...@yunikorn.apache.org
