You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by "manirajv06 (via GitHub)" <gi...@apache.org> on 2023/02/15 05:56:49 UTC
[GitHub] [yunikorn-k8shim] manirajv06 opened a new pull request, #533: [YUNIKORN-1538] Add resource fairness test suites
manirajv06 opened a new pull request, #533:
URL: https://github.com/apache/yunikorn-k8shim/pull/533
### What is this PR for?
Added test suites to verify the resource fairness (application.sort.policy=fair) among various applications with pending requests in a limited resource queue.
### What type of PR is it?
* [ ] - Improvement
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1538
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] pbacsko closed pull request #533: [YUNIKORN-1538] Add resource fairness test suites
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko closed pull request #533: [YUNIKORN-1538] Add resource fairness test suites
URL: https://github.com/apache/yunikorn-k8shim/pull/533
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] pbacsko commented on a diff in pull request #533: [YUNIKORN-1538] Add resource fairness test suites
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #533:
URL: https://github.com/apache/yunikorn-k8shim/pull/533#discussion_r1108600824
##########
test/e2e/resource_fairness/resource_fairness_suite_test.go:
##########
@@ -0,0 +1,97 @@
+/*
+ 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 resourcefairness_test
+
+import (
+ "path/filepath"
+ "testing"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/reporters"
+ "github.com/onsi/gomega"
+ v1 "k8s.io/api/core/v1"
+
+ "github.com/apache/yunikorn-core/pkg/common/configs"
+ "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"
+)
+
+func init() {
+ configmanager.YuniKornTestConfig.ParseFlags()
+}
+
+var oldConfigMap = new(v1.ConfigMap)
Review Comment:
Nit: just `var oldConfigMap *v1.ConfigMap`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] pbacsko commented on a diff in pull request #533: [YUNIKORN-1538] Add resource fairness test suites
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #533:
URL: https://github.com/apache/yunikorn-k8shim/pull/533#discussion_r1108608118
##########
test/e2e/resource_fairness/resource_fairness_suite_test.go:
##########
@@ -0,0 +1,97 @@
+/*
+ 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 resourcefairness_test
+
+import (
+ "path/filepath"
+ "testing"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/reporters"
+ "github.com/onsi/gomega"
+ v1 "k8s.io/api/core/v1"
+
+ "github.com/apache/yunikorn-core/pkg/common/configs"
+ "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"
+)
+
+func init() {
+ configmanager.YuniKornTestConfig.ParseFlags()
+}
+
+var oldConfigMap = new(v1.ConfigMap)
+var annotation = "ann-" + common.RandSeq(10)
+var kClient = k8s.KubeCtl{} //nolint
+var _ = BeforeSuite(func() {
+ Ω(kClient.SetClient()).To(BeNil())
Review Comment:
As we discussed, we should run all tests in a separate namespace to increase test isolation and stability.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] pbacsko commented on a diff in pull request #533: [YUNIKORN-1538] Add resource fairness test suites
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #533:
URL: https://github.com/apache/yunikorn-k8shim/pull/533#discussion_r1108600824
##########
test/e2e/resource_fairness/resource_fairness_suite_test.go:
##########
@@ -0,0 +1,97 @@
+/*
+ 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 resourcefairness_test
+
+import (
+ "path/filepath"
+ "testing"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/reporters"
+ "github.com/onsi/gomega"
+ v1 "k8s.io/api/core/v1"
+
+ "github.com/apache/yunikorn-core/pkg/common/configs"
+ "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"
+)
+
+func init() {
+ configmanager.YuniKornTestConfig.ParseFlags()
+}
+
+var oldConfigMap = new(v1.ConfigMap)
Review Comment:
just `var oldConfigMap *v1.ConfigMap`
##########
test/e2e/resource_fairness/resource_fairness_suite_test.go:
##########
@@ -0,0 +1,97 @@
+/*
+ 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 resourcefairness_test
+
+import (
+ "path/filepath"
+ "testing"
+
+ "github.com/onsi/ginkgo"
+ "github.com/onsi/ginkgo/reporters"
+ "github.com/onsi/gomega"
+ v1 "k8s.io/api/core/v1"
+
+ "github.com/apache/yunikorn-core/pkg/common/configs"
+ "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"
+)
+
+func init() {
+ configmanager.YuniKornTestConfig.ParseFlags()
+}
+
+var oldConfigMap = new(v1.ConfigMap)
+var annotation = "ann-" + common.RandSeq(10)
+var kClient = k8s.KubeCtl{} //nolint
Review Comment:
Reason for nolint?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] pbacsko commented on a diff in pull request #533: [YUNIKORN-1538] Add resource fairness test suites
Posted by "pbacsko (via GitHub)" <gi...@apache.org>.
pbacsko commented on code in PR #533:
URL: https://github.com/apache/yunikorn-k8shim/pull/533#discussion_r1108606675
##########
test/e2e/resource_fairness/resource_fairness_test.go:
##########
@@ -0,0 +1,162 @@
+/*
+ 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 resourcefairness_test
+
+import (
+ "fmt"
+ "math/rand"
+ "time"
+
+ "k8s.io/apimachinery/pkg/util/wait"
+
+ tests "github.com/apache/yunikorn-k8shim/test/e2e"
+ "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"
+)
+
+var _ = Describe("FairScheduling:", func() {
+ var kClient k8s.KubeCtl
+ var restClient yunikorn.RClient
+ var err error
+ var ns = "default"
+
+ var maxCPU int64 = 500
+ var maxMem int64 = 500
+
+ BeforeEach(func() {
+ kClient = k8s.KubeCtl{}
+ Ω(kClient.SetClient()).To(BeNil())
+ })
+
+ // Validates waitQueue order of requested app resources, according to fairAppScheduling.
+ // Step 1: Deploy 4 apps, which sum to 95% queueCPU / 35% queueMem
+ // -> Resource priority order: 1)CPU. 2)Mem
+ // Step 2: Deploy 1 more blocked pod each for apps0-2
+ // Step 3: Kill an App3 pod.
+ // Step 4: App with least cpu use is allocated pod next. Break tie using mem.
+ It("Test_Wait_Queue_Order", func() {
+ // Create appIDs
+ var apps []string
+ for j := 0; j < 4; j++ {
+ id := fmt.Sprintf("app%d-%s", j, common.RandSeq(5))
+ apps = append(apps, id)
+ }
+
+ // Initial allocation to fill ns quota
+ appPods := map[string][]string{}
+ appAllocs := map[string]map[string]float64{
+ apps[0]: {"cpu": 0.3, "mem": 0.1, "pods": 1},
+ apps[1]: {"cpu": 0.2, "mem": 0.05, "pods": 1},
+ apps[2]: {"cpu": 0.1, "mem": 0.15, "pods": 1},
+ apps[3]: {"cpu": 0.4, "mem": 0.05, "pods": 4},
+ }
+ for appID, req := range appAllocs {
+ appPods[appID] = []string{}
+ numPods := int(req["pods"])
+ By(fmt.Sprintf("[%s] Deploy %d pods", appID, numPods))
+ for i := 0; i < numPods; i++ {
+ // Calculate individual podRequest by dividing total appAlloc by numPods
+ reqCPU := int64(req["cpu"] / req["pods"] * float64(maxCPU))
+ reqMem := int64(req["mem"] / req["pods"] * float64(maxMem))
+ podName := fmt.Sprintf("%s-pod%d", appID, i)
+ appPods[appID] = append(appPods[appID], podName)
+
+ // Deploy pod
+ sleepPodConf := k8s.SleepPodConfig{
+ Name: podName, NS: ns, AppID: appID, CPU: reqCPU, Mem: reqMem}
+ initPod, podErr := k8s.InitSleepPod(sleepPodConf)
+ Ω(podErr).NotTo(HaveOccurred())
+ _, err = kClient.CreatePod(initPod, ns)
+ Ω(err).NotTo(HaveOccurred())
+ err = kClient.WaitForPodRunning(ns, podName, 30*time.Second)
+ Ω(err).NotTo(HaveOccurred())
+ }
+ }
+
+ // Submit one blocked pod for each app in random order. Each requests 0.1 qCPU and 0.05 qMem.
+ By("Submitting additional blocked pod for each app")
+ randOrder := rand.Perm(3)
+ for _, i := range randOrder {
+ cpuPct, memPct, appID := 0.1, 0.05, apps[i]
+ reqCPU, reqMem := int64(cpuPct*float64(maxCPU)), int64(memPct*float64(maxMem))
+ podNum := len(appPods[appID])
+ podName := fmt.Sprintf("%s-pod%d", appID, podNum)
+ appPods[appID] = append(appPods[appID], podName)
+
+ By(fmt.Sprintf("[%s] Submit %s", appID, podName))
+ sleepPodConf := k8s.SleepPodConfig{
+ Name: podName, NS: ns, AppID: appID, CPU: reqCPU, Mem: reqMem}
+ initPod, podErr := k8s.InitSleepPod(sleepPodConf)
+ Ω(podErr).NotTo(HaveOccurred())
+ _, err = kClient.CreatePod(initPod, ns)
+ Ω(err).NotTo(HaveOccurred())
+ err = kClient.WaitForPodPending(ns, podName, 10*time.Second)
+ Ω(err).NotTo(HaveOccurred())
+
+ // Wait till requests has been added to application
+ err := wait.PollImmediate(300*time.Millisecond, time.Duration(30)*time.Second, func() (bool, error) {
Review Comment:
Nit: `time.Second` is already duration, just write "30".
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] codecov[bot] commented on pull request #533: [YUNIKORN-1538] Add resource fairness test suites
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #533:
URL: https://github.com/apache/yunikorn-k8shim/pull/533#issuecomment-1430803891
# [Codecov](https://codecov.io/gh/apache/yunikorn-k8shim/pull/533?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#533](https://codecov.io/gh/apache/yunikorn-k8shim/pull/533?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8924806) into [master](https://codecov.io/gh/apache/yunikorn-k8shim/commit/22a00a1940224192c1386d80933a477c5691939d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (22a00a1) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #533 +/- ##
=======================================
Coverage 69.55% 69.55%
=======================================
Files 45 45
Lines 7715 7715
=======================================
Hits 5366 5366
Misses 2153 2153
Partials 196 196
```
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org