chia7712 commented on code in PR #843:
URL: https://github.com/apache/yunikorn-k8shim/pull/843#discussion_r1605411426
##########
pkg/common/constants/constants.go:
##########
@@ -36,8 +36,10 @@ const DomainYuniKornInternal =
siCommon.DomainYuniKornInternal
// Application
const LabelApp = "app"
+const CanonicalLabelApplicationID = DomainYuniKorn + "app-id"
const LabelApplicationID = "applicationId"
const AnnotationApplicationID = DomainYuniKorn + "app-id"
+const CanonicalLabelQueueName = DomainYuniKorn + "queue"
Review Comment:
Could it be moved to line#46 (after `AnnotationQueueName`) ?
##########
pkg/common/constants/constants.go:
##########
@@ -36,8 +36,10 @@ const DomainYuniKornInternal =
siCommon.DomainYuniKornInternal
// Application
const LabelApp = "app"
+const CanonicalLabelApplicationID = DomainYuniKorn + "app-id"
Review Comment:
Could we move `CanonicalLabelApplicationID` to line#42 (after
`AnnotationApplicationID`)?
##########
pkg/admission/util.go:
##########
@@ -37,26 +37,40 @@ func updatePodLabel(pod *v1.Pod, namespace string,
generateUniqueAppIds bool, de
}
sparkAppID := utils.GetPodLabelValue(pod, constants.SparkLabelAppID)
+ canonicalAppID := utils.GetPodLabelValue(pod,
constants.CanonicalLabelApplicationID)
appID := utils.GetPodLabelValue(pod, constants.LabelApplicationID)
- if sparkAppID == "" && appID == "" {
+ if canonicalAppID == "" && sparkAppID == "" && appID == "" {
// if app id not exist, generate one
// for each namespace, we group unnamed pods to one single app
- if GenerateUniqueAppId is not set
// if GenerateUniqueAppId:
// application ID convention:
${NAMESPACE}-${POD_UID}
// else
// application ID convention:
${AUTO_GEN_PREFIX}-${NAMESPACE}-${AUTO_GEN_SUFFIX}
generatedID := utils.GenerateApplicationID(namespace,
generateUniqueAppIds, string(pod.UID))
+
+ result[constants.CanonicalLabelApplicationID] = generatedID
Review Comment:
Do we need to apply `constants.CanonicalLabelApplicationID` to
`GetApplicationIDFromPod`? It seems `GetApplicationIDFromPod` don't take app id
from this new label
##########
pkg/admission/util.go:
##########
@@ -37,26 +37,40 @@ func updatePodLabel(pod *v1.Pod, namespace string,
generateUniqueAppIds bool, de
}
sparkAppID := utils.GetPodLabelValue(pod, constants.SparkLabelAppID)
+ canonicalAppID := utils.GetPodLabelValue(pod,
constants.CanonicalLabelApplicationID)
appID := utils.GetPodLabelValue(pod, constants.LabelApplicationID)
- if sparkAppID == "" && appID == "" {
+ if canonicalAppID == "" && sparkAppID == "" && appID == "" {
// if app id not exist, generate one
// for each namespace, we group unnamed pods to one single app
- if GenerateUniqueAppId is not set
// if GenerateUniqueAppId:
// application ID convention:
${NAMESPACE}-${POD_UID}
// else
// application ID convention:
${AUTO_GEN_PREFIX}-${NAMESPACE}-${AUTO_GEN_SUFFIX}
generatedID := utils.GenerateApplicationID(namespace,
generateUniqueAppIds, string(pod.UID))
+
+ result[constants.CanonicalLabelApplicationID] = generatedID
+ // Deprecated: After 1.7.0, admission controller will only add
canonical label if application ID was not set
result[constants.LabelApplicationID] = generatedID
+ } else if canonicalAppID != "" {
+ // Deprecated: Added in 1.6.0 for backward compatibility, in
case the prior shim version can't handle canonical label
+ result[constants.LabelApplicationID] = canonicalAppID
}
// if existing label exist, it takes priority over everything else
- if _, ok := existingLabels[constants.LabelQueueName]; !ok {
+ canonicalQueueName := utils.GetPodLabelValue(pod,
constants.CanonicalLabelQueueName)
+ queueName := utils.GetPodLabelValue(pod, constants.LabelQueueName)
+ if canonicalQueueName == "" && queueName == "" {
// if defaultQueueName is "", skip adding default queue name to
the pod labels
if defaultQueueName != "" {
// for undefined configuration, am_conf will add
'root.default' to retain existing behavior
// if a custom name is configured for default queue, it
will be used instead of root.default
+ result[constants.CanonicalLabelQueueName] =
defaultQueueName
Review Comment:
ditto. should `GetQueueNameFromPod` recognize it?
--
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]
