[GitHub] [incubator-yunikorn-k8shim] yangwwei commented on a change in pull request #177: [YUNIKORN-356] Add e2e tests for state aware app scheduling

[GitBox <gi...@apache.org>]

yangwwei commented on a change in pull request #177:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/177#discussion_r467622191



##########
File path: test/e2e/state_aware_app_scheduling/state_aware_app_scheduling_suite_test.go
##########
@@ -0,0 +1,104 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package stateawareappscheduling_test
+
+import (
+	"path/filepath"
+	"testing"
+
+	v1 "k8s.io/api/core/v1"
+
+	"github.com/apache/incubator-yunikorn-k8shim/test/e2e/framework/helpers/k8s"
+
+	"github.com/onsi/ginkgo/reporters"
+
+	"github.com/onsi/ginkgo"
+	"github.com/onsi/gomega"
+
+	"github.com/apache/incubator-yunikorn-k8shim/test/e2e/framework/configmanager"

Review comment:
       Please sort the imports

##########
File path: test/e2e/state_aware_app_scheduling/state_aware_app_scheduling_suite_test.go
##########
@@ -0,0 +1,104 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package stateawareappscheduling_test
+
+import (
+	"path/filepath"
+	"testing"
+
+	v1 "k8s.io/api/core/v1"
+
+	"github.com/apache/incubator-yunikorn-k8shim/test/e2e/framework/helpers/k8s"
+
+	"github.com/onsi/ginkgo/reporters"
+
+	"github.com/onsi/ginkgo"
+	"github.com/onsi/gomega"
+
+	"github.com/apache/incubator-yunikorn-k8shim/test/e2e/framework/configmanager"
+)
+
+func init() {
+	configmanager.YuniKornTestConfig.ParseFlags()
+}
+
+var k = k8s.KubeCtl{}
+var oldConfigMap *v1.ConfigMap
+
+var _ = BeforeSuite(func() {
+	By("Enabling state aware app scheduling config over config maps")
+	Ω(k.SetClient()).To(BeNil())
+	var c, err = k.GetConfigMaps(configmanager.YuniKornTestConfig.YkNamespace,
+		configmanager.DefaultYuniKornConfigMap)
+	Ω(err).NotTo(HaveOccurred())
+	Ω(c).NotTo(BeNil())
+
+	oldConfigMap = c.DeepCopy()
+	Ω(c).Should(BeEquivalentTo(oldConfigMap))
+	var stateAwareStr = "partitions:\n  -\n    name: default\n    placementrules:\n      - name: tag\n        " +
+		"value: namespace\n        create: true\n    queues:\n      - name: root\n        " +
+		"submitacl: '*'\n        properties:\n          application.sort.policy: stateaware"

Review comment:
       Can we use a more readable form for such multi-line strings?
   Such as https://yourbasic.org/golang/multiline-string/#raw-string-literals

##########
File path: test/e2e/state_aware_app_scheduling/fallback_test.go
##########
@@ -0,0 +1,117 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package stateawareappscheduling_test
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+
+	v1 "k8s.io/api/core/v1"
+
+	"github.com/apache/incubator-yunikorn-k8shim/test/e2e/framework/helpers/common"
+	"github.com/apache/incubator-yunikorn-k8shim/test/e2e/framework/helpers/k8s"
+	"github.com/apache/incubator-yunikorn-k8shim/test/e2e/framework/helpers/yunikorn"
+)
+
+var _ = Describe("FallbackTest:", func() {
+	var kClient k8s.KubeCtl
+	var restClient yunikorn.RClient
+	var sleepPodDef string
+	var err error
+	var sleepRespPod *v1.Pod
+	var ns string
+	var appsInfo map[string]interface{}
+	var r = regexp.MustCompile(`memory:(\d+) vcore:(\d+)`)
+
+	BeforeEach(func() {
+		// Initializing kubectl client
+		sleepPodDef, err = common.GetAbsPath("../testdata/sleeppod_template.yaml")
+		Ω(err).NotTo(HaveOccurred())
+		kClient = k8s.KubeCtl{}
+		Ω(kClient.SetClient()).To(BeNil())
+		// Initializing rest client
+		restClient = yunikorn.RClient{}
+		ns = "test-" + common.RandSeq(10)
+		By(fmt.Sprintf("create %s namespace", ns))
+		ns1, err1 := kClient.CreateNamespace(ns, nil)
+		Ω(err1).NotTo(HaveOccurred())
+		Ω(ns1.Status.Phase).To(Equal(v1.NamespaceActive))
+
+		By(fmt.Sprintf("Deploy the sleep pod to %s namespace", ns))
+		sleepObj, err2 := k8s.GetPodObj(sleepPodDef)
+		Ω(err2).NotTo(HaveOccurred())
+		sleepObj.Namespace = ns
+		sleepObj.ObjectMeta.Labels["applicationId"] = common.GetUUID()
+		sleepRespPod, err = kClient.CreatePod(sleepObj, ns)
+		Ω(err).NotTo(HaveOccurred())
+	})
+
+	It("Verify_App_In_Starting_State", func() {
+		err = kClient.WaitForPodBySelectorRunning(ns,
+			fmt.Sprintf("app=%s", sleepRespPod.ObjectMeta.Labels["app"]),
+			10)
+		Ω(err).NotTo(HaveOccurred())
+
+		appsInfo, err = restClient.GetAppInfo(sleepRespPod.ObjectMeta.Labels["applicationId"])
+		Ω(err).NotTo(HaveOccurred())
+		Ω(appsInfo).NotTo(BeNil())
+		By(fmt.Sprintf("Verify that the sleep pod is mapped to %s queue", ns))
+		Ω(appsInfo["applicationID"]).To(Equal(sleepRespPod.ObjectMeta.Labels["applicationId"]))
+		Ω(appsInfo["queueName"]).To(ContainSubstring(sleepRespPod.ObjectMeta.Namespace))
+		By("Verify that the job is scheduled by YuniKorn & is in starting state")
+		Ω(appsInfo["applicationState"]).To(Equal("Starting"))
+		Ω("yunikorn").To(Equal(sleepRespPod.Spec.SchedulerName))
+	}, 60)
+
+	It("Verify_App_State_Transition_To_Running_Post_Timeout", func() {
+		By("Wait for fallback timeout of 5mins")
+		err = restClient.WaitForAppStateTransition(sleepRespPod.ObjectMeta.Labels["applicationId"],
+			yunikorn.States().Application.Running,
+			360)

Review comment:
       Are 5 minutes enough? IIRC the timeout in the scheduler is 5 minutes, maybe a bit longer than that will be more stable?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org