[GitHub] [yunikorn-core] 0yukali0 commented on a diff in pull request #515: [YUNIKORN-1606] Configuration processing should support limit quota check at the same level.

["0yukali0 (via GitHub)" <gi...@apache.org>]

0yukali0 commented on code in PR #515:
URL: https://github.com/apache/yunikorn-core/pull/515#discussion_r1161128996


##########
pkg/common/configs/configvalidator.go:
##########
@@ -357,13 +357,32 @@ func checkLimit(limit Limit, currIdx int, userWildCardIdx, groupWildCardIdx *int
 	}
 	// at least some resource should be not null
 	if limit.MaxApplications == 0 && resources.IsZero(limitResource) {
-		return fmt.Errorf("invalid resource combination for limit names '%s' all resource limits are null", limit.Users)
+		return fmt.Errorf("invalid resource combination for limit %s all resource limits are null", limit.Limit)
 	}
+
+	if queue.MaxApplications != 0 && (limit.MaxApplications > queue.MaxApplications || limit.MaxApplications == 0) {
+		return fmt.Errorf("invalid MaxApplications settings for limit %s exeecd current the queue MaxApplications", limit.Limit)

Review Comment:
   In the website, the limits description is following.
   ```
   maxapplications is an unsigned integer value, larger than 1, which allows you to limit the number of running applications for the configured user or group. Specifying a zero maximum applications limit is not allowed as it would implicitly deny access. Denying access must be handled via the ACL entries.
   ```
   
   so I think that the error description is not suitable when conditions (queue.MaxApplications != 0 && limit.MaxApplications == 0) happens.
   I think this part will be like this.
   
   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)
     }
   }



-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org