You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by ch...@apache.org on 2024/05/08 06:27:54 UTC
(yunikorn-core) branch master updated: [YUNIKORN-2598] Update the unit test for CheckQueues and checkNodeSortingPolicy functions (#863)
This is an automated email from the ASF dual-hosted git repository.
chia7712 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 7f5ab21e [YUNIKORN-2598] Update the unit test for CheckQueues and checkNodeSortingPolicy functions (#863)
7f5ab21e is described below
commit 7f5ab21efc86e706e9406ac0ae194bcf7616b8c8
Author: YUN SUN <yu...@gmail.com>
AuthorDate: Wed May 8 14:27:10 2024 +0800
[YUNIKORN-2598] Update the unit test for CheckQueues and checkNodeSortingPolicy functions (#863)
Closes: #863
Signed-off-by: Chia-Ping Tsai <ch...@gmail.com>
---
pkg/common/configs/config_test.go | 8 +-
pkg/common/configs/configvalidator.go | 10 +-
pkg/common/configs/configvalidator_test.go | 217 +++++++++++++++++++++++++++++
3 files changed, 226 insertions(+), 9 deletions(-)
diff --git a/pkg/common/configs/config_test.go b/pkg/common/configs/config_test.go
index e672a202..2493be27 100644
--- a/pkg/common/configs/config_test.go
+++ b/pkg/common/configs/config_test.go
@@ -1513,7 +1513,7 @@ partitions:
`
// validate the config and check after the update
_, err = CreateConfig(data)
- assert.ErrorContains(t, err, "duplicated user name *")
+ assert.ErrorContains(t, err, "duplicated user name '*'")
data = `
partitions:
@@ -1548,7 +1548,7 @@ partitions:
`
// validate the config and check after the update
_, err = CreateConfig(data)
- assert.ErrorContains(t, err, "duplicated group name *")
+ assert.ErrorContains(t, err, "duplicated group name '*'")
data = `
partitions:
@@ -1610,7 +1610,7 @@ partitions:
`
// validate the config and check after the update
_, err = CreateConfig(data)
- assert.ErrorContains(t, err, "duplicated user name user.lastname")
+ assert.ErrorContains(t, err, "duplicated user name 'user.lastname'")
data = `
partitions:
@@ -1645,7 +1645,7 @@ partitions:
`
// validate the config and check after the update
_, err = CreateConfig(data)
- assert.ErrorContains(t, err, "duplicated group name test")
+ assert.ErrorContains(t, err, "duplicated group name 'test'")
// Make sure different queues can support same username or groupname
data = `
diff --git a/pkg/common/configs/configvalidator.go b/pkg/common/configs/configvalidator.go
index 26c85233..ec8e15a3 100644
--- a/pkg/common/configs/configvalidator.go
+++ b/pkg/common/configs/configvalidator.go
@@ -489,7 +489,7 @@ func checkLimit(limit Limit, existedUserName map[string]bool, existedGroupName m
}
if existedUserName[name] {
- return fmt.Errorf("duplicated user name %s , already existed", name)
+ return fmt.Errorf("duplicated user name '%s', already exists", name)
}
existedUserName[name] = true
@@ -506,7 +506,7 @@ func checkLimit(limit Limit, existedUserName map[string]bool, existedGroupName m
}
if existedGroupName[name] {
- return fmt.Errorf("duplicated group name %s , already existed", name)
+ return fmt.Errorf("duplicated group name '%s' , already existed", name)
}
existedGroupName[name] = true
@@ -632,7 +632,7 @@ func checkNodeSortingPolicy(partition *PartitionConfig) error {
// Check the queue names configured for compliance and uniqueness
// - no duplicate names at each branched level in the tree
// - queue name is alphanumeric (case ignore) with - and _
-// - queue name is maximum 16 char long
+// - queue name is maximum 64 char long
func checkQueues(queue *QueueConfig, level int) error {
// check the ACLs (if defined)
err := checkACL(queue.AdminACL)
@@ -654,11 +654,11 @@ func checkQueues(queue *QueueConfig, level int) error {
queueMap := make(map[string]bool)
for _, child := range queue.Queues {
if !QueueNameRegExp.MatchString(child.Name) {
- return fmt.Errorf("invalid child name %s, a name must only have alphanumeric characters,"+
+ return fmt.Errorf("invalid child name '%s', a name must only have alphanumeric characters,"+
" - or _, and be no longer than 64 characters", child.Name)
}
if queueMap[strings.ToLower(child.Name)] {
- return fmt.Errorf("duplicate child name found with name %s, level %d", child.Name, level)
+ return fmt.Errorf("duplicate child name found with name '%s', level %d", child.Name, level)
}
queueMap[strings.ToLower(child.Name)] = true
}
diff --git a/pkg/common/configs/configvalidator_test.go b/pkg/common/configs/configvalidator_test.go
index 8ddf1d3c..5e9a77df 100644
--- a/pkg/common/configs/configvalidator_test.go
+++ b/pkg/common/configs/configvalidator_test.go
@@ -2012,3 +2012,220 @@ func TestCheckQueuesStructure(t *testing.T) {
})
}
}
+
+func TestCheckQueues(t *testing.T) { //nolint:funlen
+ testCases := []struct {
+ name string
+ queue *QueueConfig
+ level int
+ expectedErrorMsg string
+ validateFunc func(t *testing.T, queue *QueueConfig)
+ }{
+ {
+ name: "Invalid ACL Format for AdminACL",
+ queue: &QueueConfig{
+ Name: "validQueue",
+ AdminACL: "admin group extra",
+ SubmitACL: "submit",
+ Queues: []QueueConfig{{Name: "validSubQueue"}},
+ },
+ level: 0,
+ expectedErrorMsg: "multiple spaces found in ACL: 'admin group extra'",
+ },
+ {
+ name: "Invalid ACL Format for SubmitACL",
+ queue: &QueueConfig{
+ Name: "validQueue",
+ AdminACL: "admin",
+ SubmitACL: "submit group extra",
+ Queues: []QueueConfig{{Name: "validSubQueue"}},
+ },
+ level: 0,
+ expectedErrorMsg: "multiple spaces found in ACL: 'submit group extra'",
+ },
+ {
+ name: "Duplicate Child Queue Names",
+ queue: &QueueConfig{
+ Name: "root",
+ AdminACL: "admin",
+ SubmitACL: "submit",
+ Queues: []QueueConfig{
+ {Name: "duplicateQueue"},
+ {Name: "duplicateQueue"},
+ },
+ },
+ level: 0,
+ expectedErrorMsg: "duplicate child name found with name 'duplicateQueue', level 0",
+ },
+ {
+ name: "Duplicate Child Queue Names at level 1",
+ queue: &QueueConfig{
+ Name: "root",
+ AdminACL: "admin",
+ SubmitACL: "submit",
+ Queues: []QueueConfig{
+ {
+ Name: "subqueue",
+ AdminACL: "admin",
+ SubmitACL: "submit",
+ Queues: []QueueConfig{
+ {Name: "duplicateQueue"},
+ {Name: "duplicateQueue"},
+ },
+ },
+ },
+ },
+ level: 0,
+ expectedErrorMsg: "duplicate child name found with name 'duplicateQueue', level 1",
+ },
+ {
+ name: "Check Limits Error With Duplicated User Name",
+ queue: &QueueConfig{
+ Name: "root",
+ AdminACL: "admin",
+ SubmitACL: "submit",
+ Queues: []QueueConfig{
+ {
+ Name: "subqueue",
+ },
+ },
+ Limits: []Limit{
+ {
+ Limit: "user-limit",
+ Users: []string{"user1", "user2", "user1"},
+ },
+ },
+ },
+ level: 0,
+ expectedErrorMsg: "duplicated user name 'user1', already exists",
+ },
+ {
+ name: "Invalid Child Queue Name Length",
+ queue: &QueueConfig{
+ Name: "root",
+ AdminACL: "admin",
+ SubmitACL: "submit",
+ Queues: []QueueConfig{
+ {Name: "thisQueueNameIsTooLongthisQueueNameIsTooLongthisQueueNameIsTooLong"},
+ },
+ },
+ level: 0,
+ expectedErrorMsg: "invalid child name 'thisQueueNameIsTooLongthisQueueNameIsTooLongthisQueueNameIsTooLong', a name must only have alphanumeric characters, - or _, and be no longer than 64 characters",
+ },
+ {
+ name: "Invalid Child Queue Name With Special Character",
+ queue: &QueueConfig{
+ Name: "root",
+ AdminACL: "admin",
+ SubmitACL: "submit",
+ Queues: []QueueConfig{
+ {Name: "queue_Name$"},
+ },
+ },
+ level: 0,
+ expectedErrorMsg: "invalid child name 'queue_Name$', a name must only have alphanumeric characters, - or _, and be no longer than 64 characters",
+ },
+ {
+ name: "Valid Multiple Queues",
+ queue: &QueueConfig{
+ Name: "root",
+ AdminACL: "admin",
+ SubmitACL: "submit",
+ Queues: []QueueConfig{
+ {Name: "queue_One"},
+ {Name: "queue-Two"},
+ },
+ },
+ level: 0,
+ validateFunc: func(t *testing.T, q *QueueConfig) {
+ assert.Equal(t, 2, len(q.Queues), "Expected two queues")
+ },
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+ err := checkQueues(tc.queue, tc.level)
+ if tc.expectedErrorMsg != "" {
+ assert.ErrorContains(t, err, tc.expectedErrorMsg, "Error message mismatch")
+ } else {
+ assert.NilError(t, err, "No error is expected")
+ }
+
+ if tc.validateFunc != nil {
+ tc.validateFunc(t, tc.queue)
+ }
+ })
+ }
+}
+
+func TestCheckNodeSortingPolicy(t *testing.T) { //nolint:funlen
+ testCases := []struct {
+ name string
+ partition *PartitionConfig
+ expectedErrorMsg string
+ validateFunc func(t *testing.T, partition *PartitionConfig)
+ }{
+ {
+ name: "Valid Sorting Policy with Positive Weights",
+ partition: &PartitionConfig{
+ NodeSortPolicy: NodeSortingPolicy{
+ Type: "fair",
+ ResourceWeights: map[string]float64{"memory": 1.0},
+ },
+ },
+ validateFunc: func(t *testing.T, p *PartitionConfig) {
+ assert.Equal(t, "fair", p.NodeSortPolicy.Type, "Expected sorting policy type to be 'fair'")
+ assert.Equal(t, 1, len(p.NodeSortPolicy.ResourceWeights), "Expected one resource weights")
+ },
+ },
+ {
+ name: "Negative Resource Weight",
+ partition: &PartitionConfig{
+ NodeSortPolicy: NodeSortingPolicy{
+ Type: "fair",
+ ResourceWeights: map[string]float64{"memory": -1.0},
+ },
+ },
+ expectedErrorMsg: "negative resource weight for memory is not allowed",
+ },
+ {
+ name: "Undefined Sorting Policy",
+ partition: &PartitionConfig{
+ NodeSortPolicy: NodeSortingPolicy{
+ Type: "undefinedPolicy",
+ ResourceWeights: map[string]float64{"memory": 1.0},
+ },
+ },
+ expectedErrorMsg: "undefined policy: undefinedPolicy",
+ },
+ {
+ name: "Valid Policy with Multiple Resources",
+ partition: &PartitionConfig{
+ NodeSortPolicy: NodeSortingPolicy{
+ Type: "binpacking",
+ ResourceWeights: map[string]float64{"memory": 2.0, "cpu": 3.0},
+ },
+ },
+ validateFunc: func(t *testing.T, p *PartitionConfig) {
+ assert.Equal(t, "binpacking", p.NodeSortPolicy.Type, "Expected sorting policy type to be 'binpacking'")
+ assert.Equal(t, 2, len(p.NodeSortPolicy.ResourceWeights), "Expected two resource weights")
+ },
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+ err := checkNodeSortingPolicy(tc.partition)
+ if tc.expectedErrorMsg != "" {
+ assert.ErrorContains(t, err, tc.expectedErrorMsg, "Error message mismatch")
+ } else {
+ assert.NilError(t, err, "No error is expected")
+ }
+
+ if tc.validateFunc != nil {
+ tc.validateFunc(t, tc.partition)
+ }
+ })
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org