From 5a9b7607b275a490f516590f53bb1529781992ca Mon Sep 17 00:00:00 2001 From: qzhu Date: Tue, 16 May 2023 15:43:15 +0800 Subject: [PATCH 1/7] [YUNIKORN-1739] Add e2e test to test originator pod will control all other placeholders. --- .../gang_scheduling/gang_scheduling_test.go | 159 +++++++++++++++++- 1 file changed, 158 insertions(+), 1 deletion(-) diff --git a/test/e2e/gang_scheduling/gang_scheduling_test.go b/test/e2e/gang_scheduling/gang_scheduling_test.go index 16a8de2b5..8b42d0b4c 100644 --- a/test/e2e/gang_scheduling/gang_scheduling_test.go +++ b/test/e2e/gang_scheduling/gang_scheduling_test.go @@ -20,6 +20,7 @@ package gangscheduling_test import ( "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "time" "github.com/onsi/ginkgo/v2" @@ -880,7 +881,7 @@ var _ = Describe("", func() { By("Verify placeholders deleted") for _, placeholders := range phNames { for _, ph := range placeholders { - deleteErr = kClient.WaitForPodTerminated(fifoQName, ph, 30*time.Second) + deleteErr = kClient.WaitForPodTerminated(ns, ph, 30*time.Second) Ω(deleteErr).NotTo(HaveOccurred(), "Placeholder %s still running", ph) } } @@ -891,6 +892,162 @@ var _ = Describe("", func() { Ω(len(appInfo.Allocations)).To(BeNumerically("==", 0)) }) + // Test to verify originator deletion will trigger placeholders cleanup + // 1. Create an originator pod + // 2. Set pod ownerreference (case1 without ownerreference, case2 with ownerreference) + // 3. Delete originator pod to trigger placeholders deletion + // 4. Verify placeholders deleted + // 5. Verify app allocation is empty + It("Verify_OriginatorDeletion_Trigger_Placeholders_Cleanup", func() { + + // case 1: originator pod without ownerreference + podResources := map[string]resource.Quantity{ + "cpu": resource.MustParse("10m"), + "memory": resource.MustParse("10M"), + } + podConf := k8s.TestPodConfig{ + Name: "gang-driver-pod" + common.RandSeq(5), + Labels: map[string]string{ + "app": "sleep-" + common.RandSeq(5), + "applicationId": "appid-" + common.RandSeq(5), + }, + Annotations: &k8s.PodAnnotation{ + TaskGroups: []v1alpha1.TaskGroup{ + { + Name: groupA + "-" + common.RandSeq(5), + MinMember: int32(3), + MinResource: podResources, + NodeSelector: map[string]string{"kubernetes.io/hostname": "unsatisfiable"}, + }, + { + Name: groupB + "-" + common.RandSeq(5), + MinMember: int32(3), + MinResource: podResources, + }, + }, + }, + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + "cpu": podResources["cpu"], + "memory": podResources["memory"], + }, + }, + } + + podTest, err := k8s.InitTestPod(podConf) + Ω(err).NotTo(HaveOccurred()) + taskGroupsMap, annErr := k8s.PodAnnotationToMap(podConf.Annotations) + Ω(annErr).NotTo(HaveOccurred()) + By(fmt.Sprintf("Deploy pod %s with task-groups: %+v", podTest.Name, taskGroupsMap[k8s.TaskGroups])) + originator, err := kClient.CreatePod(podTest, ns) + Ω(err).NotTo(HaveOccurred()) + + By("Verify appState = Accepted") + timeoutErr := restClient.WaitForAppStateTransition(defaultPartition, nsQueue, podConf.Labels["applicationId"], yunikorn.States().Application.Accepted, 20) + Ω(timeoutErr).NotTo(HaveOccurred()) + + By("Wait for placeholders running") + phNames := yunikorn.GetPlaceholderNames(podConf.Annotations, podConf.Labels["applicationId"]) + tgBNames := phNames[podConf.Annotations.TaskGroups[1].Name] + for _, ph := range tgBNames { + runErr := kClient.WaitForPodRunning(ns, ph, 30*time.Second) + Ω(runErr).NotTo(HaveOccurred()) + } + + By("Delete originator pod") + deleteErr := kClient.DeletePod(originator.Name, ns) + Ω(deleteErr).NotTo(HaveOccurred()) + + By("Verify placeholders deleted") + for _, placeholders := range phNames { + for _, ph := range placeholders { + deleteErr := kClient.WaitForPodTerminated(ns, ph, 30*time.Second) + Ω(deleteErr).NotTo(HaveOccurred(), "Placeholder %s still running", ph) + } + } + + By("Verify app allocation is empty") + appInfo, restErr := restClient.GetAppInfo(defaultPartition, nsQueue, podConf.Labels["applicationId"]) + Ω(restErr).NotTo(HaveOccurred()) + Ω(len(appInfo.Allocations)).To(BeNumerically("==", 0)) + + // case 2: originator pod with ownerreference + podConf = k8s.TestPodConfig{ + Name: "gang-driver-pod" + common.RandSeq(5), + Labels: map[string]string{ + "app": "sleep-" + common.RandSeq(5), + "applicationId": "appid-" + common.RandSeq(5), + }, + Annotations: &k8s.PodAnnotation{ + TaskGroups: []v1alpha1.TaskGroup{ + { + Name: groupA + "-" + common.RandSeq(5), + MinMember: int32(3), + MinResource: podResources, + NodeSelector: map[string]string{"kubernetes.io/hostname": "unsatisfiable"}, + }, + { + Name: groupB + "-" + common.RandSeq(5), + MinMember: int32(3), + MinResource: podResources, + }, + }, + }, + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + "cpu": podResources["cpu"], + "memory": podResources["memory"], + }, + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "ConfigMap", + Name: "driver-configmap", + UID: "driver-configmap-uid", + }, + }, + } + + podTest, err = k8s.InitTestPod(podConf) + Ω(err).NotTo(HaveOccurred()) + taskGroupsMap, annErr = k8s.PodAnnotationToMap(podConf.Annotations) + Ω(annErr).NotTo(HaveOccurred()) + By(fmt.Sprintf("Deploy pod %s with task-groups: %+v", podTest.Name, taskGroupsMap[k8s.TaskGroups])) + originator, err = kClient.CreatePod(podTest, ns) + Ω(err).NotTo(HaveOccurred()) + + By("Verify appState = Accepted") + timeoutErr = restClient.WaitForAppStateTransition(defaultPartition, nsQueue, podConf.Labels["applicationId"], yunikorn.States().Application.Accepted, 20) + Ω(timeoutErr).NotTo(HaveOccurred()) + + By("Wait for placeholders running") + phNames = yunikorn.GetPlaceholderNames(podConf.Annotations, podConf.Labels["applicationId"]) + tgBNames = phNames[podConf.Annotations.TaskGroups[1].Name] + for _, ph := range tgBNames { + runErr := kClient.WaitForPodRunning(ns, ph, 30*time.Second) + Ω(runErr).NotTo(HaveOccurred()) + } + + By("Delete originator pod") + deleteErr = kClient.DeletePod(originator.Name, ns) + Ω(deleteErr).NotTo(HaveOccurred()) + + By("Verify placeholders deleted") + for _, placeholders := range phNames { + for _, ph := range placeholders { + deleteErr := kClient.WaitForPodTerminated(ns, ph, 30*time.Second) + Ω(deleteErr).NotTo(HaveOccurred(), "Placeholder %s still running", ph) + } + } + + By("Verify app allocation is empty") + appInfo, restErr = restClient.GetAppInfo(defaultPartition, nsQueue, podConf.Labels["applicationId"]) + Ω(restErr).NotTo(HaveOccurred()) + Ω(len(appInfo.Allocations)).To(BeNumerically("==", 0)) + + }) + ginkgo.DescribeTable("", func(annotations k8s.PodAnnotation) { podConf := k8s.TestPodConfig{ Labels: map[string]string{ From c3724da27c33d2bc2f9deca264baff67b3bf0acd Mon Sep 17 00:00:00 2001 From: qzhu Date: Tue, 16 May 2023 16:03:46 +0800 Subject: [PATCH 2/7] Fix golint --- test/e2e/gang_scheduling/gang_scheduling_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/e2e/gang_scheduling/gang_scheduling_test.go b/test/e2e/gang_scheduling/gang_scheduling_test.go index 8b42d0b4c..893ddb47f 100644 --- a/test/e2e/gang_scheduling/gang_scheduling_test.go +++ b/test/e2e/gang_scheduling/gang_scheduling_test.go @@ -20,9 +20,10 @@ package gangscheduling_test import ( "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" @@ -961,7 +962,7 @@ var _ = Describe("", func() { By("Verify placeholders deleted") for _, placeholders := range phNames { for _, ph := range placeholders { - deleteErr := kClient.WaitForPodTerminated(ns, ph, 30*time.Second) + deleteErr = kClient.WaitForPodTerminated(ns, ph, 30*time.Second) Ω(deleteErr).NotTo(HaveOccurred(), "Placeholder %s still running", ph) } } @@ -1036,7 +1037,7 @@ var _ = Describe("", func() { By("Verify placeholders deleted") for _, placeholders := range phNames { for _, ph := range placeholders { - deleteErr := kClient.WaitForPodTerminated(ns, ph, 30*time.Second) + deleteErr = kClient.WaitForPodTerminated(ns, ph, 30*time.Second) Ω(deleteErr).NotTo(HaveOccurred(), "Placeholder %s still running", ph) } } From 5a6da826c42cef65dce3ccd5f97e1626d19032b1 Mon Sep 17 00:00:00 2001 From: qzhu Date: Tue, 16 May 2023 17:48:55 +0800 Subject: [PATCH 3/7] fix test. --- .../gang_scheduling/gang_scheduling_test.go | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/test/e2e/gang_scheduling/gang_scheduling_test.go b/test/e2e/gang_scheduling/gang_scheduling_test.go index 893ddb47f..15042d7b4 100644 --- a/test/e2e/gang_scheduling/gang_scheduling_test.go +++ b/test/e2e/gang_scheduling/gang_scheduling_test.go @@ -895,7 +895,7 @@ var _ = Describe("", func() { // Test to verify originator deletion will trigger placeholders cleanup // 1. Create an originator pod - // 2. Set pod ownerreference (case1 without ownerreference, case2 with ownerreference) + // 2. Set pod ownerreference without ownerreference // 3. Delete originator pod to trigger placeholders deletion // 4. Verify placeholders deleted // 5. Verify app allocation is empty @@ -972,8 +972,23 @@ var _ = Describe("", func() { Ω(restErr).NotTo(HaveOccurred()) Ω(len(appInfo.Allocations)).To(BeNumerically("==", 0)) + }) + + // Test to verify originator deletion will trigger placeholders cleanup + // 1. Create an originator pod + // 2. Set pod ownerreference with ownerreference, take configmap for example + // 3. Delete originator pod to trigger placeholders deletion + // 4. Verify placeholders deleted + // 5. Verify app allocation is empty + It("Verify_OriginatorDeletionWithOwnerreference_Trigger_Placeholders_Cleanup", func() { // case 2: originator pod with ownerreference - podConf = k8s.TestPodConfig{ + + // case 1: originator pod without ownerreference + podResources := map[string]resource.Quantity{ + "cpu": resource.MustParse("10m"), + "memory": resource.MustParse("10M"), + } + podConf := k8s.TestPodConfig{ Name: "gang-driver-pod" + common.RandSeq(5), Labels: map[string]string{ "app": "sleep-" + common.RandSeq(5), @@ -1010,28 +1025,28 @@ var _ = Describe("", func() { }, } - podTest, err = k8s.InitTestPod(podConf) + podTest, err := k8s.InitTestPod(podConf) Ω(err).NotTo(HaveOccurred()) - taskGroupsMap, annErr = k8s.PodAnnotationToMap(podConf.Annotations) + taskGroupsMap, annErr := k8s.PodAnnotationToMap(podConf.Annotations) Ω(annErr).NotTo(HaveOccurred()) By(fmt.Sprintf("Deploy pod %s with task-groups: %+v", podTest.Name, taskGroupsMap[k8s.TaskGroups])) - originator, err = kClient.CreatePod(podTest, ns) + originator, err := kClient.CreatePod(podTest, ns) Ω(err).NotTo(HaveOccurred()) By("Verify appState = Accepted") - timeoutErr = restClient.WaitForAppStateTransition(defaultPartition, nsQueue, podConf.Labels["applicationId"], yunikorn.States().Application.Accepted, 20) + timeoutErr := restClient.WaitForAppStateTransition(defaultPartition, nsQueue, podConf.Labels["applicationId"], yunikorn.States().Application.Accepted, 20) Ω(timeoutErr).NotTo(HaveOccurred()) By("Wait for placeholders running") - phNames = yunikorn.GetPlaceholderNames(podConf.Annotations, podConf.Labels["applicationId"]) - tgBNames = phNames[podConf.Annotations.TaskGroups[1].Name] + phNames := yunikorn.GetPlaceholderNames(podConf.Annotations, podConf.Labels["applicationId"]) + tgBNames := phNames[podConf.Annotations.TaskGroups[1].Name] for _, ph := range tgBNames { runErr := kClient.WaitForPodRunning(ns, ph, 30*time.Second) Ω(runErr).NotTo(HaveOccurred()) } By("Delete originator pod") - deleteErr = kClient.DeletePod(originator.Name, ns) + deleteErr := kClient.DeletePod(originator.Name, ns) Ω(deleteErr).NotTo(HaveOccurred()) By("Verify placeholders deleted") @@ -1043,10 +1058,9 @@ var _ = Describe("", func() { } By("Verify app allocation is empty") - appInfo, restErr = restClient.GetAppInfo(defaultPartition, nsQueue, podConf.Labels["applicationId"]) + appInfo, restErr := restClient.GetAppInfo(defaultPartition, nsQueue, podConf.Labels["applicationId"]) Ω(restErr).NotTo(HaveOccurred()) Ω(len(appInfo.Allocations)).To(BeNumerically("==", 0)) - }) ginkgo.DescribeTable("", func(annotations k8s.PodAnnotation) { From c56f4aeca981ff36b646a600bd058d73181332bb Mon Sep 17 00:00:00 2001 From: qzhu Date: Tue, 16 May 2023 17:49:50 +0800 Subject: [PATCH 4/7] fix --- test/e2e/gang_scheduling/gang_scheduling_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/e2e/gang_scheduling/gang_scheduling_test.go b/test/e2e/gang_scheduling/gang_scheduling_test.go index 15042d7b4..e783e4547 100644 --- a/test/e2e/gang_scheduling/gang_scheduling_test.go +++ b/test/e2e/gang_scheduling/gang_scheduling_test.go @@ -900,7 +900,6 @@ var _ = Describe("", func() { // 4. Verify placeholders deleted // 5. Verify app allocation is empty It("Verify_OriginatorDeletion_Trigger_Placeholders_Cleanup", func() { - // case 1: originator pod without ownerreference podResources := map[string]resource.Quantity{ "cpu": resource.MustParse("10m"), @@ -982,8 +981,6 @@ var _ = Describe("", func() { // 5. Verify app allocation is empty It("Verify_OriginatorDeletionWithOwnerreference_Trigger_Placeholders_Cleanup", func() { // case 2: originator pod with ownerreference - - // case 1: originator pod without ownerreference podResources := map[string]resource.Quantity{ "cpu": resource.MustParse("10m"), "memory": resource.MustParse("10M"), From 897be4aad168bce8cdbe1027421e5285f730b739 Mon Sep 17 00:00:00 2001 From: qzhu Date: Wed, 17 May 2023 10:51:19 +0800 Subject: [PATCH 5/7] create configmap --- .../gang_scheduling/gang_scheduling_test.go | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/test/e2e/gang_scheduling/gang_scheduling_test.go b/test/e2e/gang_scheduling/gang_scheduling_test.go index e783e4547..53b30642c 100644 --- a/test/e2e/gang_scheduling/gang_scheduling_test.go +++ b/test/e2e/gang_scheduling/gang_scheduling_test.go @@ -1012,18 +1012,35 @@ var _ = Describe("", func() { "memory": podResources["memory"], }, }, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: "v1", - Kind: "ConfigMap", - Name: "driver-configmap", - UID: "driver-configmap-uid", - }, + } + + // create a configmap as ownerreference + testConfigmap := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + UID: "test-cm-uid", + }, + Data: map[string]string{ + "test": "test", }, } + defer kClient.DeleteConfigMap(testConfigmap.Name, ns) + + testConfigmap, err := kClient.CreateConfigMap(testConfigmap, ns) + Ω(err).NotTo(HaveOccurred()) podTest, err := k8s.InitTestPod(podConf) Ω(err).NotTo(HaveOccurred()) + + podTest.OwnerReferences = []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "ConfigMap", + Name: testConfigmap.Name, + UID: testConfigmap.UID, + }, + } + taskGroupsMap, annErr := k8s.PodAnnotationToMap(podConf.Annotations) Ω(annErr).NotTo(HaveOccurred()) By(fmt.Sprintf("Deploy pod %s with task-groups: %+v", podTest.Name, taskGroupsMap[k8s.TaskGroups])) From 36ddb44e0454629ca94d146e9f2d4aec23da5277 Mon Sep 17 00:00:00 2001 From: qzhu Date: Wed, 17 May 2023 11:10:49 +0800 Subject: [PATCH 6/7] Fix golint --- test/e2e/gang_scheduling/gang_scheduling_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/e2e/gang_scheduling/gang_scheduling_test.go b/test/e2e/gang_scheduling/gang_scheduling_test.go index 53b30642c..02fd3daa5 100644 --- a/test/e2e/gang_scheduling/gang_scheduling_test.go +++ b/test/e2e/gang_scheduling/gang_scheduling_test.go @@ -1024,7 +1024,10 @@ var _ = Describe("", func() { "test": "test", }, } - defer kClient.DeleteConfigMap(testConfigmap.Name, ns) + defer func() { + err := kClient.DeleteConfigMap(testConfigmap.Name, ns) + Ω(err).NotTo(HaveOccurred()) + }() testConfigmap, err := kClient.CreateConfigMap(testConfigmap, ns) Ω(err).NotTo(HaveOccurred()) From b81c9b3ebdfd0ef073ed947b4df29544623c42eb Mon Sep 17 00:00:00 2001 From: qzhu Date: Wed, 17 May 2023 12:02:56 +0800 Subject: [PATCH 7/7] Trigger again. --- test/e2e/gang_scheduling/gang_scheduling_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/gang_scheduling/gang_scheduling_test.go b/test/e2e/gang_scheduling/gang_scheduling_test.go index 02fd3daa5..1653c1ed3 100644 --- a/test/e2e/gang_scheduling/gang_scheduling_test.go +++ b/test/e2e/gang_scheduling/gang_scheduling_test.go @@ -895,7 +895,7 @@ var _ = Describe("", func() { // Test to verify originator deletion will trigger placeholders cleanup // 1. Create an originator pod - // 2. Set pod ownerreference without ownerreference + // 2. Not set pod ownerreference // 3. Delete originator pod to trigger placeholders deletion // 4. Verify placeholders deleted // 5. Verify app allocation is empty