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

Adding cleanupOnFailure to PhaseInfo #333

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Conversation

hamersaw
Copy link
Contributor

TL;DR

Adding a cleanupOnFailure field to the PhaseInfo reported by each task evaluation. This is useful to track situations where Flyte marks a task as FAILED but it should still be cleaned up during the workflow abort. An example is ImagePullBackoff where a k8s Pod requests an image that does not exist. Flyte will mark the Pod as a failure, but it is not cleaned up until the workflow CR is garbage collected. The Podwill continually reattempt to download the missing image (with backoff) maintaining it's hold on resources.

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#3239

Follow-up issue

NA

@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Merging #333 (5b60013) into master (f5f4182) will increase coverage by 1.29%.
The diff coverage is 33.33%.

❗ Current head 5b60013 differs from pull request most recent head 113947f. Consider uploading reports for the commit 113947f to get more accurate results

@@            Coverage Diff             @@
##           master     #333      +/-   ##
==========================================
+ Coverage   62.67%   63.96%   +1.29%     
==========================================
  Files         146      146              
  Lines       12248     9930    -2318     
==========================================
- Hits         7676     6352    -1324     
+ Misses       3987     2993     -994     
  Partials      585      585              
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 21.50% <30.43%> (-1.90%) ⬇️
go/tasks/pluginmachinery/flytek8s/pod_helper.go 69.81% <100.00%> (-3.21%) ⬇️

... and 128 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 April 18, 2023 13:58
@hamersaw hamersaw merged commit 84030bd into master Apr 20, 2023
@hamersaw hamersaw deleted the bug/delete-image-pull-backoff branch April 20, 2023 20:20
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
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.

2 participants