Skip to content
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

Introduce configurable TTLSecondsAfterFinished for tasks #2252

Closed
10 tasks
RealAnna opened this issue Oct 9, 2023 · 11 comments · Fixed by #2404
Closed
10 tasks

Introduce configurable TTLSecondsAfterFinished for tasks #2252

RealAnna opened this issue Oct 9, 2023 · 11 comments · Fixed by #2404
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@RealAnna
Copy link
Contributor

RealAnna commented Oct 9, 2023

Currently, Keptn Tasks are never deleted. As a user, I may want to have them automatically cleaned up after a certain time. See https://keptn.slack.com/archives/CNRCGFU3U/p1696847482466449

We could introduce a configurable timeout in the helm chart of the lifecycle operator, and also allow users to customize/override it in the KeptnTaskDefinition specs.

For more info check TTLSecondsAfterFinished

AC

  • add an env var in the main of the lifecycle operator to inject default values to the TaskController, call it DefaultTTLSecondsAfterFinished (defaults to 10min)
  • set the var from the pod env (make sure to modify the deployment both in the helm chart and in the config folder)
  • modify the helm chart to set the pod env using a value (like done here for scheduling gates )
  • modify the TaskDefinitionSpec to add TTLSecondsAfterFinished
  • in the controller, if the KeptnTaskDefinition contains a TTLSecondsAfterFinished pass this value at job creation, otherwise use DefaultTTLSecondsAfterFinished
  • unit test
  • fix helm tests accordingly, and regenerate crd docs using this script

DoD

  • user can configure a global behavior for task cleanup
  • user can override such behavior in a single KeptnTaskDefinition
  • feature is documented
@RealAnna RealAnna added enhancement New feature or request status: ready-for-refinement Issue is relevant for the next backlog refinment labels Oct 9, 2023
@RealAnna RealAnna changed the title introduce configurable TTLSecondsAfterFinished for tasks Introduce configurable TTLSecondsAfterFinished for tasks Oct 9, 2023
@RealAnna RealAnna added the good first issue Good for newcomers label Oct 9, 2023
@mowies mowies added this to the 0.9 milestone Oct 9, 2023
@Cyanopus
Copy link

Cyanopus commented Oct 9, 2023

Can we expose the whole Job spec as a field in the WorkLoadinstance or the Workload so the behavior can be fine-tuned? Also, there should be an easy way to suggest this config from the annotation of the Deployment of the actual app, as these resources are dynamically generated by the controller. It will be a complicated user-experience(and not a pleasant one) if you have to modify resources for which you are not responsible for the lifecycle.

@mowies
Copy link
Member

mowies commented Oct 10, 2023

What's the reason why you want to have the complete jobspec in the workload instance or the workload? wouldn't it fit better into the KeptnTaskDefinition?

@RealAnna
Copy link
Contributor Author

RealAnna commented Oct 10, 2023

I agree with @mowies the job specs should be probably linked as a reference in the taskDefinition or directly merged in it.... In our original poc the container runner was like that, but we discussed that it was better to limit what the user can run via Keptn so this was changed at implementation time. So we should either keep it under Keptn control or enrich the container runner or swap it for a full job spec.

@mowies
Copy link
Member

mowies commented Oct 10, 2023

although an interesting scenario could be that one team creates taskdefinitions and another team controls the KeptnApps/Workloads and the app team maybe wants to control how the job lifecycle is handled by customizing some things from the workloads/apps

@prakrit55
Copy link
Member

Hey @RealAnna, I wd like to help it.

@RealAnna
Copy link
Contributor Author

RealAnna commented Oct 11, 2023

@prakrit55 we will need to refine this today and decide what to do 😸 will ping you when it is ready

@thisthat
Copy link
Member

Hey @prakrit55 have you started working on this ticket? :)

@prakrit55
Copy link
Member

Hey @prakrit55 have you started working on this ticket? :)

Hey @thisthat, everything is done in my local, just unit tests are left. I was hoping and working on #2254. If its time bound, I will make a quick pr on it.

@thisthat
Copy link
Member

Hey @prakrit55 no rush :) I went through the tickets and so that we never ping you again after the refinement ;)
Thanks for #2254 🙌

@prakrit55
Copy link
Member

prakrit55 commented Oct 25, 2023

Hey @prakrit55 no rush :) I went through the tickets and so that we never ping you again after the refinement ;)
Thanks for #2254 🙌

@thisthat , does the ttlSecondsAfterFinished creates another job after 10 seconds of removal? Its creating another job after 10s but not after 15 in ttl. Help me with a reply 🙂

@mowies
Copy link
Member

mowies commented Oct 30, 2023

ttlSecondsAfterFinished doesn't create another job. It will actually delete the job resource if its state is "failed" or "completed". That field exists on the core Job kubernetes resource and we want to make it configurable for keptn users. hope this helps :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants