Introduce new feature gate `VPAForETCD` to use VPA for etcd autoscaling #8984

voelzmo · 2023-12-20T20:38:23Z

How to categorize this PR?

/area auto-scaling
/area control-plane
/kind enhancement

What this PR does / why we need it:
The main reason why etcd currently uses HVPA is for limiting when VPA recommendations should be applied. The two use cases are preventing downscaling entirely, or limiting downscaling to the Shoot's maintenance window. Additionally, there are some "minimum change sizes" defined on the HVPA object, meaning that a change in resource requests smaller than the minimum change size will not be applied.
This PR introduces a method to achieve a similar functionality using plain VPA, making it possible to remove HVPA for etcd entirely in a next step.

In this PR we update to VPA version 1.1.1, which contains a new upstream feature, called EvictionRequirements, that allows us to limit evictions based on the scaling direction (is the workload being scaled up or being scaled down). Detailed information for this feature can be found in the original enhancement proposal.

In detail, this PR

Introduces a new featuregate VPAForETCD on gardenlet and garden-operator
Introduces a new controller in gardenlet, called VPAEvictionRequirementsController to manage the EvictionRequirements on VPA objects based on two new labels and a new annotation
takes care that the garden-operator starts the controller instead of gardenlet, if necessary

Introduces the following changes in case operators have enabled the new featuregate for the gardenlet on their Seeds:

Use plain VPA to scale etcd
Label the VPA object with
- Label autoscaling.gardener.cloud/eviction-requirements: managed-by-controller to indicate that the EvictionRequirements on this VPA object will be managed by the new controller
Annotate the VPA with eviction-requirements.autoscaling.gardener.cloud/downscale-restriction: in-maintenance-window-only in case of limiting scaling down to a Shoot's maintenance window
Annotate the VPA with eviction-requirements.autoscaling.gardener.cloud/downscale-restriction: never in case of denying scaling down entirely
- These annotations are currently assigned in the same way as for HVPA, based on the ShootClass
- Add the annotation shoot.gardener.cloud/maintenance-window to make the Shoot maintenance window accessible for the new controller
Drop the "minimum change size" requirement, as it lead to severe deadlocks in the past in which human interaction was necessary to get etcd out of a broken state

If HVPA is enabled, the VPAForETCD feature gate takes precedence, so the etcds will be scaled like described above with plain VPA and the corresponding HVPA object will be removed.

Release note:

Updated VPA to 1.1.1

A new feature gate named `VPAForETCD` is now introduced for gardenlet and gardener-operator. When enabled, VPA for etcd is used, regardless of the HVPA feature gate setting. The new VPA limits scaling down to a Shoot's maintenance window or even entirely based on the `ShootClass` in the same way as it is currently done for HVPA.

voelzmo · 2023-12-20T20:39:40Z

/hold as this uses an unreleased VPA version

ScheererJ

Thanks for the very interesting improvement.

I have a few questions/comments.

pkg/component/etcd/etcd.go

pkg/component/etcd/etcd_test.go

pkg/gardenlet/controller/vpaevictionrequirements/add.go

pkg/gardenlet/controller/vpaevictionrequirements/reconciler.go

andrerun

There is one minor fix required, one idea which may or may not be worth implementing, and one question.

go.mod

pkg/apis/core/v1beta1/constants/types_constants.go

pkg/gardenlet/controller/vpaevictionrequirements/reconciler.go

pkg/apis/core/v1beta1/constants/types_constants.go

pkg/component/etcd/etcd.go

pkg/component/etcd/etcd_test.go

pkg/gardenlet/controller/vpaevictionrequirements/reconciler.go

andrerun

LGTM!

voelzmo · 2024-01-11T09:28:20Z

Also adapted the PR description to match the changed labels.

ScheererJ

/lgtm

Then we only need to get the VPA release in.

rfranzke · 2024-01-19T08:19:22Z

/assign

rfranzke

Thanks for the PR, and sorry for joining the review late (I was on leave). Apart from the above feedback, please also add

some documentation for this new controller to https://github.com/gardener/gardener/blob/master/docs/concepts/gardenlet.md#controllers
an integration test for this new controller to https://github.com/gardener/gardener/tree/master/test/integration/gardenlet

pkg/apis/core/v1beta1/constants/types_constants.go

pkg/gardenlet/apis/config/v1alpha1/types.go

pkg/apis/core/v1beta1/constants/types_constants.go

pkg/gardenlet/controller/vpaevictionrequirements/add.go

pkg/gardenlet/controller/vpaevictionrequirements/reconciler.go

…r!=seed cluster

voelzmo · 2024-04-26T11:32:50Z

Updated to the new VPA release 1.1.1, which fixes the NPE and rebased. Also included changes for the license headers in the new files added by this commit. This is good to go from my side!

pkg/features/features.go

docs/deployment/feature_gates.md

voelzmo · 2024-04-26T12:50:34Z

/retest

rfranzke · 2024-04-26T13:35:37Z

/lgtm

gardener-prow · 2024-04-26T13:35:42Z

LGTM label has been added.

Git tree hash: 175ab6b00bdbdd48251a68a19ddb10ac2fc8fa4f

gardener-prow bot added the area/auto-scaling Auto-scaling (CA/HPA/VPA/HVPA, predominantly control plane, but also otherwise) related label Dec 20, 2023

gardener-prow bot requested a review from acumino December 20, 2023 20:38

gardener-prow bot added the area/control-plane Control plane related label Dec 20, 2023

gardener-prow bot requested a review from ScheererJ December 20, 2023 20:38

gardener-prow bot added kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 20, 2023

gardener-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 20, 2023

voelzmo force-pushed the enh/eviction-requirements-for-etcd branch from c70837a to 5bc931c Compare December 20, 2023 21:28

ScheererJ requested changes Jan 2, 2024

View reviewed changes

gardener-prow bot assigned ScheererJ Jan 2, 2024

andrerun reviewed Jan 3, 2024

View reviewed changes

ScheererJ reviewed Jan 9, 2024

View reviewed changes

andrerun approved these changes Jan 9, 2024

View reviewed changes

gardener-prow bot assigned andrerun Jan 9, 2024

gardener-prow bot added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Jan 9, 2024

gardener-prow bot requested review from andrerun and ScheererJ January 10, 2024 16:10

voelzmo force-pushed the enh/eviction-requirements-for-etcd branch from 4cdfec7 to 9ad9cfe Compare January 11, 2024 09:17

andrerun approved these changes Jan 11, 2024

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 11, 2024

ScheererJ reviewed Jan 12, 2024

View reviewed changes

gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 17, 2024

gardener-prow bot assigned rfranzke Jan 19, 2024

rfranzke requested changes Jan 19, 2024

View reviewed changes

gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 25, 2024

voelzmo force-pushed the enh/eviction-requirements-for-etcd branch from baa8120 to 5a523a6 Compare April 26, 2024 09:50

gardener-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 26, 2024

voelzmo force-pushed the enh/eviction-requirements-for-etcd branch from 5a523a6 to b6a25b8 Compare April 26, 2024 10:00

voelzmo added 9 commits April 26, 2024 12:58

Bump VPA to 1.1.1

eb4a00e

Add VPA for etcd when HVPA is disabled

6d639c4

Add VPAEvictionRequirements Controller in gardenlet

90526d7

Run make generate

24f5188

Run ./hack/update-skaffold-deps.sh

fc549eb

Add integration tests

b902702

Add documentation for VPAEvictionRequirements controller

7d21ac9

Add unit tests for VPAEvictionRequirements controller

10087ac

Install VPAEvictionController with gardener-operator if garden cluste…

f5b44cd

…r!=seed cluster

voelzmo force-pushed the enh/eviction-requirements-for-etcd branch from b6a25b8 to e228f21 Compare April 26, 2024 10:58

rfranzke reviewed Apr 26, 2024

View reviewed changes

pkg/features/features.go Outdated Show resolved Hide resolved

docs/deployment/feature_gates.md Outdated Show resolved Hide resolved

voelzmo added 4 commits April 26, 2024 13:49

Add new Featuregate VPAForETCD

afbdffb

Use featuregate VPAForETCD in operator and gardenlet

cbeaf79

Inline HVPAConfig

2ad0d2c

Run make generate

c6711cb

voelzmo force-pushed the enh/eviction-requirements-for-etcd branch from e228f21 to c6711cb Compare April 26, 2024 11:49

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Apr 26, 2024

gardener-prow bot merged commit 0545f59 into gardener:master Apr 26, 2024
17 checks passed

voelzmo mentioned this pull request May 3, 2024

Don't do HVPA specific things for etcd when VPAForETCD is enabled #9703

Merged

rfranzke mentioned this pull request May 7, 2024

[operator] Make controllerregistrar reconciler generic #9717

Merged

vlerenc mentioned this pull request May 17, 2024

Improve Node Utilization by Reducing Requests for Druid-Managed Pods gardener/etcd-druid#783

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce new feature gate `VPAForETCD` to use VPA for etcd autoscaling #8984

Introduce new feature gate `VPAForETCD` to use VPA for etcd autoscaling #8984

voelzmo commented Dec 20, 2023 •

edited

voelzmo commented Dec 20, 2023

ScheererJ left a comment

andrerun left a comment

andrerun left a comment

voelzmo commented Jan 11, 2024

ScheererJ left a comment

rfranzke commented Jan 19, 2024

rfranzke left a comment

voelzmo commented Apr 26, 2024

voelzmo commented Apr 26, 2024

rfranzke commented Apr 26, 2024

gardener-prow bot commented Apr 26, 2024

Introduce new feature gate VPAForETCD to use VPA for etcd autoscaling #8984

Introduce new feature gate VPAForETCD to use VPA for etcd autoscaling #8984

Conversation

voelzmo commented Dec 20, 2023 • edited

voelzmo commented Dec 20, 2023

ScheererJ left a comment

Choose a reason for hiding this comment

andrerun left a comment

Choose a reason for hiding this comment

andrerun left a comment

Choose a reason for hiding this comment

voelzmo commented Jan 11, 2024

ScheererJ left a comment

Choose a reason for hiding this comment

rfranzke commented Jan 19, 2024

rfranzke left a comment

Choose a reason for hiding this comment

voelzmo commented Apr 26, 2024

voelzmo commented Apr 26, 2024

rfranzke commented Apr 26, 2024

gardener-prow bot commented Apr 26, 2024

Introduce new feature gate `VPAForETCD` to use VPA for etcd autoscaling #8984

Introduce new feature gate `VPAForETCD` to use VPA for etcd autoscaling #8984

voelzmo commented Dec 20, 2023 •

edited