This is an automated email from the ASF dual-hosted git repository.
pbacsko 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 9519f7cf [YUNIKORN-2694] Improve placement rule funtion's test
coverage (#899)
9519f7cf is described below
commit 9519f7cfcf6503239aab6ef978f94ec94c826353
Author: SP12893678 <36910625+sp12893...@users.noreply.github.com>
AuthorDate: Tue Jun 25 19:29:47 2024 +0200
[YUNIKORN-2694] Improve placement rule funtion's test coverage (#899)
Closes: #899
Signed-off-by: Peter Bacsko <pbac...@cloudera.com>
---
pkg/scheduler/placement/placement_test.go | 82 +++++++++++++++++++++++++++
pkg/scheduler/placement/provided_rule_test.go | 65 +++++++++++++++++++++
pkg/scheduler/placement/rule_test.go | 12 ++++
pkg/scheduler/placement/testrule_test.go | 7 +++
4 files changed, 166 insertions(+)
diff --git a/pkg/scheduler/placement/placement_test.go
b/pkg/scheduler/placement/placement_test.go
index 14fe6ace..ef05c95e 100644
--- a/pkg/scheduler/placement/placement_test.go
+++ b/pkg/scheduler/placement/placement_test.go
@@ -38,6 +38,11 @@ func TestManagerNew(t *testing.T) {
assert.Equal(t, 2, len(man.rules), "wrong rule count for new placement
manager, no config")
assert.Equal(t, types.Provided, man.rules[0].getName(), "wrong name for
implicit provided rule")
assert.Equal(t, types.Recovery, man.rules[1].getName(), "wrong name for
implicit recovery rule")
+
+ // fail update rules with unknown rule
+ rules := []configs.PlacementRule{{Name: "unknown", Create: true}}
+ man = NewPlacementManager(rules, queueFunc)
+ assert.Equal(t, 0, len(man.rules), "wrong rule count for new placement
manager, no config")
}
func TestManagerInit(t *testing.T) {
@@ -101,6 +106,24 @@ func TestManagerInit(t *testing.T) {
assert.Equal(t, 2, len(ruleDAOs), "wrong DAO count for nil rules
config")
assert.Equal(t, ruleDAOs[0].Name, types.Provided, "expected provided
rule as first rule")
assert.Equal(t, ruleDAOs[1].Name, types.Recovery, "expected recovery
rule as second rule")
+
+ // init the manager with one rule which has parent
+ rules = []configs.PlacementRule{
+ {
+ Name: "test",
+ Create: true,
+ Parent: &configs.PlacementRule{
+ Name: "test",
+ },
+ },
+ }
+ err = man.initialise(rules)
+ assert.NilError(t, err, "Failed to re-initialize nil placement rules")
+ assert.Equal(t, 2, len(man.rules), "wrong rule count for nil rules
config")
+ ruleDAOs = man.GetRulesDAO()
+ assert.Equal(t, 2, len(ruleDAOs), "wrong DAO count for nil rules
config")
+ assert.Equal(t, ruleDAOs[0].Name, types.Test, "expected test rule as
first rule")
+ assert.Equal(t, ruleDAOs[1].Name, types.Recovery, "expected recovery
rule as second rule")
}
func TestManagerUpdate(t *testing.T) {
@@ -135,6 +158,13 @@ func TestManagerUpdate(t *testing.T) {
assert.Equal(t, 2, len(ruleDAOs), "wrong DAO count for nil rules
config")
assert.Equal(t, ruleDAOs[0].Name, types.Provided, "expected provided
rule as first rule")
assert.Equal(t, ruleDAOs[1].Name, types.Recovery, "expected recovery
rule as second rule")
+
+ // fail to update rules with unknown rule
+ rules = []configs.PlacementRule{{Name: "unknown", Create: true}}
+ err = man.UpdateRules(rules)
+ if err == nil {
+ t.Errorf("unknown rule should fail to update the rule.")
+ }
}
func TestManagerBuildRule(t *testing.T) {
@@ -379,3 +409,55 @@ partitions:
})
}
}
+
+func TestManagerPlaceApp_Error(t *testing.T) {
+ // Create the structure for the test
+ data := `
+partitions:
+ - name: default
+ queues:
+ - name: root
+ queues:
+ - name: testparent
+ submitacl: "*"
+ queues:
+ - name: testchild
+ - name: fixed
+ submitacl: "other-user "
+ parent: true
+`
+ err := initQueueStructure([]byte(data))
+ assert.NilError(t, err, "setting up the queue config failed")
+ // basic info without rules, manager should init
+ man := NewPlacementManager(nil, queueFunc)
+ if man == nil {
+ t.Fatal("placement manager create failed")
+ }
+ rules := []configs.PlacementRule{
+ {
+ Name: "user",
+ Create: false,
+ Parent: &configs.PlacementRule{
+ Name: "user",
+ Create: false,
+ Parent: &configs.PlacementRule{
+ Name: "fixed",
+ Value: "testparent",
+ },
+ },
+ },
+ }
+ user := security.UserGroup{
+ User: "testchild",
+ Groups: []string{},
+ }
+ tags := make(map[string]string)
+ err = man.UpdateRules(rules)
+ assert.NilError(t, err, "failed to update existing manager")
+ app := newApplication("app1", "default", "", user, tags, nil, "")
+ err = man.PlaceApplication(app)
+ queueName := app.GetQueuePath()
+ if err == nil || queueName != "" {
+ t.Errorf("failed placed app, queue: '%s', error: %v",
queueName, err)
+ }
+}
diff --git a/pkg/scheduler/placement/provided_rule_test.go
b/pkg/scheduler/placement/provided_rule_test.go
index ee85e2f0..82a496d9 100644
--- a/pkg/scheduler/placement/provided_rule_test.go
+++ b/pkg/scheduler/placement/provided_rule_test.go
@@ -136,6 +136,23 @@ partitions:
if err == nil {
t.Errorf("provided rule should have failed to place app, error
%v", err)
}
+ // deny filter type should got got empty queue
+ conf = configs.PlacementRule{
+ Name: "provided",
+ Filter: configs.Filter{
+ Type: filterDeny,
+ },
+ }
+ pr, err = newRule(conf)
+ if err != nil || pr == nil {
+ t.Errorf("provided rule create failed with parent name, err
%v", err)
+ }
+
+ appInfo = newApplication("app1", "default", "testchild", user, tags,
nil, "")
+ queue, err = pr.placeApplication(appInfo, queueFunc)
+ if queue != "" || err != nil {
+ t.Errorf("provided rule with deny filter type should got empty
queue, err nil")
+ }
}
func TestProvidedRuleParent(t *testing.T) {
@@ -249,6 +266,54 @@ func TestProvidedRuleParent(t *testing.T) {
if queue != "" || err == nil {
t.Errorf("provided rule placed app in incorrect queue '%s', err
%v", queue, err)
}
+
+ // failed parent rule
+ conf = configs.PlacementRule{
+ Name: "provided",
+ Create: true,
+ Parent: &configs.PlacementRule{
+ Name: "fixed",
+ Value: "testchild",
+ Parent: &configs.PlacementRule{
+ Name: "fixed",
+ Value: "testchild",
+ },
+ },
+ }
+ pr, err = newRule(conf)
+ if err != nil || pr == nil {
+ t.Errorf("provided rule create failed, err %v", err)
+ }
+
+ appInfo = newApplication("app1", "default", "unknown", user, tags, nil,
"")
+ queue, err = pr.placeApplication(appInfo, queueFunc)
+ if queue != "" || err == nil {
+ t.Errorf("provided rule placed app in incorrect queue '%s', err
%v", queue, err)
+ }
+
+ // parent name not has prefix
+ conf = configs.PlacementRule{
+ Name: "provided",
+ Create: true,
+ Parent: &configs.PlacementRule{
+ Name: "provided",
+ Create: true,
+ Parent: &configs.PlacementRule{
+ Name: "fixed",
+ Value: "root",
+ },
+ },
+ }
+ pr, err = newRule(conf)
+ if err != nil || pr == nil {
+ t.Errorf("provided rule create failed, err %v", err)
+ }
+
+ appInfo = newApplication("app1", "default", "unknown", user, tags, nil,
"")
+ queue, err = pr.placeApplication(appInfo, queueFunc)
+ if queue != "root.root.unknown.unknown" || err != nil {
+ t.Errorf("provided rule placed app in incorrect queue '%s', err
%v", queue, err)
+ }
}
func Test_providedRule_ruleDAO(t *testing.T) {
diff --git a/pkg/scheduler/placement/rule_test.go
b/pkg/scheduler/placement/rule_test.go
index 3df65c3d..308f4571 100644
--- a/pkg/scheduler/placement/rule_test.go
+++ b/pkg/scheduler/placement/rule_test.go
@@ -115,3 +115,15 @@ func TestReplaceDot(t *testing.T) {
t.Errorf("replace start or end dots failed, name: %s, ", name)
}
}
+
+func TestBasicRule(t *testing.T) {
+ rule := &basicRule{}
+ if name := rule.getName(); name != "unnamed rule" {
+ t.Errorf("expected %s, got %s", "unnamed rule", name)
+ }
+
+ dao := rule.ruleDAO()
+ if dao.Name != "unnamed rule" {
+ t.Errorf("expected %s, got %s", "unnamed rule", dao.Name)
+ }
+}
diff --git a/pkg/scheduler/placement/testrule_test.go
b/pkg/scheduler/placement/testrule_test.go
index 38453b31..8ca7e6a8 100644
--- a/pkg/scheduler/placement/testrule_test.go
+++ b/pkg/scheduler/placement/testrule_test.go
@@ -134,4 +134,11 @@ partitions:
if queue != "testchild" || err != nil {
t.Errorf("test rule placed app in incorrect queue '%s', err
%v", queue, err)
}
+
+ // invalid queueName
+ appInfo = newApplication("app1", "default", "test$child", user, tags,
nil, "")
+ queue, err = pr.placeApplication(appInfo, queueFunc)
+ if queue != "" || err == nil {
+ t.Errorf("invalid queueName should got empty queueName")
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: issues-h...@yunikorn.apache.org
