-
Notifications
You must be signed in to change notification settings - Fork 23
Add priorityClassName to remaining pods #778
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
Conversation
michaellzc
left a comment
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.
can we DRY things up a bit using partial template?
see
deploy-sourcegraph-helm/charts/sourcegraph/templates/gitserver/gitserver.StatefulSet.yaml
Lines 100 to 102 in 604350a
| {{- include "sourcegraph.nodeSelector" (list . "gitserver" ) | trim | nindent 6 }} | |
| {{- include "sourcegraph.affinity" (list . "gitserver" ) | trim | nindent 6 }} | |
| {{- include "sourcegraph.tolerations" (list . "gitserver" ) | trim | nindent 6 }} |
deploy-sourcegraph-helm/charts/sourcegraph/templates/_helpers.tpl
Lines 109 to 120 in 604350a
| {{- define "sourcegraph.nodeSelector" -}} | |
| {{- $top := index . 0 }} | |
| {{- $service := index . 1 }} | |
| {{- $globalNodeSelector := (index $top.Values "sourcegraph" "nodeSelector") }} | |
| {{- $serviceNodeSelector := (index $top.Values $service "nodeSelector") }} | |
| nodeSelector: | |
| {{- if $serviceNodeSelector }} | |
| {{- $serviceNodeSelector | toYaml | trim | nindent 2 }} | |
| {{- else if $globalNodeSelector }} | |
| {{- $globalNodeSelector | toYaml | trim | nindent 2 }} | |
| {{- end }} | |
| {{- end }} |
@michaellzc Yes, I'm working on a few fixes, starting with #780, and will continue with DRYing up our templates a bit better. This change is holding up a customer project, so I'll fast follow with the DRY updates. |
|
Hey @michaellzc, I created a partial template / helper function to DRY things up a bit. Helm templating seems to have some limitations in terms of preventing excess whitespace, so the current implementation is the cleanest way I've found so far to avoid adding whitespace when Helm whitespacing seems to be a weak point for Amp. |
|
Quick and dirty bash script to find all helm charts in the repo, template them without and with |
Linear issue FEIE-297: Add
priorityClassNameto remaining podssourcegraphtop level key, and / or under each pod's top-level key, which would override the config on thesourcegraphtop level key, so the customer could configure:Checklist
Test plan