[GitHub] [yunikorn-k8shim] wilfred-s commented on a diff in pull request #579: [YUNIKORN-1707] Ignore pods in non-labeled namespaces

["wilfred-s (via GitHub)" <gi...@apache.org>]

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


##########
pkg/common/utils/utils.go:
##########
@@ -109,6 +109,16 @@ func GetQueueNameFromPod(pod *v1.Pod) string {
 }
 
 func GetApplicationIDFromPod(pod *v1.Pod) (string, error) {
+	// if pod was tagged with ignore-application, return
+	if value := GetPodAnnotationValue(pod, constants.AnnotationIgnoreApplication); value != "" {
+		ignore, err := strconv.ParseBool(value)
+		if err != nil {
+			log.Logger().Warn("Failed to parse annotation "+constants.AnnotationIgnoreApplication, zap.Error(err))
+		} else if ignore {
+			return "", nil
+		}
+	}
+

Review Comment:
   We should add this in the `utils.GeneralPodFilter(obj)`.
   Any pod flows through that filter first. It removes the pod from the internal YuniKorn handling. Pods that do not pass that filter do not end up in the cache either (see FilteringResourceEventHandler).
   
   The plugins (Filter, PreFilter and PostBind) should also use the `GeneralPodFilter`. If the general pod filter passes an application ID must always be there.
   It gives us a more consistent way of handling pods. It also removes pods from the cache that we want to bypass YuniKorn.



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