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

Add content job template default requests and limits #634

Merged
merged 5 commits into from
Jan 6, 2025

Conversation

dbkegley
Copy link
Contributor

@dbkegley dbkegley commented Jan 6, 2025

Allow setting content job default resource requests and limits configurations via the values file.

If no defaults are provided in the values file then Kubernetes cluster/namespace defaults are still respected.

Connect resource settings provided globally by the Scheduler.*Request and Scheduler.*Limit settings or by the content runtime settings both take precedence over the defaults from the values file.

Replaces: #630

Note there is a separate discussion required to determine if we should specify sane defaults in our values file, but these settings are a good escape hatch for now. see also: #630 (comment)

{{- with $templateData.pod.hostAliases }}
hostAliases:
{{- toYaml . | nindent 8 }}
{{- end }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @samcofer I noticed while working on this PR that we forgot to add the hostAliases setting to the helm chart's job template version. Sorry about that, I'll try to remember to update those templates going forward

# -- sets the default resource requests and limits for Jobs launched by Connect
# -- to be used when none are provided in the content runtime settings
resources: {}
# requests:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to provide empty dictionaries by default for requests and limits?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@dbkegley dbkegley force-pushed the kegs/job-resources-overrides branch from 9ef04b9 to 72fc140 Compare January 6, 2025 20:51
@dbkegley dbkegley merged commit 6ac3842 into main Jan 6, 2025
7 checks passed
@dbkegley dbkegley deleted the kegs/job-resources-overrides branch January 6, 2025 22:25
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.

4 participants