Allow setting priorityClassName via helm chart #5255
Conversation
|
@Matthew-Beckett can I get your help reviewing this? 🙏🏼 |
| @@ -33,6 +33,8 @@ podAnnotations: {} | |||
| podSecurityContext: {} | |||
| # fsGroup: 2000 | |||
|
|
|||
| priorityClassName: "" | |||
There was a problem hiding this comment.
The if statement which evaluates if this is populated or not in deployment.yaml will evaluate to true and populate this invalid null string by default won't it?
Should this be commented out by default?
There was a problem hiding this comment.
Never mind docs say empty strings evaluate false.
https://helm.sh/docs/chart_template_guide/control_structures/
There was a problem hiding this comment.
TIL! So do you think this is good to merge?
There was a problem hiding this comment.
I'm so sorry this has taken so long, I could make a whole load of excuses for the delay but none would make it okay.
I am so sorry @timbrd, thanks for your contribution ❤️
I've tested this now and I'm happy for merge!
Codecov Report
@@ Coverage Diff @@
## main #5255 +/- ##
=======================================
Coverage 72.38% 72.38%
=======================================
Files 30 30
Lines 1673 1673
Branches 367 367
=======================================
Hits 1211 1211
Misses 399 399
Partials 63 63 Continue to review full report at Codecov.
|
|
@jsjoeio @Matthew-Beckett Is there anything more which has to be done or will the PR land shortly? |
Based on that merge I'd say no... 😅 This is all ready to go! |
Adds an option to the helm chart to set a pod priority class.