This is an automated email from the ASF dual-hosted git repository.
mani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-core.git
The following commit(s) were added to refs/heads/master by this push:
new 549123ea [YUNIKORN-2130] fix checkLimitResource (#704)
549123ea is described below
commit 549123ea54c88a281ed7bcbf94d175c64670f353
Author: PoAn Yang <[email protected]>
AuthorDate: Wed Nov 15 13:50:37 2023 +0530
[YUNIKORN-2130] fix checkLimitResource (#704)
If there is missing limit in middle queue, the checkLimitResource
doesn't populate parent limit to next level. Finally, we will fail
to cover grandchild cases.
Closes: #704
Signed-off-by: Manikandan R <[email protected]>
---
pkg/common/configs/configvalidator.go | 83 +++++++++++++-----------------
pkg/common/configs/configvalidator_test.go | 68 ++++++++++++++++++++++++
2 files changed, 105 insertions(+), 46 deletions(-)
diff --git a/pkg/common/configs/configvalidator.go
b/pkg/common/configs/configvalidator.go
index 0ae8e850..e2aa56d6 100644
--- a/pkg/common/configs/configvalidator.go
+++ b/pkg/common/configs/configvalidator.go
@@ -146,59 +146,50 @@ func checkLimitResource(cur QueueConfig, users
map[string]map[string]*resources.
curQueuePath = queuePath + DOT + cur.Name
}
- // Carry forward (populate) the parent limit settings to the next level
if limits are not configured for the current queue
- if len(cur.Limits) == 0 {
- if _, ok := users[queuePath]; ok {
- users[curQueuePath] = users[queuePath]
- }
- if _, ok := groups[queuePath]; ok {
- groups[curQueuePath] = groups[queuePath]
+ users[curQueuePath] = make(map[string]*resources.Resource)
+ groups[curQueuePath] = make(map[string]*resources.Resource)
+
+ // Carry forward (populate) the parent limit settings to the next level
+ for u, v := range users[queuePath] {
+ users[curQueuePath][u] = v.Clone()
+ }
+ for g, v := range groups[queuePath] {
+ groups[curQueuePath][g] = v.Clone()
+ }
+
+ // compare user & group limit setting between the current queue and
parent queue
+ for _, limit := range cur.Limits {
+ limitMaxResources, err :=
resources.NewResourceFromConf(limit.MaxResources)
+ if err != nil {
+ return err
}
- } else {
- // compare user & group limit setting between the current queue
and parent queue
- for _, limit := range cur.Limits {
- limitMaxResources, err :=
resources.NewResourceFromConf(limit.MaxResources)
- if err != nil {
- return err
- }
- for _, user := range limit.Users {
- // Is user limit setting exists?
- if userMaxResource, ok :=
users[queuePath][user]; ok {
- if
!userMaxResource.FitInMaxUndef(limitMaxResources) {
- return fmt.Errorf("user %s max
resource %s of queue %s is greater than immediate or ancestor parent maximum
resource %s", user, limitMaxResources.String(), cur.Name,
userMaxResource.String())
- }
- // Override with min resource
- if _, ok := users[curQueuePath]; !ok {
- users[curQueuePath] =
make(map[string]*resources.Resource)
- }
- users[curQueuePath][user] =
resources.ComponentWiseMinPermissive(limitMaxResources, userMaxResource)
- } else {
- if _, ok := users[curQueuePath]; !ok {
- users[curQueuePath] =
make(map[string]*resources.Resource)
- }
- users[curQueuePath][user] =
limitMaxResources
+
+ for _, user := range limit.Users {
+ // Is user limit setting exists?
+ if userMaxResource, ok := users[queuePath][user]; ok {
+ if
!userMaxResource.FitInMaxUndef(limitMaxResources) {
+ return fmt.Errorf("user %s max resource
%s of queue %s is greater than immediate or ancestor parent maximum resource
%s", user, limitMaxResources.String(), cur.Name, userMaxResource.String())
}
+ // Override with min resource
+ users[curQueuePath][user] =
resources.ComponentWiseMinPermissive(limitMaxResources, userMaxResource)
+ } else {
+ users[curQueuePath][user] = limitMaxResources
}
- for _, group := range limit.Groups {
- // Is group limit setting exists?
- if groupMaxResource, ok :=
groups[queuePath][group]; ok {
- if
!groupMaxResource.FitInMaxUndef(limitMaxResources) {
- return fmt.Errorf("group %s max
resource %s of queue %s is greater than immediate or ancestor parent maximum
resource %s", group, limitMaxResources.String(), cur.Name,
groupMaxResource.String())
- }
- // Override with min resource
- if _, ok := groups[curQueuePath]; !ok {
- groups[curQueuePath] =
make(map[string]*resources.Resource)
- }
- groups[curQueuePath][group] =
resources.ComponentWiseMinPermissive(limitMaxResources, groupMaxResource)
- } else {
- if _, ok := groups[curQueuePath]; !ok {
- groups[curQueuePath] =
make(map[string]*resources.Resource)
- }
- groups[curQueuePath][group] =
limitMaxResources
+ }
+ for _, group := range limit.Groups {
+ // Is group limit setting exists?
+ if groupMaxResource, ok := groups[queuePath][group]; ok
{
+ if
!groupMaxResource.FitInMaxUndef(limitMaxResources) {
+ return fmt.Errorf("group %s max
resource %s of queue %s is greater than immediate or ancestor parent maximum
resource %s", group, limitMaxResources.String(), cur.Name,
groupMaxResource.String())
}
+ // Override with min resource
+ groups[curQueuePath][group] =
resources.ComponentWiseMinPermissive(limitMaxResources, groupMaxResource)
+ } else {
+ groups[curQueuePath][group] = limitMaxResources
}
}
}
+
// traverse child queues
for _, child := range cur.Queues {
err := checkLimitResource(child, users, groups, curQueuePath)
diff --git a/pkg/common/configs/configvalidator_test.go
b/pkg/common/configs/configvalidator_test.go
index 05f2ad7d..bd9b17f3 100644
--- a/pkg/common/configs/configvalidator_test.go
+++ b/pkg/common/configs/configvalidator_test.go
@@ -872,6 +872,74 @@ func TestCheckLimitResource(t *testing.T) { //nolint:funlen
},
hasError: false,
},
+ {
+ name: "leaf queue user maxresources exceed grandparent
queue user maxresources",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxResources(
+ map[string]map[string]string{
+ "test-user1": {"memory": "100"},
+ "test-user2": {"memory": "100"},
+ },
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits: createLimitMaxResources(
+
map[string]map[string]string{
+ "test-user1":
{"memory": "50"},
+ },
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxResources(
+
map[string]map[string]string{
+
"test-user1": {"memory": "10"},
+
"test-user2": {"memory": "150"},
+ },
+ nil),
+ },
+ },
+ },
+ },
+ },
+ hasError: true,
+ },
+ {
+ name: "leaf queue group maxresources exceed grandparent
queue group maxresources",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxResources(
+ nil,
+ map[string]map[string]string{
+ "test-group1": {"memory":
"100"},
+ "test-group2": {"memory":
"100"},
+ }),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits: createLimitMaxResources(
+ nil,
+
map[string]map[string]string{
+ "test-group1":
{"memory": "50"},
+ }),
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxResources(
+ nil,
+
map[string]map[string]string{
+
"test-group1": {"memory": "10"},
+
"test-group2": {"memory": "150"},
+ }),
+ },
+ },
+ },
+ },
+ },
+ hasError: true,
+ },
}
for _, testCase := range testCases {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
