pbacsko commented on code in PR #563:
URL: https://github.com/apache/yunikorn-core/pull/563#discussion_r1228171403


##########
pkg/scheduler/partition_test.go:
##########
@@ -1900,7 +1956,7 @@ func setupPreemptionForRequiredNode(t *testing.T) 
(*PartitionContext, *objects.A
        // check if updated (must be after allocate call)
        assert.Equal(t, 1, len(app.GetReservations()), "ask should have been 
reserved")
        assert.Equal(t, 1, len(app.GetAskReservations(allocID2)), "ask should 
have been reserved")
-       assertUserGroupResource(t, getTestUserGroup(), 
resources.NewResourceFromMap(map[string]resources.Quantity{"vcore": 8000}))
+       internalAssertUserGroupResource(t, getTestUserGroup(), 
resources.NewResourceFromMap(map[string]resources.Quantity{"vcore": 8000}), 
getExpectedQueuesLimitsForPreemptionWithRequiredNode())

Review Comment:
   Will you rebase this based on #543? You already made some changes which are 
similar. Is this related? (You changed `partition_test.go` in #543).
   
   EDIT: OK, this will change



##########
pkg/common/configs/config_test.go:
##########
@@ -1721,3 +1722,418 @@ func TestGetConfigurationString(t *testing.T) {
                })
        }
 }
+
+func prepareUserLimitsConfig(leafQueueMaxApps uint64, leafQueueMaxResource 
string) string {
+       data := `
+partitions:
+  - name: default
+    queues:
+      - name: root
+        maxapplications: 3000
+        limits:
+          - limit:
+            users: 
+            - user1
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 500
+          - limit:
+            users:
+            - user2
+            groups:
+            - prod
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 300
+        queues:
+          - name: level1
+            maxapplications: 100
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                {memory: 10000, vcore: 1000}
+            limits:
+              - limit:
+                users: 
+                - user1
+                maxapplications: 50
+                maxresources: {memory: 1000, vcore: 100}
+            queues:
+              - name: leaf
+                maxapplications: 100
+                resources:
+                  guaranteed:
+                    {memory: 1000, vcore: 10}
+                  max:
+                    {memory: 10000, vcore: 1000}
+                limits:
+                  - limit:
+                    users: 
+                    - user1
+                    maxapplications: ` + strconv.Itoa(int(leafQueueMaxApps)) + 
`
+                    maxresources: ` + leafQueueMaxResource + `
+          - name: level2
+            maxapplications: 100
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                {memory: 10000, vcore: 1000}
+            limits:
+              - limit:
+                users: 
+                - user1
+                maxapplications: 10
+                maxresources: {memory: 1000, vcore: 100}
+`
+       return data
+}
+
+func prepareUserLimitsWithTwoLevelsConfig(leafQueueMaxApps uint64, 
leafQueueMaxResource string, userMaxResource string) string {
+       data := `
+partitions:
+  - name: default
+    queues:
+      - name: root
+        maxapplications: 3000
+        limits:
+          - limit:
+            users: 
+            - user1
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 500
+          - limit:
+            users:
+            - user2
+            groups:
+            - prod
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 300
+        queues:
+          - name: level1
+            maxapplications: 900
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                ` + leafQueueMaxResource + `
+            queues:
+              - name: leaf
+                maxapplications: 900
+                resources:
+                  guaranteed:
+                    {memory: 1000, vcore: 10}
+                  max:
+                    ` + leafQueueMaxResource + `
+                limits:
+                  - limit:
+                    users: 
+                    - user1
+                    maxapplications: ` + strconv.Itoa(int(leafQueueMaxApps)) + 
`
+                    maxresources: ` + userMaxResource + `
+          - name: level2
+            maxapplications: 100
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                {memory: 10000, vcore: 1000}
+            limits:
+              - limit:
+                users: 
+                - user1
+                maxapplications: 10
+                maxresources: {memory: 1000, vcore: 100}
+`
+       return data
+}
+func TestUserLimitsWithHierarchicalQueue(t *testing.T) {
+       // Make sure parent queue user max apps should not less than the child 
queue max resource
+       // validate the config and check after the update
+       _, err := CreateConfig(prepareUserLimitsConfig(50, "{memory: 10000, 
vcore: 100}"))
+       assert.ErrorContains(t, err, "user user1 max resource map[memory:10000 
vcore:100000] of queue leaf is greater than immediate or ancestor parent 
maximum resource map[memory:1000 vcore:100000]")
+
+       // Make sure parent queue user max apps should not less than the child 
queue max apps
+       // validate the config and check after the update
+       _, err = CreateConfig(prepareUserLimitsConfig(90, "{memory: 1000, 
vcore: 100}"))
+       assert.ErrorContains(t, err, "user user1 max applications 90 of queue 
leaf is greater than immediate or ancestor parent max applications 50")
+
+       // (More than one level check)
+       // Make sure hierarchical queue user max resource should not less than 
the child queue max resource
+       // validate the config and check after the update
+       _, err = CreateConfig(prepareUserLimitsWithTwoLevelsConfig(90, 
"{memory: 100000, vcore: 100}", "{memory: 90000, vcore: 100}"))
+       assert.ErrorContains(t, err, "user user1 max resource map[memory:90000 
vcore:100000] of queue leaf is greater than immediate or ancestor parent 
maximum resource map[memory:10000 vcore:10000000]")
+
+       // (More than one level check)
+       // Make sure hierarchical queue user max apps should not less than the 
child queue max apps
+       // validate the config and check after the update
+       _, err = CreateConfig(prepareUserLimitsWithTwoLevelsConfig(900, 
"{memory: 10000, vcore: 1000}", "{memory: 1000, vcore: 100}"))
+       assert.ErrorContains(t, err, "user user1 max applications 900 of queue 
leaf is greater than immediate or ancestor parent max applications 500")
+
+       // Special case: make sure wildcard user with hierarchical queue check
+       data := `
+partitions:
+  - name: default
+    queues:
+      - name: root
+        maxapplications: 3000
+        limits:
+          - limit:
+            users: 
+            - "*"
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 500
+        queues:
+          - name: level1
+            maxapplications: 900
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                {memory: 100000, vcore: 1000}
+            queues:
+              - name: leaf
+                maxapplications: 900
+                resources:
+                  guaranteed:
+                    {memory: 1000, vcore: 10}
+                  max:
+                    {memory: 100000, vcore: 1000}
+                limits:
+                  - limit:
+                    users: 
+                    - "*"
+                    maxapplications: 90
+                    maxresources: {memory: 90000, vcore: 100}
+          - name: level2
+            maxapplications: 100
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                {memory: 10000, vcore: 1000}
+            limits:
+              - limit:
+                users: 
+                - user1
+                maxapplications: 10
+                maxresources: {memory: 1000, vcore: 100}
+`
+       // validate the config and check after the update
+       _, err = CreateConfig(data)
+       assert.ErrorContains(t, err, "user * max resource map[memory:90000 
vcore:100000] of queue leaf is greater than immediate or ancestor parent 
maximum resource map[memory:10000 vcore:10000000]")
+}
+
+func prepareGroupLimitsConfig(leafQueueMaxApps uint64, leafQueueMaxResource 
string) string {
+       data := `
+partitions:
+  - name: default
+    queues:
+      - name: root
+        maxapplications: 3000
+        limits:
+          - limit:
+            groups: 
+            - group1
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 500
+          - limit:
+            users:
+            - user2
+            groups:
+            - prod
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 300
+        queues:
+          - name: level1
+            maxapplications: 100
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                {memory: 10000, vcore: 1000}
+            limits:
+              - limit:
+                groups: 
+                - group1
+                maxapplications: 50
+                maxresources: {memory: 1000, vcore: 100}
+            queues:
+              - name: leaf
+                maxapplications: 100
+                resources:
+                  guaranteed:
+                    {memory: 1000, vcore: 10}
+                  max:
+                    {memory: 10000, vcore: 1000}
+                limits:
+                  - limit:
+                    groups: 
+                    - group1
+                    maxapplications: ` + strconv.Itoa(int(leafQueueMaxApps)) + 
`
+                    maxresources: ` + leafQueueMaxResource + `
+          - name: level2
+            maxapplications: 100
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                {memory: 10000, vcore: 1000}
+            limits:
+              - limit:
+                groups: 
+                - group1
+                maxapplications: 10
+                maxresources: {memory: 1000, vcore: 100}
+`
+       return data
+}
+
+func prepareGroupLimitsWithTwoLevelsConfig(leafQueueMaxApps uint64, 
leafQueueMaxResources string, groupMaxResources string) string {
+       // (More than one level check)
+       // Make sure hierarchical queue user max resource should not less than 
the child queue max resource
+       data := `
+partitions:
+  - name: default
+    queues:
+      - name: root
+        maxapplications: 3000
+        limits:
+          - limit:
+            groups: 
+            - group1
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 500
+          - limit:
+            users:
+            - user2
+            groups:
+            - prod
+            maxresources: {memory: 10000, vcore: 10000}
+            maxapplications: 300
+        queues:
+          - name: level1
+            maxapplications: 900
+            resources:
+              guaranteed:
+                {memory: 1000, vcore: 10}
+              max:
+                ` + leafQueueMaxResources + `
+            queues:
+              - name: leaf
+                maxapplications: 900
+                resources:
+                  guaranteed:
+                    {memory: 1000, vcore: 10}
+                  max:
+                    ` + leafQueueMaxResources + `
+                limits:
+                  - limit:
+                    groups: 
+                    - group1
+                    maxapplications: ` + strconv.Itoa(int(leafQueueMaxApps)) + 
`
+                    maxresources: ` + groupMaxResources + `

Review Comment:
   Placeholders



-- 
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]

Reply via email to