[
https://issues.apache.org/jira/browse/YUNIKORN-1351?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17821767#comment-17821767
]
Craig Condit edited comment on YUNIKORN-1351 at 2/28/24 4:57 PM:
-----------------------------------------------------------------
My thoughts on V2:
We shouldn't introduce a configuration flag for the admission controller;
that's going to cause more confusion than anything else. Instead we should
strive to be consistent. The admission controller currently *only* sets the
Application ID, and then only if if it is auto-generated. Unfortunately, it
does set the non-canonical label version. We should update this to generate
both the {{yunikorn.apache.org/app-id}} *label* and {{applicationId}} *label*
and in 1.7.0, we can switch to generating only the
{{yunikorn.apache.org/app-id}} {*}label{*}. We can infer that if a user has not
supplied an Application ID explicitly, they are not concerned with where that
metadata gets stored. We need to generate both in 1.6.0 to avoid an upgrade
problem where the admission controller gets started first, and starts
generating labels which the 1.5.0 YuniKorn scheduler does not understand. Once
1.7.0 is released, we can assume that any running YuniKorn schedulers *do*
understand the new labels. In all cases, we need to have both the scheduler and
admission controller read from all possible locations.
For {{{}disableStateAware{}}}, we don't need to do anything; state-aware
scheduling will be gone by the 1.6.0 release.
We absolutely *must not* change {{schedulingPolicyParameters}} (or other
configurations) to the new format. This will just lead to mass confusion, as
this is codified in external tooling and we would now have multiple legal
namespaced identifiers with little clarity as to which is the correct one.
Just to be sure I'm interpreting your table correctly, I'm going to restate
what labels/ annotations we will support in a slightly different format. For
each entry, I'll list the canonical representation first (and bolded), followed
by the compatibility versions:
* Application
** *{{yunikorn.apache.org/app-id}} [label]* – generated by AM in 1.6.0+
** {{yunikorn.apache.org/app-id}} [annotation]
** {{applicationId}} [label] – generated by AM in 1.6.0, not in 1.7.0+
** {{spark-app-selector}} [label] – required for compatibility with Spark apps
* Queue
** *{{yunikorn.apache.org/queue}} [label]*
** {{yunikorn.apache.org/queue}} [annotation]
** {{queue}} [label]
* DisableStateAware flag – feature to be removed in 1.6.0, *remove
documentation*
** [none]
* Placeholder flag – used internally by scheduler only, *remove documentation*
** *{{internal.yunikorn.apache.org/placeholder}} [annotation]* – generate in
1.6.0+
** {{yunikorn.apache.org/placeholder}} [annotation] – parse in 1.6.0, remove
in 1.7.0
** {{placeholder}} [label] – parse in 1.6.0, remove in 1.7.0
* Task Group Name – publicly documented interface for external applications
** *{{yunikorn.apache.org/task-group-name}} [annotation]*
* Task Groups – publicly documented interface for external applications
** *{{yunikorn.apache.org/task-groups}} [annotation]*
* Scheduling Policy Parameters – publicly documented interface for external
applications
** *{{yunikorn.apache.org/schedulingPolicyParameters}} [annotation]*
* Allow-preemption flag – publicly documented interface for Pod / PriorityClass
** *{{yunikorn.apache.org/allow-preemption}} [annotation]*
was (Author: ccondit):
My thoughts on V2:
We shouldn't introduce a configuration flag for the admission controller;
that's going to cause more confusion than anything else. Instead we should
strive to be consistent. The admission controller currently *only* sets the
Application ID, and then only if if it is auto-generated. Unfortunately, it
does set the non-canonical label version, which is unfortunate. We should
update this to generate both the {{yunikorn.apache.org/app-id}} *label* and
{{applicationId}} *label* and in 1.7.0, we can switch to generating only the
{{yunikorn.apache.org/app-id}} {*}label{*}. We can infer that if a user has not
supplied an Application ID explicitly, they are not concerned with where that
metadata gets stored. We need to generate both in 1.6.0 to avoid an upgrade
problem where the admission controller gets started first, and starts
generating labels which the 1.5.0 YuniKorn scheduler does not understand. Once
1.7.0 is released, we can assume that any running YuniKorn schedulers *do*
understand the new labels. In all cases, we need to have both the scheduler and
admission controller read from all possible locations.
For {{{}disableStateAware{}}}, we don't need to do anything; state-aware
scheduling will be gone by the 1.6.0 release.
We absolutely *must not* change {{schedulingPolicyParameters}} (or other
configurations) to the new format. This will just lead to mass confusion as
this is codified in external tooling and having multiple legal namespaced
identifiers with little clarity as to which is the correct one.
Just to be sure I'm interpreting your table correctly, I'm going to restate
what labels/ annotations we will support in a slightly different format. For
each entry, I'll list the canonical representation first (and bolded), followed
by the compatibility versions:
* Application
** *{{yunikorn.apache.org/app-id}} [label]* – generated by AM in 1.6.0+
** {{yunikorn.apache.org/app-id}} [annotation]
** {{applicationId}} [label] – generated by AM in 1.6.0, not in 1.7.0+
** {{spark-app-selector}} [label] – required for compatibility with Spark apps
* Queue
** *{{yunikorn.apache.org/queue}} [label]*
** {{yunikorn.apache.org/queue}} [annotation]
** {{queue}} [label]
* DisableStateAware flag – feature to be removed in 1.6.0, *remove
documentation*
** [none]
* Placeholder flag – used internally by scheduler only, *remove documentation*
** *{{internal.yunikorn.apache.org/placeholder}} [annotation]* – generate in
1.6.0+
** {{yunikorn.apache.org/placeholder}} [annotation] – parse in 1.6.0, remove
in 1.7.0
** {{placeholder}} [label] – parse in 1.6.0, remove in 1.7.0
* Task Group Name – publicly documented interface for external applications
** *{{yunikorn.apache.org/task-group-name}} [annotation]*
* Task Groups – publicly documented interface for external applications
** *{{yunikorn.apache.org/task-groups}} [annotation]*
* Scheduling Policy Parameters – publicly documented interface for external
applications
** *{{yunikorn.apache.org/schedulingPolicyParameters}} [annotation]*
* Allow-preemption flag – publicly documented interface for Pod / PriorityClass
** *{{yunikorn.apache.org/allow-preemption}} [annotation]*
> Define policy for when to use annotations vs. labels
> ----------------------------------------------------
>
> Key: YUNIKORN-1351
> URL: https://issues.apache.org/jira/browse/YUNIKORN-1351
> Project: Apache YuniKorn
> Issue Type: Improvement
> Components: shim - kubernetes
> Reporter: Wilfred Spiegelenburg
> Assignee: Yu-Lin Chen
> Priority: Major
> Labels: newbie, pull-request-available
> Attachments: [Design Doc] Define Usage of Labels and Annotations in
> YuniKorn.pdf, [Design Doc][v2] Define policy for when to use annotations vs
> labels.pdf
>
>
> Currently, we have very inconsistent use of labels vs. annotations in
> YuniKorn. Additionally, some values are namespaced under yunikorn.apache.org
> while some are not. We will likely need to keep legacy values around for
> backwards compatibility but we should define a strategy for defining
> canonical representations of this metadata as well as a developer-oriented
> pollcy around when to use labels vs. annotations.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: issues-h...@yunikorn.apache.org
