wilfred-s commented on code in PR #628:
URL: https://github.com/apache/yunikorn-k8shim/pull/628#discussion_r1316825589


##########
pkg/appmgmt/general/podevent_handler.go:
##########
@@ -130,7 +131,8 @@ func (p *PodEventHandler) addPod(pod *v1.Pod, eventSource 
EventSource) interface
                                Metadata: appMeta,
                        })
                } else {
-                       if 
conf.GetSchedulerConf().GetSingleUserPerApplication() && app.GetUser() != 
appMeta.User {
+                       if app.GetApplicationID() != 
constants.AutoGenAppPrefix+"-"+pod.Namespace+"-"+constants.AutoGenAppSuffix &&
+                               
conf.GetSchedulerConf().GetSingleUserPerApplication() && app.GetUser() != 
appMeta.User {

Review Comment:
   Almost there: the generated ID check is not right yet.
   If `admissionController.filtering.generateUniqueAppId` is **false** we 
should not check the user at all if the appID is the same as the generated one. 
However if the setting is **true** we should check that the user is the same as 
it is a user set appID. Otherwise we still leave an option to bypass the user 
check.
   Wrapping the appID check in a simple function (in: appID, out: boolean) that 
does the above is recommended.
   
   I also recommend an order swap of the checks to improve performance:
   1.  conf
   2. appID (conf + content)
   3. name



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