-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: introduce configurable TTLSecondsAfterFinished for tasks #2404
feat: introduce configurable TTLSecondsAfterFinished for tasks #2404
Conversation
Signed-off-by: Griffin <prakritimandal611@gmail.com>
✅ Deploy Preview for keptn-lifecycle-toolkit ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2404 +/- ##
==========================================
+ Coverage 85.50% 85.75% +0.25%
==========================================
Files 161 161
Lines 10166 10202 +36
==========================================
+ Hits 8692 8749 +57
+ Misses 1196 1183 -13
+ Partials 278 270 -8
... and 24 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @prakrit55 for the PR. I left some comments to simplify the implementation moving the logic of handling a default value to K8s instead of dealing with it ourselves :)
lifecycle-operator/apis/lifecycle/v1alpha3/keptntaskdefinition_types.go
Outdated
Show resolved
Hide resolved
lifecycle-operator/controllers/lifecycle/keptntask/controller.go
Outdated
Show resolved
Hide resolved
lifecycle-operator/controllers/lifecycle/keptntask/job_utils.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Griffin <prakritimandal611@gmail.com>
hey @thisthat, you can have a look now, I have applied the changes |
Signed-off-by: Griffin <prakritimandal611@gmail.com>
test/integration/TTLSecondsAfterFinished-in-jobs/00-assert.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: Griffin <prakritimandal611@gmail.com>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
LGTM Thanks for the help @prakrit55 🙌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contribution!
Thanks, you guys too for helping me out .😊 |
Description
Fixes #2252
How to test
Please describe how to run the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also provide information about any automatic tests that you added.
Checklist
into multiple PRs)
see Contribution Guide
the Contribution Guide
Summary
This allows the user to clean up jobs after it has failed or completed after a specified time( held by user in TaskDefinition).
Fixes #2252
Checks
into multiple PRs)
see Contribution Guide)
the Contribution Guide
from the Netlify deploy preview