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 0df1d599 [YUNIKORN-2273] checkLimitMaxApplications fails if there is
missing limit in a middle queue (#760)
0df1d599 is described below
commit 0df1d599aed6b8797d3ce4575fafa41be9bad6d9
Author: PoAn Yang <[email protected]>
AuthorDate: Thu Jan 18 10:48:46 2024 +0530
[YUNIKORN-2273] checkLimitMaxApplications fails if there is missing limit
in a middle queue (#760)
Closes: #760
Signed-off-by: Manikandan R <[email protected]>
---
pkg/common/configs/configvalidator.go | 88 +++++----
pkg/common/configs/configvalidator_test.go | 275 +++++++++++++++++++++++++++--
2 files changed, 303 insertions(+), 60 deletions(-)
diff --git a/pkg/common/configs/configvalidator.go
b/pkg/common/configs/configvalidator.go
index 5e7a45b0..13de4fe4 100644
--- a/pkg/common/configs/configvalidator.go
+++ b/pkg/common/configs/configvalidator.go
@@ -171,7 +171,6 @@ func checkLimitResource(cur QueueConfig, users
map[string]map[string]*resources.
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 if wildcardMaxResource, ok :=
users[queuePath][common.Wildcard]; user != common.Wildcard && ok {
if
!wildcardMaxResource.FitInMaxUndef(limitMaxResources) {
@@ -236,54 +235,53 @@ func checkLimitMaxApplications(cur QueueConfig, users
map[string]map[string]uint
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]
- }
- } else {
- // compare user & group limit setting between the current queue
and parent queue
- for _, limit := range cur.Limits {
- limitMaxApplications := limit.MaxApplications
- for _, user := range limit.Users {
- // Is user limit setting exists?
- if userMaxApplications, ok :=
users[queuePath][user]; ok {
- if userMaxApplications != 0 &&
(userMaxApplications < limitMaxApplications || limitMaxApplications == 0) {
- return fmt.Errorf("user %s max
applications %d of queue %s is greater than immediate or ancestor parent max
applications %d", user, limitMaxApplications, cur.Name, userMaxApplications)
- }
- // Override with min resource
- if _, ok := users[curQueuePath]; !ok {
- users[curQueuePath] =
make(map[string]uint64)
- }
- users[curQueuePath][user] =
common.Min(limitMaxApplications, userMaxApplications)
- } else {
- if _, ok := users[curQueuePath]; !ok {
- users[curQueuePath] =
make(map[string]uint64)
- }
- users[curQueuePath][user] =
limitMaxApplications
+ users[curQueuePath] = make(map[string]uint64)
+ groups[curQueuePath] = make(map[string]uint64)
+
+ // Carry forward (populate) the parent limit settings to the next level
+ // queuePath is the parent queue path and curQueuePath is the current
queue path.
+ // For example, queuePath is root and curQueuePath is root.current.
+ for u, maxapplications := range users[queuePath] {
+ users[curQueuePath][u] = maxapplications
+ }
+ for g, maxapplications := range groups[queuePath] {
+ groups[curQueuePath][g] = maxapplications
+ }
+
+ // compare user & group limit setting between the current queue and
parent queue
+ for _, limit := range cur.Limits {
+ limitMaxApplications := limit.MaxApplications
+ for _, user := range limit.Users {
+ // Is user limit setting exists?
+ if userMaxApplications, ok := users[queuePath][user];
ok {
+ if userMaxApplications != 0 &&
(userMaxApplications < limitMaxApplications || limitMaxApplications == 0) {
+ return fmt.Errorf("user %s max
applications %d of queue %s is greater than immediate or ancestor parent max
applications %d", user, limitMaxApplications, cur.Name, userMaxApplications)
}
+ users[curQueuePath][user] = limitMaxApplications
+ } else if wildcardMaxApplications, ok :=
users[queuePath][common.Wildcard]; user != common.Wildcard && ok {
+ if wildcardMaxApplications != 0 &&
(wildcardMaxApplications < limitMaxApplications || limitMaxApplications == 0) {
+ return fmt.Errorf("user %s max
applications %d of queue %s is greater than wildcard max applications %d of
immediate or ancestor parent queue", user, limitMaxApplications, cur.Name,
wildcardMaxApplications)
+ }
+ users[curQueuePath][user] = limitMaxApplications
+ } else {
+ users[curQueuePath][user] = limitMaxApplications
}
+ }
- for _, group := range limit.Groups {
- // Is user limit setting exists?
- if groupMaxApplications, ok :=
groups[queuePath][group]; ok {
- if groupMaxApplications != 0 &&
(groupMaxApplications < limitMaxApplications || limitMaxApplications == 0) {
- return fmt.Errorf("group %s max
applications %d of queue %s is greater than immediate or ancestor parent max
applications %d", group, limitMaxApplications, cur.Name, groupMaxApplications)
- }
- // Override with min resource
- if _, ok := groups[curQueuePath]; !ok {
- groups[curQueuePath] =
make(map[string]uint64)
- }
- groups[curQueuePath][group] =
common.Min(limitMaxApplications, groupMaxApplications)
- } else {
- if _, ok := groups[curQueuePath]; !ok {
- groups[curQueuePath] =
make(map[string]uint64)
- }
- groups[curQueuePath][group] =
limitMaxApplications
+ for _, group := range limit.Groups {
+ // Is user limit setting exists?
+ if groupMaxApplications, ok :=
groups[queuePath][group]; ok {
+ if groupMaxApplications != 0 &&
(groupMaxApplications < limitMaxApplications || limitMaxApplications == 0) {
+ return fmt.Errorf("group %s max
applications %d of queue %s is greater than immediate or ancestor parent max
applications %d", group, limitMaxApplications, cur.Name, groupMaxApplications)
}
+ groups[curQueuePath][group] =
limitMaxApplications
+ } else if wildcardMaxApplications, ok :=
groups[queuePath][common.Wildcard]; group != common.Wildcard && ok {
+ if wildcardMaxApplications != 0 &&
(wildcardMaxApplications < limitMaxApplications || limitMaxApplications == 0) {
+ return fmt.Errorf("group %s max
applications %d of queue %s is greater than wildcard max applications %d of
immediate or ancestor parent queue", group, limitMaxApplications, cur.Name,
wildcardMaxApplications)
+ }
+ groups[curQueuePath][group] =
limitMaxApplications
+ } else {
+ groups[curQueuePath][group] =
limitMaxApplications
}
}
}
diff --git a/pkg/common/configs/configvalidator_test.go
b/pkg/common/configs/configvalidator_test.go
index 8703ab59..99001dff 100644
--- a/pkg/common/configs/configvalidator_test.go
+++ b/pkg/common/configs/configvalidator_test.go
@@ -1194,12 +1194,37 @@ func createLimitMaxApplications(users
map[string]uint64, groups map[string]uint6
func TestCheckLimitMaxApplications(t *testing.T) { //nolint:funlen
testCases := []struct {
- name string
- config QueueConfig
- hasError bool
+ name string
+ config QueueConfig
+ errMsg string
}{
{
- name: "leaf queue user group maxapplications are within
parent queue user group maxapplications",
+ name: "leaf queue user group maxapplications are within
immediate parent queue user group maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ map[string]uint64{"test-user": 100,
"*": 100},
+ map[string]uint64{"test-group": 100,
"*": 100}),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits:
createLimitMaxApplications(
+
map[string]uint64{"test-user": 100, "test-user2": 100},
+
map[string]uint64{"test-group": 100, "test-group2": 100}),
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxApplications(
+
map[string]uint64{"test-user": 100, "test-user2": 100},
+
map[string]uint64{"test-group": 100, "test-group2": 100}),
+ },
+ },
+ },
+ },
+ },
+ },
+ {
+ name: "leaf queue user group maxapplications are within
ancestor parent queue user group maxapplications",
config: QueueConfig{
Name: "parent",
Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
@@ -1207,22 +1232,24 @@ func TestCheckLimitMaxApplications(t *testing.T) {
//nolint:funlen
Queues: []QueueConfig{
{
Name: "child1",
- Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
-
map[string]uint64{"test-group": 100}),
Queues: []QueueConfig{
{
- Name: "child2",
- Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
-
map[string]uint64{"test-group": 100}),
+ Name: "childA",
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 50},
+
map[string]uint64{"test-group": 50}),
+ },
+ {
+ Name: "childB",
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 10},
+
map[string]uint64{"test-group": 10}),
},
},
},
},
},
- hasError: false,
},
{
- name: "leaf queue user maxapplications exceed parent
queue user maxapplications",
+ name: "leaf queue user maxapplications exceed immediate
parent queue user maxapplications",
config: QueueConfig{
Name: "parent",
Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
@@ -1235,7 +1262,7 @@ func TestCheckLimitMaxApplications(t *testing.T) {
//nolint:funlen
},
},
},
- hasError: true,
+ errMsg: "is greater than immediate or ancestor parent
max applications",
},
{
name: "leaf queue group maxapplications exceed parent
queue group maxapplications",
@@ -1251,15 +1278,233 @@ func TestCheckLimitMaxApplications(t *testing.T) {
//nolint:funlen
},
},
},
- hasError: true,
+ errMsg: "is greater than immediate or ancestor parent
max applications",
+ },
+ {
+ name: "leaf queue user maxapplications are exceed
grandparent queue user maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
+ map[string]uint64{"test-group": 100}),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 150},
+
map[string]uint64{"test-group": 50}),
+ },
+ },
+ },
+ },
+ },
+ errMsg: "is greater than immediate or ancestor parent
max applications",
+ },
+ {
+ name: "leaf queue group maxapplications are exceed
grandparent queue group maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
+ map[string]uint64{"test-group": 100}),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 50},
+
map[string]uint64{"test-group": 150}),
+ },
+ },
+ },
+ },
+ },
+ errMsg: "is greater than immediate or ancestor parent
max applications",
+ },
+ {
+ name: "leaf queue user maxapplications exceed parent
queue wildcard maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ map[string]uint64{"test-user1": 100,
"*": 100},
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits:
createLimitMaxApplications(
+
map[string]uint64{"test-user1": 50, "test-user2": 150},
+ nil),
+ },
+ },
+ },
+ errMsg: "is greater than wildcard max applications",
+ },
+ {
+ name: "leaf queue group maxapplications exceed parent
queue wildcard maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ nil,
+ map[string]uint64{"test-group1": 100,
"*": 100}),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits:
createLimitMaxApplications(
+ nil,
+
map[string]uint64{"test-group1": 50, "test-group2": 150}),
+ },
+ },
+ },
+ errMsg: "is greater than wildcard max applications",
+ },
+ {
+ name: "leaf queue user maxapplications exceed
grandparent queue wildcard maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ map[string]uint64{"test-user1": 100,
"*": 100},
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits:
createLimitMaxApplications(
+
map[string]uint64{"test-user1": 50},
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxApplications(
+
map[string]uint64{"test-user1": 10, "test-user2": 150},
+ nil),
+ },
+ },
+ },
+ },
+ },
+ errMsg: "is greater than wildcard max applications",
+ },
+ {
+ name: "leaf queue group maxapplications exceed
grandparent queue wildcard maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ nil,
+ map[string]uint64{"test-group1": 100,
"*": 100}),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits:
createLimitMaxApplications(
+ nil,
+
map[string]uint64{"test-group1": 50}),
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxApplications(
+ nil,
+
map[string]uint64{"test-group1": 10, "test-group2": 150}),
+ },
+ },
+ },
+ },
+ },
+ errMsg: "is greater than wildcard max applications",
+ },
+ {
+ name: "leaf queue wildcard maxapplications is within
immediate parent queue wildcard maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ map[string]uint64{"*": 100},
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits:
createLimitMaxApplications(
+ map[string]uint64{"*":
50},
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxApplications(
+
map[string]uint64{"*": 10},
+ nil),
+ },
+ },
+ },
+ },
+ },
+ },
+ {
+ name: "leaf queue wildcard maxapplications is within
grandparent queue wildcard maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ map[string]uint64{"*": 100},
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxApplications(
+
map[string]uint64{"*": 10},
+ nil),
+ },
+ },
+ },
+ },
+ },
+ },
+ {
+ name: "leaf queue wildcard maxapplications exceed
immediate parent queue wildcard maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ map[string]uint64{"*": 100},
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Limits:
createLimitMaxApplications(
+ map[string]uint64{"*":
150},
+ nil),
+ },
+ },
+ },
+ errMsg: "is greater than immediate or ancestor parent
max applications",
+ },
+ {
+ name: "leaf queue wildcard maxapplications exceed
grandparent queue wildcard maxapplications",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: createLimitMaxApplications(
+ map[string]uint64{"*": 100},
+ nil),
+ Queues: []QueueConfig{
+ {
+ Name: "child1",
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxApplications(
+
map[string]uint64{"*": 150},
+ nil),
+ },
+ },
+ },
+ },
+ },
+ errMsg: "is greater than immediate or ancestor parent
max applications",
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
err := checkLimitMaxApplications(testCase.config,
make(map[string]map[string]uint64), make(map[string]map[string]uint64),
common.Empty)
- if testCase.hasError {
- assert.ErrorContains(t, err, "is greater than
immediate or ancestor parent max applications")
+ if testCase.errMsg != "" {
+ assert.ErrorContains(t, err, testCase.errMsg)
} else {
assert.NilError(t, err)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
