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

Adding activeDeadlineSeconds to build and pod spec #958

Merged
merged 15 commits into from
Aug 2, 2022

Conversation

daraghlowe
Copy link
Contributor

What are we doing?

@sambhav
Copy link
Contributor

sambhav commented May 13, 2022

I believe we need both active deadline seconds and ttlSecondsAfterFinished. The former stops builds that are long running and the latter cleans up pods in terminal state.

We would also need to expose these fields in the image spec. We would also need to document what happens with the build controller if we introduce active deadline seconds and the pod is terminated before completion.

Lastly we also need to expose these fields in the image spec.

@daraghlowe
Copy link
Contributor Author

daraghlowe commented May 16, 2022

Thanks for the feedback @samj1912

Unfortunately ttlSecondsAfterFinished only works with jobs.

https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/

I tested it out anyway to be sure and got this error:

ValidationError(Pod.spec): unknown field "ttlSecondsAfterFinished" in io.k8s.api.core.v1.PodSpec

We're planning on addressing this in our implementation of kpack by creating a new controller to clean up the pods.

We would also need to document what happens with the build controller if we introduce active deadline seconds and the pod is terminated before completion

The container that's running exits with exit code 2 when the deadline seconds are met and the overall pod has a status of Init:Error. I didn't see anything in the logs on the kpack controller when the pod was killed. Is there something in particular I should be looking at here and where does it need to be documented?

Lastly we also need to expose these fields in the image spec.

Updated the image spec also 👍🏻

docs/image.md Outdated Show resolved Hide resolved
@daraghlowe daraghlowe requested a review from sambhav June 2, 2022 14:12
docs/build.md Show resolved Hide resolved
@daraghlowe daraghlowe requested a review from ncarlson June 8, 2022 12:58
@matthewmcnew matthewmcnew added this to the kpack 0.7.0 milestone Jun 30, 2022
@matthewmcnew matthewmcnew merged commit 46df537 into buildpacks-community:main Aug 2, 2022
@matthewmcnew
Copy link
Collaborator

Thanks @daraghlowe !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set a timeout on the builds
6 participants