Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement][K8S] Custom label of a K8S task can be passed to the pod #15369

Merged
merged 2 commits into from
Dec 28, 2023

Conversation

Gallardot
Copy link
Member

Purpose of the pull request

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

Signed-off-by: Gallardot <gallardot@apache.org>
@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d6dea46) 38.13% compared to head (68f77f8) 38.11%.

❗ Current head 68f77f8 differs from pull request most recent head 37f941c. Consider uploading reports for the commit 37f941c to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##                dev   #15369      +/-   ##
============================================
- Coverage     38.13%   38.11%   -0.02%     
+ Complexity     4698     4697       -1     
============================================
  Files          1300     1300              
  Lines         44775    44778       +3     
  Branches       4800     4800              
============================================
- Hits          17073    17068       -5     
- Misses        25848    25858      +10     
+ Partials       1854     1852       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Radeity Radeity added improvement make more easy to user or prompt friendly ready-to-merge labels Dec 27, 2023
@Radeity Radeity added this to the 3.2.1 milestone Dec 27, 2023
Copy link

sonarcloud bot commented Dec 27, 2023

labelMap.put(LAYER_LABEL, LAYER_LABEL_VALUE);
labelMap.put(NAME_LABEL, k8sJobName);
Map<String, String> jobLabelMap = new HashMap<>();
jobLabelMap.put(LAYER_LABEL, LAYER_LABEL_VALUE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @Gallardot I suggest that the following statement be executed at the end, so that when the user customizes the same label name, the one in ds will prevail, because ds will need to perform other operations with this label in the future. WDYT? cc @Radeity

jobLabelMap.put(LAYER_LABEL, LAYER_LABEL_VALUE);
jobLabelMap.put(NAME_LABEL, k8sJobName);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep user-defined labels higher priority. If it's conflict with some ds' future feature, we can add some check during task definition or throw run-time exception during task execution. Anyway, now it just behaves like identifier, like label job-name for pod which is auto-labeled by Kubernetes controller, current order is reasonable for me.

@Radeity Radeity merged commit 93ea5f6 into apache:dev Dec 28, 2023
52 checks passed
@Gallardot Gallardot deleted the label-pod branch December 28, 2023 05:52
Gallardot added a commit to Gallardot/dolphinscheduler that referenced this pull request Mar 14, 2024
Gallardot pushed a commit to Gallardot/dolphinscheduler that referenced this pull request Mar 14, 2024
[Improvement][K8S] Custom label of a K8S task can be passed to the pod (apache#15369)

See merge request logan/devops/apache/dolphinscheduler!8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend improvement make more easy to user or prompt friendly ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement][K8S] Custom label of a K8S task can be passed to the pod
4 participants