pbacsko commented on code in PR #871:
URL: https://github.com/apache/yunikorn-k8shim/pull/871#discussion_r1698409117
##########
pkg/common/utils/utils.go:
##########
@@ -104,12 +104,20 @@ func IsAssignedPod(pod *v1.Pod) bool {
}
func GetQueueNameFromPod(pod *v1.Pod) string {
+ // Queue name can be defined in multiple places
+ // The queue name is determined by the following order
+ // 1. Label: constants.CanonicalLabelQueueName
+ // 2. Annotation: constants.AnnotationQueueName
+ // 3. Label: constants.LabelQueueName
queueName := ""
- if an := GetPodLabelValue(pod, constants.LabelQueueName); an != "" {
- queueName = an
- } else if qu := GetPodAnnotationValue(pod,
constants.AnnotationQueueName); qu != "" {
- queueName = qu
+ if canonicalLabelQueueName := GetPodLabelValue(pod,
constants.CanonicalLabelQueueName); canonicalLabelQueueName != "" {
+ queueName = canonicalLabelQueueName
+ } else if annotationQueueName := GetPodAnnotationValue(pod,
constants.AnnotationQueueName); annotationQueueName != "" {
+ queueName = annotationQueueName
+ } else if labelQueueName := GetPodLabelValue(pod,
constants.LabelQueueName); labelQueueName != "" {
+ queueName = labelQueueName
}
+
return queueName
Review Comment:
Do we validate differenent values for the annotation/label? Now the order is
different, so label->app vs app->label results in different appIDs being
chosen. I don't think this is something which is very likely to happen IRL but
who knows. If someone messes this up, the selected appID will be different, so
at least a WARN is useful.
##########
pkg/common/utils/utils.go:
##########
@@ -154,15 +163,30 @@ func GetApplicationIDFromPod(pod *v1.Pod) string {
}
}
- // Application ID can be defined in annotation
- appID := GetPodAnnotationValue(pod, constants.AnnotationApplicationID)
+ // Application ID can be defined in multiple places
+ // The application ID is determined by the following order.
+ // 1. Label: constants.CanonicalLabelApplicationID
+ // 2. Annotation: constants.AnnotationApplicationID
+ // 3. Label: constants.LabelApplicationID
+ // 4. Label: constants.SparkLabelAppID
Review Comment:
Do we validate differenent values for the annotation/label? Now the order is
different, so label->app vs app->label results in different appIDs being
chosen. I don't think this is something which is very likely to happen IRL but
who knows. If someone messes this up, the selected appID will be different, so
at least a WARN is useful.
--
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]
