Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

persisting k8s plugin phase, version, and reason #331

Merged
merged 7 commits into from
Mar 27, 2023

Conversation

hamersaw
Copy link
Contributor

TL;DR

This PR adds support for persisting state of k8s plugins including Phase, PhaseVersion, and Reason. This is used to detect scenarios where the Reason has been updated and update the PhaseVersion to ensure that the latest Reason is reported to FlyteAdmin in a TaskExecutionEvent.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

^^^

Tracking Issue

flyteorg/flyte#3440

Follow-up issue

NA

Signed-off-by: Daniel Rammer <daniel@union.ai>
Signed-off-by: Daniel Rammer <daniel@union.ai>
Signed-off-by: Daniel Rammer <daniel@union.ai>
Signed-off-by: Daniel Rammer <daniel@union.ai>
Signed-off-by: Daniel Rammer <daniel@union.ai>
Signed-off-by: Daniel Rammer <daniel@union.ai>
@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Merging #331 (dc33395) into master (1329476) will increase coverage by 1.10%.
The diff coverage is 39.13%.

❗ Current head dc33395 differs from pull request most recent head 3b3c7a1. Consider uploading reports for the commit 3b3c7a1 to get more accurate results

@@            Coverage Diff             @@
##           master     #331      +/-   ##
==========================================
+ Coverage   62.87%   63.98%   +1.10%     
==========================================
  Files         146      146              
  Lines       12168     9902    -2266     
==========================================
- Hits         7651     6336    -1315     
+ Misses       3940     2984     -956     
- Partials      577      582       +5     
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
go/tasks/pluginmachinery/core/phase.go 22.09% <0.00%> (-3.49%) ⬇️
...s/plugins/k8s/kfoperators/tensorflow/tensorflow.go 75.00% <ø> (-0.48%) ⬇️
go/tasks/plugins/k8s/ray/config.go 50.00% <ø> (ø)
go/tasks/plugins/k8s/ray/ray.go 77.01% <0.00%> (-3.45%) ⬇️
go/tasks/pluginmachinery/flytek8s/pod_helper.go 69.81% <36.00%> (-5.47%) ⬇️
go/tasks/plugins/k8s/pod/plugin.go 78.46% <62.06%> (-1.94%) ⬇️

... and 125 files with indirect coverage changes

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

@hamersaw hamersaw marked this pull request as ready for review March 14, 2023 22:31
pingsutw
pingsutw previously approved these changes Mar 27, 2023
EngHabu
EngHabu previously approved these changes Mar 27, 2023
Signed-off-by: Daniel Rammer <daniel@union.ai>
@hamersaw hamersaw dismissed stale reviews from EngHabu and pingsutw via 3b3c7a1 March 27, 2023 20:36
@hamersaw hamersaw merged commit 18a594e into master Mar 27, 2023
@hamersaw hamersaw deleted the bug/k8s-plugin-state-persistence branch March 27, 2023 23:04
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
* using PhaseVersion to ensure Reason updates are sent

Signed-off-by: Daniel Rammer <daniel@union.ai>

* refactored and fixed tests

Signed-off-by: Daniel Rammer <daniel@union.ai>

* fixed linter

Signed-off-by: Daniel Rammer <daniel@union.ai>

* added docs

Signed-off-by: Daniel Rammer <daniel@union.ai>

* corrected missing old usecase

Signed-off-by: Daniel Rammer <daniel@union.ai>

* actually this will work

Signed-off-by: Daniel Rammer <daniel@union.ai>

* added missing increment on phsae version - thanks yee

Signed-off-by: Daniel Rammer <daniel@union.ai>

---------

Signed-off-by: Daniel Rammer <daniel@union.ai>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants