feat(opentelemetry-collector): add VPA and in-place resize support#2076
feat(opentelemetry-collector): add VPA and in-place resize support#2076boqu wants to merge 8 commits intoopen-telemetry:mainfrom
Conversation
|
@TylerHelmuth Thank you for your review. I pushed all the changes according to your review. Please let me know. |
|
The CI failed due missing VPA CRD. See https://github.com/open-telemetry/opentelemetry-helm-charts/actions/runs/22117983560/job/63930896483?pr=2076. VPA CRDs are not part of core Kubernetes. Is it ok to add VPA CRDs in the CI test cluster? |
|
I would be strongly against bringing non-standard crds to this chart. Is there currently a way to install VPA separately? |
|
@boqu does |
I added a minimal stub VPA CRDs here. But it's failing with below error. If my understanding is correct, helm fails when resolving all resource types against the cluster API servers before applying anything. So it looks like we still need to install the CRDs before the CI test. |
I think we only need to install the CRDs in the github action to run the CI test. If this is not acceptable, I can simply remove CI test. |
|
I install the VPA CRD in the github setup action. The CI test can pass now. |
|
|
||
| # Vertical Pod Autoscaler - https://kubernetes.io/docs/concepts/workloads/autoscaling/#vertical-pod-autoscaling | ||
| verticalPodAutoscaler: | ||
| enabled: false |
There was a problem hiding this comment.
I have misunderstood how this feature is implemented in kubernetes. Is this feature not available in a standard +1.27 k8s install? What are the additional CRDs it needs?
There was a problem hiding this comment.
No, the VPA doesn't come with kubernetes in default. It need to be installed separately. See https://kubernetes.io/docs/concepts/workloads/autoscaling/#scaling-workloads-vertically and https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler. The CRD can be found here.
There was a problem hiding this comment.
We've typically been against adding direct support for external CRDs, but seeing as how this is a kubernetes-owned CRD I'm ok adding this feature. @open-telemetry/helm-approvers what do you think?
There was a problem hiding this comment.
Can I know if we can move this PR forward or anything outstanding concern?
There was a problem hiding this comment.
I'd appreciate if we can move this PR forward.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
e094768 to
a5e0b92
Compare
|
Any chance we can move this PR forward? |
Summary
resizePolicysupport for Kubernetes in-place pod resource resize (>= 1.27)VPA
templates/vpa.yamltemplate gated onautoscaling.k8s.io/v1API capability andverticalPodAutoscaler.enabledrecommenders,controlledResources,maxAllowed,minAllowed,updatePolicy, and fullcontainerPoliciesoverridevpaKindhelper in_helpers.tplvalues.schema.jsonci/vpa-deployment-values.yamlIn-place resize
resizePolicyfield on the collector container in_pod.tplNotRequiredandRestartContainerrestart policies per resource