From fe7b6e8e17c33157300ea5c6ba8f91cbc8d7c844 Mon Sep 17 00:00:00 2001 From: rrajesh Date: Tue, 3 Sep 2024 21:27:45 +0530 Subject: [PATCH 1/7] [YUNIKORN-2714] Added E2E test to verify queue name with special characters --- .../user_group_limit/user_group_limit_test.go | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/e2e/user_group_limit/user_group_limit_test.go b/test/e2e/user_group_limit/user_group_limit_test.go index e4d74413e..27055e88d 100644 --- a/test/e2e/user_group_limit/user_group_limit_test.go +++ b/test/e2e/user_group_limit/user_group_limit_test.go @@ -641,6 +641,32 @@ var _ = ginkgo.Describe("UserGroupLimit", func() { checkUsage(groupTestType, url.QueryEscape(validGroup), sandboxQueue1, []*v1.Pod{usergroup1Sandbox1Pod1}) }) + ginkgo.It("Verify_Queue_Name_With_Special_Characters", func() { + ginkgo.By("Create a queue with a name that includes all allowed special characters") + queueName := "queue-#_@/:" + + yunikorn.UpdateCustomConfigMapWrapper(oldConfigMap, "", func(sc *configs.SchedulerConfig) error { + // remove placement rules so we can control queue + sc.Partitions[0].PlacementRules = nil + + var err error + if err = common.AddQueue(sc, "default", "root", configs.QueueConfig{ + Name: queueName, + Resources: configs.Resources{Guaranteed: map[string]string{"memory": fmt.Sprintf("%dM", 200)}}, + Properties: map[string]string{"preemption.delay": "1s"}, + }); err != nil { + return err + } + return nil + }) + + ginkgo.By("Fetch the queue information using the REST API") + allQueues, err := restClient.GetQueues("default") + gomega.Ω(err).NotTo(gomega.HaveOccurred()) + ginkgo.By("Verify that the queue information is returned correctly") + gomega.Ω(allQueues.Children[0].QueueName).To(gomega.Equal("root." + queueName)) + }) + ginkgo.AfterEach(func() { tests.DumpClusterInfoIfSpecFailed(suiteName, []string{ns.Name}) From 3a7eaf8769b928256b3a8e84c54e34c7358bef64 Mon Sep 17 00:00:00 2001 From: rrajesh Date: Tue, 3 Sep 2024 21:33:40 +0530 Subject: [PATCH 2/7] [YUNIKORN-2714] Added E2E test to verify queue name with special characters --- test/e2e/user_group_limit/user_group_limit_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/user_group_limit/user_group_limit_test.go b/test/e2e/user_group_limit/user_group_limit_test.go index 27055e88d..594275400 100644 --- a/test/e2e/user_group_limit/user_group_limit_test.go +++ b/test/e2e/user_group_limit/user_group_limit_test.go @@ -659,12 +659,13 @@ var _ = ginkgo.Describe("UserGroupLimit", func() { } return nil }) - ginkgo.By("Fetch the queue information using the REST API") allQueues, err := restClient.GetQueues("default") gomega.Ω(err).NotTo(gomega.HaveOccurred()) ginkgo.By("Verify that the queue information is returned correctly") gomega.Ω(allQueues.Children[0].QueueName).To(gomega.Equal("root." + queueName)) + Qerr := restClient.WaitforQueueToAppear("default", "root."+queueName, 20) + gomega.Ω(Qerr).NotTo(gomega.HaveOccurred()) }) ginkgo.AfterEach(func() { From 3e00ea4f3b44eaecf6b801c6f4e37bed8b500d1a Mon Sep 17 00:00:00 2001 From: rrajesh Date: Wed, 4 Sep 2024 11:43:15 +0530 Subject: [PATCH 3/7] [YUNIKORN-2714] Added E2E test to verify queue name with special characters --- test/e2e/user_group_limit/user_group_limit_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/e2e/user_group_limit/user_group_limit_test.go b/test/e2e/user_group_limit/user_group_limit_test.go index 594275400..916128354 100644 --- a/test/e2e/user_group_limit/user_group_limit_test.go +++ b/test/e2e/user_group_limit/user_group_limit_test.go @@ -663,7 +663,14 @@ var _ = ginkgo.Describe("UserGroupLimit", func() { allQueues, err := restClient.GetQueues("default") gomega.Ω(err).NotTo(gomega.HaveOccurred()) ginkgo.By("Verify that the queue information is returned correctly") - gomega.Ω(allQueues.Children[0].QueueName).To(gomega.Equal("root." + queueName)) + // loop through allQueues.children to find the queue with the name we created + for _, queue := range allQueues.Children { + if queue.QueueName == "root."+queueName { + gomega.Ω(queue.QueueName).To(gomega.Equal("root." + queueName)) + } else { + gomega.Ω(queue.QueueName).NotTo(gomega.Equal("root." + queueName)) + } + } Qerr := restClient.WaitforQueueToAppear("default", "root."+queueName, 20) gomega.Ω(Qerr).NotTo(gomega.HaveOccurred()) }) From fd40135768110d34176cbe5b1959f45bae9365a7 Mon Sep 17 00:00:00 2001 From: rrajesh Date: Wed, 4 Sep 2024 17:10:35 +0530 Subject: [PATCH 4/7] [YUNIKORN-2714] Added E2E test to verify queue name with special characters --- test/e2e/user_group_limit/user_group_limit_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/e2e/user_group_limit/user_group_limit_test.go b/test/e2e/user_group_limit/user_group_limit_test.go index 916128354..9be8a3d1f 100644 --- a/test/e2e/user_group_limit/user_group_limit_test.go +++ b/test/e2e/user_group_limit/user_group_limit_test.go @@ -643,7 +643,7 @@ var _ = ginkgo.Describe("UserGroupLimit", func() { ginkgo.It("Verify_Queue_Name_With_Special_Characters", func() { ginkgo.By("Create a queue with a name that includes all allowed special characters") - queueName := "queue-#_@/:" + queueName := "root_test22-a_b_#_c_#_d__e@dom:ain" yunikorn.UpdateCustomConfigMapWrapper(oldConfigMap, "", func(sc *configs.SchedulerConfig) error { // remove placement rules so we can control queue @@ -671,8 +671,9 @@ var _ = ginkgo.Describe("UserGroupLimit", func() { gomega.Ω(queue.QueueName).NotTo(gomega.Equal("root." + queueName)) } } - Qerr := restClient.WaitforQueueToAppear("default", "root."+queueName, 20) - gomega.Ω(Qerr).NotTo(gomega.HaveOccurred()) + queueInfo, err := restClient.GetQueue("default", "root."+queueName, false) + gomega.Ω(err).NotTo(gomega.HaveOccurred()) + gomega.Ω(queueInfo.QueueName).To(gomega.Equal("root." + queueName)) }) ginkgo.AfterEach(func() { From 2a057c74afcdd554244133bd5778a2d1dbde5cc7 Mon Sep 17 00:00:00 2001 From: rrajesh Date: Thu, 5 Sep 2024 11:32:52 +0530 Subject: [PATCH 5/7] [YUNIKORN-2714] Added E2E test to verify queue name with special characters --- .../user_group_limit/user_group_limit_test.go | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/test/e2e/user_group_limit/user_group_limit_test.go b/test/e2e/user_group_limit/user_group_limit_test.go index 9be8a3d1f..85892a6a4 100644 --- a/test/e2e/user_group_limit/user_group_limit_test.go +++ b/test/e2e/user_group_limit/user_group_limit_test.go @@ -25,9 +25,11 @@ import ( "runtime" "time" + "github.com/apache/yunikorn-k8shim/test/e2e/framework/configmanager" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/apache/yunikorn-core/pkg/common/configs" "github.com/apache/yunikorn-core/pkg/common/resources" @@ -643,12 +645,10 @@ var _ = ginkgo.Describe("UserGroupLimit", func() { ginkgo.It("Verify_Queue_Name_With_Special_Characters", func() { ginkgo.By("Create a queue with a name that includes all allowed special characters") - queueName := "root_test22-a_b_#_c_#_d__e@dom:ain" - + queueName := "root_test22-a_b_#_c_#_d_/_e@dom:ain" yunikorn.UpdateCustomConfigMapWrapper(oldConfigMap, "", func(sc *configs.SchedulerConfig) error { // remove placement rules so we can control queue sc.Partitions[0].PlacementRules = nil - var err error if err = common.AddQueue(sc, "default", "root", configs.QueueConfig{ Name: queueName, @@ -671,11 +671,53 @@ var _ = ginkgo.Describe("UserGroupLimit", func() { gomega.Ω(queue.QueueName).NotTo(gomega.Equal("root." + queueName)) } } - queueInfo, err := restClient.GetQueue("default", "root."+queueName, false) + queueInfo, err := restClient.GetQueue("default", "root."+url.QueryEscape(queueName), false) gomega.Ω(err).NotTo(gomega.HaveOccurred()) gomega.Ω(queueInfo.QueueName).To(gomega.Equal("root." + queueName)) }) + ginkgo.It("Verify_Queue_Name_With_Disallowed_Special_Characters", func() { + ginkgo.By("Attempt to create a queue with a name that includes disallowed special characters") + invalidConfig := ` + partitions: + - name: default + placementrules: + - name: tag + value: namespace + create: true + queues: + - name: root_test22-a_b_#_c_#_d__e@dom:ain$ + submitacl: '*' +` + queueName := "root_test22-a_b_#_c_#_d__e@dom:ain$" + invalidConfigData := map[string]string{"queues.yaml": invalidConfig} + invalidConfigMap := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.ConfigMapName, + Namespace: configmanager.YuniKornTestConfig.YkNamespace, + }, + Data: invalidConfigData, + } + _, invalidConfigErr := kClient.UpdateConfigMap(invalidConfigMap, configmanager.YuniKornTestConfig.YkNamespace) + gomega.Ω(invalidConfigErr).Should(gomega.HaveOccurred()) + ginkgo.By("Verify that the queue was not created") + _, err := restClient.GetQueue("default", "root."+url.QueryEscape(queueName), false) + gomega.Ω(err).To(gomega.HaveOccurred()) // Expect an error + queueName2 := "root_test22" + yunikorn.UpdateCustomConfigMapWrapper(oldConfigMap, "", func(sc *configs.SchedulerConfig) error { + // remove placement rules so we can control queue + sc.Partitions[0].PlacementRules = nil + var err error + if err = common.AddQueue(sc, "default", "root", configs.QueueConfig{ + Name: queueName2, + Resources: configs.Resources{Guaranteed: map[string]string{"memory": fmt.Sprintf("%dM", 200)}}, + Properties: map[string]string{"preemption.delay": "1s"}, + }); err != nil { + return err + } + return nil + }) + }) ginkgo.AfterEach(func() { tests.DumpClusterInfoIfSpecFailed(suiteName, []string{ns.Name}) From 0afbc2ea7b9973d76e1daa7646808702eb4c324f Mon Sep 17 00:00:00 2001 From: rrajesh Date: Thu, 5 Sep 2024 15:41:42 +0530 Subject: [PATCH 6/7] [YUNIKORN-2714] Goimport issue fix --- test/e2e/user_group_limit/user_group_limit_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/e2e/user_group_limit/user_group_limit_test.go b/test/e2e/user_group_limit/user_group_limit_test.go index 85892a6a4..70191ba34 100644 --- a/test/e2e/user_group_limit/user_group_limit_test.go +++ b/test/e2e/user_group_limit/user_group_limit_test.go @@ -25,12 +25,6 @@ import ( "runtime" "time" - "github.com/apache/yunikorn-k8shim/test/e2e/framework/configmanager" - "github.com/onsi/ginkgo/v2" - "github.com/onsi/gomega" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/apache/yunikorn-core/pkg/common/configs" "github.com/apache/yunikorn-core/pkg/common/resources" "github.com/apache/yunikorn-core/pkg/webservice/dao" @@ -38,11 +32,18 @@ import ( amconf "github.com/apache/yunikorn-k8shim/pkg/admission/conf" "github.com/apache/yunikorn-k8shim/pkg/common/constants" tests "github.com/apache/yunikorn-k8shim/test/e2e" + "github.com/apache/yunikorn-k8shim/test/e2e/framework/configmanager" "github.com/apache/yunikorn-k8shim/test/e2e/framework/helpers/common" "github.com/apache/yunikorn-k8shim/test/e2e/framework/helpers/k8s" "github.com/apache/yunikorn-k8shim/test/e2e/framework/helpers/yunikorn" siCommon "github.com/apache/yunikorn-scheduler-interface/lib/go/common" "github.com/apache/yunikorn-scheduler-interface/lib/go/si" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) type TestType int From f4e840b85c4325849696b8f02fa39a0721e3cf95 Mon Sep 17 00:00:00 2001 From: rrajesh Date: Fri, 6 Sep 2024 18:15:55 +0530 Subject: [PATCH 7/7] [YUNIKORN-2714] Removing the dead code --- test/e2e/user_group_limit/user_group_limit_test.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/test/e2e/user_group_limit/user_group_limit_test.go b/test/e2e/user_group_limit/user_group_limit_test.go index 70191ba34..eac84c2ba 100644 --- a/test/e2e/user_group_limit/user_group_limit_test.go +++ b/test/e2e/user_group_limit/user_group_limit_test.go @@ -661,17 +661,6 @@ var _ = ginkgo.Describe("UserGroupLimit", func() { return nil }) ginkgo.By("Fetch the queue information using the REST API") - allQueues, err := restClient.GetQueues("default") - gomega.Ω(err).NotTo(gomega.HaveOccurred()) - ginkgo.By("Verify that the queue information is returned correctly") - // loop through allQueues.children to find the queue with the name we created - for _, queue := range allQueues.Children { - if queue.QueueName == "root."+queueName { - gomega.Ω(queue.QueueName).To(gomega.Equal("root." + queueName)) - } else { - gomega.Ω(queue.QueueName).NotTo(gomega.Equal("root." + queueName)) - } - } queueInfo, err := restClient.GetQueue("default", "root."+url.QueryEscape(queueName), false) gomega.Ω(err).NotTo(gomega.HaveOccurred()) gomega.Ω(queueInfo.QueueName).To(gomega.Equal("root." + queueName))