Feat: Apply labels and annotations set via task decorator to pods and CR objects #6421
Conversation
…ons=...) to pods and CR objects Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Code Review Agent Run Status
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6421 +/- ##
==========================================
- Coverage 58.48% 58.47% -0.01%
==========================================
Files 940 940
Lines 71562 71575 +13
==========================================
+ Hits 41850 41855 +5
- Misses 26531 26537 +6
- Partials 3181 3183 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
Signed-off-by: Fabio Grätz <fabiogratz@googlemail.com>
fg91
changed the title
| }) | ||
|
|
||
| t.Run("Task template K8s metadata overwrites object metadata", func(t *testing.T) { | ||
| o := &v1.Pod{ |
There was a problem hiding this comment.
Labels/Annotations set via @task(pod_template=PodTemplate(...)) are overwritten by @task(labels=..., annotations=...).
| }) | ||
|
|
||
| t.Run("Task template K8s metadata overwritten by task execution metadata", func(t *testing.T) { | ||
| o := &v1.Pod{} |
There was a problem hiding this comment.
Labels/annotations set via @task(labels=..., ...) are overwritten by pyflyte run --labels ... --annotations ....
Code Review Agent Run #c6bea5Actionable Suggestions - 1
Additional Suggestions - 4
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Changelist by BitoThis pull request implements the following key changes.
|
| 37, // [37:37] is the sub-list for extension type_name | ||
| 37, // [37:37] is the sub-list for extension extendee | ||
| 0, // [0:37] is the sub-list for field type_name | ||
| 19, // 10: flyteidl.core.TaskMetadata.metadata:type_name -> flyteidl.core.K8sObjectMetadata |
There was a problem hiding this comment.
The TaskMetadata.metadata field is incorrectly mapped to K8sObjectMetadata at index 19 in the dependency indexes array. This will cause incorrect type mapping in the protobuf initialization.
Code suggestion
Check the AI-generated fix before applying
| 19, // 10: flyteidl.core.TaskMetadata.metadata:type_name -> flyteidl.core.K8sObjectMetadata |
Code Review Run #c6bea5
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Sovietaced
left a comment
Sovietaced
left a comment
There was a problem hiding this comment.
This seems reasonable to me. Labels/annotations set at the launch plan would apply to all the CRs since they get inherited by the workflow CR. So this should let you deviate on a per task basis.
Tracking issue
Closes #6238
Why are the changes needed?
Today, as a Flyte user, I have the following options to set labels/annotations on the pods/CRD objects of K8s tasks in a flyte workflow execution:
Set via pod template:
This sets labels/annotations on the pods of individual tasks.
For distributed tasks (like pytorch, ray, ...) this sets the metadata not on the CRD object but its pod template spec.
Set via
pyflyte run --labels ... --annotations ...This applies the metadata to all K8s objects in a flyte workflow execution, including task pods and task CRD objects. However, this mechanism doesn't work on an individual task level.
What changes were proposed in this pull request?
As a Flyte user, I would like to be able to specify specific labels/annotations for individual k8s task CRD objects like pytorch jobs, ray job, ... (the same way I already can today for pods via the pod template). For this aim, the following arguments are added to the task decorator:
These labels and annotations are added to the object metadata of the object executing the task (Pod or CRD object).
This PR adds a
metadatafield of typeK8sObjectMetadatato theTaskMetadataproto message and applies this additional metadata in the plugin manager'saddObjectMetadatafunction.How was this patch tested?
Related PRs
flyteorg/flytekit#3243
Summary by Bito
This pull request adds a new metadata field to the TaskMetadata proto, enabling users to apply custom labels and annotations to Kubernetes objects. The enhancement spans multiple language implementations including Go, TypeScript, Python, and Rust, providing more granular control over metadata for both pods and custom resources.Unit tests added: False
Estimated effort to review (1-5, lower is better): 5