Rename Helm HPA option + fixes to Helm READMEs #523
Conversation
Signed-off-by: Eero Tamminen <[email protected]>
Signed-off-by: Eero Tamminen <[email protected]>
eero-t
changed the title
| @@ -15,7 +15,7 @@ | |||
|
|
|||
| ## Introduction | |||
|
|
|||
| `horizontalPodAutoscaler` option enables HPA scaling for relevant service components: | |||
| `autoscaling` option enables HPA scaling for relevant service components: | |||
| https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/ | |||
There was a problem hiding this comment.
I would like to use just hpa instead of autoscaling, and then open that as Horizontal Pod Autoscaling in the text. autoscaling is to generic and can mean horizontal or vertical pod, or cluster (node) autoscaling.
There was a problem hiding this comment.
Is cluster node autoscaling something that Helm charts would control (install rules for), and something that could be added to OPEA Helm charts later on? And if yes, why that would not be also be under autoscaling option:
autoscaling.scaleNodes: trueautoscaling.maxNodes: 99
?
There was a problem hiding this comment.
autoscaling.hpa is good. We can extend the autoscaling in the future then, if needed.
There was a problem hiding this comment.
Also, these are OPEA application Helm charts, so I think it's quite clear that it's the application (deployments) that are being scaled, instead of (potentially unrelated) nodes...
There was a problem hiding this comment.
yes, that is the case. My point is that autoscaling is too generic. If this is about HPA autoscaling, lets have the hpa in the name. So, hpa or autoscaling.hpa are both fine.
There was a problem hiding this comment.
autoscaling.hpais good. We can extend the autoscaling in the future then, if needed.
As cluster scaling is not really an application install setting, there should be no confusion with that.
Are there also other relevant ways of (auto) scaling k8s applications than HPA?
(.enabled -> .hpa change is OK to me, but I'm not yet seeing compelling reason for it i.e. to differ from NIM Helm charts.)
There was a problem hiding this comment.
Yeah, the NIM point is valid. Let's keep it similar.
Description
Rename
horizontalPodAutoscaleroption asautoscaling, to align it with the name used in Nvidia Helm charts:https://github.com/NVIDIA/nim-deploy/blob/main/helm/nim-llm/README.md#autoscaling-parameters
And fix how
booloptions are shown in Helm READMEs.Issues
n/a.Type of change
Fixes README typos, changes (very rarely used) Helm option name:
Dependencies
n/a.Tests
Change is automated (+ some README editing to align table columns):
git grep -l horizontalPodAutoscaler | xargs sed -i s/horizontalPodAutoscaler/autoscaling/gAnd from the diff it's clearly doing the right thing so there should be no need for testing. If things worked earlier, they still work.