This is an automated email from the ASF dual-hosted git repository.
wilfreds 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 418714df [YUNIKORN-1973] Queue Limits level checks (#646)
418714df is described below
commit 418714df43c8f22fa58227223b4e0279c943cf86
Author: Manikandan R <[email protected]>
AuthorDate: Thu Sep 21 18:17:48 2023 +1000
[YUNIKORN-1973] Queue Limits level checks (#646)
The configuration for a limit set on sibling queues should not interact
with any queue but the parent and its own children.
Change the internal structure to take the full path into account.
Closes: #646
Signed-off-by: Wilfred Spiegelenburg <[email protected]>
---
pkg/common/configs/config_test.go | 4 +-
pkg/common/configs/configvalidator.go | 187 +++++++++++--------
pkg/common/configs/configvalidator_test.go | 287 +++++++++++++----------------
pkg/webservice/handlers_test.go | 95 ++++++++++
4 files changed, 333 insertions(+), 240 deletions(-)
diff --git a/pkg/common/configs/config_test.go
b/pkg/common/configs/config_test.go
index ef0fa645..5e4ddf4b 100644
--- a/pkg/common/configs/config_test.go
+++ b/pkg/common/configs/config_test.go
@@ -1037,7 +1037,7 @@ partitions:
_, err := CreateConfig(data)
assert.ErrorContains(t, err, "invalid MaxApplications settings for
limit")
- // Make sure limit max apps not set, but queue limit set, will fail.
+ // Make sure limit max apps not set, but queue limit set. will not fail.
data = `
partitions:
- name: default
@@ -1071,7 +1071,7 @@ partitions:
`
// validate the config and check after the update
_, err = CreateConfig(data)
- assert.ErrorContains(t, err, "MaxApplications is 0")
+ assert.NilError(t, err, "No error should be thrown even when queue
maxapps is set but not for user or group")
// Make sure limit max apps set, but queue limit not set, will not fail.
data = `
diff --git a/pkg/common/configs/configvalidator.go
b/pkg/common/configs/configvalidator.go
index 7bab772a..571e37b4 100644
--- a/pkg/common/configs/configvalidator.go
+++ b/pkg/common/configs/configvalidator.go
@@ -140,57 +140,70 @@ func checkQueueResource(cur QueueConfig, parentM
*resources.Resource) (*resource
return curG, nil
}
-func checkLimitResource(cur QueueConfig, parent *QueueConfig, users
map[string]*resources.Resource, groups map[string]*resources.Resource) error {
- // Collect user and group limits of parent queue
- if parent != nil {
- for _, limit := range parent.Limits {
- parentLimitMaxResources, err :=
resources.NewResourceFromConf(limit.MaxResources)
+func checkLimitResource(cur QueueConfig, users
map[string]map[string]*resources.Resource, groups
map[string]map[string]*resources.Resource, queuePath string) error {
+ var curQueuePath string
+ if cur.Name == RootQueue {
+ curQueuePath = RootQueue
+ } else {
+ 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 {
+ limitMaxResources, err :=
resources.NewResourceFromConf(limit.MaxResources)
if err != nil {
return err
}
for _, user := range limit.Users {
- users[user] = parentLimitMaxResources
- }
- for _, group := range limit.Groups {
- groups[group] = parentLimitMaxResources
- }
- }
- }
-
- // 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[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())
+ // 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
}
- // Override with min resource
- users[user] =
resources.ComponentWiseMinPermissive(limitMaxResources, userMaxResource)
- } else {
- users[user] = limitMaxResources
}
- }
- for _, group := range limit.Groups {
- // Is group limit setting exists?
- if groupMaxResource, ok := groups[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())
+ 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
}
- // Override with min resource
- groups[group] =
resources.ComponentWiseMinPermissive(limitMaxResources, groupMaxResource)
- } else {
- groups[group] = limitMaxResources
}
}
}
// traverse child queues
for _, child := range cur.Queues {
- err := checkLimitResource(child, &cur, users, groups)
+ err := checkLimitResource(child, users, groups, curQueuePath)
if err != nil {
return err
}
@@ -212,49 +225,68 @@ func checkQueueMaxApplications(cur QueueConfig) error {
return nil
}
-func checkLimitMaxApplications(cur QueueConfig, parent *QueueConfig, users
map[string]uint64, groups map[string]uint64) error {
- // Collect user and group limits of parent queue
- if parent != nil {
- for _, limit := range parent.Limits {
- parentLimitMaxApplications := limit.MaxApplications
- for _, user := range limit.Users {
- users[user] = parentLimitMaxApplications
- }
- for _, group := range limit.Groups {
- groups[group] = parentLimitMaxApplications
- }
- }
+func checkLimitMaxApplications(cur QueueConfig, users
map[string]map[string]uint64, groups map[string]map[string]uint64, queuePath
string) error {
+ var curQueuePath string
+ if cur.Name == RootQueue {
+ curQueuePath = RootQueue
+ } else {
+ curQueuePath = queuePath + DOT + cur.Name
}
- // 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[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)
+ // 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[user] = common.Min(limitMaxApplications,
userMaxApplications)
- } else {
- users[user] = limitMaxApplications
}
- }
- for _, group := range limit.Groups {
- // Is group limit setting exists?
- if groupMaxApplications, ok := groups[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)
+
+ 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
}
- groups[group] =
common.Min(limitMaxApplications, groupMaxApplications)
- } else {
- groups[group] = limitMaxApplications
}
}
}
// traverse child queues
for _, child := range cur.Queues {
- err := checkLimitMaxApplications(child, &cur, users, groups)
+ err := checkLimitMaxApplications(child, users, groups,
curQueuePath)
if err != nil {
return err
}
@@ -514,14 +546,10 @@ func checkLimit(limit Limit, existedUserName
map[string]bool, existedGroupName m
}
}
// at least some resource should be not null
- if limit.MaxApplications == 0 && resources.IsZero(limitResource) {
+ if limit.MaxApplications == 0 && len(limit.MaxResources) == 0 {
return fmt.Errorf("invalid resource combination for limit %s
all resource limits are null", limit.Limit)
}
- if limit.MaxApplications == 0 {
- return fmt.Errorf("MaxApplications is 0 in '%s' limit name, it
should be between 1 - %d", limit.Limit, queue.MaxApplications)
- }
-
if queue.MaxApplications != 0 && queue.MaxApplications <
limit.MaxApplications {
return fmt.Errorf("invalid MaxApplications settings for limit
%s exceed current the queue MaxApplications", limit.Limit)
}
@@ -759,10 +787,11 @@ func Validate(newConfig *SchedulerConfig) error {
return err
}
- if err = checkLimitResource(partition.Queues[0], nil,
make(map[string]*resources.Resource), make(map[string]*resources.Resource));
err != nil {
+ if err = checkLimitResource(partition.Queues[0],
make(map[string]map[string]*resources.Resource),
make(map[string]map[string]*resources.Resource), common.Empty); err != nil {
return err
}
- if err = checkLimitMaxApplications(partition.Queues[0], nil,
make(map[string]uint64), make(map[string]uint64)); err != nil {
+
+ if err = checkLimitMaxApplications(partition.Queues[0],
make(map[string]map[string]uint64), make(map[string]map[string]uint64),
common.Empty); err != nil {
return err
}
// write back the partition to keep changes
diff --git a/pkg/common/configs/configvalidator_test.go
b/pkg/common/configs/configvalidator_test.go
index c89b4563..05f2ad7d 100644
--- a/pkg/common/configs/configvalidator_test.go
+++ b/pkg/common/configs/configvalidator_test.go
@@ -24,6 +24,7 @@ import (
"gotest.tools/v3/assert"
+ "github.com/apache/yunikorn-core/pkg/common"
"github.com/apache/yunikorn-core/pkg/common/resources"
)
@@ -744,51 +745,47 @@ func TestCheckLimitResource(t *testing.T) {
//nolint:funlen
hasError bool
}{
{
- name: "leaf queue user group maxresoruces are within
parent queue user group maxresources",
+ name: "leaf queue user group maxresources are within
immediate parent queue user group maxresources",
config: QueueConfig{
Name: "parent",
- Limits: []Limit{
- {
- Limit: "user-limit",
- Users:
[]string{"test-user"},
- MaxResources:
map[string]string{"memory": "100"},
- },
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"100"}},
+
map[string]map[string]string{"test-group": {"memory": "100"}}),
+ Queues: []QueueConfig{
{
- Limit: "group-limit",
- Groups:
[]string{"test-group"},
- MaxResources:
map[string]string{"memory": "100"},
+ Name: "child1",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"50"}},
+
map[string]map[string]string{"test-group": {"memory": "50"}}),
+ Queues: []QueueConfig{
+ {
+ Name: "child2",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"10"}},
+
map[string]map[string]string{"test-group": {"memory": "10"}}),
+ },
+ },
},
},
+ },
+ hasError: false,
+ },
+ {
+ name: "leaf queue user group maxresources are within
ancestor parent queue user group maxresources",
+ config: QueueConfig{
+ Name: "parent",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"100"}},
+
map[string]map[string]string{"test-group": {"memory": "100"}}),
Queues: []QueueConfig{
{
Name: "child1",
- Limits: []Limit{
+ Queues: []QueueConfig{
{
- Limit:
"user-limit",
- Users:
[]string{"test-user"},
- MaxResources:
map[string]string{"memory": "50"},
+ Name: "childA",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"50"}},
+
map[string]map[string]string{"test-group": {"memory": "50"}}),
},
{
- Limit:
"group-limit",
- Groups:
[]string{"test-group"},
- MaxResources:
map[string]string{"memory": "50"},
- },
- },
- Queues: []QueueConfig{
- {
- Name: "child2",
- Limits: []Limit{
- {
-
Limit: "user-limit",
-
Users: []string{"test-user"},
-
MaxResources: map[string]string{"memory": "10"},
- },
- {
-
Limit: "group-limit",
-
Groups: []string{"test-group"},
-
MaxResources: map[string]string{"memory": "10"},
- },
- },
+ Name: "childB",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"10"}},
+
map[string]map[string]string{"test-group": {"memory": "10"}}),
},
},
},
@@ -800,79 +797,86 @@ func TestCheckLimitResource(t *testing.T) {
//nolint:funlen
name: "leaf queue user maxresources exceed parent queue
user maxresources",
config: QueueConfig{
Name: "parent",
- Limits: []Limit{
- {
- Limit: "user-limit",
- Users:
[]string{"test-user"},
- MaxResources:
map[string]string{"memory": "100"},
- },
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"100"}},
+
map[string]map[string]string{"test-group": {"memory": "100"}}),
+ Queues: []QueueConfig{
{
- Limit: "group-limit",
- Groups:
[]string{"test-group"},
- MaxResources:
map[string]string{"memory": "100"},
+ Name: "child1",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"200"}},
+
map[string]map[string]string{"test-group": {"memory": "50"}}),
},
},
+ },
+ hasError: true,
+ },
+ {
+ name: "leaf queue group maxresources exceed parent
queue group maxresources",
+ config: QueueConfig{
+ Name: "parent",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"100"}},
+
map[string]map[string]string{"test-group": {"memory": "100"}}),
Queues: []QueueConfig{
{
Name: "child1",
- Limits: []Limit{
- {
- Limit:
"user-limit",
- Users:
[]string{"test-user"},
- MaxResources:
map[string]string{"memory": "200"},
- },
- {
- Limit:
"group-limit",
- Groups:
[]string{"test-group"},
- MaxResources:
map[string]string{"memory": "50"},
- },
- },
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"50"}},
+
map[string]map[string]string{"test-group": {"memory": "200"}}),
},
},
},
hasError: true,
},
{
- name: "leaf queue group maxresources exceed parent
queue group maxresources",
+ name: "queues at same level maxresources can be greater
or less than or equal to the other but with in immediate parent",
config: QueueConfig{
Name: "parent",
- Limits: []Limit{
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"100"}},
+
map[string]map[string]string{"test-group": {"memory": "100"}}),
+ Queues: []QueueConfig{
{
- Limit: "user-limit",
- Users:
[]string{"test-user"},
- MaxResources:
map[string]string{"memory": "100"},
+ Name: "child1",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"50"}},
+
map[string]map[string]string{"test-group": {"memory": "50"}}),
},
{
- Limit: "group-limit",
- Groups:
[]string{"test-group"},
- MaxResources:
map[string]string{"memory": "100"},
+ Name: "child2",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"60"}},
+
map[string]map[string]string{"test-group": {"memory": "60"}}),
},
},
+ },
+ hasError: false,
+ },
+ {
+ name: "queues at same level maxresources can be greater
or less than or equal to the other but with in ancestor parent",
+ config: QueueConfig{
+ Name: "parent",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"100"}},
+
map[string]map[string]string{"test-group": {"memory": "100"}}),
Queues: []QueueConfig{
{
Name: "child1",
- Limits: []Limit{
+ Queues: []QueueConfig{
{
- Limit:
"user-limit",
- Users:
[]string{"test-user"},
- MaxResources:
map[string]string{"memory": "50"},
+ Name: "childA",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"50"}},
+
map[string]map[string]string{"test-group": {"memory": "50"}}),
},
{
- Limit:
"group-limit",
- Groups:
[]string{"test-group"},
- MaxResources:
map[string]string{"memory": "200"},
+ Name: "childB",
+ Limits:
createLimitMaxResources(map[string]map[string]string{"test-user": {"memory":
"60"}},
+
map[string]map[string]string{"test-group": {"memory": "60"}}),
},
},
},
},
},
- hasError: true,
+ hasError: false,
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
- err := checkLimitResource(testCase.config, nil,
make(map[string]*resources.Resource), make(map[string]*resources.Resource))
+ err := checkLimitResource(testCase.config,
make(map[string]map[string]*resources.Resource),
make(map[string]map[string]*resources.Resource), common.Empty)
if testCase.hasError {
assert.ErrorContains(t, err, "is greater than
immediate or ancestor parent maximum resource")
} else {
@@ -882,6 +886,41 @@ func TestCheckLimitResource(t *testing.T) { //nolint:funlen
}
}
+func createLimitMaxResources(users map[string]map[string]string, groups
map[string]map[string]string) []Limit {
+ var limits []Limit
+ for user, limit := range users {
+ var users []string
+ users = append(users, user)
+ limit := Limit{Limit: "user-limit", Users: users, MaxResources:
limit}
+ limits = append(limits, limit)
+ }
+ for group, limit := range groups {
+ var groups []string
+ groups = append(groups, group)
+ limit := Limit{Limit: "group-limit", Groups: groups,
MaxResources: limit}
+ limits = append(limits, limit)
+ }
+ return limits
+}
+
+func createLimitMaxApplications(users map[string]uint64, groups
map[string]uint64) []Limit {
+ var limits []Limit
+ for user, limit := range users {
+ var users []string
+ users = append(users, user)
+ limit := Limit{Limit: "user-limit", Users: users,
MaxApplications: limit}
+ limits = append(limits, limit)
+ }
+
+ for group, limit := range groups {
+ var groups []string
+ groups = append(groups, group)
+ limit := Limit{Limit: "group-limit", Groups: groups,
MaxApplications: limit}
+ limits = append(limits, limit)
+ }
+ return limits
+}
+
func TestCheckLimitMaxApplications(t *testing.T) { //nolint:funlen
testCases := []struct {
name string
@@ -892,48 +931,18 @@ func TestCheckLimitMaxApplications(t *testing.T) {
//nolint:funlen
name: "leaf queue user group maxapplications are within
parent queue user group maxapplications",
config: QueueConfig{
Name: "parent",
- Limits: []Limit{
- {
- Limit: "user-limit",
- Users:
[]string{"test-user"},
- MaxApplications: 100,
- },
- {
- Limit: "group-limit",
- Groups:
[]string{"test-group"},
- MaxApplications: 100,
- },
- },
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
+ map[string]uint64{"test-group": 100}),
Queues: []QueueConfig{
{
Name: "child1",
- Limits: []Limit{
- {
- Limit:
"user-limit",
- Users:
[]string{"test-user"},
-
MaxApplications: 100,
- },
- {
- Limit:
"group-limit",
- Groups:
[]string{"test-group"},
-
MaxApplications: 100,
- },
- },
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
+
map[string]uint64{"test-group": 100}),
Queues: []QueueConfig{
{
Name: "child2",
- Limits: []Limit{
- {
-
Limit: "user-limit",
-
Users: []string{"test-user"},
-
MaxApplications: 100,
- },
- {
-
Limit: "group-limit",
-
Groups: []string{"test-group"},
-
MaxApplications: 100,
- },
- },
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
+
map[string]uint64{"test-group": 100}),
},
},
},
@@ -945,33 +954,13 @@ func TestCheckLimitMaxApplications(t *testing.T) {
//nolint:funlen
name: "leaf queue user maxapplications exceed parent
queue user maxapplications",
config: QueueConfig{
Name: "parent",
- Limits: []Limit{
- {
- Limit: "user-limit",
- Users:
[]string{"test-user"},
- MaxApplications: 100,
- },
- {
- Limit: "group-limit",
- Groups:
[]string{"test-group"},
- MaxApplications: 100,
- },
- },
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
+ map[string]uint64{"test-group": 100}),
Queues: []QueueConfig{
{
Name: "child1",
- Limits: []Limit{
- {
- Limit:
"user-limit",
- Users:
[]string{"test-user"},
-
MaxApplications: 200,
- },
- {
- Limit:
"group-limit",
- Groups:
[]string{"test-group"},
-
MaxApplications: 50,
- },
- },
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 200},
+
map[string]uint64{"test-group": 50}),
},
},
},
@@ -981,33 +970,13 @@ func TestCheckLimitMaxApplications(t *testing.T) {
//nolint:funlen
name: "leaf queue group maxapplications exceed parent
queue group maxapplications",
config: QueueConfig{
Name: "parent",
- Limits: []Limit{
- {
- Limit: "user-limit",
- Users:
[]string{"test-user"},
- MaxApplications: 100,
- },
- {
- Limit: "group-limit",
- Groups:
[]string{"test-group"},
- MaxApplications: 100,
- },
- },
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 100},
+ map[string]uint64{"test-group": 100}),
Queues: []QueueConfig{
{
Name: "child1",
- Limits: []Limit{
- {
- Limit:
"user-limit",
- Users:
[]string{"test-user"},
-
MaxApplications: 50,
- },
- {
- Limit:
"group-limit",
- Groups:
[]string{"test-group"},
-
MaxApplications: 200,
- },
- },
+ Limits:
createLimitMaxApplications(map[string]uint64{"test-user": 50},
+
map[string]uint64{"test-group": 200}),
},
},
},
@@ -1017,7 +986,7 @@ func TestCheckLimitMaxApplications(t *testing.T) {
//nolint:funlen
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
- err := checkLimitMaxApplications(testCase.config, nil,
make(map[string]uint64), make(map[string]uint64))
+ 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")
} else {
@@ -1231,7 +1200,7 @@ func TestCheckLimits(t *testing.T) { //nolint:funlen
},
},
},
- errMsg: "MaxApplications is 0",
+ errMsg: "",
},
{
name: "group maxapplications is 0",
@@ -1256,7 +1225,7 @@ func TestCheckLimits(t *testing.T) { //nolint:funlen
},
},
},
- errMsg: "MaxApplications is 0",
+ errMsg: "",
},
{
name: "user wildcard is not last entry",
diff --git a/pkg/webservice/handlers_test.go b/pkg/webservice/handlers_test.go
index a0199f85..d7f8eedc 100644
--- a/pkg/webservice/handlers_test.go
+++ b/pkg/webservice/handlers_test.go
@@ -197,6 +197,60 @@ partitions:
vcore: 10000
`
+const userGroupLimitsConfig = `
+partitions:
+ - name: default
+ queues:
+ - name: root
+ parent: true
+ submitacl: '*'
+ queues:
+ - name: parent1
+ parent: true
+ limits:
+ - limit: ""
+ users:
+ - test_user
+ maxapplications: 0
+ maxresources:
+ cpu: "200"
+`
+
+const userGroupLimitsInvalidConfig = `
+partitions:
+ - name: default
+ queues:
+ - name: root
+ parent: true
+ submitacl: '*'
+ queues:
+ - name: parent1
+ parent: true
+ limits:
+ - limit: ""
+ users:
+ - test_user
+ maxapplications: 1
+ maxresources:
+ cpu: "0"
+`
+
+const userGroupLimitsInvalidConfig1 = `
+partitions:
+ - name: default
+ queues:
+ - name: root
+ parent: true
+ submitacl: '*'
+ queues:
+ - name: parent1
+ parent: true
+ limits:
+ - limit: ""
+ users:
+ - test_user
+`
+
const rmID = "rm-123"
const policyGroup = "default-policy-group"
const queueName = "root.default"
@@ -266,6 +320,47 @@ func TestValidateConf(t *testing.T) {
}
}
+func TestUserGroupLimits(t *testing.T) {
+ confTests := []struct {
+ content string
+ expectedResponse dao.ValidateConfResponse
+ }{
+ {
+ content: userGroupLimitsConfig,
+ expectedResponse: dao.ValidateConfResponse{
+ Allowed: true,
+ Reason: common.Empty,
+ },
+ },
+ {
+ content: userGroupLimitsInvalidConfig,
+ expectedResponse: dao.ValidateConfResponse{
+ Allowed: false,
+ Reason: "MaxResources is zero in '' limit, all
resource types are zero",
+ },
+ },
+ {
+ content: userGroupLimitsInvalidConfig1,
+ expectedResponse: dao.ValidateConfResponse{
+ Allowed: false,
+ Reason: "invalid resource combination for
limit all resource limits are null",
+ },
+ },
+ }
+ for _, test := range confTests {
+ // No err check: new request always returns correctly
+ //nolint: errcheck
+ req, _ := http.NewRequest("POST", "",
strings.NewReader(test.content))
+ resp := &MockResponseWriter{}
+ validateConf(resp, req)
+ var vcr dao.ValidateConfResponse
+ err := json.Unmarshal(resp.outputBytes, &vcr)
+ assert.NilError(t, err, "failed to unmarshal
ValidateConfResponse from response body")
+ assert.Equal(t, vcr.Allowed, test.expectedResponse.Allowed,
"allowed flag incorrect")
+ assert.Equal(t, vcr.Reason, test.expectedResponse.Reason,
"response text not as expected")
+ }
+}
+
func TestApplicationHistory(t *testing.T) {
// make sure the history is nil when we finish this test
defer ResetIMHistory()
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
