yangwwei commented on pull request #202: URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/202#issuecomment-717758396
> Instead of this, I expanded the GeneralPredicate and added each predicate separately. I skipped one: PodFitsPorts, what has been replaced by PodFitsHostPorts, and since we don't support K8s 1.0, I think we don't need that. Hi @kingamarton I am not sure if it is a good idea to replace `GeneralPred`, I just checked K8s 1.17 code, https://github.com/kubernetes/kubernetes/blob/a42a619a5f2bde896bcc63a990fb3b42d55dffe3/pkg/scheduler/algorithmprovider/defaults/defaults.go#L40-L54. It still uses `GeneralPred`, not the separate ones. One difference I can see is in `GeneralPred`: https://github.com/kubernetes/kubernetes/blob/a42a619a5f2bde896bcc63a990fb3b42d55dffe3/pkg/scheduler/algorithm/predicates/predicates.go#L1144-L1189. It runs a bunch of things: 1. PodFitsResources 2. PodFitsHost 3. PodFitsHostPorts 4. PodMatchNodeSelector what you have in the PR: 1. PodFitsResourcesPred 2. PodFitsHostPortsPred 3. MatchNodeSelectorPred where misses `PodFitsHost`. Even you add this one, I still have some concern there might be something out of our radar, could you please take a closer look at the predicates functions and let me know if this is a safe change? Predicates are invoked in every scheduling cycle, we need to be extremely careful while making changes. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
