feat(chart): add optional VerticalPodAutoscaler support - #6592
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @shcherbak. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Control plane components are usually sized manually/statically based on cluster scale, not autoscaled with VPA. There are no practical reasons. More importan VPA is a 3rd party controller with own CRDs, it shouldnt be a dependency in between projects. There is no explanation on why or benefits of external dns with VPA not clear at all If any maintainer see a valid reason to depend and/or support VPA resources we could discuss /hold |
|
Thanks for the review, @ivankatliarchuk - fair questions, let me explain the motivation and address the dependency concern directly. Why VPA helps here external-dns' resource footprint isn't fixed like a typical control-plane component - it scales with the number of watched sources (Ingress/Service/Gateway API/CRDs) and the size of the managed zones per provider. In practice that means:
On the CRD dependency concern This is opt-in and gated behind verticalPodAutoscaler.enabled (default false). When disabled - which is the default and unchanged behavior for every existing user - the template isn't rendered at all, and the chart has no runtime or install-time dependency on the VPA CRD. Nobody who doesn't explicitly enable this needs the VPA controller installed. Happy to add an explicit README note about needing the VPA CRDs installed when the flag is turned on, if that helps. Happy to also default updatePolicy.updateMode to something more conservative (e.g. Initial or Off instead of Auto) if that addresses the "control planes shouldn't be auto-resized live" concern - open to feedback on which default is safest. Let me know if this addresses the concern or if there's additional context you'd want documented before reconsidering the hold. |
Adds optional VerticalPodAutoscaler (VPA) support to the external-dns Helm chart.