wilfred-s commented on code in PR #489:
URL: https://github.com/apache/yunikorn-core/pull/489#discussion_r1058062419
##########
pkg/scheduler/objects/queue_test.go:
##########
@@ -1545,6 +1545,67 @@ func TestApplyConf(t *testing.T) {
assert.Assert(t, root.guaranteedResource == nil)
}
+func TestNewConfiguredQueue(t *testing.T) {
+ // check variable assignment
+ properties := getProperties()
+ resourceConf := getResourceConf()
+ parentConfig := configs.QueueConfig{
+ Name: "PARENT_QUEUE",
+ Parent: true,
+ MaxApplications: uint64(32),
+ ChildTemplate: configs.ChildTemplate{
+ Properties: properties,
+ Resources: configs.Resources{
+ Max: resourceConf,
+ Guaranteed: resourceConf,
+ },
+ },
+ }
+ parent, err := NewConfiguredQueue(parentConfig, nil)
+ assert.NilError(t, err, "failed to create queue: %v", err)
+ assert.Equal(t, parent.Name, "parent_queue")
+ assert.Equal(t, parent.QueuePath, "parent_queue")
+ assert.Equal(t, parent.isManaged, true)
+ assert.Equal(t, parent.maxRunningApps, uint64(32))
+ assert.Assert(t, reflect.DeepEqual(properties,
parent.template.GetProperties()))
+ // turn resouce config into resource struct
+ resourceStruct, err := resources.NewResourceFromConf(resourceConf)
+ assert.NilError(t, err, "failed to create new resource from config:
%v", err)
Review Comment:
move with the line that creates the resource conf (1551) for clarity
##########
pkg/scheduler/objects/queue_test.go:
##########
@@ -1545,6 +1545,67 @@ func TestApplyConf(t *testing.T) {
assert.Assert(t, root.guaranteedResource == nil)
}
+func TestNewConfiguredQueue(t *testing.T) {
+ // check variable assignment
+ properties := getProperties()
+ resourceConf := getResourceConf()
+ parentConfig := configs.QueueConfig{
+ Name: "PARENT_QUEUE",
+ Parent: true,
+ MaxApplications: uint64(32),
+ ChildTemplate: configs.ChildTemplate{
+ Properties: properties,
+ Resources: configs.Resources{
+ Max: resourceConf,
+ Guaranteed: resourceConf,
+ },
+ },
+ }
+ parent, err := NewConfiguredQueue(parentConfig, nil)
+ assert.NilError(t, err, "failed to create queue: %v", err)
+ assert.Equal(t, parent.Name, "parent_queue")
+ assert.Equal(t, parent.QueuePath, "parent_queue")
+ assert.Equal(t, parent.isManaged, true)
+ assert.Equal(t, parent.maxRunningApps, uint64(32))
+ assert.Assert(t, reflect.DeepEqual(properties,
parent.template.GetProperties()))
+ // turn resouce config into resource struct
+ resourceStruct, err := resources.NewResourceFromConf(resourceConf)
+ assert.NilError(t, err, "failed to create new resource from config:
%v", err)
+ assert.Assert(t, reflect.DeepEqual(resourceStruct,
parent.template.GetMaxResource()))
+ assert.Assert(t, reflect.DeepEqual(resourceStruct,
parent.template.GetGuaranteedResource()))
+
+ // case 0: leaf can use template
+ leafConfig := configs.QueueConfig{
+ Name: "leaf_queue",
+ Parent: false,
+ Properties: getProperties(),
+ Resources: configs.Resources{
+ Max: getResourceConf(),
+ Guaranteed: getResourceConf(),
+ },
+ }
+ childLeaf, err := NewConfiguredQueue(leafConfig, parent)
+ assert.NilError(t, err, "failed to create queue: %v", err)
+ assert.Equal(t, childLeaf.QueuePath, "parent_queue.leaf_queue")
+ assert.Assert(t, childLeaf.template == nil)
+ assert.Assert(t, reflect.DeepEqual(childLeaf.properties,
parent.template.GetProperties()))
+ assert.Assert(t, reflect.DeepEqual(childLeaf.maxResource,
parent.template.GetMaxResource()))
+ assert.Assert(t, reflect.DeepEqual(childLeaf.guaranteedResource,
parent.template.GetGuaranteedResource()))
Review Comment:
use `resources.Equals()` for the check. If by coincidence we get a zero
value for a quantity the config and the set value will not be the same as we
ignore zero max and guaranteed. It is safer in the long run as it tests
expected behaviour.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
