Remove CPU limits from gardener components

[stoyanr]

How to categorize this PR?

/kind enhancement

What this PR does / why we need it:

• Removes CPU limits from all components to prevent CPU throttling due to reaching limits.

The following will be added in separate PRs:

• Remove or adjust memory limits from all components to match actual usage on landscapes.
• Fix VerticalPodAutoscaler resources to use RequestsOnly instead of RequestsAndLimits
• Fix VerticalPodAutoscaler and Hvpa resources to remove MaxAllowed where it's present

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

CPU limits from all gardener components have been removed to prevent CPU throttling due to reaching limits.

[gardener-robot]

@stoyanr Label area/controlplane does not exist.

[gardener-robot]

@stoyanr Label area/controlplane does not exist.

[stoyanr]

/invite @danielfoehrKn @voelzmo

[gardener-robot]

@stoyanr Label area/controlplane does not exist.

[stoyanr]

I consider this PR ready for review now.

[rfranzke]

/lgtm

[vlerenc]

LOL, thanks @rfranzke.

(1) #255 was a band-aid from mid 2018 for the poor VPA v1 for the actual ticket...

(2) #16 that we couldn't implement with the former VPA v1 and so we did this, because we hit...

(3) #79, where KCM stopped working under load

cc @voelzmo we might want to rid is of all that scalingClass workarounds now with VPA v2, if we can make it work at scale and as we want it to work.

So funny/shocking how long this topic is now with us. I see Gaurav made the changes. He was the first of the 3 colleagues on the auto-scaling topic - not in parallel - in sequence over 4 years. :-)

Time to give it a new shot and improve or ideally get rid of all these hacks.

[stoyanr]

Time to give it a new shot and improve or ideally get rid of all these hacks.

@vlerenc I will take a look, and perhaps improve it in a separate PR.

[vlerenc]

Thanks @stoyanr. I am just a bit cautious here as well.

The point is that it may still be necessary to have "decent" requests, "oriented" at the number/size of the nodes, so that we start in the correct ballpark and avoid lengthy up-scaling iterations with also the current VPAv2 (unless tuned). I say that because I remember many years before that e.g. waking up clusters (after hibernation) took very long to scale up and become operational again. Scaling classes "catapulted" control planes into the right ballpark. As long as VPAv2 is making poor recommendations or isn't bumping up faster, most of that code would probably still be required. We can only really rid ourselves of it once VPAv2 overcomes it current problems. At least, that's my gut feeling.

[stoyanr]

As long as VPAv2 is making poor recommendations or isn't bumping up faster, most of that code would probably still be required. We can only really rid ourselves of it once VPAv2 overcomes it current problems.

OK, in this case I will postpone this for later.

[stoyanr]

@vlerenc I examined the scalingClass implementation and played with it a bit, it seems that when HVPA is enabled (it's a feature gate in gardenlet that I believe is enabled by default), the scaling class is only really taken into account the first time the deployment is created. If the deployment already exists, the resources are copied from the existing deployment. This means that if the cluster started with a low initial number of nodes, increasing the number of nodes later will have no effect on the resources in the deployment. Hibernating / waking up will certainly have no effect either.

I am rather skeptical of this complex implementation (scaling class + HVPA), but as agreed, let's take another look and possibly change it once we are more confident of VPA recommendations.

[vlerenc]

@stoyanr My memory is from many years back with end users complaining in Slack and whatever you have found out today counts (not my memory), because we may have influenced the behaviour directly or indirectly many times over since 2018.

[stoyanr]

@vlerenc @rfranzke The new etcd-druid release has been merged and I removed the few TODOs in the PR (now possible with the new etcd-druid version). I consider the PR ready to be merged. Could I get an approval or do you have any other open points?

[rfranzke]

/lgtm
/approve
/hold

@vlerenc any objections? If not please /hold cancel so that the PR gets auto-merged by Prow.

[gardener-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rfranzke]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vlerenc]

/hold cancel
@stoyanr Thanks for the PR. 👍 Let's roll it our carefully now, watching for possible issues, increased node usage, etc. please.
@rfranzke I have not hold this one (in fact, I lgtm'd it, because that's what we want - I can only check from the "intention" level and not every line anyway/am no expert), but you wanted to double-check because of Prow's automation?

[rfranzke]

I simply wasn't sure whether you are fine merging this PR now because there were quite some follow-up questions after your /lgtm and I couldn't follow everything in detail :)