Adding support for per-task PodTemplate configuration #308

hamersaw · 2023-01-11T11:28:42Z

TL;DR

This PR adds support for configuring task executions using a per-task defined PodTemplate and / or PodTemplate name. The differences are client vs. server side application respectively.

Type

Bug Fix
Feature
Plugin

Are all requirements met?

Complete description

Currently supported task types:

Tracking Issue

flyteorg/flyte#3123

Follow-up issue

NA

Signed-off-by: Daniel Rammer <[email protected]>

codecov · 2023-01-11T22:00:43Z

Codecov Report

Merging #308 (f246012) into master (471a262) will increase coverage by 1.21%.
The diff coverage is 66.22%.

❗ Current head f246012 differs from pull request most recent head 02854c4. Consider uploading reports for the commit 02854c4 to get more accurate results

@@            Coverage Diff             @@
##           master     #308      +/-   ##
==========================================
+ Coverage   63.03%   64.25%   +1.21%     
==========================================
  Files         148      146       -2     
  Lines       12153     9841    -2312     
==========================================
- Hits         7661     6323    -1338     
+ Misses       3912     2939     -973     
+ Partials      580      579       -1

Flag	Coverage Δ
unittests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...tasks/pluginmachinery/flytek8s/container_helper.go	`88.43% <52.38%> (-1.28%)`	⬇️
go/tasks/pluginmachinery/flytek8s/pod_helper.go	`72.54% <54.62%> (-11.52%)`	⬇️
go/tasks/plugins/k8s/pod/plugin.go	`81.73% <76.00%> (-1.15%)`	⬇️
...sks/pluginmachinery/flytek8s/pod_template_store.go	`100.00% <100.00%> (ø)`
go/tasks/plugins/array/k8s/subtask.go	`30.35% <100.00%> (-0.14%)`	⬇️
go/tasks/plugins/k8s/kfoperators/mpi/mpi.go	`75.00% <100.00%> (+3.79%)`	⬆️
...o/tasks/plugins/k8s/kfoperators/pytorch/pytorch.go	`73.07% <100.00%> (+4.50%)`	⬆️
...s/plugins/k8s/kfoperators/tensorflow/tensorflow.go	`75.58% <100.00%> (+3.78%)`	⬆️
go/tasks/plugins/k8s/ray/ray.go	`79.66% <100.00%> (+0.55%)`	⬆️
go/tasks/plugins/array/inputs.go	`83.33% <0.00%> (-4.91%)`	⬇️
... and 129 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Daniel Rammer <[email protected]>

ByronHsu

nit: the name of this function is forgotten to change. Can you also fix this?

go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow.go

go/tasks/pluginmachinery/flytek8s/pod_helper.go

Signed-off-by: Daniel Rammer <[email protected]>

go/tasks/pluginmachinery/flytek8s/pod_helper.go

Signed-off-by: Daniel Rammer <[email protected]>

fg91 · 2023-02-07T15:39:42Z

Thanks a lot @hamersaw 🙏 🙂

ByronHsu · 2023-02-07T19:42:34Z

Thank you so much @hamersaw

flixr · 2023-02-09T19:07:53Z

very nice, thanks @hamersaw 😄

* implemented Signed-off-by: Daniel Rammer <[email protected]> * unit tests working Signed-off-by: Daniel Rammer <[email protected]> * updated flyteidl Signed-off-by: Daniel Rammer <[email protected]> * fixed lint issues Signed-off-by: Daniel Rammer <[email protected]> * updated documentation Signed-off-by: Daniel Rammer <[email protected]> * removing unnecessarily commented lines Signed-off-by: Daniel Rammer <[email protected]> * updated unit tests Signed-off-by: Daniel Rammer <[email protected]> * fixed lint issues Signed-off-by: Daniel Rammer <[email protected]> * updated docs Signed-off-by: Daniel Rammer <[email protected]> * if user provides PodTemplate name and it doesn't exist we should fail Signed-off-by: Daniel Rammer <[email protected]> * updated for new flyteidl definition Signed-off-by: Daniel Rammer <[email protected]> * updated flyteidl Signed-off-by: Daniel Rammer <[email protected]> * working with either container or pod passed in task template Signed-off-by: Daniel Rammer <[email protected]> * cleaned up definitions Signed-off-by: Daniel Rammer <[email protected]> * fixing up unit tests Signed-off-by: Daniel Rammer <[email protected]> * fixed unit tests Signed-off-by: Daniel Rammer <[email protected]> * fixed lint issues Signed-off-by: Daniel Rammer <[email protected]> * removed dead code Signed-off-by: Daniel Rammer <[email protected]> * removing more dead code Signed-off-by: Daniel Rammer <[email protected]> * remove more dead code Signed-off-by: Daniel Rammer <[email protected]> * correctly applying resources for container and pod tasks Signed-off-by: Daniel Rammer <[email protected]> * updated flyteidl version Signed-off-by: Daniel Rammer <[email protected]> * moved sidecar parsing to pod plugin Signed-off-by: Daniel Rammer <[email protected]> * fixed unit tests Signed-off-by: Daniel Rammer <[email protected]> * updating semantics to address review comments Signed-off-by: Daniel Rammer <[email protected]> --------- Signed-off-by: Daniel Rammer <[email protected]>

hamersaw added 5 commits January 11, 2023 05:01

implemented

1ea39af

Signed-off-by: Daniel Rammer <[email protected]>

unit tests working

0231bf8

Signed-off-by: Daniel Rammer <[email protected]>

updated flyteidl

d1047f7

Signed-off-by: Daniel Rammer <[email protected]>

Merge branch 'master' into feature/task-pod-template

ff69656

Signed-off-by: Daniel Rammer <[email protected]>

fixed lint issues

ec010e7

Signed-off-by: Daniel Rammer <[email protected]>

hamersaw added 2 commits January 13, 2023 11:30

updated documentation

602feac

Signed-off-by: Daniel Rammer <[email protected]>

removing unnecessarily commented lines

2f486a8

Signed-off-by: Daniel Rammer <[email protected]>

hamersaw mentioned this pull request Jan 13, 2023

Updating PodTemplateStore API flyteorg/flytepropeller#516

Merged

8 tasks

hamersaw added 3 commits January 13, 2023 14:27

updated unit tests

5a36297

Signed-off-by: Daniel Rammer <[email protected]>

fixed lint issues

57351fb

Signed-off-by: Daniel Rammer <[email protected]>

updated docs

9dbd1fc

Signed-off-by: Daniel Rammer <[email protected]>

hamersaw marked this pull request as ready for review January 13, 2023 21:00

hamersaw requested review from eapolinario, EngHabu, kumare3 and wild-endeavor January 13, 2023 21:01

hamersaw added 3 commits January 13, 2023 19:50

if user provides PodTemplate name and it doesn't exist we should fail

1b65e71

Signed-off-by: Daniel Rammer <[email protected]>

updated for new flyteidl definition

6c25a83

Signed-off-by: Daniel Rammer <[email protected]>

updated flyteidl

f19716f

Signed-off-by: Daniel Rammer <[email protected]>

ByronHsu reviewed Jan 24, 2023

View reviewed changes

go/tasks/plugins/k8s/kfoperators/tensorflow/tensorflow.go Show resolved Hide resolved

go/tasks/pluginmachinery/flytek8s/pod_helper.go Outdated Show resolved Hide resolved

hamersaw added 9 commits January 24, 2023 13:26

working with either container or pod passed in task template

c8528b9

Signed-off-by: Daniel Rammer <[email protected]>

cleaned up definitions

2cc6563

Signed-off-by: Daniel Rammer <[email protected]>

fixing up unit tests

be7c455

Signed-off-by: Daniel Rammer <[email protected]>

fixed unit tests

2d88777

Signed-off-by: Daniel Rammer <[email protected]>

fixed lint issues

c410d97

Signed-off-by: Daniel Rammer <[email protected]>

removed dead code

100f2d0

Signed-off-by: Daniel Rammer <[email protected]>

removing more dead code

94f5236

Signed-off-by: Daniel Rammer <[email protected]>

remove more dead code

63118d6

Signed-off-by: Daniel Rammer <[email protected]>

correctly applying resources for container and pod tasks

0718751

Signed-off-by: Daniel Rammer <[email protected]>

hamersaw added 4 commits January 30, 2023 15:52

updated flyteidl version

c33fed2

Signed-off-by: Daniel Rammer <[email protected]>

moved sidecar parsing to pod plugin

70040ae

Signed-off-by: Daniel Rammer <[email protected]>

Merge branch 'master' into feature/task-pod-template

a04d903

Signed-off-by: Daniel Rammer <[email protected]>

fixed unit tests

8f6f3a3

Signed-off-by: Daniel Rammer <[email protected]>

EngHabu reviewed Feb 2, 2023

View reviewed changes

go/tasks/pluginmachinery/flytek8s/pod_helper.go Outdated Show resolved Hide resolved

go/tasks/pluginmachinery/flytek8s/pod_helper.go Outdated Show resolved Hide resolved

go/tasks/pluginmachinery/flytek8s/pod_helper.go Show resolved Hide resolved

updating semantics to address review comments

02854c4

Signed-off-by: Daniel Rammer <[email protected]>

hamersaw requested a review from EngHabu February 3, 2023 17:09

EngHabu approved these changes Feb 6, 2023

View reviewed changes

hamersaw merged commit c4abd4f into master Feb 6, 2023

hamersaw deleted the feature/task-pod-template branch February 6, 2023 16:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for per-task PodTemplate configuration #308

Adding support for per-task PodTemplate configuration #308

hamersaw commented Jan 11, 2023 •

edited

Loading

codecov bot commented Jan 11, 2023 •

edited

Loading

ByronHsu left a comment

fg91 commented Feb 7, 2023

ByronHsu commented Feb 7, 2023

flixr commented Feb 9, 2023

Adding support for per-task PodTemplate configuration #308

Adding support for per-task PodTemplate configuration #308

Conversation

hamersaw commented Jan 11, 2023 • edited Loading

TL;DR

Type

Are all requirements met?

Complete description

Tracking Issue

Follow-up issue

codecov bot commented Jan 11, 2023 • edited Loading

Codecov Report

ByronHsu left a comment

Choose a reason for hiding this comment

fg91 commented Feb 7, 2023

ByronHsu commented Feb 7, 2023

flixr commented Feb 9, 2023

hamersaw commented Jan 11, 2023 •

edited

Loading

codecov bot commented Jan 11, 2023 •

edited

Loading