-
Notifications
You must be signed in to change notification settings - Fork 27
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
Conversation
{{- with $templateData.pod.hostAliases }} | ||
hostAliases: | ||
{{- toYaml . | nindent 8 }} | ||
{{- end }} |
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.
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
charts/rstudio-connect/values.yaml
Outdated
# -- 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: |
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.
Do we need to provide empty dictionaries by default for requests
and limits
?
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.
Good catch!
9ef04b9
to
72fc140
Compare
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
andScheduler.*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)