Add finalizer to avoid premature CRD Deletion #5788
Conversation
Signed-off-by: Rafael Raposo <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5788 +/- ##
=======================================
Coverage 36.31% 36.32%
=======================================
Files 1304 1304
Lines 110048 110049 +1
=======================================
+ Hits 39964 39974 +10
+ Misses 65928 65919 -9
Partials 4156 4156
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
concern if propeller never picks up the FlyteWorkflow CRD then the CRD won't be able to get garbage collected. |
| @@ -129,6 +129,9 @@ func PrepareFlyteWorkflow(data interfaces.ExecutionData, flyteWorkflow *v1alpha1 | |||
| acceptAtWrapper := v1.NewTime(data.ExecutionParameters.AcceptedAt) | |||
| flyteWorkflow.AcceptedAt = &acceptAtWrapper | |||
|
|
|||
| // Add finalizer | |||
There was a problem hiding this comment.
Doesn't this already happen here?
SetFinalizerIfEmpty(mutableW, FinalizerKey)
https://github.com/unionai/flyte/blob/14eaa16a43bff3ada4e93c6c6421c6b35471bf30/flytepropeller/pkg/controller/handler.go#L128
There was a problem hiding this comment.
@EngHabu I believe in their scenario, the CRD is getting deleted before propeller has had the chance to evaluate (and set the finalizer for) the external workflow
Tracking issue
Closes #5786
Why are the changes needed?
To make sure Workflows aren't prematurely deleted by Flyteadmin and FlytePropeller can properly transitions the states to
AbortedWhat changes were proposed in this pull request?
How was this patch tested?
Unit Tested
Tested in a staging environment manually.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link