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 2cda6640 [YUNIKORN-1959] add check for maxapplications and
maxresources(#638)
2cda6640 is described below
commit 2cda66401279c548c63cc9484a309b5ddc766d4d
Author: PoAn Yang <[email protected]>
AuthorDate: Thu Sep 7 17:12:59 2023 +0530
[YUNIKORN-1959] add check for maxapplications and maxresources(#638)
* maxapplications can't be zero
* maxresources fields can't be all zero
Closes: #638
Signed-off-by: Manikandan R <[email protected]>
---
pkg/common/configs/config_test.go | 2 +-
pkg/common/configs/configvalidator.go | 16 ++++---
pkg/common/configs/configvalidator_test.go | 71 +++++++++++++++++++++++++-----
3 files changed, 69 insertions(+), 20 deletions(-)
diff --git a/pkg/common/configs/config_test.go
b/pkg/common/configs/config_test.go
index a0b9b4a2..ef0fa645 100644
--- a/pkg/common/configs/config_test.go
+++ b/pkg/common/configs/config_test.go
@@ -1071,7 +1071,7 @@ partitions:
`
// validate the config and check after the update
_, err = CreateConfig(data)
- assert.ErrorContains(t, err, "MaxApplications is 0 in limit name")
+ assert.ErrorContains(t, err, "MaxApplications is 0")
// 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 a7f304e6..7bab772a 100644
--- a/pkg/common/configs/configvalidator.go
+++ b/pkg/common/configs/configvalidator.go
@@ -509,19 +509,21 @@ func checkLimit(limit Limit, existedUserName
map[string]bool, existedGroupName m
zap.Error(err))
return err
}
+ if resources.IsZero(limitResource) {
+ return fmt.Errorf("MaxResources is zero in '%s' limit,
all resource types are zero", limit.Limit)
+ }
}
// at least some resource should be not null
if limit.MaxApplications == 0 && resources.IsZero(limitResource) {
return fmt.Errorf("invalid resource combination for limit %s
all resource limits are null", limit.Limit)
}
- if queue.MaxApplications != 0 {
- if limit.MaxApplications > queue.MaxApplications {
- return fmt.Errorf("invalid MaxApplications settings for
limit %s exeecd current the queue MaxApplications", limit.Limit)
- }
- if limit.MaxApplications == 0 {
- return fmt.Errorf("MaxApplications is 0 in limit name
%s, it should be 1 ~ %d", limit.Limit, queue.MaxApplications)
- }
+ 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)
}
// If queue is RootQueue, the queue.Resources.Max will be null, we
don't need to check for root queue
diff --git a/pkg/common/configs/configvalidator_test.go
b/pkg/common/configs/configvalidator_test.go
index 893bd7e3..c89b4563 100644
--- a/pkg/common/configs/configvalidator_test.go
+++ b/pkg/common/configs/configvalidator_test.go
@@ -1077,6 +1077,49 @@ func TestCheckLimits(t *testing.T) { //nolint:funlen
},
errMsg: "",
},
+ {
+ name: "partial fields in maxresources are 0",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: []Limit{
+ {
+ Limit: "user-limit",
+ Users:
[]string{"test-user"},
+ MaxApplications: 1,
+ MaxResources:
map[string]string{"memory": "100", "cpu": "0", "nvidia.com/gpu": "0"},
+ },
+ },
+ },
+ errMsg: "",
+ },
+ {
+ name: "all fields in maxresources are 0",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: []Limit{
+ {
+ Limit: "user-limit",
+ Users:
[]string{"test-user"},
+ MaxApplications: 1,
+ MaxResources:
map[string]string{"memory": "0", "cpu": "0"},
+ },
+ },
+ },
+ errMsg: "MaxResources is zero",
+ },
+ {
+ name: "both maxresources and maxresources are 0",
+ config: QueueConfig{
+ Name: "parent",
+ Limits: []Limit{
+ {
+ Limit: "user-limit",
+ Users: []string{"test-user"},
+ },
+ },
+ },
+ errMsg: "invalid resource combination",
+ },
{
name: "user maxresources exceed queue limits",
config: QueueConfig{
@@ -1086,14 +1129,16 @@ func TestCheckLimits(t *testing.T) { //nolint:funlen
},
Limits: []Limit{
{
- Limit: "user-limit",
- Users:
[]string{"test-user"},
- MaxResources:
map[string]string{"memory": "200"},
+ Limit: "user-limit",
+ Users:
[]string{"test-user"},
+ MaxApplications: 1,
+ MaxResources:
map[string]string{"memory": "200"},
},
{
- Limit: "group-limit",
- Groups:
[]string{"test-group"},
- MaxResources:
map[string]string{"memory": "100"},
+ Limit: "group-limit",
+ Groups:
[]string{"test-group"},
+ MaxApplications: 1,
+ MaxResources:
map[string]string{"memory": "100"},
},
},
},
@@ -1108,14 +1153,16 @@ func TestCheckLimits(t *testing.T) { //nolint:funlen
},
Limits: []Limit{
{
- Limit: "user-limit",
- Users:
[]string{"test-user"},
- MaxResources:
map[string]string{"memory": "100"},
+ Limit: "user-limit",
+ Users:
[]string{"test-user"},
+ MaxApplications: 1,
+ MaxResources:
map[string]string{"memory": "100"},
},
{
- Limit: "group-limit",
- Groups:
[]string{"test-group"},
- MaxResources:
map[string]string{"memory": "200"},
+ Limit: "group-limit",
+ Groups:
[]string{"test-group"},
+ MaxApplications: 1,
+ MaxResources:
map[string]string{"memory": "200"},
},
},
},
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
